From 02a38df61976f6bbd1e5d2555a182e0a1411de57 Mon Sep 17 00:00:00 2001 From: Christian Persch Date: Wed, 13 Oct 2010 13:14:11 +0200 Subject: Make sure the surfaces own their pixels When creating a surface for pixel data, make sure the surface keeps a reference to the pixels, either by directly owning them or by keeping a reference to the GdkPixbuf owning them. This fixes a crash when rendering some SVG files (e.g. [1]) to a recording or pdf surface. Fixes https://bugs.freedesktop.org/show_bug.cgi?id=30071 . ==27565== Unaddressable byte(s) found during client check request ==27565== at 0x427E2C0: _cairo_debug_check_image_surface_is_defined (cairo-debug.c:125) ==27565== by 0x42B5749: _cairo_surface_acquire_source_image (cairo-surface.c:1447) ==27565== by 0x42BC119: _cairo_surface_snapshot_copy_on_write (cairo-surface-snapshot.c:125) ==27565== by 0x42B407E: _cairo_surface_detach_snapshot (cairo-surface.c:329) ==27565== by 0x42B3FE9: _cairo_surface_detach_snapshots (cairo-surface.c:314) ==27565== by 0x42B49D0: cairo_surface_finish (cairo-surface.c:715) ==27565== by 0x42B48EF: cairo_surface_destroy (cairo-surface.c:645) ==27565== by 0x42A16DA: _cairo_pattern_fini (cairo-pattern.c:346) ==27565== by 0x42A21D2: cairo_pattern_destroy (cairo-pattern.c:828) ==27565== by 0x4281FD8: _cairo_gstate_fini (cairo-gstate.c:229) ==27565== by 0x428211F: _cairo_gstate_restore (cairo-gstate.c:290) ==27565== by 0x4276D86: cairo_restore (cairo.c:583) ==27565== by 0x40390B0: rsvg_cairo_pop_discrete_layer (rsvg-cairo-draw.c:1003) ==27565== by 0x40380CD: rsvg_cairo_render_path (rsvg-cairo-draw.c:639) ==27565== by 0x4035C4D: rsvg_render_path (rsvg-base.c:2067) ==27565== by 0x40287FE: _rsvg_node_rect_draw (rsvg-shapes.c:445) ==27565== by 0x4029E89: rsvg_node_draw (rsvg-structure.c:69) ==27565== by 0x4029F34: _rsvg_node_draw_children (rsvg-structure.c:87) ==27565== by 0x4029E89: rsvg_node_draw (rsvg-structure.c:69) ==27565== by 0x402A9A9: rsvg_node_svg_draw (rsvg-structure.c:326) ==27565== by 0x4029E89: rsvg_node_draw (rsvg-structure.c:69) ==27565== by 0x4039D49: rsvg_handle_render_cairo_sub (rsvg-cairo-render.c:234) ==27565== by 0x4039DA1: rsvg_handle_render_cairo (rsvg-cairo-render.c:256) ==27565== by 0x804A06A: main (rsvg-convert.c:319) ==27565== Address 0x6c6b028 is not stack'd, malloc'd or (recently) free'd [1] http://websvn.kde.org/*checkout*/trunk/KDE/kdegames/libkdegames/carddecks/svg-oxygen-white/oxygen-white.svgz?revision=896352 --- rsvg-cairo-draw.c | 92 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 59 insertions(+), 33 deletions(-) (limited to 'rsvg-cairo-draw.c') diff --git a/rsvg-cairo-draw.c b/rsvg-cairo-draw.c index f15589a6..e42b2985 100644 --- a/rsvg-cairo-draw.c +++ b/rsvg-cairo-draw.c @@ -42,6 +42,8 @@ #include +static const cairo_user_data_key_t surface_pixel_data_key; + static void _rsvg_cairo_set_shape_antialias (cairo_t * cr, ShapeRenderingProperty aa) { @@ -655,13 +657,16 @@ rsvg_cairo_render_image (RsvgDrawingCtx * ctx, const GdkPixbuf * pixbuf, guchar *cairo_pixels; cairo_format_t format; cairo_surface_t *surface; - static const cairo_user_data_key_t key; int j; RsvgBbox bbox; if (pixbuf == NULL) return; + cairo_pixels = g_try_malloc (4 * width * height); + if (!cairo_pixels) + return; + rsvg_bbox_init (&bbox, state->affine); bbox.x = pixbuf_x; bbox.y = pixbuf_y; @@ -679,13 +684,9 @@ rsvg_cairo_render_image (RsvgDrawingCtx * ctx, const GdkPixbuf * pixbuf, else format = CAIRO_FORMAT_ARGB32; - cairo_pixels = g_try_malloc (4 * width * height); - if (!cairo_pixels) - return; - surface = cairo_image_surface_create_for_data ((unsigned char *) cairo_pixels, format, width, height, 4 * width); - cairo_surface_set_user_data (surface, &key, cairo_pixels, (cairo_destroy_func_t) g_free); + cairo_surface_set_user_data (surface, &surface_pixel_data_key, cairo_pixels, (cairo_destroy_func_t) g_free); for (j = height; j; j--) { guchar *p = gdk_pixels; @@ -777,6 +778,10 @@ rsvg_cairo_generate_mask (cairo_t * cr, RsvgMask * self, RsvgDrawingCtx * ctx, R double sx, sy, sw, sh; gboolean nest = cr != render->initial_cr; + pixels = g_try_malloc0 (height * rowstride); + if (pixels == NULL) + return; + if (self->maskunits == objectBoundingBox) _rsvg_push_view_box (ctx, 1, 1); @@ -788,9 +793,9 @@ rsvg_cairo_generate_mask (cairo_t * cr, RsvgMask * self, RsvgDrawingCtx * ctx, R if (self->maskunits == objectBoundingBox) _rsvg_pop_view_box (ctx); - pixels = g_new0 (guint8, height * rowstride); surface = cairo_image_surface_create_for_data (pixels, CAIRO_FORMAT_ARGB32, width, height, rowstride); + cairo_surface_set_user_data (surface, &surface_pixel_data_key, pixels, (cairo_destroy_func_t) g_free); mask_cr = cairo_create (surface); save_cr = render->cr; @@ -847,7 +852,6 @@ rsvg_cairo_generate_mask (cairo_t * cr, RsvgMask * self, RsvgDrawingCtx * ctx, R nest ? 0 : render->offset_x, nest ? 0 : render->offset_y); cairo_surface_destroy (surface); - g_free (pixels); } static void @@ -887,22 +891,31 @@ rsvg_cairo_push_render_stack (RsvgDrawingCtx * ctx) else { guchar *pixels; int rowstride = render->width * 4; - pixels = g_new0 (guint8, render->width * render->height * 4); + GdkPixbuf *pixbuf; + + pixels = g_try_malloc0 (render->width * render->height * 4); + if (pixels == NULL) + return; /* not really correct, but the best we can do here */ + + /* The pixbuf takes ownership of @pixels */ + pixbuf = gdk_pixbuf_new_from_data (pixels, + GDK_COLORSPACE_RGB, + TRUE, + 8, + render->width, + render->height, + rowstride, + (GdkPixbufDestroyNotify) rsvg_pixmap_destroy, + NULL); + render->pixbuf_stack = g_list_prepend (render->pixbuf_stack, pixbuf); surface = cairo_image_surface_create_for_data (pixels, CAIRO_FORMAT_ARGB32, render->width, render->height, rowstride); - render->pixbuf_stack = - g_list_prepend (render->pixbuf_stack, - gdk_pixbuf_new_from_data (pixels, - GDK_COLORSPACE_RGB, - TRUE, - 8, - render->width, - render->height, - rowstride, - (GdkPixbufDestroyNotify) rsvg_pixmap_destroy, - NULL)); + /* Also keep a reference to the pixbuf which owns the pixels */ + cairo_surface_set_user_data (surface, &surface_pixel_data_key, + g_object_ref (pixbuf), + (cairo_destroy_func_t) g_object_unref); } child_cr = cairo_create (surface); cairo_surface_destroy (surface); @@ -929,7 +942,6 @@ rsvg_cairo_pop_render_stack (RsvgDrawingCtx * ctx) RsvgCairoRender *render = (RsvgCairoRender *) ctx->render; cairo_t *child_cr = render->cr; gboolean lateclip = FALSE; - GdkPixbuf *output = NULL; cairo_surface_t *surface = NULL; RsvgState *state = rsvg_current_state (ctx); gboolean nest; @@ -945,6 +957,7 @@ rsvg_cairo_pop_render_stack (RsvgDrawingCtx * ctx) if (state->filter) { GdkPixbuf *pixbuf = render->pixbuf_stack->data; + GdkPixbuf *output; render->pixbuf_stack = g_list_remove (render->pixbuf_stack, pixbuf); @@ -957,6 +970,10 @@ rsvg_cairo_pop_render_stack (RsvgDrawingCtx * ctx) gdk_pixbuf_get_width (output), gdk_pixbuf_get_height (output), gdk_pixbuf_get_rowstride (output)); + cairo_surface_set_user_data (surface, &surface_pixel_data_key, + g_object_ref (output), + (cairo_destroy_func_t) g_object_unref); + } else surface = cairo_get_target (child_cr); @@ -991,7 +1008,6 @@ rsvg_cairo_pop_render_stack (RsvgDrawingCtx * ctx) render->bb_stack = g_list_delete_link (render->bb_stack, render->bb_stack); if (state->filter) { - g_object_unref (output); cairo_surface_destroy (surface); } } @@ -1031,9 +1047,29 @@ rsvg_cairo_get_image_of_node (RsvgDrawingCtx * ctx, RsvgCairoRender *render; rowstride = width * 4; - pixels = g_new0 (guint8, width * height * 4); + pixels = g_try_malloc0 (width * height * 4); + if (pixels == NULL) + return NULL; + + /* no colorspace conversion necessary. this is just a convenient + holder of ARGB data with a width, height, & stride */ + img = gdk_pixbuf_new_from_data (pixels, + GDK_COLORSPACE_RGB, + TRUE, + 8, + width, + height, + rowstride, + (GdkPixbufDestroyNotify) rsvg_pixmap_destroy, + NULL); + surface = cairo_image_surface_create_for_data (pixels, CAIRO_FORMAT_ARGB32, width, height, rowstride); + /* Also keep a reference to the pixbuf which owns the pixels */ + cairo_surface_set_user_data (surface, &surface_pixel_data_key, + g_object_ref (img), + (cairo_destroy_func_t) g_object_unref); + cr = cairo_create (surface); cairo_surface_destroy (surface); @@ -1044,16 +1080,6 @@ rsvg_cairo_get_image_of_node (RsvgDrawingCtx * ctx, rsvg_node_draw (drawable, ctx, 0); rsvg_state_pop (ctx); - /* no colorspace conversion necessary. this is just a convenient - holder of ARGB data with a width, height, & stride */ - img = gdk_pixbuf_new_from_data (pixels, - GDK_COLORSPACE_RGB, - TRUE, - 8, - width, - height, - rowstride, (GdkPixbufDestroyNotify) rsvg_pixmap_destroy, NULL); - cairo_destroy (cr); rsvg_render_free (ctx->render); -- cgit v1.2.1