Skip to content
This repository has been archived by the owner on Jun 2, 2023. It is now read-only.
This repository has been archived by the owner on Jun 2, 2023. It is now read-only.

Can't have multiple Infospot or panorama link with the same image source #263

Closed
2 of 5 tasks
juanRabaa opened this issue Oct 28, 2020 · 12 comments · Fixed by #308
Closed
2 of 5 tasks

Can't have multiple Infospot or panorama link with the same image source #263

juanRabaa opened this issue Oct 28, 2020 · 12 comments · Fixed by #308
Assignees
Labels

Comments

@juanRabaa
Copy link

juanRabaa commented Oct 28, 2020

Description

When I add more than one Infospot with the same image source, some of them don't appear. They do exist, I can focus them, but they are transparent.

With this code, one of the Infospots won't appear. It also happens with more than 2. I tried it with 3 Infospot with the same image source, and the second one ended up being transparent

panorama.link( panorama_video, new Vector3(3883.71, 745.13, -3047.48), 1000, "assets/play-button.png" );
const videoTrigger = new Infospot( 1000, "assets/play-button.png" );
videoTrigger.position.set( 5000.00, 800, 8000 );
panorama.add( videoTrigger );
Panolens version

Master

Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • Windows
@GiorgosK
Copy link

Have noticed the same even in one of the examples from @pchen66 https://codepen.io/pchen66/pen/dRYNNG
sometimes the 1st image (on left of the pillar) appears sometimes it does not
If the two images are different there is no problem

@juanRabaa
Copy link
Author

It also seems like the one InfoSpot that gets rendered, gets deformed (width and height changed)

@leonexcc
Copy link
Contributor

leonexcc commented Oct 30, 2020

I think i found the problem for this. The ImageLoader uses a cache and if he finds an image in the cache he doesn't check if it is already completely loaded. But if the image is not completely loaded the width and height is 0 and the rate is NaN and the Infospot will not be rendered correctly.

This is my fix:

diff --git a/src/loaders/ImageLoader.js b/src/loaders/ImageLoader.js
index a4e1344..dcb1efc 100644
--- a/src/loaders/ImageLoader.js
+++ b/src/loaders/ImageLoader.js
@@ -40,13 +40,22 @@ const ImageLoader = {
         if ( cached !== undefined ) {

             if ( onLoad ) {
-
-                setTimeout( function () {
-
-                    onProgress( { loaded: 1, total: 1 } );
-                    onLoad( cached );
-
-                }, 0 );
+
+                if ( cached.complete ) {
+                    setTimeout( function () {
+
+                        onProgress( { loaded: 1, total: 1 } );
+                        onLoad( cached );
+
+                    }, 0 );
+                } else {
+                    cached.addEventListener( 'load', function () {
+
+                        onProgress( { loaded: 1, total: 1 } );
+                        onLoad( cached );
+
+                    }, false );
+                }

             }

@juanRabaa
Copy link
Author

Dude good job, that works for me at the moment. You've just saved me a headache

I think i found the problem for this. The ImageLoader uses a cache and if he finds an image in the cache he doesn't check if it is already completely loaded. But if the image is not completely loaded the width and height is 0 and the rate is NaN and the Infospot will not be rendered correctly.

This is my fix:

diff --git a/src/loaders/ImageLoader.js b/src/loaders/ImageLoader.js
index a4e1344..dcb1efc 100644
--- a/src/loaders/ImageLoader.js
+++ b/src/loaders/ImageLoader.js
@@ -40,13 +40,22 @@ const ImageLoader = {
         if ( cached !== undefined ) {

             if ( onLoad ) {
-
-                setTimeout( function () {
-
-                    onProgress( { loaded: 1, total: 1 } );
-                    onLoad( cached );
-
-                }, 0 );
+
+                if ( cached.complete ) {
+                    setTimeout( function () {
+
+                        onProgress( { loaded: 1, total: 1 } );
+                        onLoad( cached );
+
+                    }, 0 );
+                } else {
+                    cached.addEventListener( 'load', function () {
+
+                        onProgress( { loaded: 1, total: 1 } );
+                        onLoad( cached );
+
+                    }, false );
+                }

             }

@leonexcc
Copy link
Contributor

I had to change the if ( cached.complete ) { to if ( cached.complete && cached.src ) { for external resources.

@lucychen0103
Copy link

lucychen0103 commented Feb 16, 2021

@leonexcc Hi, I'm new to coding. I don't understand what part to directly paste into my <script></script> code. Thanks, sorry!

@leonexcc
Copy link
Contributor

You have to change the src/loaders/ImageLoader.js and remove the lines with - and add the lines with + instead. This is something that has to changed in panolens. I can provide a Pull request if there is a chance it will be merged. Lately there are no changes to this library.

@lucychen0103
Copy link

@flyandi is taking over the project. Hopefully this will be solved soon. Thanks!

@flyandi
Copy link
Collaborator

flyandi commented Feb 19, 2021

Do u want to submit a PR for this?

@leonexcc
Copy link
Contributor

Can't provide some for the next two weeks. But i can look into this after that.

@leonexcc
Copy link
Contributor

leonexcc commented Mar 3, 2021

I provided a PR. Please let me know if that works or if i have to change anything.

flyandi added a commit that referenced this issue Mar 21, 2021
Fixes a caching problem in the ImageLoader (see #263)
@lucychen0103
Copy link

Is this included in the panolens min file? For some reason the fix doesn't work for me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants