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

Fix crash on mobile devices supporting S3TC for GLES3 renderer #62909

Merged
merged 1 commit into from
Jul 18, 2022

Conversation

minosvasilias
Copy link

Applies changes made in #32255 to rasterizer_storage_gles3.cpp, fixes #28308 for GLES3 builds.

The fix in that PR was only implemented for the GLES2 renderer. The same issue can however also occur on GLES3.
Mobile devices supporting S3TC compression will expect S3TC compressed files and crash if those are not found, complaining about missing s3tc.stex import files.

Since above PR was cherry picked for 3.4.3, it may be sensible to include this in 3.x.
Not relevant for master as far as i am aware.

@akien-mga akien-mga merged commit c7d143d into godotengine:3.x Jul 18, 2022
@akien-mga
Copy link
Member

Thanks!

@Malcolmnixon
Copy link
Contributor

Isnt the real problem that the texture-loader isnt respecting the texture-format selected by the project and bundled with the application, and instead insists on using the first one supported by the OS/renderer?

This just hit Quest 2 which aparently just started advertising s3tc support in their latest update. Testing shows that if the s3tc textures are bundled in the APK then the programs run just fine using the s3tc textures.

It just seems a little odd to fix by claiming that Android devices don't support s3tc when some of them do and work fine.

@akien-mga
Copy link
Member

akien-mga commented Jul 19, 2022

See the discussion in #32255.

This is the "lazy" fix to make it easy to keep supporting multiple devices with a single APK (all devices support ETC/ETC2, but only a tiny fraction support S3TC). For example here if Quest 2 supports S3TC but Quest 1 doesn't, you'll need either two separate exports with different config options, or accept that your Quest 1 users will pay the price of S3TC textures they can't use, and Quest 2 users pay the price of ETC2 textures they're not using because S3TC took precedence.

A more complex solution could be worked on if someone is interested to actually properly add support for including S3TC textures in the APK, but it's not clear what benefit users would have from this aside from doubling the size of their APK.

@akien-mga
Copy link
Member

Cherry-picked for 3.4.5.

@Calinou
Copy link
Member

Calinou commented Jul 19, 2022

A more complex solution could be worked on if someone is interested to actually properly add support for including S3TC textures in the APK, but it's not clear what benefit users would have from this aside from doubling the size of their APK.

S3TC has slightly better quality than ETC2 (and is probably a bit faster to decode). Both support compressing transparent textures. On the other hand, S3TC is much better on all aspects compared to ETC1 (including transparency support, which ETC1 lacks). This can make its use interesting for GLES2 projects that only need to run on the Quest.

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

Successfully merging this pull request may close these issues.

4 participants