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

BasisModule.isFormatSupported() always returns true #272

Closed
donmccurdy opened this issue Jul 16, 2020 · 12 comments
Closed

BasisModule.isFormatSupported() always returns true #272

donmccurdy opened this issue Jul 16, 2020 · 12 comments

Comments

@donmccurdy
Copy link
Contributor

The function seems to return true for any input — even if it's not one of the expected enum values. For example:

this.basisModule.isFormatSupported(1234, 5678);
// → true

Perhaps a bug, or not implemented yet?

@donmccurdy
Copy link
Contributor Author

More context — I'm getting an error when trying to transcode a UASTC texture to PVRTC. This method had returned true, which I assumed meant it was supported as a target. That's fine if it's not, I just need to detect it. Decoding to RGBA32 worked fine on this device.

@MarkCallow
Copy link
Collaborator

This function tells you if the transcoder supports the format, i.e. is that format's transcoder compiled in to the module you are using. It does not give any information about what formats the platform you are running on supports.

The UASTC to PVRTC1 encoder is always compiled in so it will return true for

this.basisModule.isFormatSupported(TranscodeTarget.PVRTC1_4_RGBA, TextureFormat.UASTC4x4);

Perhaps Safari/WebKit do not support WEBGL_compressed_texture_PVRTC. You need to use WebGL extension queries to see whether a particular format is supported in the platform.

As to what happens in the JS/c++ interface when you give unknown enum values, as you did in your example, I don't know. Mappings from strings for the known values to underlying enum values are declared in embind. The underlying c++ function will return false for unknown basis_tex_formats (the 2nd argument) so I don't understand why your example returns true. If the basis_tex_format is ETC1S, it will also return false for unknown transcoder_texture_formats (the first argument.) However it will return true for an unknown first argument when the second is UASTC4x4. That is the only bug I can see.

@donmccurdy
Copy link
Contributor Author

The device does support WEBGL_compressed_texture_PVRTC, but transcoding to PVRTC fails. I think you can reproduce this by transcoding the KTX logo UASTC sample to PVRTC.

I can't find a combination of inputs to isFormatSupported that will ever return false.

@donmccurdy
Copy link
Contributor Author

Related: mrdoob/three.js#19846

@MarkCallow
Copy link
Collaborator

When you say "fails" do you mean false is returned?

@donmccurdy
Copy link
Contributor Author

Unsure if it was false or null, but yes, something like that. It doesn't return image data.

@MarkCallow
Copy link
Collaborator

MarkCallow commented Jul 16, 2020

The only combinations of inputs, using the set of enums defined in the JS interface, that should return false are

this.basisModule.isFormatSupported(TranscodeTarget.PVRTC2_4_RGB, TextureFormat.UASTC4x4);
this.basisModule.isFormatSupported(TranscodeTarget.PVRTC2_4_RGB, TextureFormat.ETC1S);
this.basisModule.isFormatSupported(TranscodeTarget.PVRTC2_4_RGBA, TextureFormat.UASTC4x4);
this.basisModule.isFormatSupported(TranscodeTarget.PVRTC2_4_RGBA, TextureFormat.ETC1S);
this.basisModule.isFormatSupported(TranscodeTarget.PVRTC2_4_RGBA, TextureFormat.UASTC4x4);
this.basisModule.isFormatSupported(TranscodeTarget.EAC_R11, TextureFormat.ETC1S);
this.basisModule.isFormatSupported(TranscodeTarget.EAC_RG11, TextureFormat.ETC1S);

[Above list has been corrected].

because these are not compiled into the module and PVRTC2 is not supported for UASTC transcoding.

I will first look into why the PVRTC1 transcoding fails before I spend any more time on this. I hope I can reproduce it natively otherwise it becomes hard to debug. If you are building the transcoder yourself, you can set BASISU_FORCE_DEVEL_MESSAGES to 1 at the top of lib/basisu/transcoder/basisu_transcoder.h to get messages that may tell you why it is failing.

@lexaknyazev
Copy link
Member

Why isn't ETC1S -> EAC_R11 supported? The BasisU has it implemented.

@MarkCallow
Copy link
Collaborator

MarkCallow commented Jul 16, 2020

It was not compiled into the the transcoder module in the basisu repo and I copied the settings. I thought, mistakenly it appears, that R11 & RG11 were not supported by WebGL. They're under the ETC extension. We'll have to see how much this adds to the size of the transcoder.

@MarkCallow
Copy link
Collaborator

MarkCallow commented Jul 16, 2020

@donmccurdy, the PVRTC1 transcoders (both from ETC1S and UASTC) only support power of 2 dimensions. The ETC1S texture is (ktx_app) is 2048 * 2048 while the UASTC texture (ktx_document) is 1000x1392. Hence the former works, the latter doesn't. There is a decode flag to request the transcoder pad the texture to next power of 2 but it is not implemented yet. The comment, from Rich, says "we're going to support it."

@donmccurdy
Copy link
Contributor Author

That explains things, thank you! I'll check for power of 2 and fall back to RGBA32 if necessary for now.

@donmccurdy
Copy link
Contributor Author

Closing, doesn't seem like any change is needed here. Thanks!

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

No branches or pull requests

3 participants