From 7295b6c88cbadb188871162d4d76e7c50faef9d1 Mon Sep 17 00:00:00 2001 From: HybridDog <3192173+HybridDog@users.noreply.github.com> Date: Mon, 18 Nov 2024 00:02:53 +0100 Subject: [PATCH] Remove unused and rarely used irrlicht color functions (#15442) SColor.h contains many functions which are unused and/or perform linear operations on non-linear 8 bit sRGB color values, such as the plus operator and `SColor::getInterpolated()`, and there is no documentation about missing gamma correction. Some of these functions are not called or called only once: * `getAverage(s16 color)`: Unused * `SColor::getLightness()`: Unused * `SColor::getAverage()`: Claims to determine a color's average intensity but calculates something significantly different since SColor represents non-linear sRGB values. * `SColor::getInterpolated_quadratic()`: Claims to interpolate between colors but uses the sRGB color space, which is neither physically nor perceptually linear. * `SColorf::getInterpolated_quadratic()`: Unused * `SColorf::setColorComponentValue()`: Unused Removing or inlining these functions can simplify the code and documenting gamma-incorrect operations can reduce confusion about what the functions do. This commit does the following: * Remove the above-mentioned unused functions * Inline `SColor::getAverage()` into `CIrrDeviceLinux::TextureToMonochromeCursor()` * Rename `SColor::getLuminance()` into `SColor::getBrightness()` since it does not determine a color's luminance but calculates something which differs significantly from physical luminance since SColor represents non-linear sRGB values. * Inline `SColor::getInterpolated_quadratic()` into `GameUI::update()`, where it is only used for the alpha value calculation for fading * Document gamma-incorrect behaviour in docstrings --- irr/include/SColor.h | 105 +++++------------------------------- irr/src/CIrrDeviceLinux.cpp | 3 +- src/client/camera.h | 2 +- src/client/gameui.cpp | 8 +-- src/client/imagesource.cpp | 2 +- 5 files changed, 20 insertions(+), 100 deletions(-) diff --git a/irr/include/SColor.h b/irr/include/SColor.h index a2dd52603..41bca1a8c 100644 --- a/irr/include/SColor.h +++ b/irr/include/SColor.h @@ -224,12 +224,6 @@ inline u32 getBlue(u16 color) return (color & 0x1F); } -//! Returns the average from a 16 bit A1R5G5B5 color -inline s32 getAverage(s16 color) -{ - return ((getRed(color) << 3) + (getGreen(color) << 3) + (getBlue(color) << 3)) / 3; -} - //! Class representing a 32 bit ARGB color. /** The color values for alpha, red, green, and blue are stored in a single u32. So all four values may be between 0 and 255. @@ -275,24 +269,12 @@ public: 0 means no blue, 255 means full blue. */ u32 getBlue() const { return color & 0xff; } - //! Get lightness of the color in the range [0,255] - f32 getLightness() const - { - return 0.5f * (core::max_(core::max_(getRed(), getGreen()), getBlue()) + core::min_(core::min_(getRed(), getGreen()), getBlue())); - } - - //! Get luminance of the color in the range [0,255]. - f32 getLuminance() const + //! Get an approximate brightness value of the color in the range [0,255] + f32 getBrightness() const { return 0.3f * getRed() + 0.59f * getGreen() + 0.11f * getBlue(); } - //! Get average intensity of the color in the range [0,255]. - u32 getAverage() const - { - return (getRed() + getGreen() + getBlue()) / 3; - } - //! Sets the alpha component of the Color. /** The alpha component defines how transparent a color should be. \param a The alpha value of the color. 0 is fully transparent, 255 is fully opaque. */ @@ -362,9 +344,9 @@ public: /** \return True if this color is smaller than the other one */ bool operator<(const SColor &other) const { return (color < other.color); } - //! Adds two colors, result is clamped to 0..255 values + //! Adds two colors in a gamma-incorrect way /** \param other Color to add to this color - \return Addition of the two colors, clamped to 0..255 values */ + \return Sum of the two non-linear colors, clamped to 0..255 values */ SColor operator+(const SColor &other) const { return SColor(core::min_(getAlpha() + other.getAlpha(), 255u), @@ -374,7 +356,9 @@ public: } //! Interpolates the color with a f32 value to another color - /** \param other: Other color + /** Note that the interpolation is neither physically nor perceptually + linear since it happens directly in the sRGB color space. + \param other: Other color \param d: value between 0.0f and 1.0f. d=0 returns other, d=1 returns this, values between interpolate. \return Interpolated color. */ SColor getInterpolated(const SColor &other, f32 d) const @@ -387,34 +371,6 @@ public: (u32)core::round32(other.getBlue() * inv + getBlue() * d)); } - //! Returns interpolated color. ( quadratic ) - /** \param c1: first color to interpolate with - \param c2: second color to interpolate with - \param d: value between 0.0f and 1.0f. */ - SColor getInterpolated_quadratic(const SColor &c1, const SColor &c2, f32 d) const - { - // this*(1-d)*(1-d) + 2 * c1 * (1-d) + c2 * d * d; - d = core::clamp(d, 0.f, 1.f); - const f32 inv = 1.f - d; - const f32 mul0 = inv * inv; - const f32 mul1 = 2.f * d * inv; - const f32 mul2 = d * d; - - return SColor( - core::clamp(core::floor32( - getAlpha() * mul0 + c1.getAlpha() * mul1 + c2.getAlpha() * mul2), - 0, 255), - core::clamp(core::floor32( - getRed() * mul0 + c1.getRed() * mul1 + c2.getRed() * mul2), - 0, 255), - core::clamp(core::floor32( - getGreen() * mul0 + c1.getGreen() * mul1 + c2.getGreen() * mul2), - 0, 255), - core::clamp(core::floor32( - getBlue() * mul0 + c1.getBlue() * mul1 + c2.getBlue() * mul2), - 0, 255)); - } - //! set the color by expecting data in the given format /** \param data: must point to valid memory containing color information in the given format \param format: tells the format in which data is available @@ -508,7 +464,7 @@ public: SColorf(f32 r, f32 g, f32 b, f32 a = 1.0f) : r(r), g(g), b(b), a(a) {} - //! Constructs a color from 32 bit Color. + //! Constructs a color from 32 bit Color without gamma correction /** \param c: 32 bit color from which this SColorf class is constructed from. */ SColorf(SColor c) @@ -520,7 +476,7 @@ public: a = c.getAlpha() * inv; } - //! Converts this color to a SColor without floats. + //! Converts this color to a SColor without gamma correction SColor toSColor() const { return SColor((u32)core::round32(a * 255.0f), (u32)core::round32(r * 255.0f), (u32)core::round32(g * 255.0f), (u32)core::round32(b * 255.0f)); @@ -558,7 +514,9 @@ public: } //! Interpolates the color with a f32 value to another color - /** \param other: Other color + /** Note that the interpolation is neither physically nor perceptually + linear if it happens directly in the sRGB color space. + \param other: Other color \param d: value between 0.0f and 1.0f \return Interpolated color. */ SColorf getInterpolated(const SColorf &other, f32 d) const @@ -569,45 +527,6 @@ public: other.g * inv + g * d, other.b * inv + b * d, other.a * inv + a * d); } - //! Returns interpolated color. ( quadratic ) - /** \param c1: first color to interpolate with - \param c2: second color to interpolate with - \param d: value between 0.0f and 1.0f. */ - inline SColorf getInterpolated_quadratic(const SColorf &c1, const SColorf &c2, - f32 d) const - { - d = core::clamp(d, 0.f, 1.f); - // this*(1-d)*(1-d) + 2 * c1 * (1-d) + c2 * d * d; - const f32 inv = 1.f - d; - const f32 mul0 = inv * inv; - const f32 mul1 = 2.f * d * inv; - const f32 mul2 = d * d; - - return SColorf(r * mul0 + c1.r * mul1 + c2.r * mul2, - g * mul0 + c1.g * mul1 + c2.g * mul2, - b * mul0 + c1.b * mul1 + c2.b * mul2, - a * mul0 + c1.a * mul1 + c2.a * mul2); - } - - //! Sets a color component by index. R=0, G=1, B=2, A=3 - void setColorComponentValue(s32 index, f32 value) - { - switch (index) { - case 0: - r = value; - break; - case 1: - g = value; - break; - case 2: - b = value; - break; - case 3: - a = value; - break; - } - } - //! Returns the alpha component of the color in the range 0.0 (transparent) to 1.0 (opaque) f32 getAlpha() const { return a; } diff --git a/irr/src/CIrrDeviceLinux.cpp b/irr/src/CIrrDeviceLinux.cpp index e88902936..0b9ef5b71 100644 --- a/irr/src/CIrrDeviceLinux.cpp +++ b/irr/src/CIrrDeviceLinux.cpp @@ -1951,7 +1951,8 @@ Cursor CIrrDeviceLinux::TextureToMonochromeCursor(irr::video::ITexture *tex, con XPutPixel(sourceImage, x, y, 0); } else // color { - if (pixelCol.getAverage() >= 127) + if ((pixelCol.getRed() + pixelCol.getGreen() + + pixelCol.getBlue()) / 3 >= 127) XPutPixel(sourceImage, x, y, 1); else XPutPixel(sourceImage, x, y, 0); diff --git a/src/client/camera.h b/src/client/camera.h index 9a8e2d36a..3715bebad 100644 --- a/src/client/camera.h +++ b/src/client/camera.h @@ -48,7 +48,7 @@ struct Nametag return bgcolor.value(); else if (!use_fallback) return video::SColor(0, 0, 0, 0); - else if (textcolor.getLuminance() > 186) + else if (textcolor.getBrightness() > 186) // Dark background for light text return video::SColor(50, 50, 50, 50); else diff --git a/src/client/gameui.cpp b/src/client/gameui.cpp index 3408ba196..c2693c6ae 100644 --- a/src/client/gameui.cpp +++ b/src/client/gameui.cpp @@ -201,10 +201,10 @@ void GameUI::update(const RunStats &stats, Client *client, MapDrawControl *draw_ status_y - status_height, status_x + status_width, status_y)); // Fade out - video::SColor final_color = m_statustext_initial_color; - final_color.setAlpha(0); - video::SColor fade_color = m_statustext_initial_color.getInterpolated_quadratic( - m_statustext_initial_color, final_color, m_statustext_time / statustext_time_max); + video::SColor fade_color = m_statustext_initial_color; + f32 d = m_statustext_time / statustext_time_max; + fade_color.setAlpha(static_cast( + fade_color.getAlpha() * (1.0f - d * d))); guitext_status->setOverrideColor(fade_color); guitext_status->enableOverrideColor(true); } diff --git a/src/client/imagesource.cpp b/src/client/imagesource.cpp index 0f4e81f97..a14dac290 100644 --- a/src/client/imagesource.cpp +++ b/src/client/imagesource.cpp @@ -566,7 +566,7 @@ static void apply_hue_saturation(video::IImage *dst, v2u32 dst_pos, v2u32 size, for (u32 x = dst_pos.X; x < dst_pos.X + size.X; x++) { if (colorize) { - f32 lum = dst->getPixel(x, y).getLuminance() / 255.0f; + f32 lum = dst->getPixel(x, y).getBrightness() / 255.0f; if (norm_l < 0) { lum *= norm_l + 1.0f;