[cairo] _cairo_color_compute_shorts fails with FPU set to
behdad at behdad.org
Wed Aug 30 00:45:41 PDT 2006
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
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
> if (f == 1.0)
> i = N;
> 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:
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
> 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.
"Commandment Three says Do Not Kill, Amendment Two says Blood Will Spill"
-- Dan Bern, "New American Language"
More information about the cairo