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

[Siarhei Siamashka]

On Friday 19 February 2010, Luca Barbato wrote:
> On 02/19/2010 12:31 PM, Siarhei Siamashka wrote:
> > 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.
> >
> > 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.
>
> The stddev variation seems to say that's not always true or at least the
> rounding changes are present.

Well, pixman_fixed_16_16_t should be able to handle numbers at least in 
[-32768.0, +32767.0] range, so any rounding errors should be irrelevant
here.

For radial gradients alone, this change should be completely safe unless I'm
mistaken. I'm not sure about other types of gradients, but the code itself
(explicit casts everywhere) suggests that the other types of gradients are
likely to be fine too.

> I wonder if that difference is worth having a configure switch to change
> the types so who is more concerned to precision (and has an output
> target large enough to have such difference perceptible) could use the
> larger formats and who would enjoy the speedup without concerns could
> use the smaller one.

I think it is preferable to have the same code on all platforms whenever
it is possible. If the gradients are proved to work safely with
pixman_fixed_16_16_t data type (and no platforms suffer any performance
penalty), then it is better to use it everywhere for better maintainability.

Maybe it even makes sense to get rid of fixed point part completely and do
all the calculations entirely in floating point for radial gradients. There
are too many options to evaluate :)

-- 
Best regards,
Siarhei Siamashka