Review Request: KColorSpace::KHCY::KHCY(const & QColor) constructor floating point precision error
mw_triad at users.sourceforge.net
Thu Jun 25 17:21:56 BST 2009
(Please do not quote my e-mail address unobfuscated in message bodies.)
Benoit Jacob wrote:
> Guys, maybe I see this a bit late, but the file in SVN still uses
> operator== between qreal's and that's a "no go".
In this case I'm not sure I agree. I /want/ to treat the values as not
equal even if they differ by a small amount; as long as the math stays
consistent, we should get a reasonable chroma. Probably the best thing
would be to simply build the whole module with -ffloat-store and leave
it at that. But that adds complication to the build system that's only
needed for one platform.
(Besides, this code is already slow, and simultaneously something you
might want in a critical path. I'm not entirely comfortable making it
Hmm... right now you pointed out that the code as-is should always work.
And for the moment, it /is/ working (and there is now a unit test in
place to catch if it starts misbehaving the same way again).
I'm really not sure I want to change this. Right now, the precision of
QColor means it shouldn't ever break. Conversely, if QColor ever gets to
where 0.9999999999, 0.9999999975, 1.0 can be represented, I'm worried
that a fuzzy compare will wrongly consider this "pure white", which /it
isn't/. (In fact, this color has chroma == 1.0.)
> *** But I just want to avoid division by 0 ! So for an expression like
> a/(b-c) the check that I want is really b==c, right ?
More specifically, catch anything that will cause the result of that
expression to be greater than one. For now. If we ever go HDR, then the
constraints need to be re-thought. (Maybe. Actually I don't think chroma
can exceed 1.0 unless you have "darklight", i.e. negative RGB values.)
Additionally, treating arbitrarily-small values as 0 would be /wrong/,
since there is no constraint against having the not-quite-black color
1.7e-50, 1.4e-50, 1.9e-42. This color has a pretty strong chroma despite
that it's luma is ludicrously small.
So you can't just look at the denominator, you have to look at the result.
The original problem occurred due to loss of precision when the r,g,b
channels should have been (in fact, were) equal, but imprecise
conversion resulted in the luma being slightly off. The problem is
you're dealing with behavior near a singularity, which isn't easy to get
On a side note, this problem occurs due to trying to model light in an
LDR color space. Moving to HDR (assuming it is implemented correctly)
would make this problem go away :-).
> *** What if I know that my numbers are between -1 and 1, or even
> between 0 and 1, and i know furthermore that a number can be declared
> "negligible" as soon as it is negligible when compared to 1? (Note:
> this is the case in kcolorspaces.cpp)
No it isn't the case. The last assumption is wrong; see above.
"Who wants to sing?" -- Orcs (Warcraft II)
More information about the kde-core-devel