From df03fdf3d7768ce08ec94db1dc461fa62f3fec43 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Mon, 11 Apr 2022 11:04:20 -0700 Subject: [PATCH] mipgen: fixups / clarification regarding sRGB. The `g_linearized` variable was being used for two purposes: to denote the linearity of the source format AND the linearity of the destination format. This was confusing and is now split is `sourceIsLinear` and `destIsLinear`. This change has no effect on the the look of the suzanne demo. --- libs/image/include/image/ColorTransform.h | 4 +-- .../include/imageio/BlockCompression.h | 33 ++++++++++++++++++- libs/imageio/src/BlockCompression.cpp | 2 +- samples/CMakeLists.txt | 6 ++-- tools/mipgen/src/main.cpp | 31 +++++++++++------ 5 files changed, 59 insertions(+), 17 deletions(-) diff --git a/libs/image/include/image/ColorTransform.h b/libs/image/include/image/ColorTransform.h index 334c21b014..8986cfd303 100644 --- a/libs/image/include/image/ColorTransform.h +++ b/libs/image/include/image/ColorTransform.h @@ -158,8 +158,8 @@ inline filament::math::float3 linearToSRGB(const filament::math::float3& color) return sRGBColor; } -// Creates a n-channel sRGB image from a linear floating-point image. -// The source image can have more than N channels, but only the first N are converted to sRGB. +// Creates a N-channel sRGB image from a linear floating-point image. +// The source image can have more than N channels, but only the first 3 are converted to sRGB. template std::unique_ptr fromLinearTosRGB(const LinearImage& image) { const size_t w = image.getWidth(); diff --git a/libs/imageio/include/imageio/BlockCompression.h b/libs/imageio/include/imageio/BlockCompression.h index 1b742590e5..e952cc1b16 100644 --- a/libs/imageio/include/imageio/BlockCompression.h +++ b/libs/imageio/include/imageio/BlockCompression.h @@ -116,7 +116,7 @@ enum class AstcSemantic { struct AstcConfig { AstcPreset quality; AstcSemantic semantic; - filament::math::ushort2 blocksize; + filament::math::ushort2 blocksize; bool srgb; }; @@ -182,6 +182,37 @@ struct CompressionConfig { AstcConfig astc; S3tcConfig s3tc; EtcConfig etc; + bool isLinear() const { + if (type == ASTC) { + return !astc.srgb; + } + switch (type == S3TC ? s3tc.format : etc.format) { + case CompressedFormat::R11_EAC: + case CompressedFormat::SIGNED_R11_EAC: + case CompressedFormat::RG11_EAC: + case CompressedFormat::SIGNED_RG11_EAC: + case CompressedFormat::RGB8_ETC2: + case CompressedFormat::RGB8_ALPHA1_ETC2: + case CompressedFormat::RGBA8_ETC2_EAC: + case CompressedFormat::RGB_S3TC_DXT1: + case CompressedFormat::RGBA_S3TC_DXT1: + case CompressedFormat::RGBA_S3TC_DXT3: + case CompressedFormat::RGBA_S3TC_DXT5: + return true; + case CompressedFormat::SRGB8_ETC2: + case CompressedFormat::SRGB8_ALPHA1_ETC: + case CompressedFormat::SRGB8_ALPHA8_ETC2_EAC: + case CompressedFormat::SRGB_S3TC_DXT1: + case CompressedFormat::SRGB_ALPHA_S3TC_DXT1: + case CompressedFormat::SRGB_ALPHA_S3TC_DXT3: + case CompressedFormat::SRGB_ALPHA_S3TC_DXT5: + return false; + default: + // This should be unreachable. + assert(false && "ASTC formats are already handled"); + return false; + } + } }; bool parseOptionString(const std::string& options, CompressionConfig* config); diff --git a/libs/imageio/src/BlockCompression.cpp b/libs/imageio/src/BlockCompression.cpp index f4e0e17f0f..7c1b7b6068 100644 --- a/libs/imageio/src/BlockCompression.cpp +++ b/libs/imageio/src/BlockCompression.cpp @@ -381,7 +381,7 @@ static void extract4x4RGBA(uint8_t* dst, const LinearImage& source, uint32_t x0, // - DXT1 with no alpha (16 input pixels in 64 bits of output, 6:1) // - DXT5 with alpha (16 input pixels into 128 bits of output, 4:1) // -// TODO: investigate using something more capable than STB (eg AMD Compressenator, bimg, libsquish) +// TODO: remove this in favor of basisu CompressedTexture s3tcCompress(const LinearImage& original, S3tcConfig config) { const bool dxt5 = config.format == CompressedFormat::RGBA_S3TC_DXT5; LinearImage source = extendToFourChannels(original); diff --git a/samples/CMakeLists.txt b/samples/CMakeLists.txt index 69f49be69f..dae5b24f2a 100644 --- a/samples/CMakeLists.txt +++ b/samples/CMakeLists.txt @@ -151,9 +151,9 @@ add_mesh("assets/models/monkey/monkey.obj" "suzanne.filamesh") # Use a RGBA compression format for Metal and Vulkan support. set (COMPRESSION "--compression=s3tc_rgba_dxt5") add_ktxfiles("assets/models/monkey/albedo.png" "albedo_s3tc.ktx" "${COMPRESSION}") -add_ktxfiles("assets/models/monkey/roughness.png" "roughness.ktx" "${COMPRESSION};--grayscale") -add_ktxfiles("assets/models/monkey/metallic.png" "metallic.ktx" "${COMPRESSION};--grayscale") -add_ktxfiles("assets/models/monkey/ao.png" "ao.ktx" "${COMPRESSION};--grayscale") +add_ktxfiles("assets/models/monkey/roughness.png" "roughness.ktx" "${COMPRESSION};--grayscale;--linear") +add_ktxfiles("assets/models/monkey/metallic.png" "metallic.ktx" "${COMPRESSION};--grayscale;--linear") +add_ktxfiles("assets/models/monkey/ao.png" "ao.ktx" "${COMPRESSION};--grayscale;--linear") add_pngfile("assets/models/monkey/normal.png" "normal.png") diff --git a/tools/mipgen/src/main.cpp b/tools/mipgen/src/main.cpp index 493c7d6ebd..e829ada83f 100644 --- a/tools/mipgen/src/main.cpp +++ b/tools/mipgen/src/main.cpp @@ -48,7 +48,7 @@ static bool g_addAlpha = false; static bool g_stripAlpha = false; static bool g_grayscale = false; static bool g_ktxContainer = false; -static bool g_linearized = false; +static bool g_sourceIsLinear = false; static bool g_quietMode = false; static uint32_t g_mipLevelCount = 0; @@ -71,13 +71,13 @@ Options: --license, -L print copyright and license information --linear, -l - assume that image pixels are already linearized + specifies that the source image is linear (converts to floats without transformation) --page, -p generate HTML page for review purposes (mipmap.html) --quiet, -q suppress console output from the mipgen tool --grayscale, -g - create a single-channel image and do not perform gamma correction + create a single-channel image --format=[exr|hdr|rgbm|psd|png|dds|ktx], -f [exr|hdr|rgbm|psd|png|dds|ktx] specify output file format, inferred from output pattern if omitted --kernel=[box|nearest|hermite|gaussian|normals|mitchell|lanczos|min], -k [filter] @@ -190,7 +190,7 @@ static int handleArguments(int argc, char* argv[]) { license(); exit(0); case 'l': - g_linearized = true; + g_sourceIsLinear = true; break; case 'g': g_grayscale = true; @@ -273,7 +273,7 @@ int main(int argc, char* argv[]) { g_ktxContainer = true; g_formatSpecified = true; } else if (!g_formatSpecified) { - g_format = ImageEncoder::chooseFormat(outputPattern, g_linearized); + g_format = ImageEncoder::chooseFormat(outputPattern, g_sourceIsLinear); } if (!g_quietMode) { @@ -282,7 +282,7 @@ int main(int argc, char* argv[]) { ifstream inputStream(inputPath.getPath(), ios::binary); LinearImage sourceImage = ImageDecoder::decode(inputStream, inputPath.getPath(), - g_linearized ? ImageDecoder::ColorSpace::LINEAR : ImageDecoder::ColorSpace::SRGB); + g_sourceIsLinear ? ImageDecoder::ColorSpace::LINEAR : ImageDecoder::ColorSpace::SRGB); if (!sourceImage.isValid()) { cerr << "Unable to open image: " << inputPath.getPath() << endl; return 1; @@ -337,15 +337,25 @@ int main(int argc, char* argv[]) { .pixelDepth = 0, }; size_t componentCount = sourceImage.getChannels(); + + // Try to choose an internal format that has the same transformation function as the + // source format. This varible may be adjusted later, after the destination format has + // been resolved. + bool destIsLinear = g_sourceIsLinear; + if (componentCount == 1) { info.glFormat = info.glBaseInternalFormat = Ktx1Bundle::RED; info.glInternalFormat = Ktx1Bundle::R8; + destIsLinear = true; } else if (componentCount == 3) { info.glFormat = info.glBaseInternalFormat = Ktx1Bundle::RGB; - info.glInternalFormat = Ktx1Bundle::RGB8; + info.glInternalFormat = destIsLinear ? Ktx1Bundle::RGB8 : Ktx1Bundle::SRGB8; } else if (componentCount == 4) { info.glFormat = info.glBaseInternalFormat = Ktx1Bundle::RGBA; - info.glInternalFormat = Ktx1Bundle::RGBA8; + info.glInternalFormat = destIsLinear ? Ktx1Bundle::RGBA8 : Ktx1Bundle::SRGB8_ALPHA8; + } else { + cerr << "Bad component count." << endl; + return 1; } #ifdef IMAGEIO_SUPPORTS_BLOCK_COMPRESSION CompressionConfig config {}; @@ -359,6 +369,7 @@ int main(int argc, char* argv[]) { // glFormat should be 0, and glBaseInternalFormat should be RED, RG, RGB, or RGBA. // The glInternalFormat field is the only field that specifies the actual format. info.glFormat = 0; + destIsLinear = config.isLinear(); } #else if (!g_compression.empty()) { @@ -387,11 +398,11 @@ int main(int argc, char* argv[]) { return; } #endif - if (g_grayscale && g_linearized) { + if (g_grayscale && destIsLinear) { data = fromLinearToGrayscale(image); } else if (g_grayscale) { data = fromLinearTosRGB(image); - } else if (g_linearized) { + } else if (destIsLinear) { if (componentCount == 3) { data = fromLinearToRGB(image); } else {