Review Request: KColorSpace::KHCY::KHCY(const & QColor) constructor floating point precision error

[Matthew Woehlke]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/868/#review1362
-----------------------------------------------------------


This seems overly complicated (and also wrong if >8-bit color precision is in play). Can the 'r', 'g', 'b' qreals be trusted, or do they have similar problems? If yes, I think it works (better) to simply replace the entire 'if (0.0 == y || 1.0 == y)' test with 'if (r == g && g == b)'?

- Matthew


On 2009-06-21 22:44:32, Michael Kreitzer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/868/
> -----------------------------------------------------------
> 
> (Updated 2009-06-21 22:44:32)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> The constructor KColorSpace::KHCY::KHCY(const & QColor) has a floating point precision error exposed by the -ftree-pre flag in gcc 4.4 which is enabled with -O2 optimizations. The result is an invalid chroma calculation in the corner case of full white (1.0, 1.0, 1.0 in rgb). The patch works around this by performing an integer comparison of r, g, and b and setting chroma to 0.0 if all 3 are equal.
> 
> 
> This addresses bugs 194703 and 195522.
>     https://bugs.kde.org/show_bug.cgi?id=194703
>     https://bugs.kde.org/show_bug.cgi?id=195522
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdeui/colors/kcolorspaces.cpp 985087 
> 
> Diff: http://reviewboard.kde.org/r/868/diff
> 
> 
> Testing
> -------
> 
> This patch corrects the test case code in bug 195522. It also removes all color errors when applied to my system wide kdelibs library.
> 
> 
> Thanks,
> 
> Michael
> 
>