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.

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

LoadingManager: wrong order of events #5438

Closed
jppresents opened this issue Oct 12, 2014 · 10 comments
Closed

LoadingManager: wrong order of events #5438

jppresents opened this issue Oct 12, 2014 · 10 comments

Comments

@jppresents
Copy link

Hi,

Maybe I am getting this wrong, but shouldn't the loading manager first call the onLoad function and after that the onProgress function?

My Reason:
I load a json file (containing materials and a geometry) and I set the onLoad callback. In this callback I place the stuff in my scene and in some array to later modify it.

Now I wan to use the DefaultLoadingManager to start my application when everything is loaded.
I do it this way:
THREE.DefaultLoadingManager.onProgress = function ( item, loaded, total ) {
if (loaded == total) {
start();
}

Since the order in which the callbacks are called is "progress" and after that "loaded" the LoadingManager starts the application before everything is setup properly and it (sometimes, depending on the load order) crashes.

If I am just using this wrong, feel free to point me in the right direction - if not, I'll be happy to switch the order of the callbacks and create a pull request.

@jppresents
Copy link
Author

It might be more problematic than that. Just switching the order of onLoad and onProgress does not fix my problem. My LoadCallback ( on a JSONLoader ) is still called after the OnProgress is called (with total == completed).

Also I tried the "onLoad" function of the DefaultLoadingManager - this is sometimes even called when not everything is loaded (I think because while locally testing some objects load so fast, they trigger even before all loads are requested)

A console.log in the onProgress and the onLoad Callbacks of the DefaultLoadingManager (sometimes!) result in this:

DefaulLoadingManafger.onProgress textures/marker.png 1 1
DefaulLoadingManafger.onLoad
DefaulLoadingManafger.onProgress textures/floortexture.png 2 6
DefaulLoadingManafger.onProgress textures/floortexture.png 3 6
DefaulLoadingManafger.onProgress textures/walltexture.png 4 6
DefaulLoadingManafger.onProgress textures/floortexture.png 5 6
DefaulLoadingManafger.onProgress textures/walltexture.png 6 6
DefaulLoadingManafger.onLoad
DefaulLoadingManafger.onProgress textures/avatar.png 7 7
DefaulLoadingManafger.onLoad

This is the log code: console.log('DefaulLoadingManafger.onProgress', item, loaded, total );

From what I gather in the code the "onLoad" should only fire if everything is loaded - but the Loadingmanager simply does not know, that there will be more to load. (Because the loading of item 1 is done, before the loading of item 2 is even requested).

(This is using the unmodified three.js 68)

@mrdoob
Copy link
Owner

mrdoob commented Oct 13, 2014

From what I gather in the code the "onLoad" should only fire if everything is loaded - but the Loadingmanager simply does not know, that there will be more to load. (Because the loading of item 1 is done, before the loading of item 2 is even requested).

Can you provide upload somewhere a test so we can take a look?

@jppresents
Copy link
Author

Hello,

Sorry for taking a three days to respond.

I am using Windows 7 64bit.

With Firefox it seems to be fine every time.
But with Chrome Version 37.0.2062.124 m the problem occurs.
Testing IE 11 (11.0.9600.17280) there is the same problem as in chrome.

Here is the demo of the bug (or the demo of my code not understanding the loading manager):
Zip download: http://jppresents.net/static/defaultLoadingManager/loader.zip
Live site: http://jppresents.net/static/defaultLoadingManager/loadertest.html

Demo consists of: loadertest.html (contains the relevant script), a mini json model + texture and the current release three.js (68).

Expected console output is:

loaded floor # 1
loaded floor # 2
loaded floor # 3
loaded floor # 4
loaded floor # 5
loaded floor # 6
loaded floor # 7
1 / 7
2 / 7
3 / 7
4 / 7
5 / 7
6 / 7
7 / 7
all loading done (says the DefaultLoadingManager)

At least this is what happens the first time (and for me 2 more times refreshing in chrome) and then, refreshing more often (hitting f5 a few times in chrome for example) console-output like this happens:

loaded floor # 1
1 / 1
all loading done (says the DefaultLoadingManager)
loaded floor # 2
loaded floor # 3
loaded floor # 4
loaded floor # 5
loaded floor # 6
loaded floor # 7

So I can't use the progress or the onSetup of the defaultLoadingManager to safely ensure that everything is loaded, before I start my rendering loop.

Thanks for taking the time to look at this.

@gero3
Copy link
Contributor

gero3 commented Oct 17, 2014

The real problem is that JSON Laoder doesn't support the loaderManager.

@jppresents
Copy link
Author

@mrdoob
I think this is no at a question anymore, it's a bug. (pinging you, because you tagged it a question)

As shown above the loadingManager does not work (at least not every time) with IE and Chrome. (only firefox.)

I must admit, that I am not sure what gero3 means.
What he posted before (and then edited) was:
"Hmm, seems like DefaultLoadingManager expects that all the loading events to be asynchronous but it seems like chrome and IE11 don't follow that rule." that made a lot more sense to me.

@gero3
Copy link
Contributor

gero3 commented Oct 31, 2014

It was wrong that is why i deleted it.

What happens:

  • You ask to load json 1 and DefaultLoadingManager doesn't count
  • You ask to load json 2 and DefaultLoadingManager doesn't count
  • You ask to load json 3 and DefaultLoadingManager doesn't count
  • You ask to load json 4 and DefaultLoadingManager doesn't count
  • You ask to load json 5 and DefaultLoadingManager doesn't count
  • When it loads json 1, It finds image1, the image gets asked to load and DefaultLoadingManager raises total to 1 .
  • Then it loads image 1, DefaultLoadingManager raises loaded to 1 and thinks it is finished.

So this is not correct.

What should happen is:

  • You ask to load json 1 and DefaultLoadingManager raises total to 1 .
  • You ask to load json 2 and DefaultLoadingManager raises total to 2 .
  • You ask to load json 3 and DefaultLoadingManager raises total to 3 .
  • You ask to load json 4 and DefaultLoadingManager raises total to 4 .
  • You ask to load json 5 and DefaultLoadingManager raises total to 5 .
  • When it loads json 1, It finds image 1, the image gets asked to load and DefaultLoadingManager raises total to 6 and DefaultLoadingManager raises loaded to 1.
  • Then it loads image 1, DefaultLoadingManager raises loaded to 2.
  • When it loads json 2, It finds image 2, the image gets asked to load and DefaultLoadingManager raises total to 7 and DefaultLoadingManager raises loaded to 3.
  • Then it loads image 2, DefaultLoadingManager raises loaded to 4.
  • When it loads json 3, It finds image 3, the image gets asked to load and DefaultLoadingManager raises total to 8 and DefaultLoadingManager raises loaded to 5.
  • Then it loads image 3, DefaultLoadingManager raises loaded to 6.
  • When it loads json 4, It finds image 4, the image gets asked to load and DefaultLoadingManager raises total to 9 and DefaultLoadingManager raises loaded to 7.
  • Then it loads image 4, DefaultLoadingManager raises loaded to 8.
  • When it loads json 5, It finds image 5, the image gets asked to load and DefaultLoadingManager raises total to 10 and DefaultLoadingManager raises loaded to 9.
  • Then it loads image 5, DefaultLoadingManager raises loaded to 10.

@jppresents
Copy link
Author

Thank you for the clarification, I understand it now. (And still think that
this should be labeled as a bug.)

On Fri, Oct 31, 2014 at 10:26 AM, gero3 notifications@github.com wrote:

It was wrong that is why i deleted it.

What happens:

  • You ask to load json 1 and DefaultLoadingManager doesn't count
  • You ask to load json 2 and DefaultLoadingManager doesn't count
  • You ask to load json 3 and DefaultLoadingManager doesn't count
  • You ask to load json 4 and DefaultLoadingManager doesn't count
  • You ask to load json 5 and DefaultLoadingManager doesn't count
  • When it loads json 1, It finds image1, the image gets asked to load
    and DefaultLoadingManager raises total to 1 .
  • Then it loads image 1, DefaultLoadingManager raises loaded to 1 and
    thinks it is finished.

So this is not correct.

What should happen is:

  • You ask to load json 1 and DefaultLoadingManager raises total to 1 .

  • You ask to load json 2 and DefaultLoadingManager raises total to 2 .

  • You ask to load json 3 and DefaultLoadingManager raises total to 3 .

  • You ask to load json 4 and DefaultLoadingManager raises total to 4 .

    You ask to load json 5 and DefaultLoadingManager raises total to 5 .

    When it loads json 1, It finds image 1, the image gets asked to load
    and DefaultLoadingManager raises total to 6 and DefaultLoadingManager

    raises loaded to 1.

    Then it loads image 1, DefaultLoadingManager raises loaded to 2.

    When it loads json 2, It finds image 2, the image gets asked to load
    and DefaultLoadingManager raises total to 7 and DefaultLoadingManager

    raises loaded to 3.

    Then it loads image 2, DefaultLoadingManager raises loaded to 4.

    When it loads json 3, It finds image 3, the image gets asked to load
    and DefaultLoadingManager raises total to 8 and DefaultLoadingManager

    raises loaded to 5.

    Then it loads image 3, DefaultLoadingManager raises loaded to 6.

    When it loads json 4, It finds image 4, the image gets asked to load
    and DefaultLoadingManager raises total to 9 and DefaultLoadingManager

    raises loaded to 7.

    Then it loads image 4, DefaultLoadingManager raises loaded to 8.

    When it loads json 5, It finds image 5, the image gets asked to load
    and DefaultLoadingManager raises total to 10 and DefaultLoadingManager
    raises loaded to 9.

  • Then it loads image 5, DefaultLoadingManager raises loaded to 10.


Reply to this email directly or view it on GitHub
#5438 (comment).

@mrdoob
Copy link
Owner

mrdoob commented Oct 31, 2014

I'd call it a Enhancement ;)
JSONLoader needs to start using LoadingManager.

@jppresents
Copy link
Author

Agreed :), thank you.

@dubejf
Copy link
Contributor

dubejf commented Jul 23, 2015

I tried running the linked example, it seems to give the correct result. Maybe the example was modified since first reported?

In any case, JSONLoader now implements LoadingManager. This should be fixed. #5758

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

No branches or pull requests

4 participants