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

WebGLRenderer: Use ETC2 for ETC1 textures. #28021

Merged
merged 2 commits into from
Mar 31, 2024
Merged

WebGLRenderer: Use ETC2 for ETC1 textures. #28021

merged 2 commits into from
Mar 31, 2024

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Mar 28, 2024

Related issue: #27162 (comment)

Description

The idea of this PR is to stop use WEBGL_compressed_texture_etc1 and always rely on WEBGL_compressed_texture_etc by treating ETC1 as ETC2 textures. According to https://www.khronos.org/assets/uploads/developers/library/2012-siggraph-opengl-es-bof/Ericsson-ETC2-SIGGRAPH_Aug12.pdf, slide 10, that should work:

ETC2 is backward compatible with ETC1: If you have an old ETC1 texture, you can load it as an ETC2 texture and the hardware will decode it correctly.

The motivation behind this change is to remove the dependency of RGB_ETC1_Format to gl. compressedTexImage2D() so we can eventually use gl.texStorage*() for ETC1 textures.

This change also includes the WebGL backend of WebGPURenderer.

Copy link

github-actions bot commented Mar 28, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
671.7 kB (166.7 kB) 671.6 kB (166.7 kB) -92 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
451.1 kB (109 kB) 451 kB (109 kB) -92 B

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 28, 2024

@donmccurdy I'm afraid I'm not sure how to test this change since on my devices the KTX examples do not output ETC1. Do you have an idea how we can verify if this PR actually works?

@@ -703,7 +703,7 @@ function WebGLTextures( _gl, extensions, state, properties, capabilities, utils,
let mipmap;
const mipmaps = texture.mipmaps;

const useTexStorage = ( texture.isVideoTexture !== true && glInternalFormat !== RGB_ETC1_Format );
Copy link
Collaborator Author

@Mugen87 Mugen87 Mar 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify: This is the bit (the RGB_ETC1_Format check) I want to get rid of^^.

@Mugen87 Mugen87 added this to the r164 milestone Mar 28, 2024
@donmccurdy
Copy link
Collaborator

@Mugen87 the override in KTX2Loader can be removed with this PR:

// https://github.com/mrdoob/three.js/pull/22928
this.workerConfig.etc1Supported = false;

If your devices are still not producing ETC1 after that, I think I could test this on my devices this weekend.

Thank you!

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 28, 2024

Thanks! After removing the override I still get RGBA_BPTC but I can force now ETC1 by setting all flags like bptcSupported in detectSupport() to false except for etc1Supported.

The PR seems to work but it would be good if you could test this change, too!

@donmccurdy
Copy link
Collaborator

Confirmed – ETC1 is working in WebGL 2 on my iPhone 11. I believe if you use a KTX2 file encoded with ETC1S / "Basis LZ" (rather than UASTC) then the first transcoding choices, if available, will be ETC2 or ETC1.

@Mugen87 Mugen87 marked this pull request as ready for review March 31, 2024 09:46
@Mugen87 Mugen87 merged commit 6d188b5 into mrdoob:dev Mar 31, 2024
12 checks passed
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.

None yet

2 participants