-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
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
bugfix: ETC1 is not supported with compressedTexSubImage2D() #27162
Conversation
…g to WEBGL_compressed_texture_etc1 extension
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
Diff for I'm not immediately sure why that demo renders as it does. One limitation of this fix will be that color management and sRGB decoding for ETC1 textures isn't possible without Related: |
compressedTexSubImage2D() with ETC1 format fails on Android devices, webgl outputs an error and doesn't load a texture! See the screenshot below: Yes, I get a color managment problem with sRGB textures in ETC1 format and I have to convert them to linear space manually in a shader. However, the same issue with color management exists for PVRTC textures even without my bugfix. The previous versions of three.js had SRGBToLinear function call in a shader after fetching an srgb texture and everything was fine. In the latest version S3TC (DXT1/5) and ASTC are converted to linear space correctly when textures are loaded but ETC1 and PVRTC - no, they should be converted manually in a shader. |
It's mainly the test failure that I'm concerned about. But re-running the screenshot from the
Yes. Note that I don't expect three.js will bring inline sRGB decoding back, so you'd need to do this decoding manually in your shaders, after your PR. That isn't a blocker for this PR, in my opinion, but should perhaps be a consideration when deciding what texture formats to use. |
I was also surprised to see the change in screenshots!
Decoding manually in a shader is not a problem, I can do it. The problem is that the newer versions break functionality. When upgraded three.js I was very surprised that my content doesn't work at all on Android devices (ETC1) and have the wrong gamma on Apple (PVRTC). I have been developing my software product on three.js for past 4 years and I have a lot of legacy content that is encoded in ETC1 and PVRTC. Yes, three.js has the tests but it doesn't run them on android and ios. And, believe me, every new version breaks something on mobile devices. |
Agreed that we should merge this fix, once we know what's happening with the test failure. It may also be worth logging a warning about ETC1 textures when |
I think at some point we should stop support for ETC1. |
Once WebGPU is available on mobile, do we know if ETC1 is expected to work fully (with sRGB support) there? |
@Kangz Any ideas? |
Yes, WebGPU requires either BC or (ETC2+ASTC) support and there are the
sRGB variants of the ETC2 formats
https://gpuweb.github.io/gpuweb/#dom-gputextureformat-etc2-rgb8unorm-srgb
(note ETC2 is a superset of ETC1)
…On Tue, Nov 14, 2023, 08:54 mrdoob ***@***.***> wrote:
@Kangz <https://github.com/Kangz> Any ideas?
—
Reply to this email directly, view it on GitHub
<#27162 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADTN2XVGKPNADCJOIM27JLYEMPBTAVCNFSM6AAAAAA7ETQ5MSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBZGY4TGNZVGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
That makes me even more confident to deprecate the ETC1 formats. |
Sorry I might be misunderstanding... but if ETC2 is a superset of ETC1, doesn't that mean WebGPU will support ETC1 just fine, and it would be premature for us to deprecate ETC1 support? Or — and I hope I am not being very dense here 😰 — could ETC1 data be uploaded with |
I want to revisit this issue and give the idea of @donmccurdy a try. Meaning internally using
My major motivation behind this change is to get rid of the |
ETC1 is not supported with compressedTexSubImage2D() according to WEBGL_compressed_texture_etc1 extension
(see https://developer.mozilla.org/en-US/docs/Web/API/WEBGL_compressed_texture_etc1)