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

KTX2Loader: Fix .minFilter default for untranscoded compressed textures #29904

Merged

Conversation

donmccurdy
Copy link
Collaborator

@donmccurdy donmccurdy commented Nov 15, 2024

KTX2Loader implements three 'paths' for textures:

  1. Uncompressed DataTexture (F16, F32, Uint8, ...)
  2. Raw CompressedTexture (ETC1/2, ASTC, BCn, ...)
  3. Universal/transcoded CompressedTexture (BasisLZ/ETC1S, UASTC)

Basis UASTC HDR textures may take path (2) or (3) depending on device support for ASTC HDR formats, since the pre-transcode data is also valid ASTC data. In case (2), we weren't correctly setting .minFilter defaults for textures without mipmaps.

@Mugen87 Mugen87 added this to the r171 milestone Nov 15, 2024
@donmccurdy donmccurdy merged commit 5b51d6f into mrdoob:dev Nov 15, 2024
11 checks passed
@donmccurdy donmccurdy deleted the fix/ktx2loader-compressed-texture-mipmaps branch November 15, 2024 23:18
@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 16, 2024

Uncompressed DataTexture (F16, F32, Uint8, ...)

WebGLRenderer makes sure the use case of #29884 works by having two separate code paths (compressed vs uncompressed) for uploading data of CompressedTexturess, see:

if ( texture.format !== RGBAFormat ) {
if ( glFormat !== null ) {
if ( useTexStorage ) {
if ( dataReady ) {
state.compressedTexSubImage2D( _gl.TEXTURE_2D, i, 0, 0, mipmap.width, mipmap.height, glFormat, mipmap.data );
}
} else {
state.compressedTexImage2D( _gl.TEXTURE_2D, i, glInternalFormat, mipmap.width, mipmap.height, 0, mipmap.data );
}
} else {
console.warn( 'THREE.WebGLRenderer: Attempt to load unsupported compressed texture format in .uploadTexture()' );
}
} else {
if ( useTexStorage ) {
if ( dataReady ) {
state.texSubImage2D( _gl.TEXTURE_2D, i, 0, 0, mipmap.width, mipmap.height, glFormat, glType, mipmap.data );
}
} else {
state.texImage2D( _gl.TEXTURE_2D, i, glInternalFormat, mipmap.width, mipmap.height, 0, glFormat, glType, mipmap.data );
}
}

TBH, I was never happy with the exception handling of RGBAFormat since I think it's more clear when instances of CompressedTextures actually hold compressed data (and thus use a compressed format).

I wish we could avoid CompressedTexture with RGBAFormat since that would allow us a more clean implementation on the renderer side. In WebGPURenderer there is currently no separate RGBAFormat handling so the use case of #29884 does not work yet (neither in the WebGPU and WebGL backend, see https://jsfiddle.net/20j6an3x/). We could fix this on the renderer side but imo it would be more clear if compressed texture loaders return uncompressed data as instances of DataTexture.

@donmccurdy
Copy link
Collaborator Author

I think we could change KTX2Loader to return THREE.DataTexture instead of THREE.CompressedTexture conditionally, depending on the selected transcoding format. The only obstacle I see is that we'd need a new THREE.DataCubeTexture class, to handle all three cases here:

if ( container.faceCount === 6 ) {
texture = new CompressedCubeTexture( faces, format, type );
} else {
const mipmaps = faces[ 0 ].mipmaps;
texture = container.layerCount > 1
? new CompressedArrayTexture( mipmaps, width, height, container.layerCount, format, type )
: new CompressedTexture( mipmaps, width, height, format, type );
}

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

Successfully merging this pull request may close these issues.

Exr conversion to ktx2 added to background Mac system render exception
2 participants