[cairo-commit] 5 commits - src/cairo-quartz-image-surface.c src/cairo-quartz-surface.c

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Fri Dec 11 14:14:26 UTC 2020


 src/cairo-quartz-image-surface.c |   39 +++++++--------
 src/cairo-quartz-surface.c       |  100 +++++++++++++++++++--------------------
 2 files changed, 69 insertions(+), 70 deletions(-)

New commits:
commit e22d7212acb454daccc088619ee147af03883974
Merge: f7054c892 dccaa9179
Author: Uli Schlachter <psychon at znc.in>
Date:   Fri Dec 11 14:14:24 2020 +0000

    Merge branch 'bigsur-use-after-free' into 'master'
    
    Ref and destroy the cairo surface handed off to CoreGraphics.
    
    Closes #420
    
    See merge request cairo/cairo!52

commit dccaa9179bcf08cb6909305de28c0495523339bc
Author: John Ralls <jralls at ceridwen.us>
Date:   Sat Dec 5 11:44:19 2020 -0800

    Quartz Better manage acquired/replayed surface.

diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c
index ceb0973e8..7a9f52401 100644
--- a/src/cairo-quartz-surface.c
+++ b/src/cairo-quartz-surface.c
@@ -795,6 +795,7 @@ _cairo_surface_to_cgimage (cairo_surface_t       *source,
     cairo_status_t status;
     cairo_image_surface_t *image_surface;
     void *image_data, *image_extra;
+    cairo_bool_t acquired = FALSE;
 
     if (source->backend && source->backend->type == CAIRO_SURFACE_TYPE_QUARTZ_IMAGE) {
 	cairo_quartz_image_surface_t *surface = (cairo_quartz_image_surface_t *) source;
@@ -841,33 +842,50 @@ _cairo_surface_to_cgimage (cairo_surface_t       *source,
 						      &image_extra);
 	if (unlikely (status))
 	    return status;
+	acquired = TRUE;
+    }
+
+    if (image_surface->width == 0 || image_surface->height == 0) {
+	*image_out = NULL;
+	if (acquired)
+	    _cairo_surface_release_source_image (source, image_surface, image_extra);
+	else
+	    cairo_surface_destroy (&image_surface->base);
+
+	return status;
     }
 
     image_data = _cairo_malloc_ab (image_surface->height, image_surface->stride);
     if (unlikely (!image_data))
+    {
+	if (acquired)
+	    _cairo_surface_release_source_image (source, image_surface, image_extra);
+	else
+	    cairo_surface_destroy (&image_surface->base);
+
 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+    }
 
     memcpy (image_data, image_surface->data,
 	    image_surface->height * image_surface->stride);
-    if (image_surface->width == 0 || image_surface->height == 0) {
-	*image_out = NULL;
-	DataProviderReleaseCallback (image_data, image_data, 0);
-    } else {
-	*image_out = CairoQuartzCreateCGImage (image_surface->format,
-					       image_surface->width,
-					       image_surface->height,
-					       image_surface->stride,
-					       image_data,
-					       TRUE,
-					       NULL,
-					       DataProviderReleaseCallback,
-					       image_data);
-
-	/* TODO: differentiate memory error and unsupported surface type */
-	if (unlikely (*image_out == NULL))
-	    status = CAIRO_INT_STATUS_UNSUPPORTED;
-    }
-    _cairo_surface_release_source_image (source, image_surface, image_extra);
+    *image_out = CairoQuartzCreateCGImage (image_surface->format,
+					   image_surface->width,
+					   image_surface->height,
+					   image_surface->stride,
+					   image_data,
+					   TRUE,
+					   NULL,
+					   DataProviderReleaseCallback,
+					   image_data);
+
+    /* TODO: differentiate memory error and unsupported surface type */
+    if (unlikely (*image_out == NULL))
+	status = CAIRO_INT_STATUS_UNSUPPORTED;
+
+    if (acquired)
+	_cairo_surface_release_source_image (source, image_surface, image_extra);
+    else
+	cairo_surface_destroy (&image_surface->base);
 
     return status;
 }
commit 1ddfccca31e23820a30e8f618e216fe2931b49b2
Author: John Ralls <jralls at ceridwen.us>
Date:   Mon Nov 30 17:16:00 2020 -0800

    Quartz image drawing: Remove containers for cairo_surface_t.
    
    Since we now copy the data that CGImage needs we don't need to
    keep the surface around anymore, nor release it or the image in the
    DataProviderReleaseCallback.

diff --git a/src/cairo-quartz-image-surface.c b/src/cairo-quartz-image-surface.c
index 7d8c71f1a..ad23621af 100644
--- a/src/cairo-quartz-image-surface.c
+++ b/src/cairo-quartz-image-surface.c
@@ -49,17 +49,9 @@
 #define SURFACE_ERROR_INVALID_SIZE (_cairo_surface_create_in_error(_cairo_error(CAIRO_STATUS_INVALID_SIZE)))
 #define SURFACE_ERROR_INVALID_FORMAT (_cairo_surface_create_in_error(_cairo_error(CAIRO_STATUS_INVALID_FORMAT)))
 
-typedef struct {
-    cairo_surface_t *surface;
-    void *image_data;
-} quartz_image_info_t;
-
 static void
-DataProviderReleaseCallback (void *info, const void *data, size_t size)
+DataProviderReleaseCallback (void *image_info, const void *data, size_t size)
 {
-    quartz_image_info_t *image_info = (quartz_image_info_t *) info;
-    cairo_surface_destroy (image_info->surface);
-    free (image_info->image_data);
     free (image_info);
 }
 
@@ -95,9 +87,8 @@ _cairo_quartz_image_surface_finish (void *asurface)
 {
     cairo_quartz_image_surface_t *surface = (cairo_quartz_image_surface_t *) asurface;
 
-    /* the imageSurface will be destroyed by the data provider's release callback */
     CGImageRelease (surface->image);
-
+    cairo_surface_destroy (surface->imageSurface);
     return CAIRO_STATUS_SUCCESS;
 }
 
@@ -154,35 +145,29 @@ _cairo_quartz_image_surface_flush (void *asurface,
     cairo_quartz_image_surface_t *surface = (cairo_quartz_image_surface_t *) asurface;
     CGImageRef oldImage = surface->image;
     CGImageRef newImage = NULL;
-    quartz_image_info_t *image_info;
-
+    void *image_data;
+    const unsigned int size = surface->imageSurface->height * surface->imageSurface->stride;
     if (flags)
 	return CAIRO_STATUS_SUCCESS;
 
     /* XXX only flush if the image has been modified. */
 
-    image_info = _cairo_malloc (sizeof (quartz_image_info_t));
-    if (unlikely (!image_info))
+    image_data = _cairo_malloc_ab ( surface->imageSurface->height,
+				    surface->imageSurface->stride);
+    if (unlikely (!image_data))
 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
 
-    image_info->surface = _cairo_surface_snapshot ((cairo_surface_t*)surface->imageSurface);
-    image_info->image_data = _cairo_malloc_ab (surface->imageSurface->height,
-					       surface->imageSurface->stride);
-    if (unlikely (!image_info->image_data))
-	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
-
-    memcpy (image_info->image_data, surface->imageSurface->data,
+    memcpy (image_data, surface->imageSurface->data,
 	    surface->imageSurface->height * surface->imageSurface->stride);
-
     newImage = CairoQuartzCreateCGImage (surface->imageSurface->format,
 					 surface->imageSurface->width,
 					 surface->imageSurface->height,
 					 surface->imageSurface->stride,
-					 image_info->image_data,
+					 image_data,
 					 TRUE,
 					 NULL,
 					 DataProviderReleaseCallback,
-					 image_info);
+					 image_data);
 
     surface->image = newImage;
     CGImageRelease (oldImage);
@@ -326,8 +311,7 @@ cairo_quartz_image_surface_create (cairo_surface_t *surface)
     cairo_image_surface_t *image_surface;
     int width, height, stride;
     cairo_format_t format;
-    unsigned char *data;
-    quartz_image_info_t *image_info;
+    void *image_data;
 
     if (surface->status)
 	return surface;
@@ -340,7 +324,6 @@ cairo_quartz_image_surface_create (cairo_surface_t *surface)
     height = image_surface->height;
     stride = image_surface->stride;
     format = image_surface->format;
-    data = image_surface->data;
 
     if (!_cairo_quartz_verify_surface_size(width, height))
 	return SURFACE_ERROR_INVALID_SIZE;
@@ -357,24 +340,19 @@ cairo_quartz_image_surface_create (cairo_surface_t *surface)
 
     memset (qisurf, 0, sizeof(cairo_quartz_image_surface_t));
 
-    image_info = _cairo_malloc (sizeof (quartz_image_info_t));
-    if (unlikely (!image_info))
-	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
-
-    image_info->surface = _cairo_surface_snapshot (surface);
-    image_info->image_data = _cairo_malloc_ab (height, stride);
-    if (unlikely (!image_info->image_data))
+    image_data = _cairo_malloc_ab (height, stride);
+    if (unlikely (!image_data))
 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
 
-    memcpy (image_info->image_data, data, height * stride);
+    memcpy (image_data, image_surface->data, height * stride);
     image = CairoQuartzCreateCGImage (format,
 				      width, height,
 				      stride,
-				      image_info->image_data,
+				      image_data,
 				      TRUE,
 				      NULL,
 				      DataProviderReleaseCallback,
-				      image_info);
+				      image_data);
 
     if (!image) {
 	free (qisurf);
diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c
index 2fa376bb8..ceb0973e8 100644
--- a/src/cairo-quartz-surface.c
+++ b/src/cairo-quartz-surface.c
@@ -778,23 +778,10 @@ CairoQuartzCreateGradientFunction (const cairo_gradient_pattern_t *gradient,
 			     &gradient_callbacks);
 }
 
-/* Obtain a CGImageRef from a #cairo_surface_t * */
-
-typedef struct {
-    cairo_surface_t *surface;
-    cairo_image_surface_t *image_out;
-    void *image_extra;
-    void *image_data;
-} quartz_source_image_t;
-
 static void
 DataProviderReleaseCallback (void *info, const void *data, size_t size)
 {
-    quartz_source_image_t *source_img = info;
-    _cairo_surface_release_source_image (source_img->surface, source_img->image_out, source_img->image_extra);
-    cairo_surface_destroy (source_img->surface);
-    free (source_img->image_data);
-    free (source_img);
+    free (info);
 }
 
 static cairo_status_t
@@ -806,8 +793,8 @@ _cairo_surface_to_cgimage (cairo_surface_t       *source,
 			   CGImageRef            *image_out)
 {
     cairo_status_t status;
-    quartz_source_image_t *source_img;
     cairo_image_surface_t *image_surface;
+    void *image_data, *image_extra;
 
     if (source->backend && source->backend->type == CAIRO_SURFACE_TYPE_QUARTZ_IMAGE) {
 	cairo_quartz_image_surface_t *surface = (cairo_quartz_image_surface_t *) source;
@@ -829,19 +816,12 @@ _cairo_surface_to_cgimage (cairo_surface_t       *source,
 	}
     }
 
-    source_img = _cairo_malloc (sizeof (quartz_source_image_t));
-    if (unlikely (source_img == NULL))
-	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
-
-    source_img->surface = _cairo_surface_snapshot (source);
-
     if (source->type == CAIRO_SURFACE_TYPE_RECORDING) {
 	image_surface = (cairo_image_surface_t *)
 	    cairo_image_surface_create (format, extents->width, extents->height);
 	if (unlikely (image_surface->base.status)) {
 	    status = image_surface->base.status;
 	    cairo_surface_destroy (&image_surface->base);
-	    free (source_img);
 	    return status;
 	}
 
@@ -851,51 +831,43 @@ _cairo_surface_to_cgimage (cairo_surface_t       *source,
 							    NULL);
 	if (unlikely (status)) {
 	    cairo_surface_destroy (&image_surface->base);
-	    free (source_img);
 	    return status;
 	}
 
-	source_img->image_out = image_surface;
-	source_img->image_data = NULL;
-
 	cairo_matrix_init_identity (matrix);
     }
     else {
-	status = _cairo_surface_acquire_source_image (source_img->surface,
-						      &source_img->image_out,
-						      &source_img->image_extra);
-	if (unlikely (status)) {
-	    free (source_img);
+	status = _cairo_surface_acquire_source_image (source, &image_surface,
+						      &image_extra);
+	if (unlikely (status))
 	    return status;
-	}
     }
 
-    source_img->image_data = malloc (source_img->image_out->height * source_img->image_out->stride);
-    if (unlikely (!source_img->image_data))
+    image_data = _cairo_malloc_ab (image_surface->height, image_surface->stride);
+    if (unlikely (!image_data))
 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
 
-    memcpy (source_img->image_data, source_img->image_out->data,
-	    source_img->image_out->height * source_img->image_out->stride);
-    if (source_img->image_out->width == 0 || source_img->image_out->height == 0) {
+    memcpy (image_data, image_surface->data,
+	    image_surface->height * image_surface->stride);
+    if (image_surface->width == 0 || image_surface->height == 0) {
 	*image_out = NULL;
-	DataProviderReleaseCallback (source_img,
-				     source_img->image_out->data,
-				     source_img->image_out->height * source_img->image_out->stride);
+	DataProviderReleaseCallback (image_data, image_data, 0);
     } else {
-	*image_out = CairoQuartzCreateCGImage (source_img->image_out->format,
-					       source_img->image_out->width,
-					       source_img->image_out->height,
-					       source_img->image_out->stride,
-					       source_img->image_data,
+	*image_out = CairoQuartzCreateCGImage (image_surface->format,
+					       image_surface->width,
+					       image_surface->height,
+					       image_surface->stride,
+					       image_data,
 					       TRUE,
 					       NULL,
 					       DataProviderReleaseCallback,
-					       source_img);
+					       image_data);
 
 	/* TODO: differentiate memory error and unsupported surface type */
 	if (unlikely (*image_out == NULL))
 	    status = CAIRO_INT_STATUS_UNSUPPORTED;
     }
+    _cairo_surface_release_source_image (source, image_surface, image_extra);
 
     return status;
 }
commit b5e84a97833e8e1d082f4409383b09f9827ada09
Author: John Ralls <jralls at ceridwen.us>
Date:   Mon Nov 30 15:13:41 2020 -0800

    Quartz: Ensure that image data and surface are available to draw.
    
    Snapshot the cairo_surface_t and copy the image data to provide to
    the CGDataProvider so that it is independent of the cairo_surface_t's
    lifetime.
    
    Closes https://gitlab.freedesktop.org/cairo/cairo/-/issues/420

diff --git a/src/cairo-quartz-image-surface.c b/src/cairo-quartz-image-surface.c
index 84d56c9b4..7d8c71f1a 100644
--- a/src/cairo-quartz-image-surface.c
+++ b/src/cairo-quartz-image-surface.c
@@ -49,11 +49,18 @@
 #define SURFACE_ERROR_INVALID_SIZE (_cairo_surface_create_in_error(_cairo_error(CAIRO_STATUS_INVALID_SIZE)))
 #define SURFACE_ERROR_INVALID_FORMAT (_cairo_surface_create_in_error(_cairo_error(CAIRO_STATUS_INVALID_FORMAT)))
 
+typedef struct {
+    cairo_surface_t *surface;
+    void *image_data;
+} quartz_image_info_t;
+
 static void
 DataProviderReleaseCallback (void *info, const void *data, size_t size)
 {
-    cairo_surface_t *surface = (cairo_surface_t *) info;
-    cairo_surface_destroy (surface);
+    quartz_image_info_t *image_info = (quartz_image_info_t *) info;
+    cairo_surface_destroy (image_info->surface);
+    free (image_info->image_data);
+    free (image_info);
 }
 
 static cairo_surface_t *
@@ -147,24 +154,35 @@ _cairo_quartz_image_surface_flush (void *asurface,
     cairo_quartz_image_surface_t *surface = (cairo_quartz_image_surface_t *) asurface;
     CGImageRef oldImage = surface->image;
     CGImageRef newImage = NULL;
+    quartz_image_info_t *image_info;
 
     if (flags)
 	return CAIRO_STATUS_SUCCESS;
 
     /* XXX only flush if the image has been modified. */
 
-    /* To be released by the ReleaseCallback */
-    cairo_surface_reference ((cairo_surface_t*) surface->imageSurface);
+    image_info = _cairo_malloc (sizeof (quartz_image_info_t));
+    if (unlikely (!image_info))
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+
+    image_info->surface = _cairo_surface_snapshot ((cairo_surface_t*)surface->imageSurface);
+    image_info->image_data = _cairo_malloc_ab (surface->imageSurface->height,
+					       surface->imageSurface->stride);
+    if (unlikely (!image_info->image_data))
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+
+    memcpy (image_info->image_data, surface->imageSurface->data,
+	    surface->imageSurface->height * surface->imageSurface->stride);
 
     newImage = CairoQuartzCreateCGImage (surface->imageSurface->format,
 					 surface->imageSurface->width,
 					 surface->imageSurface->height,
 					 surface->imageSurface->stride,
-					 surface->imageSurface->data,
+					 image_info->image_data,
 					 TRUE,
 					 NULL,
 					 DataProviderReleaseCallback,
-					 surface->imageSurface);
+					 image_info);
 
     surface->image = newImage;
     CGImageRelease (oldImage);
@@ -309,6 +327,7 @@ cairo_quartz_image_surface_create (cairo_surface_t *surface)
     int width, height, stride;
     cairo_format_t format;
     unsigned char *data;
+    quartz_image_info_t *image_info;
 
     if (surface->status)
 	return surface;
@@ -338,20 +357,24 @@ cairo_quartz_image_surface_create (cairo_surface_t *surface)
 
     memset (qisurf, 0, sizeof(cairo_quartz_image_surface_t));
 
-    /* In case the create_cgimage fails, this ref will
-     * be released via the callback (which will be called in
-     * case of failure.)
-     */
-    cairo_surface_reference (surface);
+    image_info = _cairo_malloc (sizeof (quartz_image_info_t));
+    if (unlikely (!image_info))
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+
+    image_info->surface = _cairo_surface_snapshot (surface);
+    image_info->image_data = _cairo_malloc_ab (height, stride);
+    if (unlikely (!image_info->image_data))
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
 
+    memcpy (image_info->image_data, data, height * stride);
     image = CairoQuartzCreateCGImage (format,
 				      width, height,
 				      stride,
-				      data,
+				      image_info->image_data,
 				      TRUE,
 				      NULL,
 				      DataProviderReleaseCallback,
-				      image_surface);
+				      image_info);
 
     if (!image) {
 	free (qisurf);
diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c
index f697d157e..2fa376bb8 100644
--- a/src/cairo-quartz-surface.c
+++ b/src/cairo-quartz-surface.c
@@ -784,6 +784,7 @@ typedef struct {
     cairo_surface_t *surface;
     cairo_image_surface_t *image_out;
     void *image_extra;
+    void *image_data;
 } quartz_source_image_t;
 
 static void
@@ -791,6 +792,8 @@ DataProviderReleaseCallback (void *info, const void *data, size_t size)
 {
     quartz_source_image_t *source_img = info;
     _cairo_surface_release_source_image (source_img->surface, source_img->image_out, source_img->image_extra);
+    cairo_surface_destroy (source_img->surface);
+    free (source_img->image_data);
     free (source_img);
 }
 
@@ -830,7 +833,7 @@ _cairo_surface_to_cgimage (cairo_surface_t       *source,
     if (unlikely (source_img == NULL))
 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
 
-    source_img->surface = source;
+    source_img->surface = _cairo_surface_snapshot (source);
 
     if (source->type == CAIRO_SURFACE_TYPE_RECORDING) {
 	image_surface = (cairo_image_surface_t *)
@@ -853,7 +856,7 @@ _cairo_surface_to_cgimage (cairo_surface_t       *source,
 	}
 
 	source_img->image_out = image_surface;
-	source_img->image_extra = NULL;
+	source_img->image_data = NULL;
 
 	cairo_matrix_init_identity (matrix);
     }
@@ -867,6 +870,12 @@ _cairo_surface_to_cgimage (cairo_surface_t       *source,
 	}
     }
 
+    source_img->image_data = malloc (source_img->image_out->height * source_img->image_out->stride);
+    if (unlikely (!source_img->image_data))
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+
+    memcpy (source_img->image_data, source_img->image_out->data,
+	    source_img->image_out->height * source_img->image_out->stride);
     if (source_img->image_out->width == 0 || source_img->image_out->height == 0) {
 	*image_out = NULL;
 	DataProviderReleaseCallback (source_img,
@@ -877,7 +886,7 @@ _cairo_surface_to_cgimage (cairo_surface_t       *source,
 					       source_img->image_out->width,
 					       source_img->image_out->height,
 					       source_img->image_out->stride,
-					       source_img->image_out->data,
+					       source_img->image_data,
 					       TRUE,
 					       NULL,
 					       DataProviderReleaseCallback,
commit 32c12c617a2551e1238e52c570bd5c42a6c0cbef
Author: John Ralls <jralls at ceridwen.us>
Date:   Mon Nov 30 14:10:18 2020 -0800

    Ensure _cairo_quartz_surface_create_internal always nulls imageSurfaceEquiv.

diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c
index 65d03080a..f697d157e 100644
--- a/src/cairo-quartz-surface.c
+++ b/src/cairo-quartz-surface.c
@@ -2273,11 +2273,13 @@ _cairo_quartz_surface_create_internal (CGContextRef cgContext,
     surface->extents.width = width;
     surface->extents.height = height;
     surface->virtual_extents = surface->extents;
+    surface->imageData = NULL;
+    surface->imageSurfaceEquiv = NULL;
+
 
     if (IS_EMPTY (surface)) {
 	surface->cgContext = NULL;
 	surface->cgContextBaseCTM = CGAffineTransformIdentity;
-	surface->imageData = NULL;
 	surface->base.is_clear = TRUE;
 	return surface;
     }
@@ -2290,9 +2292,6 @@ _cairo_quartz_surface_create_internal (CGContextRef cgContext,
     surface->cgContext = cgContext;
     surface->cgContextBaseCTM = CGContextGetCTM (cgContext);
 
-    surface->imageData = NULL;
-    surface->imageSurfaceEquiv = NULL;
-
     return surface;
 }
 


More information about the cairo-commit mailing list