[cairo-commit] 2 commits - src/cairo-xcb-surface-render.c test/Makefile.am test/Makefile.sources test/xcb-stress-cache.c test/xcb-stress-cache.ref.png

Uli Schlachter psychon at kemper.freedesktop.org
Mon Jan 17 13:21:50 PST 2011


 src/cairo-xcb-surface-render.c |    2 
 test/Makefile.am               |    1 
 test/Makefile.sources          |    1 
 test/xcb-stress-cache.c        |  118 +++++++++++++++++++++++++++++++++++++++++
 test/xcb-stress-cache.ref.png  |binary
 5 files changed, 121 insertions(+), 1 deletion(-)

New commits:
commit 7f83b4e949b85fc604e9a7841c566eebd0aa1452
Author: Uli Schlachter <psychon at znc.in>
Date:   Mon Jan 17 18:38:16 2011 +0100

    xcb: Don't finish snapshots when they are detached
    
    Some code might own a reference to the snapshot when it is
    detached. For this reason, we shouldn't finish the snapshot except
    when its reference count drops to zero.
    
    This avoids destroying source patterns which get evicted from the
    cache while acquiring the mask.
    
    Fixes xcb-stress-cache.
    
    Big "thank you" to Andrea Canciani for helping in figuring this one out.
    
    Signed-off-by: Uli Schlachter <psychon at znc.in>

diff --git a/src/cairo-xcb-surface-render.c b/src/cairo-xcb-surface-render.c
index 1a71996..425473a 100644
--- a/src/cairo-xcb-surface-render.c
+++ b/src/cairo-xcb-surface-render.c
@@ -1308,7 +1308,7 @@ _cairo_xcb_surface_picture (cairo_xcb_surface_t *target,
 
     _cairo_surface_attach_snapshot (source,
 				    &picture->base,
-				    cairo_surface_finish);
+				    NULL);
 
 setup_picture:
     filter = pattern->base.filter;
commit 44095f3dde22b2c379902e56adc47408b630c5e7
Author: Uli Schlachter <psychon at znc.in>
Date:   Sun Jan 16 20:52:01 2011 +0100

    Add a test case for a bug in the xcb backend
    
    This was found via cairo-perf-micro which sometimes triggered this bug in its
    mask-similar_image-* test.
    
    Signed-off-by: Uli Schlachter <psychon at znc.in>

diff --git a/test/Makefile.am b/test/Makefile.am
index 8914f07..3704704 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -1429,6 +1429,7 @@ REFERENCE_IMAGES = \
 	user-font.ref.png \
 	user-font.svg.ref.png \
 	user-font.xlib.ref.png \
+	xcb-stress-cache.ref.png \
 	xcb-surface-source.rgb24.ref.png \
 	xcb-surface-source.argb32.ref.png \
 	xcomposite-projection.ref.png \
diff --git a/test/Makefile.sources b/test/Makefile.sources
index 76a5273..279b228 100644
--- a/test/Makefile.sources
+++ b/test/Makefile.sources
@@ -294,6 +294,7 @@ test_sources = \
 	user-font-proxy.c				\
 	user-font-rescale.c				\
 	white-in-noop.c					\
+	xcb-stress-cache.c				\
 	xcomposite-projection.c				\
 	xlib-expose-event.c 				\
 	zero-alpha.c					\
diff --git a/test/xcb-stress-cache.c b/test/xcb-stress-cache.c
new file mode 100644
index 0000000..3c7ee15
--- /dev/null
+++ b/test/xcb-stress-cache.c
@@ -0,0 +1,118 @@
+/*
+ * Copyright © 2011 Uli Schlachter
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy,
+ * modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Author: Uli Schlachter <psychon at znc.in>
+ */
+
+#include "cairo-test.h"
+
+#define WIDTH 512
+#define HEIGHT 512
+
+/* This is a random pick. This results in 512 (width) * 512 (height) *
+ * 2 (surfaces per run) * 4 (ARGB32) * ITERATIONS = 200 MiB of image data. */
+#define ITERATIONS 100
+
+/* This tries to trigger a bug in the xcb backend where a Picture is freed to
+ * early. It goes something like this:
+ *
+ * - _composite_mask calls _cairo_xcb_picture_for_pattern to get a xcb_picture_t
+ *   for the source.
+ * - _cairo_xcb_picture_for_pattern calls _cairo_xcb_surface_picture which calls
+ *   _cairo_xcb_screen_store_surface_picture which adds the picture to a cache.
+ * - _cairo_xcb_surface_picture also attached the picture as a snapshot to
+ *   the source surface using cairo_surface_finish as detach_func.
+ * - _composite_mask calls _cairo_xcb_picture_for_pattern to get a xcb_picture_t
+ *   for the mask.
+ * - The resulting picture surface is added to the cache again, but the cache is
+ *   already full, so a random cache entry is picked and removed.
+ * - The surface that was added before is picked and gets fed to
+ *   _surface_cache_entry_destroy.
+ * - This calls _cairo_surface_detach_snapshot which causes the
+ *   detach_func from above to be called, so the surface is finished and the
+ *   associated picture is FreePicture'd.
+ * - _composite_mask now uses a Picture that was already freed.
+ *
+ * So this depends on the screen's surface cache to be full which is why we do
+ * all this looping.
+ */
+
+static cairo_surface_t *
+create_image ()
+{
+    cairo_surface_t *surface;
+    cairo_t *cr;
+
+    surface = cairo_image_surface_create (CAIRO_FORMAT_ARGB32, WIDTH, HEIGHT);
+    /* Paint something random to the image */
+    cr = cairo_create (surface);
+    cairo_paint (cr);
+    cairo_set_source_rgb (cr, 0, 1, 1);
+    cairo_rectangle (cr, 0, 0, WIDTH/2.0, HEIGHT/2.0);
+    cairo_fill (cr);
+    cairo_set_source_rgb (cr, 1, 0, 1);
+    cairo_rectangle (cr, WIDTH/2.0, HEIGHT/2.0, WIDTH/2.0, HEIGHT/2.0);
+    cairo_fill (cr);
+    cairo_destroy (cr);
+
+    return surface;
+}
+
+static cairo_surface_t *
+dirty_cache (cairo_t *cr)
+{
+    cairo_surface_t *surface;
+
+    /* Set a source surface... */
+    surface = create_image ();
+    cairo_set_source_surface (cr, surface, 0, 0);
+    cairo_surface_destroy (surface);
+    /* ...and create a mask surface, so that we can hit the early FreePicture */
+    surface = create_image ();
+    cairo_mask_surface (cr, surface, 0, 0);
+
+    return surface;
+}
+
+static cairo_test_status_t
+draw (cairo_t *cr, int width, int height)
+{
+    int i;
+    cairo_surface_t *array[ITERATIONS];
+
+    /* We have to keep the associated cairo_surface_t alive so that they aren't
+     * removed from the cache */
+    for (i = 0; i < ITERATIONS; i++)
+	array[i] = dirty_cache (cr);
+    for (i = 0; i < ITERATIONS; i++)
+	cairo_surface_destroy (array[i]);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+CAIRO_TEST (xcb_stress_cache,
+	    "Stress test for a image surface cache in cairo-xcb",
+	    "xcb, stress", /* keywords */
+	    NULL, /* requirements */
+	    2, 2,
+	    NULL, draw)
diff --git a/test/xcb-stress-cache.ref.png b/test/xcb-stress-cache.ref.png
new file mode 100644
index 0000000..850ce59
Binary files /dev/null and b/test/xcb-stress-cache.ref.png differ


More information about the cairo-commit mailing list