[Pixman] [PATCH 0/18] Add iterators to images

Soeren Sandmann sandmann at cs.au.dk
Sat Jan 15 16:14:53 PST 2011


Andrea Canciani <ranma42 at gmail.com> writes:

> I also tried to implement the "parameter, then color" idea:
> http://cgit.freedesktop.org/~ranma42/pixman/commit/?h=wip/itergradient&id=3bcd40c2a5842519167c6ab37d2ad4628667b108
> 
> It is currently far from perfect (in particular the "-1" hardcoded to clear...),
> but it seems to provide a small but consistent performance improvement.
> I put some TODOs related both to performance and correctness in the
> commit message. I forgot to mention that this would make it easy to
> really compute 16 bits components (thus having true wide support,
> instead of expanding the narrow result) by having two gradient walker
> parameter->color functions.

This looks like a very good idea to me. 

Aside from the performance improvement, the code is also somewhat
simpler. Passing the parameters as floating point, either single or
double precision would not only avoid the unnecessary conversion to
fixed-point, but also allow the "-1" issue to be solved since NaN
could be used for that purpose.

I'm less convinced about the destination buffer thing; I'll send some
comments on that later.

> > Overall, the patch set looks good. It's just not everything seems to be rosy
> > to me, I can see (and accept) some of the compromises it introduces.
> 
> I've been playing around with the iterators4 branch and it looks very
> promising to me, but I noticed two minor coding issues.
> 
> In 9f3e73a2f147265020466ee094abd868a77f52ed the new get_scanline
> functions  {conical,radial,linear_gradient}_get_scanline_* are added.
> I think that a more consistent naming would be better.
> In the same commit, _pixman_solid_fill_iter_init () is added twice in
> pixman-private.h

Thanks, I have fixed both issues in the iterators4 branch.


Soren


More information about the Pixman mailing list