[cairo] _cairo_color_compute_shorts fails with FPU set to single precision

[Behdad Esfahbod]

On Wed, 2006-08-30 at 02:00 -0400, Carl Worth wrote:
> So we need to tweak the computation very slightly in order to ensure
> that 1.0 maps correctly. The current implementation of
> _cairo_color_compute_shorts attempts a tweak by using the following
> computation instead:
> 
> 	i = floor (f * (N+1-epsilon))
> 
> In this particular case, we're mapping to a 16-bit integer so N is
> 65535. And in the current implementation the value for epsilon is
> 1e-5, (chosen as something near 1/65535 I suppose).

Not really.  1e-5 is actually a (1e-5/65536) epsilon.  It's just as you
said, very small yet representable in doubles.


> Behdad has cooked up some patches that attempt to address this by
> using a slightly different approach of:
> 
> 	i = floor (f * ((N+1) * (1.0 - epsilon)))
> 
> Where the code actually uses DBL_EPSILON as defined by float.h for its
> epsilon value, (or 1e-7 if DBL_EPSILON is not found in a system
> header). This seems like an improvement at first, but it won't
> actually help with the original bug report which was triggered by
> setting the FPU to single-precision mode at run time. So getting a
> compile-time constant based on what float.h advertises won't help I
> think.

Good point.  I ignored the question of how FPU was set to
single-precision in the first place.  In the back of my mind I assumed
that someone's using 32bit types as doubles on some weird platform.


> I'm open to suggestions on how to correctly fix this bug.
> 
> One idea I have is to simply special case the one value we know is
> problematic:
> 
> 	if (f == 1.0)
> 	    i = N;
> 	else
> 	    i = floor (f * (N+1));

I think this is way better than the current code and my proposal.


> The branch is certainly not lovely, but I'm hoping that this

Or better yet, use what Ali suggests.  Or even without a branch:

        int32_t i;
        i = f * 65536.0;
        i -= i == 65536;

> floating-to-fixed conversion for colors won't be a hot spot, (unlike
> the floating-to-fixed conversion of _cairo_fixed_from_double which is
> used much more often and is already a known hot spot for some
> architectures).
> 
> Meanwhile, since we know we're dealing with non-negative values here,
> can't we just drop the floor() and rely on the rounding-toward-zero
> truncation mandated by the C specification?

I think we can.

> -Carl

-- 
behdad
http://behdad.org/

"Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill"
        -- Dan Bern, "New American Language"