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

Editor - broken TGA support in Map properties of a material #26312

Closed
GitHubDragonFly opened this issue Jun 21, 2023 · 3 comments · Fixed by #26316
Closed

Editor - broken TGA support in Map properties of a material #26312

GitHubDragonFly opened this issue Jun 21, 2023 · 3 comments · Fixed by #26316

Comments

@GitHubDragonFly
Copy link
Contributor

Description

TGA textures are not handled properly in Map properties of a material.
This is the error in Firefox browser on Windows computer:

THREE.WebGLState: TypeError: WebGL2RenderingContext.texSubImage2D: Argument 7 is not valid for any of the 7-argument overloads.
texSubImage2D https://threejs.org/build/three.module.js:23147
uploadTexture https://threejs.org/build/three.module.js:24540
setTexture2D https://threejs.org/build/three.module.js:23917
setValueT1 https://threejs.org/build/three.module.js:18444
upload https://threejs.org/build/three.module.js:18999
setProgram https://threejs.org/build/three.module.js:29794
renderBufferDirect https://threejs.org/build/three.module.js:28645
renderObject https://threejs.org/build/three.module.js:29344
renderObjects https://threejs.org/build/three.module.js:29313
renderScene https://threejs.org/build/three.module.js:29182
render https://threejs.org/build/three.module.js:28987
renderToCanvas https://threejs.org/editor/js/libs/ui.three.js:944
setValue https://threejs.org/editor/js/libs/ui.three.js:160
loadFile https://threejs.org/editor/js/libs/ui.three.js:84

The 1st code below is the current code in the ui.three.js file (starting at line 75) that is passing improper texture.
The 2nd code is what works for me but can probably be changed.

Reproduction steps

  1. Go to https://threejs.org/editor/
  2. Load any model and try to apply a TGA texture to one of its Map properties

Code

			} else if ( extension === 'tga' ) {

				reader.addEventListener( 'load', function ( event ) {

					const canvas = new TGALoader().parse( event.target.result );

					const texture = new THREE.CanvasTexture( canvas, mapping );
					texture.sourceFile = file.name;

					scope.setValue( texture );

					if ( scope.onChangeCallback ) scope.onChangeCallback( texture );

				}, false );

				reader.readAsArrayBuffer( file );
			} else if ( extension === 'tga' ) {

				reader.addEventListener( 'load', function ( event ) {

					const tga_texture = new TGALoader().parse( event.target.result );

					let canvas = document.createElement( 'canvas' );
					let ctx = canvas.getContext( '2d' );
					canvas.width = tga_texture.width;
					canvas.height = tga_texture.height;

					let imgData = new ImageData( new Uint8ClampedArray( tga_texture.data ), tga_texture.width, tga_texture.height );

					ctx.putImageData( imgData, 0, 0 );

					const texture = new THREE.CanvasTexture( canvas, mapping );
					texture.sourceFile = file.name;

					scope.setValue( texture );

					if ( scope.onChangeCallback ) scope.onChangeCallback( texture );

				}, false );

				reader.readAsArrayBuffer( file );

Live example

Screenshots

No response

Version

r153

Device

Desktop

Browser

Firefox

OS

Windows

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 22, 2023

Good catch! Your suggested fix works however it should not be necessary to use CanvasTexture. TGALoader is normally returning an instance of DataTexture so maybe:

const texData = new TGALoader().parse( event.target.result );

const texture = new THREE.DataTexture( texData.data, texData.width, texData.height, THREE.RGBAFormat, THREE.UnsignedByteType, mapping );
texture.needsUpdate = true;

texture.generateMipmaps = true;
texture.flipY = true;
texture.magFilter = THREE.LinearFilter;
texture.minFilter = THREE.LinearMipmapLinearFilter;
texture.colorSpace = THREE.SRGBColorSpace;

texture.sourceFile = file.name;

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 22, 2023

Actually, there is a simpler approach. I'll file a PR.

@GitHubDragonFly
Copy link
Contributor Author

Cool.

You might consider taking a look at another fix I suggested in #19781

It works on my end as it is but could possibly be improved or simplified.

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

Successfully merging a pull request may close this issue.

2 participants