[cairo-commit] 5 commits - src/cairo-xcb-private.h src/cairo-xcb-screen.c src/cairo-xcb-surface-render.c src/cairo-xlib-xcb-surface.c test/Makefile.sources test/xcb-snapshot-assert.c test/xcb-snapshot-assert.ref.png
Uli Schlachter
psychon at kemper.freedesktop.org
Sat Jul 9 01:35:13 PDT 2011
src/cairo-xcb-private.h | 23 ++++++
src/cairo-xcb-screen.c | 13 +++
src/cairo-xcb-surface-render.c | 50 -------------
src/cairo-xlib-xcb-surface.c | 145 ++++++++++++++++++++++++++++++++++++++-
test/Makefile.sources | 1
test/xcb-snapshot-assert.c | 67 ++++++++++++++++++
test/xcb-snapshot-assert.ref.png |binary
7 files changed, 249 insertions(+), 50 deletions(-)
New commits:
commit 508990af8d83c83ae6ea0c3e66bd736d3446027d
Author: Uli Schlachter <psychon at znc.in>
Date: Fri Jul 8 21:31:47 2011 +0200
xcb: Don't use xcb surfaces as snapshots
Eventually someone might try to paint to the xcb surface again. However,
_cairo_surface_begin_modification doesn't like that:
cairo-surface.c:385: _cairo_surface_begin_modification: Assertion
`surface->snapshot_of == ((void *)0)' failed.
There was only a single place in the xcb backend where a cairo_xcb_surface_t
could be used as a snapshot, so the _cairo_surface_has_snapshot that checked for
such a surface can be removed, too.
This does *not* remove all snapshots from the xcb backend, but all the remaining
snapshots are instances of cairo_xcb_picture_t. These surfaces are only ever
created internally and thus can't be modified by users directly.
Fixes: xcb-snapshot-assert
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 e8d6c52..4f4b1dc 100644
--- a/src/cairo-xcb-surface-render.c
+++ b/src/cairo-xcb-surface-render.c
@@ -1016,17 +1016,6 @@ _cairo_xcb_surface_picture (cairo_xcb_surface_t *target,
cairo_xcb_picture_t *picture;
cairo_filter_t filter;
- {
- cairo_xcb_surface_t *snapshot;
-
- snapshot = (cairo_xcb_surface_t *)
- _cairo_surface_has_snapshot (source, &_cairo_xcb_surface_backend);
- if (snapshot != NULL) {
- if (snapshot->screen == target->screen)
- source = &snapshot->base;
- }
- }
-
picture = (cairo_xcb_picture_t *)
_cairo_surface_has_snapshot (source, &_cairo_xcb_picture_backend);
if (picture != NULL) {
@@ -2533,17 +2522,6 @@ _upload_image_inplace (cairo_xcb_surface_t *surface,
return CAIRO_INT_STATUS_UNSUPPORTED;
{
- cairo_xcb_surface_t *snapshot;
-
- snapshot = (cairo_xcb_surface_t *)
- _cairo_surface_has_snapshot (pattern->surface, &_cairo_xcb_surface_backend);
- if (snapshot != NULL) {
- if (snapshot->screen == surface->screen)
- return CAIRO_INT_STATUS_UNSUPPORTED;
- }
- }
-
- {
cairo_xcb_picture_t *snapshot;
snapshot = (cairo_xcb_picture_t *)
@@ -2609,12 +2587,6 @@ _upload_image_inplace (cairo_xcb_surface_t *surface,
_cairo_xcb_screen_put_gc (surface->screen, image->depth, gc);
_cairo_xcb_connection_release (surface->connection);
- if (surface->width == image->width && surface->height == image->height &&
- extents->width == image->width && extents->height == image->height)
- {
- _cairo_surface_attach_snapshot (&image->base, &surface->base, NULL);
- }
-
return CAIRO_STATUS_SUCCESS;
}
commit 5b8c01ec777538a110c5dc79fee04294b29f9721
Author: Uli Schlachter <psychon at znc.in>
Date: Fri Jul 8 22:04:29 2011 +0200
Add a test case that asserts on xcb
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/test/Makefile.sources b/test/Makefile.sources
index 7027389..0dee384 100644
--- a/test/Makefile.sources
+++ b/test/Makefile.sources
@@ -299,6 +299,7 @@ test_sources = \
user-font-rescale.c \
white-in-noop.c \
xcb-stress-cache.c \
+ xcb-snapshot-assert.c \
xcomposite-projection.c \
xlib-expose-event.c \
zero-alpha.c \
diff --git a/test/xcb-snapshot-assert.c b/test/xcb-snapshot-assert.c
new file mode 100644
index 0000000..e1ddb73
--- /dev/null
+++ b/test/xcb-snapshot-assert.c
@@ -0,0 +1,67 @@
+/*
+ * 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.h"
+#include "cairo-test.h"
+
+static cairo_surface_t *
+create_image (int width, int height)
+{
+ 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_set_source_rgb (cr, 0, 1, 1);
+ cairo_paint (cr);
+ cairo_destroy (cr);
+
+ return surface;
+}
+
+static cairo_test_status_t
+draw (cairo_t *cr, int width, int height)
+{
+ /* Image has to have same geometry as xcb surface to be added as a snapshot */
+ cairo_set_source_surface (cr, create_image (width, height), 0, 0);
+
+ /* This attaches the tested xcb surface as a snapshot */
+ cairo_paint (cr);
+
+ /* Now cairo is modifying a snapshot which fails an
+ * assert in _cairo_surface_begin_modification */
+ cairo_paint (cr);
+
+ return CAIRO_TEST_SUCCESS;
+}
+
+CAIRO_TEST (xcb_snapshot_assert,
+ "Test a wrong _cairo_surface_attach_snapshot call",
+ "xcb", /* keywords */
+ NULL, /* requirements */
+ 2, 2,
+ NULL, draw)
diff --git a/test/xcb-snapshot-assert.ref.png b/test/xcb-snapshot-assert.ref.png
new file mode 100644
index 0000000..850ce59
Binary files /dev/null and b/test/xcb-snapshot-assert.ref.png differ
commit 4153de46892b47b5b21fbef8939ef732935bfe03
Author: Uli Schlachter <psychon at znc.in>
Date: Fri Jul 8 16:51:19 2011 +0200
xcb: Track cairo_xcb_picture_t surfaces
When e.g. using an image surface as the source for a xcb surface, a
cairo_xcb_picture_t is created and attached to that image surface as a snapshot.
This contains the Picture that was created on the X11 server.
However, as soon as the cairo_xcb_picture_t's cairo_xcb_screen_t is finished and
destroyed, this picture can't be used anymore. This commit now makes sure all
these Pictures are freed when the screen is finished.
This was found because my X server's memory usage grew quite large. Every time
the app was done drawing, it destroyed its last surface which also destroyed the
last reference to the cairo_xcb_screen_t. This meant that the existing Picture
snapshots couldn't be used anymore, but they were still kept around and used up
memory until there wasn't any free memory left.
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/src/cairo-xcb-private.h b/src/cairo-xcb-private.h
index 82e05c3..3f085e0 100644
--- a/src/cairo-xcb-private.h
+++ b/src/cairo-xcb-private.h
@@ -123,6 +123,8 @@ struct _cairo_xcb_picture {
int x0, y0;
int x, y;
+
+ cairo_list_t link;
};
#if CAIRO_HAS_XLIB_XCB_FUNCTIONS
@@ -189,6 +191,7 @@ struct _cairo_xcb_screen {
cairo_list_t link;
cairo_list_t surfaces;
+ cairo_list_t pictures;
};
struct _cairo_xcb_connection {
diff --git a/src/cairo-xcb-screen.c b/src/cairo-xcb-screen.c
index b61f20b..efe42cf 100644
--- a/src/cairo-xcb-screen.c
+++ b/src/cairo-xcb-screen.c
@@ -62,6 +62,18 @@ _cairo_xcb_screen_finish (cairo_xcb_screen_t *screen)
cairo_surface_destroy (surface);
}
+ while (! cairo_list_is_empty (&screen->pictures)) {
+ cairo_surface_t *surface;
+
+ surface = &cairo_list_first_entry (&screen->pictures,
+ cairo_xcb_picture_t,
+ link)->base;
+
+ cairo_surface_reference (surface);
+ cairo_surface_finish (surface);
+ cairo_surface_destroy (surface);
+ }
+
for (i = 0; i < screen->solid_cache_size; i++)
cairo_surface_destroy (screen->solid_cache[i].picture);
@@ -231,6 +243,7 @@ _cairo_xcb_screen_get (xcb_connection_t *xcb_connection,
sizeof (struct pattern_cache_entry));
cairo_list_init (&screen->link);
cairo_list_init (&screen->surfaces);
+ cairo_list_init (&screen->pictures);
if (connection->flags & CAIRO_XCB_HAS_DRI2)
screen->device = _xcb_drm_device (xcb_connection, xcb_screen);
diff --git a/src/cairo-xcb-surface-render.c b/src/cairo-xcb-surface-render.c
index 0c31820..e8d6c52 100644
--- a/src/cairo-xcb-surface-render.c
+++ b/src/cairo-xcb-surface-render.c
@@ -85,6 +85,7 @@ _cairo_xcb_picture_finish (void *abstract_surface)
cairo_status_t status;
status = _cairo_xcb_connection_acquire (connection);
+ cairo_list_del (&surface->link);
if (unlikely (status))
return status;
@@ -125,6 +126,8 @@ _cairo_xcb_picture_create (cairo_xcb_screen_t *screen,
&screen->connection->device,
_cairo_content_from_pixman_format (pixman_format));
+ cairo_list_add (&surface->link, &screen->pictures);
+
surface->screen = screen;
surface->picture = _cairo_xcb_connection_get_xid (screen->connection);
surface->pixman_format = pixman_format;
commit e775db35d9306b74867f981a08d253562b15cffd
Author: Uli Schlachter <psychon at znc.in>
Date: Fri Jul 8 17:09:45 2011 +0200
xcb: Move cairo_xcb_picture_t to cairo-xcb-private.h
The next commit will make cairo-xcb-screen.c use this struct and add new
members. Splitting off the move into its own commits makes that easier to
understand.
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/src/cairo-xcb-private.h b/src/cairo-xcb-private.h
index 45b7bbe..82e05c3 100644
--- a/src/cairo-xcb-private.h
+++ b/src/cairo-xcb-private.h
@@ -63,6 +63,7 @@ typedef struct _cairo_xcb_connection cairo_xcb_connection_t;
typedef struct _cairo_xcb_font cairo_xcb_font_t;
typedef struct _cairo_xcb_screen cairo_xcb_screen_t;
typedef struct _cairo_xcb_surface cairo_xcb_surface_t;
+typedef struct _cairo_xcb_picture cairo_xcb_picture_t;
typedef struct _cairo_xcb_shm_mem_pool cairo_xcb_shm_mem_pool_t;
typedef struct _cairo_xcb_shm_info cairo_xcb_shm_info_t;
@@ -105,6 +106,25 @@ struct _cairo_xcb_surface {
cairo_list_t link;
};
+struct _cairo_xcb_picture {
+ cairo_surface_t base;
+
+ cairo_xcb_screen_t *screen;
+ xcb_render_picture_t picture;
+ xcb_render_pictformat_t xrender_format;
+ pixman_format_code_t pixman_format;
+
+ int width, height;
+
+ cairo_extend_t extend;
+ cairo_filter_t filter;
+ cairo_bool_t has_component_alpha;
+ xcb_render_transform_t transform;
+
+ int x0, y0;
+ int x, y;
+};
+
#if CAIRO_HAS_XLIB_XCB_FUNCTIONS
typedef struct _cairo_xlib_xcb_surface {
cairo_surface_t base;
diff --git a/src/cairo-xcb-surface-render.c b/src/cairo-xcb-surface-render.c
index ad7454f..0c31820 100644
--- a/src/cairo-xcb-surface-render.c
+++ b/src/cairo-xcb-surface-render.c
@@ -59,25 +59,6 @@
* extension if it is available.
*/
-typedef struct _cairo_xcb_picture {
- cairo_surface_t base;
-
- cairo_xcb_screen_t *screen;
- xcb_render_picture_t picture;
- xcb_render_pictformat_t xrender_format;
- pixman_format_code_t pixman_format;
-
- int width, height;
-
- cairo_extend_t extend;
- cairo_filter_t filter;
- cairo_bool_t has_component_alpha;
- xcb_render_transform_t transform;
-
- int x0, y0;
- int x, y;
-} cairo_xcb_picture_t;
-
static inline cairo_xcb_connection_t *
_picture_to_connection (cairo_xcb_picture_t *picture)
{
commit 5b9205cc52f50f997c9cd6c5a64faf783d83310f
Author: Uli Schlachter <psychon at znc.in>
Date: Sun Jul 3 17:45:58 2011 +0200
xlib-xcb: Register a XCloseDisplay hook
This commits makes the xlib-xcb backend produce its own cairo_device_t. This
per-Display* device is used to manage the registration of a XCloseDisplay hook
via XAddExtension/XESetCloseDisplay in the same way that the xlib backend does
this. The device is necessary to see if we already registered an extension.
This fixes weird errors when running cairo-test-suite with -a -s. They were
caused because the backend didn't see the XCloseDisplay and the next
XOpenDisplay happened to create a xcb_connection_t with the same address as the
last display. This caused the xcb backend to assume lots of wrongness.
This commit makes use of _cairo_xlib_display_mutex which is otherwise compiled
in but not used anywhere when xlib-xcb is enabled.
Patch v2: Fixed the xcb_device == NULL case and made sure the xcb_device is only
finished on XCloseDisplay, not when all xlib-xcb surfaces are destroyed.
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/src/cairo-xlib-xcb-surface.c b/src/cairo-xlib-xcb-surface.c
index 8fe0e50..0ffbc4c 100644
--- a/src/cairo-xlib-xcb-surface.c
+++ b/src/cairo-xlib-xcb-surface.c
@@ -47,6 +47,22 @@
#include "cairo-xlib-xrender-private.h"
#include <X11/Xlib-xcb.h>
+#include <X11/Xlibint.h> /* For XESetCloseDisplay */
+
+struct cairo_xlib_xcb_display_t {
+ cairo_device_t base;
+
+ Display *dpy;
+ cairo_device_t *xcb_device;
+ XExtCodes *codes;
+
+ cairo_list_t link;
+};
+typedef struct cairo_xlib_xcb_display_t cairo_xlib_xcb_display_t;
+
+/* List of all cairo_xlib_xcb_display_t alive,
+ * protected by _cairo_xlib_display_mutex */
+static cairo_list_t displays;
static cairo_surface_t *
_cairo_xlib_xcb_surface_create (void *dpy,
@@ -245,6 +261,124 @@ static const cairo_surface_backend_t _cairo_xlib_xcb_surface_backend = {
_cairo_xlib_xcb_surface_glyphs,
};
+static void
+_cairo_xlib_xcb_display_finish (void *abstract_display)
+{
+ cairo_xlib_xcb_display_t *display = (cairo_xlib_xcb_display_t *) abstract_display;
+
+ CAIRO_MUTEX_LOCK (_cairo_xlib_display_mutex);
+ cairo_list_del (&display->link);
+ CAIRO_MUTEX_UNLOCK (_cairo_xlib_display_mutex);
+
+ cairo_device_destroy (display->xcb_device);
+ display->xcb_device = NULL;
+
+ XESetCloseDisplay (display->dpy, display->codes->extension, NULL);
+}
+
+static int
+_cairo_xlib_xcb_close_display(Display *dpy, XExtCodes *codes)
+{
+ cairo_xlib_xcb_display_t *display;
+
+ CAIRO_MUTEX_LOCK (_cairo_xlib_display_mutex);
+ cairo_list_foreach_entry (display,
+ cairo_xlib_xcb_display_t,
+ &displays,
+ link)
+ {
+ if (display->dpy == dpy)
+ {
+ /* _cairo_xlib_xcb_display_finish will lock the mutex again
+ * -> deadlock (This mutex isn't recursive) */
+ cairo_device_reference (&display->base);
+ CAIRO_MUTEX_UNLOCK (_cairo_xlib_display_mutex);
+
+ /* Make sure the xcb and xlib-xcb devices are finished */
+ cairo_device_finish (display->xcb_device);
+ cairo_device_finish (&display->base);
+ cairo_device_destroy (&display->base);
+ return 0;
+ }
+ }
+ CAIRO_MUTEX_UNLOCK (_cairo_xlib_display_mutex);
+
+ return 0;
+}
+
+static const cairo_device_backend_t _cairo_xlib_xcb_device_backend = {
+ CAIRO_DEVICE_TYPE_XLIB,
+
+ NULL,
+ NULL,
+
+ NULL, /* flush */
+ _cairo_xlib_xcb_display_finish,
+ free, /* destroy */
+};
+
+static cairo_device_t *
+_cairo_xlib_xcb_device_create (Display *dpy, cairo_device_t *xcb_device)
+{
+ cairo_xlib_xcb_display_t *display = NULL;
+ cairo_device_t *device;
+
+ if (xcb_device == NULL)
+ return NULL;
+
+ CAIRO_MUTEX_INITIALIZE ();
+
+ CAIRO_MUTEX_LOCK (_cairo_xlib_display_mutex);
+ if (displays.next == NULL) {
+ cairo_list_init (&displays);
+ }
+
+ cairo_list_foreach_entry (display,
+ cairo_xlib_xcb_display_t,
+ &displays,
+ link)
+ {
+ if (display->dpy == dpy) {
+ /* Maintain MRU order. */
+ if (displays.next != &display->link)
+ cairo_list_move (&display->link, &displays);
+
+ /* Grab a reference for our caller */
+ device = cairo_device_reference (&display->base);
+ assert (display->xcb_device == xcb_device);
+ goto unlock;
+ }
+ }
+
+ display = malloc (sizeof (cairo_xlib_xcb_display_t));
+ if (unlikely (display == NULL)) {
+ device = _cairo_device_create_in_error (CAIRO_STATUS_NO_MEMORY);
+ goto unlock;
+ }
+
+ display->codes = XAddExtension (dpy);
+ if (unlikely (display->codes == NULL)) {
+ device = _cairo_device_create_in_error (CAIRO_STATUS_NO_MEMORY);
+ free (display);
+ goto unlock;
+ }
+
+ _cairo_device_init (&display->base, &_cairo_xlib_xcb_device_backend);
+
+ XESetCloseDisplay (dpy, display->codes->extension, _cairo_xlib_xcb_close_display);
+
+ display->dpy = dpy;
+ display->xcb_device = cairo_device_reference(xcb_device);
+
+ cairo_list_add (&display->link, &displays);
+ device = &display->base;
+
+unlock:
+ CAIRO_MUTEX_UNLOCK (_cairo_xlib_display_mutex);
+
+ return device;
+}
+
static cairo_surface_t *
_cairo_xlib_xcb_surface_create (void *dpy,
void *scr,
@@ -265,9 +399,12 @@ _cairo_xlib_xcb_surface_create (void *dpy,
_cairo_surface_init (&surface->base,
&_cairo_xlib_xcb_surface_backend,
- xcb->device,
+ _cairo_xlib_xcb_device_create (dpy, xcb->device),
xcb->content);
+ /* _cairo_surface_init() got another reference to the device, drop ours */
+ cairo_device_destroy (surface->base.device);
+
surface->display = dpy;
surface->screen = scr;
surface->visual = visual;
@@ -604,13 +741,15 @@ void
cairo_xlib_device_debug_set_precision (cairo_device_t *device,
int precision)
{
- cairo_xcb_device_debug_set_precision (device, precision);
+ cairo_xlib_xcb_display_t *display = (cairo_xlib_xcb_display_t *) device;
+ cairo_xcb_device_debug_set_precision (display->xcb_device, precision);
}
int
cairo_xlib_device_debug_get_precision (cairo_device_t *device)
{
- return cairo_xcb_device_debug_get_precision (device);
+ cairo_xlib_xcb_display_t *display = (cairo_xlib_xcb_display_t *) device;
+ return cairo_xcb_device_debug_get_precision (display->xcb_device);
}
#endif /* CAIRO_HAS_XLIB_XCB_FUNCTIONS */
More information about the cairo-commit
mailing list