Three.js: clampToMaxSize function in WebGLTextures may need type check

[WJsjtu]

Description of the problem

In uploadTexture function for WebGLTextures
https://github.com/mrdoob/three.js/blob/23be4d6b8484e2912c3ac760720d1ccc6af2f693/src/renderers/webgl/WebGLTextures.js#L429-L435
clampToMaxSize is called for any texture on texture.image.
I've figure out the texture type could only be one of CanvasTexture , VideoTexture and DataTexture for uploadTexture.
So the logic problem comes here, clampToMaxSize function given an DataTexture.image ( an object with width, height, data properties ) will cause error on drawImage if the size is larger than capabilities.maxTextureSize.
https://github.com/mrdoob/three.js/blob/23be4d6b8484e2912c3ac760720d1ccc6af2f693/src/renderers/webgl/WebGLTextures.js#L18-L33

The example for SkinnedMesh in doc will go to clampToMaxSize with DataTexture.image as image argument but its size is not larger than capabilities.maxTextureSize which makes it not trigger the error.
Maybe it is not practical to set extreme large number of bones, but I think logically this should be fixed.

makePowerOfTwo function do this well.

Three.js version

• [x] Dev
• [x] r90

Any thoughts?

[Mugen87]

You mean this error, right?

https://jsfiddle.net/f2Lommf5/2175/

[WJsjtu]

Yes, you are right!
I guess such code as (image instanceof HTMLImageElement || image instanceof HTMLCanvasElement || image instanceof ImageBitmap) is needed.

[Mugen87]

I guess this would fix the issue. We only use canvas for specific image types.

function clampToMaxSize( image, maxSize ) {

    if ( image.width > maxSize || image.height > maxSize ) {

        var scale = maxSize / Math.max( image.width, image.height );

        if ( image instanceof HTMLImageElement || image instanceof HTMLCanvasElement || image instanceof ImageBitmap ) {

            // Warning: Scaling through the canvas will only work with images that use
            // premultiplied alpha.

            var canvas = document.createElementNS( 'http://www.w3.org/1999/xhtml', 'canvas' );
            canvas.width = Math.max( Math.floor( image.width * scale ), 1 );
            canvas.height = Math.max( Math.floor( image.height * scale ), 1 );

            var context = canvas.getContext( '2d' );
            context.drawImage( image, 0, 0, image.width, image.height, 0, 0, canvas.width, canvas.height );

            console.warn( 'THREE.WebGLRenderer: image is too big (' + image.width + 'x' + image.height + '). Resized to ' + canvas.width + 'x' + canvas.height, image );

            return canvas;

        } else {

            var width = Math.max( Math.floor( image.width * scale ), 1 );
            var height = Math.max( Math.floor( image.height * scale ), 1 );

            console.warn( 'THREE.WebGLRenderer: image is too big (' + image.width + 'x' + image.height + '). Resized to ' + width + 'x' + height, image );

            image.width = width;
            image.height = height;

        }

    }

    return image;

}

[WJsjtu]

Does makePowerOfTwo also need to care about DataTexture.image if textureNeedsPowerOfTwo(image) === true ( Is this possible ? ) and isPowerOfTwo(image) === false?
https://github.com/mrdoob/three.js/blob/23be4d6b8484e2912c3ac760720d1ccc6af2f693/src/renderers/webgl/WebGLTextures.js#L48-L68
Currently it only deals with HTMLImageElement, HTMLCanvasElement and ImageBitmap.

[Mugen87]

Maybe. @WestLangley @mrdoob What do you think about this?

[mrdoob]

So the problem is that clampToMaxSize() and makePowerOfTwo() breaks when passing a DataTexture.image?

[WJsjtu]

clampToMaxSize() breaks and makePowerOfTwo() does nothing to DataTexture.image.

[WestLangley]

POT is only required for mipmapping. DataTextures are typically neither mipmapped nor resized -- right?. So I do not understand what the issue is...

[WJsjtu]

@WestLangley Well , thank you for pointing out that the DataTextures do not need power-of-two process. So you can take care of the first three comments of this issue and ignore others.

The issue is about clampToMaxSize function which is called on any texture's image including DataTextures.image without the type check of the image.
When a DataTextures.image with size larger than capabilities.maxTextureSize is passed to clampToMaxSize. clampToMaxSize will call Canvas.drawImage on an JS object ( DataTextures.image ). And this lets the console reporting the error:

Uncaught TypeError: Failed to execute 'drawImage' on 'CanvasRenderingContext2D': The provided value is not of type '(CSSImageValue or HTMLImageElement or SVGImageElement or HTMLVideoElement or HTMLCanvasElement or ImageBitmap or OffscreenCanvas)'

[WestLangley]

I do not think DataTextures should ever be resized.

[WJsjtu]

@WestLangley Even the DataTextures are larger than capabilities.maxTextureSize?
This will cause warning in console.

WebGL: INVALID_VALUE: texImage2D: width or height out of range

[WestLangley]

Resizing a DataTexture would destroy the data. If the DataTexture is too big, then that is a user error.

[Mugen87]

It's not just a warning, clampToMaxSize() throws a runtime error: https://jsfiddle.net/f2Lommf5/2175/

My suggestion was to add at least a more robust version of clampToMaxSize() that avoids the runtime error and reports a warning instead (_image is too big..._).

[Mugen87]

Closing. If more users complain, we can still make clampToMaxSize() more robust. For now let's keep the code as it is.

[mrdoob]

My suggestion was to add at least a more robust version of clampToMaxSize() that avoids the runtime error and reports a warning instead (_image is too big..._).

That sounds good to me 👍

[Mugen87]

Okay, i'll make a PR then 😊