[cairo] GSoC: Scan converting rasteriser update

M Joonas Pihlaja jpihlaja at cc.helsinki.fi
Wed Oct 29 09:43:04 PDT 2008


On Sat, 18 Oct 2008, Jeff Muizelaar wrote:

> Here are some more comments on your latest code:

Thanks.  I've taken your advice to heart and decrufted the code 
with this wonderful and slightly scary new tool I've learnt since 
upgrading git: rebase --interactive.  :)

http://cgit.freedesktop.org/~joonas/cairo/log/?h=spans-pruning

The short of it is: from 60 odd rambling commits to 13 with less 
rambling and more commit.  Distilling out the goodness took a lot 
longer than I expected on account of what I now realise were just 
numerous messy commits.  Mostly submarine white space fixes and 
tangential changes that had to be split out and moved about.

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

I've always had a separate branch for the stroke-to-polygon 
feature that I merged from into spans.  It's still available as 
the stroke-to-polygon branch in my repo, but on further 
consideration I've decided to pull the stroking changes 
from spans entirely.

Namely, the grumbling about the code having stroking to polygons 
and stroking to traps interleaved and harder to read is certainly 
valid.  The intention was more that the patch that adds the 
stroking to polygons be easy to read rather than the resulting 
code, but in retrospect that was just stupid of me.

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

Ew.. I'm not doing that.  That's nasty. :)

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

I think this is the way to go, but would require some shuffling 
of the stroker with planning this time.

> Cosmetic comments

Fixed all these.

I hope the new spans-pruning branch proves easier to review now 
that there's less irrelevant clutter in there.  Things I know 
about and intend to still do are:

- Get rid of cairo_delta_spans_t by rewriting the mono scan
converter to do its own bookkeeping.

- Sort out the 64 bit arithmetic issues and use cairo's 
implementations rather than rolling new ones on the spot.

- Rebase to master from 1.6.4.

Cheers,

Joonas


More information about the cairo mailing list