From 89403ca23b0271443494a087f3b504f370fc4e28 Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:35:22 -0400 Subject: [PATCH] Fix RGBGFX to support `-C/--color-curve` in `-r/--reverse` mode (#2110) Clarifies error message when RGBGFX reverse mode has `-c` mismatching `-p`. --- include/gfx/rgba.hpp | 28 +++++----------- src/gfx/main.cpp | 2 +- src/gfx/pal_spec.cpp | 8 ++--- src/gfx/reverse.cpp | 23 +++++++++---- src/gfx/rgba.cpp | 46 +++++++++++++++++++++----- test/gfx/reverse_color_mismatch.2bpp | Bin 0 -> 16 bytes test/gfx/reverse_color_mismatch.err | 3 ++ test/gfx/reverse_color_mismatch.flags | 3 ++ test/gfx/reverse_color_mismatch.pal | 1 + 9 files changed, 75 insertions(+), 39 deletions(-) create mode 100644 test/gfx/reverse_color_mismatch.2bpp create mode 100644 test/gfx/reverse_color_mismatch.err create mode 100644 test/gfx/reverse_color_mismatch.flags create mode 100644 test/gfx/reverse_color_mismatch.pal diff --git a/include/gfx/rgba.hpp b/include/gfx/rgba.hpp index 87f1f602..6a3c67b8 100644 --- a/include/gfx/rgba.hpp +++ b/include/gfx/rgba.hpp @@ -20,23 +20,6 @@ struct Rgba { explicit constexpr Rgba(uint32_t rgba = 0) : red(rgba >> 24), green(rgba >> 16), blue(rgba >> 8), alpha(rgba) {} - // CGB colors are RGB555, so we use bit 15 to signify that the color is transparent instead - // Since the rest of the bits don't matter then, we return 0x8000 (1 << 15) exactly. - static constexpr uint16_t transparent = 0b1'00000'00000'00000; - - static constexpr Rgba fromCGBColor(uint16_t color) { - constexpr auto _5to8 = [](uint8_t channel) -> uint8_t { - channel &= 0b11111; // For caller's convenience - return channel << 3 | channel >> 2; - }; - return { - _5to8(color), - _5to8(color >> 5), - _5to8(color >> 10), - static_cast(color & transparent ? 0x00 : 0xFF), - }; - } - // Returns this RGBA as a 32-bit number that can be printed in hex (`#%08x`) // to yield its CSS representation (`#rrggbbaa`). uint32_t toCSS() const { @@ -45,15 +28,22 @@ struct Rgba { }; return shl(red, 24) | shl(green, 16) | shl(blue, 8) | shl(alpha, 0); } - bool operator==(Rgba const &rhs) const { return toCSS() == rhs.toCSS(); } + // We allow some leeway to consider colors as transparent or opaque, + // but intermediate alpha values are still ambiguous. static constexpr uint8_t transparency_threshold = 0x10; bool isTransparent() const { return alpha < transparency_threshold; } static constexpr uint8_t opacity_threshold = 0xF0; bool isOpaque() const { return alpha >= opacity_threshold; } bool isAmbiguous() const { return isTransparent() == isOpaque(); } - // Computes the equivalent CGB color, respects the color curve depending on options + + // CGB colors are RGB555, so we use bit 15 to signify that the color is transparent instead + // Since the rest of the bits don't matter then, we return 0x8000 (1 << 15) exactly. + static constexpr uint16_t transparent = 0b1'00000'00000'00000; + // Computes the equivalent RGB888 color; respects the color curve depending on argument + static Rgba fromCGBColor(uint16_t color, bool useColorCurve); + // Computes the equivalent RGB555 color; respects the color curve depending on options uint16_t cgbColor() const; bool isGray() const { return red == green && green == blue; } diff --git a/src/gfx/main.cpp b/src/gfx/main.cpp index dac70e03..b807e12f 100644 --- a/src/gfx/main.cpp +++ b/src/gfx/main.cpp @@ -528,7 +528,7 @@ static void verboseOutputConfig() { for (auto const &pal : options.palSpec) { fputs("\t\t", stderr); for (auto const &color : pal) { - if (color) { + if (color.has_value()) { fprintf(stderr, "#%06x, ", color->toCSS() >> 8); } else { fputs("#none, ", stderr); diff --git a/src/gfx/pal_spec.cpp b/src/gfx/pal_spec.cpp index 9ac1fa95..045881c4 100644 --- a/src/gfx/pal_spec.cpp +++ b/src/gfx/pal_spec.cpp @@ -542,10 +542,10 @@ static void parseGBCFile(char const *filename, std::filebuf &file) { } options.palSpec.push_back({ - Rgba::fromCGBColor(toWord(buf[0], buf[1])), - Rgba::fromCGBColor(toWord(buf[2], buf[3])), - Rgba::fromCGBColor(toWord(buf[4], buf[5])), - Rgba::fromCGBColor(toWord(buf[6], buf[7])), + Rgba::fromCGBColor(toWord(buf[0], buf[1]), false), + Rgba::fromCGBColor(toWord(buf[2], buf[3]), false), + Rgba::fromCGBColor(toWord(buf[4], buf[5]), false), + Rgba::fromCGBColor(toWord(buf[6], buf[7]), false), }); } } diff --git a/src/gfx/reverse.cpp b/src/gfx/reverse.cpp index 4bc1de96..b5595b85 100644 --- a/src/gfx/reverse.cpp +++ b/src/gfx/reverse.cpp @@ -119,10 +119,6 @@ void reverse() { warnx("Tile deduplication is enabled, but no tilemap is provided"); } - if (options.useColorCurve) { - warnx("The color curve is not yet supported in reverse mode"); - } - if (options.inputSlice.left != 0 || options.inputSlice.top != 0 || options.inputSlice.height != 0) { warnx("\"Sliced-off\" pixels are ignored in reverse mode"); @@ -230,14 +226,14 @@ void reverse() { palSize ); } - // Expand the colors + // Expand the little-endian RGB555 colors to RGB888 auto &palette = palettes.emplace_back(); std::generate( palette.begin(), palette.begin() + options.nbColorsPerPal, [&buf, i = 0]() mutable { i += 2; - return Rgba::fromCGBColor(buf[i - 2] | buf[i - 1] << 8); // little-endian + return Rgba::fromCGBColor(buf[i - 2] | buf[i - 1] << 8, options.useColorCurve); } ); } @@ -251,7 +247,20 @@ void reverse() { } if (options.hasExplicitPalSpec() && palettes != options.palSpec) { - warnx("Colors in the palette file do not match those specified with '-c'"); + // The explicit `-c` pal spec does not match the input `-p` palette file. + // Check whether their 8-to-5-bit-reduced GB colors nevertheless match. + std::vector, 4>> palSpecQuantized(options.palSpec); + for (auto &pal : palSpecQuantized) { + for (auto &color : pal) { + if (color.has_value()) { + color = Rgba::fromCGBColor(color->cgbColor(), options.useColorCurve); + } + } + } + warnx( + "Colors %s the palette file do not match those specified with '-c'", + palettes == palSpecQuantized ? "reversed from" : "in" + ); // This spacing aligns "...versus with `-c`" above the column of `-c` palettes fputs("Colors specified in the palette file: ...versus with '-c':\n", stderr); for (size_t i = 0; i < palettes.size() || i < options.palSpec.size(); ++i) { diff --git a/src/gfx/rgba.cpp b/src/gfx/rgba.cpp index 1526e0fc..736fdf97 100644 --- a/src/gfx/rgba.cpp +++ b/src/gfx/rgba.cpp @@ -24,16 +24,24 @@ std::string toCGB(uint16_t color) { return buf; } -// Based on inverting the "Modern - Accurate" formula used by SameBoy -// since commit b5a611c5db46d6a0649d04d24d8d6339200f9ca1 (Dec 2020), -// with gaps in the scale curve filled by polynomial interpolation. +// Copied from the "Modern - Accurate" (`GB_COLOR_CORRECTION_MODERN_ACCURATE`) +// formula used by SameBoy in its `scale_channel_with_curve` function since +// commit b5a611c5db46d6a0649d04d24d8d6339200f9ca1 (Dec 2020). +// clang-format off: vertically align columns of values +static std::array color_curve{ + 0, 6, 12, 20, 28, 36, 45, 56, 66, 76, 88, 100, 113, 125, 137, 149, + 161, 172, 182, 192, 202, 210, 218, 225, 232, 238, 243, 247, 250, 252, 254, 255, +}; +// clang-format on + +// Inverted `color_curve`, with gaps filled by polynomial interpolation. // clang-format off: vertically align columns of values static std::array reverse_curve{ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, - 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 3, 3, 3, - 3, 3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 5, 5, 5, 5, - 5, 5, 5, 6, 6, 6, 6, 6, 6, 7, 7, 7, 7, 7, 7, 7, - 7, 8, 8, 8, 8, 8, 8, 9, 9, 9, 9, 9, 10, 10, 10, 10, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 3, 3, 3, + 3, 3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 5, 5, 5, 5, + 5, 5, 5, 6, 6, 6, 6, 6, 6, 7, 7, 7, 7, 7, 7, 7, + 7, 8, 8, 8, 8, 8, 8, 9, 9, 9, 9, 9, 10, 10, 10, 10, 10, 10, 11, 11, 11, 11, 11, 11, 12, 12, 12, 12, 12, 13, 13, 13, 13, 13, 14, 14, 14, 14, 14, 14, 15, 15, 15, 15, 15, 16, 16, 16, 16, 16, 16, 17, 17, 17, 17, 17, 18, 18, 18, 18, 18, 18, 19, 19, @@ -48,6 +56,28 @@ static std::array reverse_curve{ }; // clang-format on +Rgba Rgba::fromCGBColor(uint16_t color, bool useColorCurve) { + uint8_t r = color & 0b11111, g = (color >> 5) & 0b11111, b = (color >> 10) & 0b11111; + if (useColorCurve) { + r = color_curve[r]; + g = color_curve[g]; + b = color_curve[b]; + if (g != b) { + g = round(pow((pow(g / 255.0, 2.2) * 3 + pow(b / 255.0, 2.2)) / 4, 1 / 2.2) * 255); + } + } else { + r = r << 3 | r >> 2; + g = g << 3 | g >> 2; + b = b << 3 | b >> 2; + } + return { + r, + g, + b, + static_cast(color & transparent ? 0x00 : 0xFF), + }; +} + uint16_t Rgba::cgbColor() const { if (isTransparent()) { return transparent; diff --git a/test/gfx/reverse_color_mismatch.2bpp b/test/gfx/reverse_color_mismatch.2bpp new file mode 100644 index 0000000000000000000000000000000000000000..01d633b27e8ea9b17084fc911d0c8cc43a4170a9 GIT binary patch literal 16 KcmZQzKm`B*5C8!H literal 0 HcmV?d00001 diff --git a/test/gfx/reverse_color_mismatch.err b/test/gfx/reverse_color_mismatch.err new file mode 100644 index 00000000..c9cc6441 --- /dev/null +++ b/test/gfx/reverse_color_mismatch.err @@ -0,0 +1,3 @@ +warning: Colors reversed from the palette file do not match those specified with '-c' +Colors specified in the palette file: ...versus with '-c': +[#adceefff, , , ] [#abcdefff, , , ] diff --git a/test/gfx/reverse_color_mismatch.flags b/test/gfx/reverse_color_mismatch.flags new file mode 100644 index 00000000..5b34593f --- /dev/null +++ b/test/gfx/reverse_color_mismatch.flags @@ -0,0 +1,3 @@ +-c #abcdef +-s 1 +-p reverse_color_mismatch.pal diff --git a/test/gfx/reverse_color_mismatch.pal b/test/gfx/reverse_color_mismatch.pal new file mode 100644 index 00000000..cb91089b --- /dev/null +++ b/test/gfx/reverse_color_mismatch.pal @@ -0,0 +1 @@ +5w \ No newline at end of file