mrdoob / three.js

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

Overriding default Image Loading during Geometry Import. #3412

Closed bhouston closed 9 years ago

bhouston commented 11 years ago

I have changed the requirements for this change from the original text (see "OLD TEXT" below) to just requiring the ability to intercept image loading calls during a geometry import. I've described the more specific and limited requirements in this comment:

https://github.com/mrdoob/three.js/issues/3412#issuecomment-17278034

OLD TEXT: So right now many of the importers create textures and fetch their images immediately during loading via ImageUtils.load*Texture().

I would like to add the option to not immediately fetch the image data for the textures. Instead I would like to trigger these after the geometry has finished loading.

I am not sure the best way to do this but it could be as simple as adding an option on ImageUtils.loadTexture() where it can just create a texture.loader = new THREE.ImageLoader() and configures it properly but never calls texture.loader.load( texture.sourceUrl, texture.image ); One can then iterate over the textures post-load and find those that have texture.loader defined and then one can call texture.loader.load(...) and upon success one can set texture.loader = undefined or something like that.

I would like to add this as an option all the importers that can load textures during loading and then in the functions on ImageUtils.load*Texture().

I think it may be useful to modify THREE.ImageLoader so that it knows when it is in the process of loading and whether it failed during loading and the reason why.

I can also add a utility function that traverses the scene graph as I described above looking for texture.loader to load that are not already in the process of loading.

gaitat commented 11 years ago

I think that this can be implemented much faster and less messy with the jquery deferred objects. But that would create an external dependency and probably that is why jquery has been left out of three.js.

crobi commented 11 years ago

Out of curiosity, why is it better to start loading textures after the geometry has finished loading?

bhouston commented 11 years ago

There are a few different reasons but one is that it allows us to analyse the textures prior to loading. Such as allowing us to potentially consolidate textures between different models, decide to load lower resolution textures than are specified, and possibly not load the textures at all if they are not formats that are supported by the web (e.g. tiff.)

We are trying to increase the general robustness of the loaders and right now there can be a lot of issues with textures, thus necessitating further post-processing.

crobi commented 11 years ago

So the main reason is to have a centralized image loading code that supports the features you listed? But you don't have to wait for the geometry to be loaded for that, do you? It'll take longer for the textures to appear if you wait.

Or is there some feature that requires to know the list of all textures before loading them? Some applications might want to load models / add them to the scene dynamically, so you never know when you are done anyway.

bhouston commented 11 years ago

@crobi It could also be done by passing in a custom ImageFactory class. That ImageFactory class could have the same interface as that provided by THREE.ImageUtils.

For example:


THREE.ImageFactory.prototype = {

  loadTexture: function( url, mapping, onLoad, onError ) {},

  loadCompressedTexture: function( url, mapping, onLoad, onError ) {},

  loadTextureCube: function( url, mapping, onLoad, onError ) {}

};

We can make the loaders take an imageFactory as a parameter and if it isn't set we do imageFactory = THREE.SimpleImageFactory(), which is just a wrapper around THREE.ImageUtils.

This would allow for people like me to extend the image handling in the various loaders without having to maintain separate forks of the loaders (which we are currently doing.) Maintaining separate forks means that it is really hard for us to contribute back improvements to the mainline Three.js project.

I think that having flexibility here is a good thing.

bhouston commented 11 years ago

I can actually implement the above proposal pretty easily, the changes to the code base are actually fairly minor and fully backwards compatible.

WestLangley commented 11 years ago

But you don't have to wait for the geometry to be loaded for that, do you? It'll take longer for the textures to appear if you wait.

What is the answer to @crobi 's question?

crobi commented 11 years ago

There's one reason for delaying the texture loading I can think of:

If you want to consolidate textures, you have to make sure the textures not only share the URL, but also all texture parameters (filtering, wrapping, ...). So either you add all of THREE.Texture properties (there's 8 of them in the constructor) as arguments to the proposed THREE.ImageFactory.loadTexture and hope the loader won't modify the properties after using the factory. Or you wait until the loader is finished (until the next draw call) and analyze all requested textures. Personally, I'd prefer the first approach.

Of course, either approach will have problems if the user modifies the texture properties after the loader is done - he might not realize that the texture object is shared across different models.

bhouston commented 11 years ago

@WestLangley I've changed the title of this bug from the original title that asked for "delayed loading" to just having the ability to intercept image loading calls during geometry import. I've also described in this comment a simple means of achieving this:

https://github.com/mrdoob/three.js/issues/3412#issuecomment-17278034

Basically we are using a custom image management system (resource pooling if you will) and it is not appropriate for our use case to load all the textures from their default URLs during a geometry import. But giving us the ability to intercept these calls allows us to change how they are handled without having to fork the importers.

bhouston commented 11 years ago

@WestLangley @crobi I'll implement the intercept approach and submit it as a PR. I think you'll be please as how minimal the changes our and the flexibility it provides. I'll do that tomorrow.