From 8e09d8bd71858eee1a3722ae7c85c31ea65da49a Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Tue, 12 Apr 2022 09:25:25 -0700 Subject: [PATCH] Fix sRGB correctness in Ktx1Reader. When creating a KTX file (e.g. with mipgen), you can of course choose whether or not you want to use an sRGB format. At load time, your app has an expectation about whether a particular texture has an sRGB format. e.g. the glTF convention is that albedo textures should be sRGB, while roughness textures are linear. Before this commit, our KTX loader would look at the app's expectation, then silently re-interpret the bits in the glTF texture in order to match this expectation. This seems very wrong. Instead, if the expectated format does not match the format in the KTX file, we now print a warning. Fixes #2422, which will also be obsoleted by our upcoming KTX2 support. --- libs/ktxreader/include/ktxreader/Ktx1Reader.h | 6 +-- libs/ktxreader/src/Ktx1Reader.cpp | 38 ++++++------------- 2 files changed, 14 insertions(+), 30 deletions(-) diff --git a/libs/ktxreader/include/ktxreader/Ktx1Reader.h b/libs/ktxreader/include/ktxreader/Ktx1Reader.h index 6cff1579f4..216c1da54b 100644 --- a/libs/ktxreader/include/ktxreader/Ktx1Reader.h +++ b/libs/ktxreader/include/ktxreader/Ktx1Reader.h @@ -55,7 +55,7 @@ namespace Ktx1Reader { * * @param engine Used to create the Filament Texture * @param ktx In-memory representation of a KTX file - * @param srgb Forces the KTX-specified format into an SRGB format if possible + * @param srgb Requests an sRGB format from the KTX file * @param callback Gets called after all texture data has been uploaded to the GPU * @param userdata Passed into the callback */ @@ -68,7 +68,7 @@ namespace Ktx1Reader { * * @param engine Used to create the Filament Texture * @param ktx In-memory representation of a KTX file - * @param srgb Forces the KTX-specified format into an SRGB format if possible + * @param srgb Requests an sRGB format from the KTX file */ Texture* createTexture(Engine* engine, Ktx1Bundle* ktx, bool srgb); @@ -80,7 +80,7 @@ namespace Ktx1Reader { bool isCompressed(const KtxInfo& info); - TextureFormat toSrgbTextureFormat(TextureFormat format); + bool isSrgbTextureFormat(TextureFormat format); TextureFormat toTextureFormat(const KtxInfo& info); diff --git a/libs/ktxreader/src/Ktx1Reader.cpp b/libs/ktxreader/src/Ktx1Reader.cpp index 16d003207c..6433e08cec 100644 --- a/libs/ktxreader/src/Ktx1Reader.cpp +++ b/libs/ktxreader/src/Ktx1Reader.cpp @@ -15,6 +15,7 @@ */ #include +#include namespace ktxreader { namespace Ktx1Reader { @@ -78,8 +79,10 @@ Texture* createTexture(Engine* engine, const Ktx1Bundle& ktx, bool srgb, const auto dataformat = toPixelDataFormat(ktxinfo); auto texformat = toTextureFormat(ktxinfo); - if (srgb) { - texformat = toSrgbTextureFormat(texformat); + if (srgb && !isSrgbTextureFormat(texformat)) { + utils::slog.w << "Requested sRGB format but KTX contains a linear format. "; + } else if (!srgb && isSrgbTextureFormat(texformat)) { + utils::slog.w << "Requested linear format but KTX contains a sRGB format. "; } Texture* texture = Texture::Builder() @@ -187,64 +190,45 @@ bool isCompressed(const KtxInfo& info) { return info.glFormat == 0; } -TextureFormat toSrgbTextureFormat(TextureFormat format) { +bool isSrgbTextureFormat(TextureFormat format) { switch(format) { // Non-compressed case Texture::InternalFormat::RGB8: - return Texture::InternalFormat::SRGB8; case Texture::InternalFormat::RGBA8: - return Texture::InternalFormat::SRGB8_A8; + return false; // ASTC case Texture::InternalFormat::RGBA_ASTC_4x4: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_4x4; case Texture::InternalFormat::RGBA_ASTC_5x4: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_5x4; case Texture::InternalFormat::RGBA_ASTC_5x5: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_5x5; case Texture::InternalFormat::RGBA_ASTC_6x5: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_6x5; case Texture::InternalFormat::RGBA_ASTC_6x6: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_6x6; case Texture::InternalFormat::RGBA_ASTC_8x5: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_8x5; case Texture::InternalFormat::RGBA_ASTC_8x6: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_8x6; case Texture::InternalFormat::RGBA_ASTC_8x8: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_8x8; case Texture::InternalFormat::RGBA_ASTC_10x5: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_10x5; case Texture::InternalFormat::RGBA_ASTC_10x6: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_10x6; case Texture::InternalFormat::RGBA_ASTC_10x8: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_10x8; case Texture::InternalFormat::RGBA_ASTC_10x10: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_10x10; case Texture::InternalFormat::RGBA_ASTC_12x10: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_12x10; case Texture::InternalFormat::RGBA_ASTC_12x12: - return Texture::InternalFormat::SRGB8_ALPHA8_ASTC_12x12; + return false; // ETC2 case Texture::InternalFormat::ETC2_RGB8: - return Texture::InternalFormat::ETC2_SRGB8; case Texture::InternalFormat::ETC2_RGB8_A1: - return Texture::InternalFormat::ETC2_SRGB8_A1; case Texture::InternalFormat::ETC2_EAC_RGBA8: - return Texture::InternalFormat::ETC2_EAC_SRGBA8; + return false; // DXT case Texture::InternalFormat::DXT1_RGB: - return Texture::InternalFormat::DXT1_SRGB; case Texture::InternalFormat::DXT1_RGBA: - return Texture::InternalFormat::DXT1_SRGBA; case Texture::InternalFormat::DXT3_RGBA: - return Texture::InternalFormat::DXT3_SRGBA; case Texture::InternalFormat::DXT5_RGBA: - return Texture::InternalFormat::DXT5_SRGBA; + return false; default: - return format; + return true; } }