[cairo] Some other bits...

[Behdad Esfahbod]

On Wed, 17 Aug 2005, Keith Packard wrote:

> On Wed, 2005-08-17 at 00:24 -0400, Behdad Esfahbod wrote:
>
> > cairo_fixed_t
> >  _cairo_fixed_from_26_6 (uint32_t i)
> >  {
> > -    return i << 10;
> > +    return i << (CAIRO_FIXED_FLOAT_BITS - 10);
>
> This should be - 6, not - 10, I think.

Fixed here.  Thanks.


> > -       factor -= factor & 0xffff0000;
> > +       factor &= 0xffff;
> >         break;
> >      case CAIRO_EXTEND_REFLECT:
> >         if (factor < 0 || factor > 65536) {
> >             if ((factor >> 16) % 2)
> > -               factor = 65536 - (factor - (factor & 0xffff0000));
> > +               factor = 65536 - (factor & 0xffff);
> >             else
> > -               factor -= factor & 0xffff0000;
> > +               factor &= 0xffff;
> >         }
>
> Aren't all of these supposed to scale with CAIRO_FIXED_FLOAT_BITS?

Right.  I've not finished the work yet.  So far the
CAIRO_FIXED_FLOAT_BITS is only defined and used in cairo-fixed.c.
I thought I may first send a draft to see whether I should spend
more time in this direction.  This particular case is also easy
to fix, but cairo-traps.c is a lot harder.

> -    cairo_fixed_48_16_t a_dx = a->edge.p2.x - a->edge.p1.x;
> -    cairo_fixed_48_16_t a_dy = a->edge.p2.y - a->edge.p1.y;
> -    cairo_fixed_48_16_t b_dx = b->edge.p2.x - b->edge.p1.x;
> -    cairo_fixed_48_16_t b_dy = b->edge.p2.y - b->edge.p1.y;
> +    cairo_fixed_48_16_t a_dx = (cairo_fixed_48_16_t) a->edge.p2.x -
> a->edge.p1.x;
> +    cairo_fixed_48_16_t a_dy = (cairo_fixed_48_16_t) a->edge.p2.y -
> a->edge.p1.y;
> +    cairo_fixed_48_16_t b_dx = (cairo_fixed_48_16_t) b->edge.p2.x -
> b->edge.p1.x;
> +    cairo_fixed_48_16_t b_dy = (cairo_fixed_48_16_t) b->edge.p2.y -
> b->edge.p1.y;
>
> Why do you believe these need to be done in 64 bits? Are you concerned
> that the subtract might overflow?

That's what I thought, but you're right, overflow is probably
happening in many places already.  I was just searching for
cairo_fixed_*_*_t and thought 48_16 is used to avoid overflow
here, but I'm wrong.

> -keith

Thanks,

--behdad
http://behdad.org/