[cairo] On fallback strategies (was Re: GSoC: Scan converting rasteriser update

[Behdad Esfahbod]

Jeff Muizelaar wrote:
> 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.

Using UNSUPPORTED does not mean we can't have
_cairo_image_fallback_do_some_part_of_filling_and_just_that().
It's not either/or.  Define UNSUPPORTED as calling cairo_surface_fallback_XXX().


> 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'm talking about that single-line function that needs to be copied in 12
different backend files, and modified every time the backend signature changes.

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

Sure, some operations may be better fallbacked using a function calling helper
functions.  But for show_glyphs for example, there's no way to do it cleaner
than using UNSUPPORTED.  The way it works currently is:

cairo_surface_show_glyphs:
	try surface_backend->show_glyphs
	cairo_surface_fallback_show_glyphs
		cairo_scaled_font_show_glyphs
			try font_backend->show_glyphs
			create a mask, add glyphs to it and paint it




Vladimir Vukicevic wrote:

> I agree with Carl here -- having the fallback invoked explicitly in
> the backend would lead to much simpler and clearer backend code.  The
> way that it's structured currently, there's magic that happens in
> multiple places, and you have to follow the UNSUPPORTED-returns back
> to wherever they were handled.  It's pretty confusing and hard to
> read, and also doesn't allow for some optimizations based on having
> more finer-grained control of when/how to invoke fallback code.

Currently the backend interface has like 15 methods, many backends implement
only five of them.  How does adding ten one-line functions make it more readable?

behdad