[PATCH] [ps/pdf] Copy the old clip before emitting the meta surface.

Chris Wilson chris at chris-wilson.co.uk
Wed Apr 16 07:08:23 PDT 2008


The surface->clip is modified in place and so we cannot just keep a
pointer to the old clip whilst replaying a meta surface (as it may
modify the contents of old_clip).

This was first reported as
https://bugzilla.mozilla.org/show_bug.cgi?id=429071.

The issue with the old clip was identified by valgrind:
==28204== Conditional jump or move depends on uninitialised value(s)
==28204==    at 0x560587E: _cairo_surface_set_clip (cairo-surface.c:2016)
==28204==    by 0x561E489: _cairo_pdf_surface_emit_pattern (cairo-pdf-surface.c:1549)
==28204==    by 0x561FCEE: _cairo_pdf_surface_show_page (cairo-pdf-surface.c:3923)
---
 src/cairo-pdf-surface.c |   53 +++++++++++++++++++++++++++++++++++-----------
 src/cairo-ps-surface.c  |   24 +++++++++++++++++---
 src/cairo-surface.c     |    1 +
 3 files changed, 61 insertions(+), 17 deletions(-)

diff --git a/src/cairo-pdf-surface.c b/src/cairo-pdf-surface.c
index f82af83..1c7a769 100644
--- a/src/cairo-pdf-surface.c
+++ b/src/cairo-pdf-surface.c
@@ -42,6 +42,7 @@
 #define _BSD_SOURCE /* for snprintf() */
 #include "cairoint.h"
 #include "cairo-pdf.h"
+#include "cairo-clip-private.h"
 #include "cairo-pdf-surface-private.h"
 #include "cairo-pdf-operators-private.h"
 #include "cairo-scaled-font-subsets-private.h"
@@ -1494,9 +1495,9 @@ _cairo_pdf_surface_emit_meta_surface (cairo_pdf_surface_t  *surface,
 {
     double old_width, old_height;
     cairo_paginated_mode_t old_paginated_mode;
-    cairo_clip_t *old_clip;
+    cairo_clip_t old_clip, *clip;
     cairo_rectangle_int_t meta_extents;
-    cairo_status_t status, status2;
+    cairo_status_t status;
     int alpha = 0;
 
     status = _cairo_surface_get_extents (meta_surface, &meta_extents);
@@ -1506,7 +1507,12 @@ _cairo_pdf_surface_emit_meta_surface (cairo_pdf_surface_t  *surface,
     old_width = surface->width;
     old_height = surface->height;
     old_paginated_mode = surface->paginated_mode;
-    old_clip = _cairo_surface_get_clip (&surface->base);
+    clip = _cairo_surface_get_clip (&surface->base);
+    if (clip != NULL) {
+	status = _cairo_clip_init_deep_copy (&old_clip, clip, &surface->base);
+	if (status)
+	    return status;
+    }
     _cairo_pdf_surface_set_size_internal (surface,
 					  meta_extents.width,
 					  meta_extents.height);
@@ -1517,14 +1523,20 @@ _cairo_pdf_surface_emit_meta_surface (cairo_pdf_surface_t  *surface,
     surface->paginated_mode = CAIRO_PAGINATED_MODE_RENDER;
     _cairo_pdf_group_resources_clear (&surface->resources);
     status = _cairo_pdf_surface_open_content_stream (surface, TRUE);
-    if (status)
+    if (status) {
+	if (clip != NULL)
+	    _cairo_clip_reset (&old_clip);
 	return status;
+    }
 
     *resource = surface->content;
     if (cairo_surface_get_content (meta_surface) == CAIRO_CONTENT_COLOR) {
 	status = _cairo_pdf_surface_add_alpha (surface, 1.0, &alpha);
-	if (status)
-	    goto CLEANUP_GROUP;
+	if (status) {
+	    if (clip != NULL)
+		_cairo_clip_reset (&old_clip);
+	    return status;
+	}
 
 	_cairo_output_stream_printf (surface->output,
 				     "q /a%d gs 0 0 0 rg 0 0 %f %f re f Q\n",
@@ -1536,21 +1548,36 @@ _cairo_pdf_surface_emit_meta_surface (cairo_pdf_surface_t  *surface,
     status = _cairo_meta_surface_replay_region (meta_surface, &surface->base,
 						CAIRO_META_REGION_NATIVE);
     assert (status != CAIRO_INT_STATUS_UNSUPPORTED);
-    if (status)
-	goto CLEANUP_GROUP;
+    if (status) {
+	if (clip != NULL)
+	    _cairo_clip_reset (&old_clip);
+	return status;
+    }
 
     status = _cairo_pdf_surface_close_content_stream (surface);
+    if (status) {
+	if (clip != NULL)
+	    _cairo_clip_reset (&old_clip);
+	return status;
+    }
 
- CLEANUP_GROUP:
     _cairo_pdf_surface_set_size_internal (surface,
 					  old_width,
 					  old_height);
     surface->paginated_mode = old_paginated_mode;
-    status2 = _cairo_surface_set_clip (&surface->base, old_clip);
-    if (status == CAIRO_STATUS_SUCCESS)
-	status = status2;
 
-    return status;
+    if (clip != NULL) {
+	status = _cairo_clip_init_deep_copy (clip, &old_clip, &surface->base);
+	_cairo_clip_reset (&old_clip);
+	if (status)
+	    return status;
+    }
+
+    status = _cairo_surface_set_clip (&surface->base, clip);
+    if (status)
+	return status;
+
+    return CAIRO_STATUS_SUCCESS;
 }
 
 static cairo_status_t
diff --git a/src/cairo-ps-surface.c b/src/cairo-ps-surface.c
index f6940bf..dd86d3a 100644
--- a/src/cairo-ps-surface.c
+++ b/src/cairo-ps-surface.c
@@ -2037,7 +2037,7 @@ _cairo_ps_surface_emit_meta_surface (cairo_ps_surface_t  *surface,
     double old_width, old_height;
     cairo_matrix_t old_cairo_to_ps;
     cairo_content_t old_content;
-    cairo_clip_t *old_clip;
+    cairo_clip_t old_clip, *clip;
     cairo_rectangle_int_t meta_extents;
     cairo_status_t status;
 
@@ -2049,7 +2049,12 @@ _cairo_ps_surface_emit_meta_surface (cairo_ps_surface_t  *surface,
     old_width = surface->width;
     old_height = surface->height;
     old_cairo_to_ps = surface->cairo_to_ps;
-    old_clip = _cairo_surface_get_clip (&surface->base);
+    clip = _cairo_surface_get_clip (&surface->base);
+    if (clip != NULL) {
+	status = _cairo_clip_init_deep_copy (&old_clip, clip, &surface->base);
+	if (status)
+	    return status;
+    }
     surface->width = meta_extents.width;
     surface->height = meta_extents.height;
     cairo_matrix_init (&surface->cairo_to_ps, 1, 0, 0, -1, 0, surface->height);
@@ -2072,8 +2077,11 @@ _cairo_ps_surface_emit_meta_surface (cairo_ps_surface_t  *surface,
     status = _cairo_meta_surface_replay_region (meta_surface, &surface->base,
 						CAIRO_META_REGION_NATIVE);
     assert (status != CAIRO_INT_STATUS_UNSUPPORTED);
-    if (status)
+    if (status) {
+	if (clip != NULL)
+	    _cairo_clip_reset (&old_clip);
 	return status;
+    }
 
     _cairo_output_stream_printf (surface->stream,
 				 "  Q\n");
@@ -2081,7 +2089,15 @@ _cairo_ps_surface_emit_meta_surface (cairo_ps_surface_t  *surface,
     surface->width = old_width;
     surface->height = old_height;
     surface->cairo_to_ps = old_cairo_to_ps;
-    status = _cairo_surface_set_clip (&surface->base, old_clip);
+
+    if (clip != NULL) {
+	status = _cairo_clip_init_deep_copy (clip, &old_clip, &surface->base);
+	_cairo_clip_reset (&old_clip);
+	if (status)
+	    return status;
+    }
+
+    status = _cairo_surface_set_clip (&surface->base, clip);
     if (status)
 	return status;
 
diff --git a/src/cairo-surface.c b/src/cairo-surface.c
index 0d5a3f8..5da9905 100644
--- a/src/cairo-surface.c
+++ b/src/cairo-surface.c
@@ -1810,6 +1810,7 @@ _cairo_surface_reset_clip (cairo_surface_t *surface)
 	return _cairo_surface_set_error (surface,CAIRO_STATUS_SURFACE_FINISHED);
 
     surface->current_clip_serial = 0;
+    surface->clip = NULL;
 
     if (surface->backend->intersect_clip_path) {
 	status = surface->backend->intersect_clip_path (surface,
-- 
1.5.4.3


--=-OVPSIn5ah7r21wsNFvCf--



More information about the cairo mailing list