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

WebGLTextures: Use gl.texStorage2D() with compressed textures. #22928

Merged
merged 1 commit into from
Dec 2, 2021

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Dec 1, 2021

Related issue: see #22790 (comment) and #21874

Description

Introduces the usage of gl.texStorage2D() with compressed textures.

@Mugen87 Mugen87 added this to the r136 milestone Dec 1, 2021
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 2, 2021

Merging for further testing.

@Mugen87 Mugen87 merged commit ee14bef into mrdoob:dev Dec 2, 2021
@mrdoob
Copy link
Owner

mrdoob commented Dec 2, 2021

Thanks!

@donmccurdy
Copy link
Collaborator

donmccurdy commented Dec 7, 2021

@Mugen87 I'm seeing a problem introduced by this PR when testing our webgl / loader / texture / ktx2 example in Safari:

Screen Shot 2021-12-06 at 8 18 50 PM

In Safari we select ETC1 as the transcode target. In Chrome and Firefox ETC1 is not supported on my device so we select DXT instead, and the problem doesn't occur. None of these browsers are exposing as many compressed formats as they should, and hopefully that will be improved in upcoming browser updates (see issues below). But the browser support is probably a moot point for now – we currently support devices that can only handle ETC1, and dropping that support is not something we should do by accident.

glCompressedTexSubimage2d supports ETC2 textures but not ETC1. Only WEBGL_compressed_texture_etc1 is available but it needs WEBGL_compressed_texture_etc.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 7, 2021

In Safari we select ETC1 as the transcode target.

Does this selection depend on the type of the WebGL rendering context? Would it be possible to select a different texture compression format when Safari supports WebGL 2?

So when using gl.texStorage2D() ETC1 is not supported and thus loaders should not select it as format. Maybe we could establish a policy that requires the usage WebGL1Renderer if users rely on ETC1?

@donmccurdy
Copy link
Collaborator

It's based on the available WebGL extensions. At least on my device those are consistent between WebGL 1 and WebGL 2:

//  Safari (macOS 12, M1 Pro)
EXT_texture_compression_rgtc
WEBGL_compressed_texture_etc1
WEBGL_compressed_texture_s3tc
WEBGL_compressed_texture_s3tc_srgb

//  Chrome, Firefox (macOS 12, M1 Pro)
EXT_texture_compression_rgtc
WEBGL_compressed_texture_s3tc
WEBGL_compressed_texture_s3tc_srgb

I haven't tested this myself, but once ANGLE/Metal is enabled the device should be able to support the following in any of the browsers above:

EXT_texture_compression_bptc
EXT_texture_compression_rgtc
WEBGL_compressed_texture_astc
WEBGL_compressed_texture_etc
WEBGL_compressed_texture_etc1
WEBGL_compressed_texture_pvrtc
WEBGL_compressed_texture_s3tc
WEBGL_compressed_texture_s3tc_srgb

So when using gl.texStorage2D() ETC1 is not supported and thus loaders should not select it as format.

We can do that. If KTX2Loader avoids ETC1 whenever renderer.isWebGL2 === true is that enough?

Maybe we could establish a policy that requires the usage WebGL1Renderer if users rely on ETC1?

Probably not necessary, I'm not sure what kind of device would (1) support WebGL 2, and (2) not support more compressed formats than ETC1. See platform support table. Even then, I guess a user could choose to keep using WebGL 2 and transcode the texture into uncompressed RGBA data.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 8, 2021

If KTX2Loader avoids ETC1 whenever renderer.isWebGL2 === true is that enough?

I would say yes. Since we rely on texStorage2D() now, it's necessary to completely avoid ETC1 with WebGL 2.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 8, 2021

I guess instead of:

etc1Supported: renderer.extensions.has( 'WEBGL_compressed_texture_etc1' ),

it would be:

etc1Supported: renderer.capabilities.isWebGL2 === false && renderer.extensions.has( 'WEBGL_compressed_texture_etc1' ), 

@donmccurdy
Copy link
Collaborator

Sounds good! Opened #22982.

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.

3 participants