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

BasisUniversal: Use RGTC compression when available #90170

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

BlueCube3310
Copy link
Contributor

Depends on #89426

Changes BasisUniversal's destination format for normal maps from (DXT5/ETC2)_RA-As-RG to RGTC_RG/ETC2_RG11, which results in a slight quality gain.
Also adds support for encoding R-channel only images and decoding them as RGTC_R/ETC2_R11.

This needs testing on mobile devices, but I suspect the difference in quality will be most significant there.

master PR diff
dxt5 rgtc nrmdiff
main rgtc_red red_diff

MRP:
basisurgtctest.zip

@BlueCube3310 BlueCube3310 requested a review from a team as a code owner April 3, 2024 12:21
@AThousandShips AThousandShips added this to the 4.x milestone Apr 3, 2024
@BlueCube3310 BlueCube3310 force-pushed the basisu-rgtc-etc2-rg branch from 3915916 to 5f125f3 Compare May 2, 2024 11:16
@BlueCube3310 BlueCube3310 force-pushed the basisu-rgtc-etc2-rg branch from 5f125f3 to 01406ff Compare June 17, 2024 12:54
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, it works as expected. The normal map texture (displayed on the right) seems more yellow but I don't know how close this is to ground truth, as we currently lack a way to have Lossless-compressed textures be imported as RG8 (Normal Map import option isn't available).

The final result (albedo with normal map) looks very close either way.

Testing project:

APKs for quick testing on Android:

Desktop

Before After (this PR)
Screenshot_20240617_174058 png webp Screenshot_20240617_174342 png webp

Mobile

Tested on Samsung Galaxy Z Fold4 (Adreno 730).

Before After (this PR)
Screenshot_20240617_174725_BasisuRGTCTest png webp Screenshot_20240617_174738_BasisuRGTCTest png webp

} else {
// No supported VRAM compression formats, decompress.
basisu_format = basist::transcoder_texture_format::cTFRGBA32;
image_format = Image::FORMAT_RGBA8;
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't Image::FORMAT_RG8 or Image::FORMAT_LA8 be used here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BasisU doesn't allow directly decompressing textures to R8/RG8, so they have to be converted after decompression. This increases the loading time slightly, though I think the tradeoff is worth it since less VRAM is wasted.

} else {
// No supported VRAM compression formats, decompress.
basisu_format = basist::transcoder_texture_format::cTFRGBA32;
image_format = Image::FORMAT_RGBA8;
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't Image::FORMAT_R8 be used here?

@BlueCube3310 BlueCube3310 force-pushed the basisu-rgtc-etc2-rg branch from 01406ff to 838a67e Compare June 17, 2024 17:25
@akien-mga akien-mga modified the milestones: 4.x, 4.4 Sep 10, 2024
@akien-mga akien-mga merged commit c1bc42b into godotengine:master Sep 11, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

5 participants