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

Creating a CubeTexture from 6 CompressedTextures doesn't work after r136 #24065

Closed
umar-ahmed opened this issue May 15, 2022 · 11 comments
Closed

Comments

@umar-ahmed
Copy link

umar-ahmed commented May 15, 2022

Describe the bug

Prior to version r136 of the library, creating a CubeTexture from 6 CompressedTextures used to work just fine. However, it seems like this change introduced in r136 changed the behaviour of how textures are loaded into the GPU when WebGL 2 APIs are available, which broke this use-case.

As a work-around, I have been able to get the cube texture to render by setting texture.isVideoTexture = true on the CubeTexture. According to this line, that forces Three to opt-out of using texStorage2D which seems to be causing the issue

Code

const loader = new KTX2Loader()
    .setTranscoderPath( 'js/libs/basis/' )
    .detectSupport( renderer );

const textures = await Promise.all([
  loader.loadAsync( './textures/nx.ktx2' ),
  loader.loadAsync( './textures/px.ktx2' ),
  loader.loadAsync( './textures/ny.ktx2' ),
  loader.loadAsync( './textures/py.ktx2' ),
  loader.loadAsync( './textures/nz.ktx2' ),
  loader.loadAsync( './textures/pz.ktx2' )
]);

textures.forEach((texture) => {
  texture.generateMipmaps = false;
  texture.encoding = THREE.LinearEncoding;
  texture.needsUpdate = true;
});

const cubeTexture = new THREE.CubeTexture(textures);
cubeTexture.encoding = textures[0].encoding;
cubeTexture.format = textures[0].format;
cubeTexture.minFilter = textures[0].minFilter;
cubeTexture.magFilter = textures[0].magFilter;
cubeTexture.generateMipmaps = false;

// HACK: Setting `isVideoTexture` opts-out of using `texStorage2D`
cubeTexture.isVideoTexture = true;

cubeTexture.needsUpdate = true;

// Set scene background
scene.background = cubeTexture

Live example

  • CodeSandbox (I'm using react-three-fiber, let me know if you want a vanilla Three.js version!)

Expected behavior

It should load the CubeTexture just fine as the scene background. Instead, the scene background is black and the following errors show in the console:

[.WebGL-0x700abe0a00] GL_INVALID_VALUE: Texture dimensions must all be greater than zero.

and

[.WebGL-0x700abe0a00] GL_INVALID_OPERATION: Level of detail outside of range.

Upgrading to r140.2, also shows this added error:

THREE.WebGLTextures: sRGB encoded textures have to use RGBAFormat and UnsignedByteType.

Screenshots

CleanShot 2022-05-15 at 10 56 44@2x

Screen.Recording.2022-05-15.at.11.01.09.AM.mov

Platform:

  • Device: Desktop
  • OS: MacOS
  • Browser: Chrome
  • Three.js version: r136
@umar-ahmed umar-ahmed changed the title Creating a CubeTexture from 6 CompressedTextures Creating a CubeTexture from 6 CompressedTextures doesn't work after r136 May 15, 2022
@CodyJasonBennett
Copy link
Contributor

CodyJasonBennett commented May 15, 2022

Can confirm the issue is isolated to changes made in that PR.

Among them, this should perhaps be state.texStorage2D( _gl.TEXTURE_CUBE_MAP, levels, glInternalFormat, cubeImage[0].mipmaps[ 0 ].width, cubeImage[0].mipmaps[ 0 ].height ):

if ( useTexStorage && allocateMemory ) {
state.texStorage2D( _gl.TEXTURE_CUBE_MAP, levels, glInternalFormat, image.width, image.height );
}

However, the cube texture is still unrenderable in this case (issues with mipmaps?). Setting .isVideoTexture to the cube texture will completely bypass code paths introduced with that PR as a workaround.

@Mugen87
Copy link
Collaborator

Mugen87 commented May 16, 2022

(I'm using react-three-fiber, let me know if you want a vanilla Three.js version!)

A vanilla version would be great! Ideally with jsfiddle.

Among them, this should perhaps be state.texStorage2D( _gl.TEXTURE_CUBE_MAP, levels, glInternalFormat, cubeImage[0].mipmaps[ 0 ].width, cubeImage[0].mipmaps[ 0 ].height ):

There is an example that uses a compressed cube map and it works fine. Using image.width and image.height in uploadCubeTexture() should work with all compressed textures. Otherwise the texture has not been configured correctly.

@LeviPesin
Copy link
Contributor

There is an example that uses a compressed cube map and it works fine.

It does not quite work for me.
Screenshot_20220516-190115_Samsung Internet
But I think this should be a separate issue?

@Mugen87
Copy link
Collaborator

Mugen87 commented May 16, 2022

Your platform probably does not fully support S3TC.

@umar-ahmed
Copy link
Author

umar-ahmed commented May 16, 2022

@Mugen87

A vanilla version would be great! Ideally with jsfiddle.

Cool, I'll get on that

There is an example that uses a compressed cube map and it works fine. Using image.width and image.height in uploadCubeTexture() should work with all compressed textures. Otherwise the texture has not been configured correctly.

I think maybe you misunderstood the specific workflow I'm trying to achieve. I can see in that example the compressed textures are loaded using the DDSLoader as one texture. But what I'm trying to achieve (and what used to work before r136) was manually constructing a CubeTexture from 6 CompressedTextures. In particular, I'm using the KTX2Loader.

From my understanding, Three.js's KTX2Loader doesn't support loading cubemaps as a single file, despite the ktx2 spec and tooling supporting it. So as a work-around, I've been loading 6 .ktx2 files using the KTX2Loader and then constructing the CubeTexture from those 6 images. In as late as r135 this worked with no problems (see the code snippet above). However the introduction of the code for using texStorage2D seems to have regressed that behaviour

I could just be misinterpreting what you're saying. I'm not super familiar with the WebGL APIs and what goes on under the hood with Three.js. If there's some specific requirements for texture for the uploadCubeTexture() function to work correctly, it would be much appreciated if you could tell me. I'm using the toktx CLI tool for the conversion, and there seems to be some options in there to configure the outputted texture file. Thanks!

@donmccurdy
Copy link
Collaborator

From my understanding, Three.js's KTX2Loader doesn't support loading cubemaps as a single file ...

Yeah, I guess this is something we should try to support, too. You're just using the cubemaps as backgrounds and not environment maps, correct?

THREE.WebGLTextures: sRGB encoded textures have to use RGBAFormat and UnsignedByteType.

Hm we must be using RGB_ETC2_Format and RGBA_ASTC_4x4_Format with sRGB encoding elsewhere... this warning may not be quite right?

@umar-ahmed
Copy link
Author

Yeah, I guess this is something we should try to support, too. You're just using the cubemaps as backgrounds and not environment maps, correct?

@donmccurdy That's correct. I'm just using it as a scene background.

Hm we must be using RGB_ETC2_Format and RGBA_ASTC_4x4_Format with sRGB encoding elsewhere... this warning may not be quite right?

You know what, I think that error goes away when I set texture.encoding = THREE.LinearEncoding; on each of the 6 textures. That's probably just my bad since I'm new to the recent color management system changes

@donmccurdy
Copy link
Collaborator

A background texture probably should be sRGB though -- so I suspect your original code was right and the warning is incorrect for this case.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 7, 2022

THREE.WebGLTextures: sRGB encoded textures have to use RGBAFormat and UnsignedByteType.

This warning is generate in WebGLTextures.verifyColorSpace(). However, there is an early out for all compressed textures. So when RGBA_ASTC_4x4_Format is used in combination with sRGBEncoding, there is no warning and the renderer should pick COMPRESSED_SRGB8_ALPHA8_ASTC_4x4_KHR.

I think the main problem is that compressed cube textures also have to use CompressedTexture and not CubeTexture. So using new THREE.CubeTexture(textures); while textures is an array of compressed textures does not work. It's required to organize the compress cube map similar to how DDSLoader works.

@donmccurdy
Copy link
Collaborator

Possible solution:

@umar-ahmed
Copy link
Author

Thanks @donmccurdy! That indeed does resolve my issue. I think composing the cube texture manually should be possible now. Although, it's probably best to load it as a single .ktx2 cubemap texture, which is now possible thanks to your work on the KTX2Loader to support that 🙏 .

Good to close this ticket out now.

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

No branches or pull requests

5 participants