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 BasisUniversal ETC RA as RG transcoding #86916

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

BlueCube3310
Copy link
Contributor

@BlueCube3310 BlueCube3310 commented Jan 7, 2024

The BasisUniversal module was incorrectly set to interpret RA_AS_RG textures as ETC2_RGBA8, which caused the following error on mobile devices:

VRAM Compressed BasisUniversal
IMG_20240107_115719 IMG_20240107_115648

This PR should fix that issue, though I haven't been able to test it yet.

MRP for testing the changes: basisu_transcode.zip

@BlueCube3310 BlueCube3310 requested a review from a team as a code owner January 7, 2024 12:03
@AThousandShips AThousandShips added this to the 4.3 milestone Jan 7, 2024
@AThousandShips AThousandShips added cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jan 7, 2024
@fire fire requested a review from a team January 7, 2024 18:20
@fire
Copy link
Member

fire commented Jan 7, 2024

I don't have a workflow for testing etc2 on a mobile device right now. Maybe the rendering team is able to check in.

It looks ok, but I've been burnt not being able to test on a device before.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (Samsung Galaxy Z Fold4), it works as expected.

Testing project: basisu_transcode_1.zip

Built APKs (ARM64 only):

Desktop

For comparison purposes. Texture is applied both as albedo and normal map for easier comparison.

VRAM Compressed (S3TC) Basis Universal
Screenshot_20240111_180928 webp Screenshot_20240111_182036 webp

Mobile

VRAM Compressed (ETC2)
Screenshot_20240111_182007_basisu_transcode_etc2 webp
Before After (this PR)
Screenshot_20240111_182118_basisu_transcode_basisu_before webp Screenshot_20240111_182127_basisu_transcode_basisu_after webp

@akien-mga akien-mga merged commit 8616487 into godotengine:master Jan 11, 2024
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@YuriSizov YuriSizov removed cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release needs testing labels Jan 24, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.2.

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jan 24, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.4.

@akien-mga akien-mga changed the title Fix BasisUniversal ETC RA as RG transcoding Fix BasisUniversal ETC RA as RG transcoding Feb 6, 2024
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.

7 participants