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
This commit is contained in:
Stefan Brüns
2020-10-05 16:56:09 +02:00
committed by paulcor
parent a3f14b2fc2
commit c3873ea313
2 changed files with 59 additions and 3 deletions

View File

@@ -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));
}
/*

View File

@@ -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
*/