From c3873ea3132e9e8b1bd7246de8eca7fbea3a966f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Br=C3=BCns?= <stefan.bruens@rwth-aachen.de>
Date: Mon, 5 Oct 2020 16:56:09 +0200
Subject: [PATCH] Use proper rounding when casting RGB values to int

When doing an RGB->HSV->RGB roundtrip, the original value should be
restored (HSV, being double, has sufficient precision).

For e.g. `RGBValue(1,2,3)`, the equivalent resulting code for blue is
`trunc(int * 255.0 / 255.0)` (cast from double to int truncates).
At least with x87 FP math and its immediate 80bit extended precision
the resulting value is ~trunc(2.9999..), i.e. 2, similar problems may
exist on other architectures with other values.

Using proper rounding avoids the error magnification.

Closes https://github.com/wxWidgets/wxWidgets/pull/2078
---
 src/common/image.cpp  |  6 ++---
 tests/image/image.cpp | 56 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/src/common/image.cpp b/src/common/image.cpp
index a189fbb57c..85ab31c7c2 100644
--- a/src/common/image.cpp
+++ b/src/common/image.cpp
@@ -3312,9 +3312,9 @@ wxImage::RGBValue wxImage::HSVtoRGB(const HSVValue& hsv)
         }
     }
 
-    return RGBValue((unsigned char)(red * 255.0),
-                    (unsigned char)(green * 255.0),
-                    (unsigned char)(blue * 255.0));
+    return RGBValue((unsigned char)wxRound(red * 255.0),
+                    (unsigned char)wxRound(green * 255.0),
+                    (unsigned char)wxRound(blue * 255.0));
 }
 
 /*
diff --git a/tests/image/image.cpp b/tests/image/image.cpp
index 64e8dc9a75..d9d7d1be0c 100644
--- a/tests/image/image.cpp
+++ b/tests/image/image.cpp
@@ -1884,6 +1884,62 @@ TEST_CASE("wxImage::Paste", "[image][paste]")
 
 }
 
+TEST_CASE("wxImage::RGBtoHSV", "[image][rgb][hsv]")
+{
+    SECTION("RGB(0,0,0) (Black) to HSV")
+    {
+       wxImage::RGBValue rgbBlack(0, 0, 0);
+       wxImage::HSVValue hsvBlack = wxImage::RGBtoHSV(rgbBlack);
+
+       CHECK(hsvBlack.value == Approx(0.0));
+       // saturation and hue are undefined
+    }
+    SECTION("RGB(255,255,255) (White) to HSV")
+    {
+       wxImage::RGBValue rgbWhite(255, 255, 255);
+       wxImage::HSVValue hsvWhite = wxImage::RGBtoHSV(rgbWhite);
+
+       CHECK(hsvWhite.saturation == Approx(0.0));
+       CHECK(hsvWhite.value == Approx(1.0));
+       // hue is undefined
+    }
+    SECTION("RGB(0,255,0) (Green) to HSV")
+    {
+       wxImage::RGBValue rgbGreen(0, 255, 0);
+       wxImage::HSVValue hsvGreen = wxImage::RGBtoHSV(rgbGreen);
+
+       CHECK(hsvGreen.hue == Approx(1.0/3.0));
+       CHECK(hsvGreen.saturation == Approx(1.0));
+       CHECK(hsvGreen.value == Approx(1.0));
+    }
+
+    SECTION("RGB to HSV to RGB")
+    {
+        const wxImage::RGBValue rgbValues[] =
+        {
+            wxImage::RGBValue(   0,   0,   0 ),
+            wxImage::RGBValue(  10,  10,  10 ),
+            wxImage::RGBValue( 255, 255, 255 ),
+            wxImage::RGBValue( 255,   0,   0 ),
+            wxImage::RGBValue(   0, 255,   0 ),
+            wxImage::RGBValue(   0,   0, 255 ),
+            wxImage::RGBValue(   1,   2,   3 ),
+            wxImage::RGBValue(  10,  20,  30 ),
+        };
+
+        for (int i = 0; i < WXSIZEOF(rgbValues); i++)
+        {
+            wxImage::RGBValue rgbValue = rgbValues[i];
+            wxImage::HSVValue hsvValue = wxImage::RGBtoHSV(rgbValue);
+            wxImage::RGBValue rgbRoundtrip = wxImage::HSVtoRGB(hsvValue);
+
+            CHECK(rgbRoundtrip.red == rgbValue.red);
+            CHECK(rgbRoundtrip.green == rgbValue.green);
+            CHECK(rgbRoundtrip.blue == rgbValue.blue);
+        }
+    }
+}
+
 /*
     TODO: add lots of more tests to wxImage functions
 */