Need to flush twice to detach snapshots
@psychon
Submitted by Uli Schlachter Assigned to Chris Wilson @ickle
Description
Created attachment 118865 Reproducer
This comes originally from https://github.com/i3/i3/pull/1990.
The attached test program has the following behavior:
$ gcc -Wall -Wextra test.c $(pkg-config --cflags --libs cairo xcb) && ./a.out a.out: ../../../../src/cairo-surface.c:1652: cairo_surface_mark_dirty_rectangle: Assertion `! _cairo_surface_has_snapshots (surface)' failed. [1] 11287 abort ./a.out $ gcc -Wall -Wextra test.c $(pkg-config --cflags --libs cairo xcb) -DUSE_WORKAROUND && ./a.out $
This program creates a cairo-xcb surface (in a way so that RENDER is not used), draws to it (this causes a fallback) and then calls cairo_surface_flush(surf); cairo_surface_mark_dirty(surf);. As the failed assertion shows, the surface has snapshots attached during cairo_surface_mark_dirty() which we explicitly disallow via an assertion.
In the second run, the USE_WORKAROUND preprocessor symbol is defined. The only difference is that this program now calls cairo_surface_flush() twice in a row so that the second call can detach the snapshot that the first flush added. This snapshot is added in _cairo_xcb_surface_flush() and is used to save the fallback surface for later use.
As far as I can see, this bug always existed. No idea why no one noticed it so far (well, how many people don't have RENDER?).
@chris: Why are we detaching snapshots in cairo_surface_flush() anyway? Wouldn't it make more sense to detach them in cairo_surface_mark_dirty(), because that's the point when the snapshots become invalid. Changing this would break _cairo_surface_snapshot(), because it uses a snapshot to do a copy-on-write, but that might count as misuse of the API. This could in turn be fixed with some kind of "modification callback chain". What do you think?
Short term, I would just remove the call to _cairo_surface_attach_snapshot() from _cairo_xcb_surface_flush() to fix the symptom of this problem, but flush() has (to me) more of a "make my drawing visible"-semantics and not a "I will definitely draw to things directly now".
Attachment 118865, "Reproducer":
test.c