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

[Matthew Woehlke]

(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 
even slower...)

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 
right.

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.

-- 
Matthew
"Who wants to sing?" -- Orcs (Warcraft II)