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

[Siarhei Siamashka]

On Friday 19 February 2010, Soeren Sandmann wrote:
> 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.

Yes, gradients code is a bit inconsistent with the regards to data types,
so I was unsure about this patch in the first place.

About the values outside [0.0, 1.0] range. If float to int conversion does not 
fit the destination integer type, floating point exceptions may be triggered
or some other nasty effects happen. Maybe it can even cause troubles with the
current double -> int64_t conversion in some really artificial corner cases.

But I guess, the top limit for the 't' value can be estimated before running
the processing loop. So that we know beforehand whether it can always fit
16.16 fixed point value safely and can either use optimized variant of
function or fallback to a slower high precision variant.

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

Now it looks to me like more than 16.16 precision might make sense in some
cases. After all, you started to get rid of 16-bit limitations (on image
sizes) in other parts of pixman.

-- 
Best regards,
Siarhei Siamashka