mrdoob / three.js

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

ImageBitmapLoader Cache key should take account of ImageBitmapOptions #16196

Open takahirox opened 5 years ago

takahirox commented 5 years ago
Description of the problem

Background

ImageBitmap is created with createImageBitmap(). createImageBitmap() can take options imageOrientation(flipY), premultiplyAlpha, and so on. The options have effect to bitmap.

https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/createImageBitmap

So ImageBitmaps created with different options parameters should be distinguished from each other even if they refer to the same url.

Problem

But Cache system in ImageBitmapLoader uses only url as cache key.

https://github.com/mrdoob/three.js/blob/dev/src/loaders/ImageBitmapLoader.js#L50 https://github.com/mrdoob/three.js/blob/dev/src/loaders/ImageBitmapLoader.js#L87

In the example below, bitmap should be distinguished from bitmap2 but identical if Cache is enabled.

var loader = new THREE.ImageBitmapLoader();

loader.setOptions( { imageOrientation: 'none' } );
loader.load( url, function ( bitmap ) {

    loader.setOptions( { imageOrientation: 'flipY' } );
    loader.load( url, function ( bitmap2 ) {

        console.log( bitmap === bitmap2 ); // Should be false

    } );

} );
Three.js version
Browser
OS
Hardware Requirements (graphics card, VR Device, ...)
Mugen87 commented 5 years ago

So ImageBitmap created with different options parameters should be distinguished from others even if they refer to the same url.

TBH, this use case sounds a bit artificial to me. Why would you load the same texture twice with different image bitmap settings? Properties like premultiplyAlpha or colorSpaceConversion highly depend on the used image. It's seems unlikely to extract the same image data in different ways...

takahirox commented 5 years ago

Yeah, may be rare case. But it's a bug. Probably accepting it as limitation and documenting may sound reasonable.