[cairo] GSoC: Scan converting rasteriser update

[Jeff Muizelaar]

On Mon, Oct 06, 2008 at 11:40:45AM -0400, Behdad Esfahbod wrote:
> Carl Worth wrote:
> > On Wed, 2008-10-01 at 15:19 -0400, Jeff Muizelaar wrote:
> >> Overall comments:
> >> - I don't really like the make_span_renderer interface. It might be better if
> >>   instead of relying on the fallback surface, we had helpers that would be called from
> >>   the image surface's fill and stroke methods.
> > 
> > Agreed.
> > 
> > This approach is a general one that could be applied liberally to
> > cairo's code base to clean it up considerably. That is, it would be nice
> > to have explicit code in each backend for each interface, calling into
> > specific helper functions, rather than just returning UNSUPPORTED and
> > relying on the fallback code to do something, (which might not be
> > optimal across all backends). And even if the fallback code doesn't
> > perform poorly, it makes the code much harder to read.
> 
> Actually I don't agree.  Having to make a call instead of just returning
> UNSUPPORTED is going to make the code for backends considerably uglier.  Right
> now one does ot even need to implement many of the methods.  Without
> UNSUPPORTED one has to copy paste a lot of cruft, that need to be changed in
> 10 places everytime the fallback implementation details change.

These implementation details are often important to a backend. For
example, the fallback path goes through a lot of work to deal with the
mismatch between SOURCE and CLEAR mask bounding that cairo and
pixman/XRender have. This wouldn't be necessary on other backends.

The UNSUPPORTED model becomes problematic as soon as a backend wants to
implement one of these methods previously unimplemented methods.
Fallbacks are a all-or-nothing choice, so implementing anything that
uses some of the previous fallback is difficult. This makes adjusting
the behaviour of a fallback difficult for a backend.

Further, I don't see why one would have to copy paste a lot of cruft
when we can have a utility function that's called. There's no reason a
fill() implementation would need to be anything more than a single
function call.

> I think the idiom is very readable and useful when you grasp it:  a function
> returning cairo_int_status_t can return UNSUPPORTED and not worry about what
> happens, while one returning cairo_status_t can't.  That simple, and a lot can
> be checked using static analysis.

That's not really true; you do need to worry about what happens. If you
have a backend that doesn't implement fill() you have to implement
composite_trapezoids(). composite_trapezoids() certainly isn't the only
logical operation to implement fill with, a backend end could, for
example, want composite_spans(), composite_flattened_polygon() or
composite_non_intersecting_polygon(). We wouldn't want to probe each
backend for each of these possible operations. 

Another example is stroke(). If you don't implement stroke() you also
have to implement composite_trapezoids().  This might make sense for
some backends, but others would prefer stroke() to be implemented in
terms of fill().

I don't really understand your comparison between returning UNSUPPORTED
and cairo_status_t though...

-Jeff