Review Request: KColorSpace::KHCY::KHCY(const & QColor) constructor floating point precision error
mrgrim at gr1m.org
Tue Jun 23 00:20:06 BST 2009
On Monday 22 June 2009 6:44:39 pm Matthew Woehlke wrote:
> Michael Kreitzer wrote:
> > I considered that as well. The only thing that worries me is IEEE 754 on
> > x86 seems woefully unreliable for direct comparisons if you're not trying
> > to do something very simple (e.g. prevent a divide by zero).
> True, but I think it will be okay in this case. If the values aren't
> identical, the chroma calculation should hopefully be meaningful.
> And this should only happen for black or white, yes? (Although you could
> get some pretty tiny-but-non-zero values of c for grays, before.) Maybe
> it would be better to always calculate c, and check after for infinity
> or NaN.
> Do you think this would be better?
> // chroma component
> c = qMax( (y - n) / y, (p - y) / (1 - y) );
> if (!(c >= 0.0 || c <= 1.0))
> c = 0.0;
I think part of the original problem was that while y was represented by
0.99999999999999995836663657655662973 when it should have been 1.0 it did
proceed with the chroma calculation, and the result was a c of 1.0. Testing
for NaN in this case would have been ineffective.
The result of 1.0 was from the second calculation in the qMax. (1.0 -
0.99999999999999995836663657655662973) / (1.0 -
0.99999999999999995836663657655662973) resulted in 1.0. This was incorrect due
to y not being correct due to precision problems. If I understand correctly in
situations where r, g, and b are identical then y should be equal to all 3 as
well (r = g = b = y). If this is not the case when it should be due to a
precision or rounding error and chroma is calculated anyways you will end up
with a bad chroma.
Honestly I cannot think of a nice solution to this without some sort of fixed
point or decimal math. I think the r = g = b comparison is probably the best
work around. For now, if you look at the disassembled output of GCC, all three
are stored on the stack immediately after returning from the QColor method
which should ensure any rounding errors are consistent between the three. Lets
hope that remains consistent.
More information about the kde-core-devel