[cairo] changed semantics of cairo_destroy

[Maarten Bosmans]

2010/8/31 M Joonas Pihlaja <jpihlaja at cc.helsinki.fi>:
>
> On Mon, 30 Aug 2010, Maarten Bosmans wrote:
>
>> Last week I chased down a bug that was causing rendering artifacts in
>> Gtk+ when used with a recent Cairo snapshot. It bisected to this
>> commit in Cairo:
>> http://cgit.freedesktop.org/cairo/commit/?id=4438cc6a49e7e902dce045706f7125a2c3e2174b
>> (Gtk bug report is here: https://bugzilla.gnome.org/show_bug.cgi?id=628291)
>
> It's not clear from the Gtk bug report how the rendering was
> incorrect.  From looking at the code, the main difference from
> flushing a win32 surface vs. not, is that flushing resets the clip on
> the drawing context.  Could the artifacts be due to missing/misclipped
> native rendering?

The main problems I could see was missing text in cellrenderers and
wrong highlighting states on buttons, for example a  button would stay
highlighted after going over it with the pointer, so after a while you
end up with all highlighted buttons in your app.

As I'm not sure where all the native drawing gets done in Gtk, I can't
say if its because of misclipped native rendering.

>> I think this change warrants an explicit notice in the release notes
>> for 1.10, as it is certainly something for Cairo users to be aware of
>> and I didn't see the change announced in one of the 1.9.x snapshot
>> notes.
>
> We discussed this change (on the irc channel #cairo, not on the
> mailing list, sorry!) and decided in the end to drop flushing from
> cairo_destroy().  As far as I recall the main motivation was that
> applications are making lots of very short lived cairo_t objects and
> flushing the surface was causing a lot of needless overhead,
> especially considering that apps should be using cairo_surface_flush()
> in all the right places anyway (since cairo 0.9.0.)  The feeling was
> that it's not technically a change in the API since the
> cairo_destroy() documentation doesn't actually say anything about
> flushing surfaces, even if it is an actual change in behaviour.

Note that I am not questioning the change in Cairo, as it is obviously
Gtk here that misused the side-effects from destroying a cairo_t. It's
just that it is a change user of Cairo should be aware of, so they
know how to fix their app (or toolkit).

>> By the way, I proposed a patch in Gtk+ to overcome this problem by adding a
>>     cairo_surface_flush (cairo_get_target (cr));
>> before every
>>     cairo_destroy (cr);
>> where relevant. Is this the correct approach (it certainly resolves
>> the bug I was seeing),
>
> Yes, this would be a correct fix.  An even better fix would be to
> flush the surface only before doing any non-cairo rendering.

Yeah, I understand. I was really going for the quick fix here though,
as Gtk3 is quite different in this respect, as Benjamin already said.

>> or should there be something done with
>> cairo_device_t?
>
> No, there's nothing to do with a cairo_device_t that would cause the
> surface to flush, because the win32 surfaces of cairo don't have a
> device.
>
> Joonas

Maarten