On Thu, Jan 6, 2011 at 3:43 AM, Soeren Sandmann <sandm...@cs.au.dk> wrote:
> Søren Sandmann <sandm...@cs.au.dk> writes:
>
>> The following patch series changes the scanline access to be based on
>> iterators instead of direct calls to virtual functions. There are
>> several benefits to this:

It also provides a natural place to initialize the colorspace conversion :)
Or, much more likely to be useful in the short term, a place where
gradient walkers can be initialized (and the color function can be
sampled if the underlying implementation benefits from this).

>
> The patches are also available in the iterators3 branch here:
>
> http://cgit.freedesktop.org/~sandmann/pixman/log/?h=iterators3

Iterators gets initialized with a buffer which can always contain at least
"width" pixels. This is sufficient for current image types, but future image
types might need additional space.

Would it be possible to have the general_composite_rect ask to the iterator
about how much memory it will need (and provide a buffer with at least this
much memory)?
Or would it be better to have the iterator allocate that memory internally
upon initialization?
In this case, we should probably have the initializer return success/error
and also have some kind of iter_fini()

I think that it would be nice to make some additional promises about
what the code outside the iterator will do with the pointers it gets from
get_scanline. Having it in the code (i.e. returning const pointers) would
be good because the compiler would be able to warn us about incorrect
use, but it would require a different get_scanline_src and get_scanline_dst,
which isn't worth it as long as we only use iterators in a single place.

Apart from this, I skimmed through the patches and noticed that sometimes
a change is fixed or improved by a following patch  (example: patch 15/18
is immediately generalized by patch 16/18). Is this intentional?

I also have some comments on specific patches, which will follow.

Andrea

PS: Well done!
_______________________________________________
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Reply via email to