[Pixman] [PATCH/RFC] Use pixman_fixed_16_16_t type in _pixman_gradient_walker_{reset/pixel}

Soeren Sandmann sandmann at daimi.au.dk
Fri Feb 19 08:40:50 PST 2010


Hi,

> From: Siarhei Siamashka <siarhei.siamashka at nokia.com>
> 
> This argument is explicitly cast to 'int' or 'int32_t' anyway when it
> gets processed, except for 'if (pos < stops[n].x)' construct.

Even in that construct, the stops[n].x value is a 16.16 fixed
point. And some of the other similar constructs do first cast to
int32. 

> And at least the value coming from radial gradients processing code
> is expected to be in [0.0, 1.0], which should fit into
> pixman_fixed_16_16_t data type nicely.

Values outside of [0.0, 1.0] can be generated, and they are handled by
the repeat code in the gradient walker. However, it really does look
like there's some dumb stuff going on with gradients.

- The type in the gradient walker is pixman_fixed_32_32_t, but the
  code consistently casts it to int32 and treats it like 16.16.

- All the individual gradients cast the value to 48.16.

So the values first get computed in 48.16, then cast to 32.32, then
used as 16.16. It looks like all of this nonsense can just become
16.16 values without losing anything. 

If we do that, we might as well fix it up everywhere, and not just in
the radial code. We should make sure that the cairo test suite
continues to pass as well as it did before making this change, just to
make sure we didn't overlook anything.


Thanks,
Soren


More information about the Pixman mailing list