mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.74k stars 35.38k forks source link

Loader Improvements #4248

Closed gregtatum closed 6 years ago

gregtatum commented 10 years ago

I'm taking a crack at improving some of the loaders, making them a little more consistent in their usage, and then writing out the docs. I'll post anything I need feedback on here.

First thing:

THREE.Loader has an addStatusElement method, which creates a DOM element that says "Loading ...". It doesn't add it to the DOM, update it, or do anything with it. I was thinking about removing that method as it doesn't appear to do anything, and is not used in the examples.

It seems that if there is any DOM loading interface it should go in THREE.LoadingManager. In fact maybe a visual interface should be a separate class that is only in the examples.

So my proposal is to remove that functionality, and if it gets put back in, put it into the examples. Thoughts?

gero3 commented 10 years ago

It gets used in the examples. https://github.com/mrdoob/three.js/search?q=statusDomElement This is the domElement that gets added when in the constructor of Loader, showstatus has been set.

Altough I agree it needs some improvements. It would be best if the manager had a simple default loading DOM. That is easily overwritten in the examples.

mrdoob commented 10 years ago

So my proposal is to remove that functionality, and if it gets put back in, put it into the examples. Thoughts?

Sounds good to me :+1:

gregtatum commented 10 years ago

How crazy would it be to look at using promises for loaders and the loading manager? I have a bunch of code going down this path and I'm wondering if it's worth pursuing. I'm not sure about dependencies and getting something that could be a self-contained promise implementation for use inside the three.js codebase. Promises seem to make things work out nice at the end of it. The end user would then have an onLoad callback, and a promise that could be used to interact with.

mrdoob commented 10 years ago

I've yet to understand promises. Do you know of a page doing a good explanation. At the moment, I don't understand the difference between callbacks and promises.

gregtatum commented 10 years ago

http://www.html5rocks.com/en/tutorials/es6/promises/ http://promises-aplus.github.io/promises-spec/ http://blog.parse.com/2013/01/29/whats-so-great-about-javascript-promises/

Essentially you can pass promises around, and you don't really have to care specifically when they are resolved. So you can load external shaders, images, models, etc, then use a promise to determine when to start the execution of your game logic or whatever else, only when the promised results have been resolved. It also helps get rid of callback hell and makes code easier to read since callbacks don't have to be crazily nested. It also makes error handling happen in a much more logical order.

A bunch of examples using Q: https://github.com/kriskowal/q/wiki/Examples-Gallery Here's a simple chaining example using Q: https://gist.github.com/jeffcogswell/8257755

kumavis commented 10 years ago

Promises vs Callbacks is a pretty opinionated topic.

Personally I feel like callbacks are the simplest implementation, and if you want the Loader to provide a promise, you can convert the operation yourself using something like q-wrap

This is how q-wrap does its magic by the way.

gregtatum commented 10 years ago

Yeah, I think you're right about it being too opinionated a topic. I wrote some code going down this path, and left it for awhile. I'll just use the q-wrap in my own stuff. That looks like a handy tool. It seems like it would be trivial to implement.

repsac commented 10 years ago

TIL: Promises.

Thanks for sharing at least. I see the appeal but because of the work I do I have to think about backwards compatibility and that companies I deal with, or their clients, may not have the most updated browsers (in fact they are normally at least a year behind) so I would be concerned about depending on a native implementation of Promises being present.

SET001 commented 9 years ago
benaadams commented 9 years ago

Isn't too hard to switch to promises in the calling code, using the existing callback system.

atm we use https://github.com/taylorhakes/promise-polyfill as a promise polyfill as its tiny

Wrap our texture loads in promises, with caching and cpu-side dumping post gpu load something like this:


var texturePromises = {};

THREE.ImageUtils.crossOrigin = "anonymous";

function SetTextureAsync( path, wrapping, uniform ) {
    var promise;
    var hashPath = path + '|' + wrapping;
    if ( texturePromises[hashPath] ) {
        promise = texturePromises[hashPath];
    } else {
        promise = LoadTextureAsync( path );
        texturePromises[hashPath] = promise;
    }
    return promise.then( updateLoadStatus )
            .then( function ( texture ) {
                texture.wrapS = texture.wrapT = wrapping;
                uniform.value = texture;
            } );
}

function SetCompressedTextureAsync( path, wrapping, uniform ) {
    var promise;
    var hashPath = path + '|' + wrapping;
    if ( texturePromises[hashPath] ) {
        promise = texturePromises[hashPath];
    } else {
        promise = LoadCompressedTextureAsync( path );
        texturePromises[hashPath] = promise;
    }
    return promise.then( updateLoadStatus )
            .then( function ( texture ) {
            texture.wrapS = texture.wrapT = wrapping;
            uniform.value = texture;
    } );
}

var LoadTextureAsync = ( function () {
    function ReleaseTexture( texture ) {
        texture.image = null;
        if ( texture.mipmaps ) {
            texture.mipmaps = null;
        }
        texture.onUpdate = undefined;
    }
    return function LoadTextureAsync( texturePath ) {
        return new Promise( function ( resolve, reject ) {
            var texture = THREE.ImageUtils.loadTexture( pathTextures + texturePath, undefined, resolve, reject );
            console.log( "* Loading " + texturePath );
            texture.onUpdate = ReleaseTexture;
        } );
    }
} )();

var LoadCompressedTextureAsync = ( function () {
    var ddsLoader = new THREE.DDSLoader();
    function ReleaseTexture( texture ) {
        if ( texture.mipmaps ) {
            texture.image = null;
            texture.mipmaps = null;
        }
        texture.onUpdate = undefined;
    }
    return function LoadCompressedTextureAsync( texturePath ) {
        return new Promise( function ( resolve, reject ) {
            var texture = ddsLoader.load( pathTextures + texturePath + '.dds' + ext, resolve, undefined, reject );
            console.log( "* Loading " + texturePath + '.dds' + ext );
            texture.onUpdate = ReleaseTexture;
        } );
    }
} )();

Have an item that holds all the start up promises:

var startupResources = [];

Create a bunch of texture uniforms and load the promises into the startupResources

var textures = {};

function downloadTextures() {
    textures["tileTexture"] = { type: "t", value: null };
    startupResources.push( SetCompressedTextureAsync( "terrain_r", 
        THREE.ClampToEdgeWrapping, textures["tileTexture"] ) );
    ...
}

Have an animate function that switches to render when the loadStage is Running

render has its own call to requestAnimationFrame so essentially this function is dumped when everything is loaded.

function animate() {
    if ( loadStage === loadStages.Running ) {
        render();
    } else if ( loadStage === loadStages.Loading ) {
        tickLoader();
        requestAnimationFrame( animate );
    }
}

In the init it kicks off all the downloads which add them to startupResources as per downloadTextures above.

We then use Promise.all( startupResources ) to wait until they all complete before switching loadStage = loadStages.Running which triggers the animate function to move to rendering rather than an animated loading screen

World.init = function init() {

    if ( setupRenderer() !== true ) return false;

    downloadExtraData();
    downloadShaders();
    downloadTextures();

    animate();

    startWorker();

    Promise.all( startupResources )
        .then( function () {
            needsResize = true;
            startupResources = texturePromises = undefined;
            document.getElementById( "loadingText" ).style.display = "none";
            document.getElementById( "progressBar" ).style.display = "none";
            document.getElementById( "logoLoading" ).classList.add( "disappear" );
            document.getElementById("topbar").classList.remove( "off" );
            setTimeout( function () {
                document.getElementById( "logoLoading" ).style.display = "none";
            }, 3000 );
            console.info( "All loaded" );

            SetUpModels();
            SetupUIEvents();

            loadStage = loadStages.Running;
        } )
        .catch( function ( e ) {
            console.error( "Error", e );
            loadStage = loadStages.Errored;
        } );

    return true;
}
lukehorvat commented 7 years ago

If promises are too opinionated, perhaps loaders could at least use the common Node.js callback style i.e. function(error, value) { }? Then three.js wouldn't need to use promises, but folks could easily promisify loaders if they wanted to.

Callback example:

var loader = new THREE.ImageLoader();

loader.load('texture.png', function(error, image) {
  if (error) {
    // Do something with the error.
  } else {
    // Do something with the image.
  }
});

Promisified example:

var util = require('util');
var loader = new THREE.ImageLoader();

util.promisify(loader.load)('texture.png')
.then(function(image) {
  // Do something with the image.
})
.catch(function(error) {
  // Do something with the error.
});
mrdoob commented 7 years ago

If promises are too opinionated, perhaps loaders could at least use the common Node.js callback style i.e. function(error, value) { }?

Is there a spec for that?

lukehorvat commented 7 years ago

The closest I've found to a spec is this. And it is explained in the Node.js documentation here.

It's just a Node.js convention, and pretty much every library that provides promisification (including ones that aren't tied to Node.js and work in the browser) follows this convention.

You can read more about it here.

looeee commented 7 years ago

I've been promisifying the current loaders for quite a while now without issues. I don't think that they need to be refactored for this.

satori99 commented 7 years ago

Hi all, One thing that springs to my mind regarding Promises, is that they allow for ES6 async / await style coding, which can help to simplify highly asynchronous code in many cases.

donmccurdy commented 6 years ago

THREE.Loader no longer has any DOM/status management code, so I think this issue can be closed? If we want to do anything different with promises vs callbacks that could be opened in a new issue.

Mugen87 commented 6 years ago

If we want to do anything different with promises vs callbacks that could be opened in a new issue.

Yeah, sounds good. Let's close this one.