[cairo] GSoC: Scan converting rasteriser update

[Jeff Muizelaar]

On Wed, Oct 01, 2008 at 03:19:04PM -0400, Jeff Muizelaar wrote:
> On Fri, Aug 29, 2008 at 02:29:07AM +0300, M Joonas Pihlaja wrote:
> > So yeah, please check out the code and come complain to me.  The
> > code is available, as always, in the spans branch of
> > http://gitweb.freedesktop.org/?p=users/joonas/cairo
> 
> Here are some comments based on a look though some of the new code a while ago:

[snip]

Here are some more comments on your latest code:

First, I think you should separate out the stroke to polygon changes. If
possible, I'd suggest keeping them in a separate branch that the spans
branch could work on top of and merge as needed.

It looks like cairo_stroker_init is overloaded to do two things now:
stroke to traps or stroke to a polygon. Is there ever a case where
stroker->traps and stro ker->polygnon would both be non-null? If so, is
it worth optimizing for doing both at the same time? If not, the
to_polygon stroker and the to_traps stroker should be futher separated.
Keeping them interleaved makes the code harder to read and ensure
correctness because of all the 'if (stroker->polygon)' and 'if
(stroker->traps)' checks.

I'm not sure how easy separating the two strokers will be, as it looks
like they share a fair amount in common. However, a good first step
could be to make a copy of the existing code, change it to stroke to
polygons and then see what common functions can be factored out. If
there's still a lot of duplication then it might be worth implementing
some sort of stroker destination object that has virtual methods for the
different operations.

Cosmetic comments
------------------

No need to have (cairo_polygon_t*)NULL casts.  I can see a documentation
value in having the casts but I don't think it is worth it. The case
that I saw would go away with changes above anyways...

cairo-spans-private.h should use cairo_private directly instead of doing
'#define I cairo_private'.

-Jeff