[cairo-bugs] [Bug 92455] New: Need to flush twice to detach snapshots

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=92455

            Bug ID: 92455
           Summary: Need to flush twice to detach snapshots
           Product: cairo
           Version: unspecified
          Hardware: Other
                OS: All
            Status: NEW
          Severity: normal
          Priority: medium
         Component: xcb backend
          Assignee: chris at chris-wilson.co.uk
          Reporter: psychon at znc.in
        QA Contact: cairo-bugs at cairographics.org

Created attachment 118865
  --> https://bugs.freedesktop.org/attachment.cgi?id=118865&action=edit
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".

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cairographics.org/archives/cairo-bugs/attachments/20151014/bdc4222e/attachment.html>