Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug with loading of <a-assets> #5251

Closed
tomfelder94 opened this issue Feb 23, 2023 · 11 comments
Closed

Bug with loading of <a-assets> #5251

tomfelder94 opened this issue Feb 23, 2023 · 11 comments

Comments

@tomfelder94
Copy link
Contributor

Description:

We found a bug in which the loading of images is not correctly handled. Because of this, the loading screen is always waiting until the timeout is fired. I attached a very simple code below, you just need to add a random image ("image.jpg") to reproduce (not reproducible on glitch, image needs to be local).

The issue seems to be in the following part of the a-assets code:

line 47 and following:

// Wait for <img>s.
imgEls = this.querySelectorAll('img');
for (i = 0; i < imgEls.length; i++) {
   imgEl = fixUpMediaElement(imgEls[i]);
   loaded.push(new Promise(function (resolve, reject) {
      // Set in cache because we won't be needing to call three.js loader if we have.
      // a loaded media element.
      THREE.Cache.add(imgEls[i].getAttribute('src'), imgEl);
      imgEl.onload = resolve;
      imgEl.onerror = reject;
   }));
}

We are not handling the case if the image has already been loaded, in this case promise is never being resolved. We should use something like the following in the promise:

if (imgEl.complete) {
    resolve();
}

Is this a known issue or are we missing something? I could create a PR for that if you wish.

  • A-Frame Version: 1.4.1
  • Platform / Device: Chrome, Macbook Pro 2019 (Intel)
  • Reproducible Code Snippet or URL:
<html>
<head>
    <script src="https://aframe.io/releases/1.4.1/aframe.min.js"></script>
</head>
<body>
    <a-scene>
        <a-assets timeout="10000">
            <!-- You got until the count of 10 to load else the show will go on without you. -->
            <img src="image.jpg">
        </a-assets>
        <a-box position="-1 0.5 -3" rotation="0 45 0" color="#4CC3D9"></a-box>
        <a-sphere position="0 1.25 -5" radius="1.25" color="#EF2D5E"></a-sphere>
        <a-cylinder position="1 0.75 -3" radius="0.5" height="1.5" color="#FFC65D"></a-cylinder>
        <a-plane position="0 0 -4" rotation="-90 0 0" width="4" height="4" color="#7BC8A4"></a-plane>
        <a-sky color="#ECECEC"></a-sky>
    </a-scene>
</body>
</html>
@dmarcos
Copy link
Member

dmarcos commented Feb 23, 2023 •
edited

How is local different than glitch? Are you serving your image through a web server? Perhaps relevant: https://aframe.io/docs/1.4.0/introduction/faq.html#why-does-my-asset-e-g-image-video-model-not-load

@tomfelder94
Copy link
Contributor Author

It is a matter of network speed and timing, that's why my glitch example was not working, sorry. If the image is small enough/ network fast enough, the image is loaded before we wait on the onload event and therefore causing the issue. I could reproduce it with the following glitch example: https://glitch.com/edit/#!/erratic-like-ocean?path=index.html%3A11%3A19

It does not happen always but sometimes does (in particular when cache is cleared), as it is a matter of timing.

Can you reproduce?

@gftruj
Copy link
Contributor

gftruj commented Feb 23, 2023

(in particular when cache is cleared)

Couldn't reproduce with a fiddle, now i see why :D
Happens for me each hard reload on windows chrome / edge, for some reason works on firefox (tried with your glitch)

@dmarcos
Copy link
Member

dmarcos commented Feb 23, 2023 •
edited

Can you try with a master build? https://cdn.jsdelivr.net/gh/aframevr/aframe@3d281849d480293d10badca644584806c3f4906a/dist/aframe-master.min.js

@gftruj
Copy link
Contributor

gftruj commented Feb 23, 2023

Same, ctrl-F5 (on chrome even the first load) results in a blue screen until a-assets timeouts (remixed glitch, preview on windows10 chrome / edge)

@tomfelder94
Copy link
Contributor Author

tomfelder94 commented Feb 24, 2023 •
edited

I tried with the master build, same problem. The thing that fixes it is if we check if the loading of the image is already complete with something like below.

if (imgEl.complete) {
    resolve();
}

Here's a glitch with the master: https://glitch.com/edit/#!/familiar-obtainable-agenda?path=index.html%3A20%3A0

@dmarcos
Copy link
Member

dmarcos commented Feb 24, 2023

Thanks! Yeah we can add such line:

  if (imgEl.complete) { 
    resolve(); 
    return;
 }

PR welcome.

@tomfelder94
Copy link
Contributor Author

Alright, will create a PR

@dmarcos
Copy link
Member

dmarcos commented Feb 26, 2023

@tomfelder94 thanks so much!

@tomfelder94 tomfelder94 mentioned this issue Feb 27, 2023
Closed
@tomfelder94
Copy link
Contributor Author

tomfelder94 commented Feb 27, 2023 •
edited

Created the PR: #5253

Wasn't sure if I was supposed to commit the dist files...

@tomfelder94 tomfelder94 mentioned this issue Feb 27, 2023
Merged
@dmarcos
Copy link
Member

dmarcos commented Feb 28, 2023

Fixed by #5251

@dmarcos dmarcos closed this as completed Feb 28, 2023
@dmarcos dmarcos mentioned this issue Jul 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dmarcos @gftruj @tomfelder94