Re: [patch] image: increase reference count for parent

Hi, Chris

I see set_parent is only called in _cairo_image_surface_clone_subimage().  The call sequence is

1. cairo_surface_map_to_image ()  // in cairo-surface.c
2. _cairo_surface_map_to_image () // in cairo-surface.
3. if backend returns image == 0, call _cairo_image_surface_clone_subimage(). // this sets image->parent
4. back in cairo_surface_map_to_image(), if image->format is CAIRO_FORMAT_INVALID, destroy image and call _cairo_image_surface_clone_subimage().

set_parent in _cairo_image_surface_clone_subimage() sets parent. But if the backend returns the image that is not NULL AND not CAIRO_FORMAT_INVALID, image->parent is not set.  In addition,
_cairo_image_clone_subimage() is the only function set parent.  I think we should either not set_parent in _cairo_image_clone_subimage() or make image->parent = NULL in cairo_surface_map_to_image() call.

Otherwise, map-to-image-fill, map-bit-to-image and few other tests seg faults on some glesv2 driver that does not support BGRA format glReadPixels.


The following is the patch that set parent to NULL in cairo_surface_map_to_image()

Thanks

Henry

From 675d1df824977f6e8d348ac78c88fa8635dfd0e2 Mon Sep 17 00:00:00 2001
From: Henry Song <[email protected]>
Date: Tue, 29 Jan 2013 17:41:00 -0800
Subject: [PATCH] surface: set surface->parent to NULL in cairo_surface_map_to_image.

We need to set surface->parent to NULL in cairo_surface_map_to_image.
Otherwise, cairo_surface_unmap_image() descreases reference
count on target surface's parent.  This has an extra decrease on
the parent's reference count and causes crashes in map-to-image-fill,
map-bit-to-image test cases for some OpenGL ES 2 drivers where
the mapped image is CAIRO_FORMAT_INVALID, and thus the original mapped
image is destroyed, a new image is created with the valid format
and parent set to the target surface.
---
 src/cairo-surface.c      |    2 ++
 test/cairo-test-runner.c |    2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/cairo-surface.c b/src/cairo-surface.c
index ffffef8..671eee4 100644
--- a/src/cairo-surface.c
+++ b/src/cairo-surface.c
@@ -793,6 +793,8 @@ cairo_surface_map_to_image (cairo_surface_t  *surface,
 	image = _cairo_image_surface_clone_subimage (surface, extents);
     }
 
+    image->parent = NULL;
+
     return &image->base;
 }
 
diff --git a/test/cairo-test-runner.c b/test/cairo-test-runner.c
index a5c6705..108cc50 100644
--- a/test/cairo-test-runner.c
+++ b/test/cairo-test-runner.c
@@ -59,7 +59,7 @@
 #ifdef _MSC_VER
 #include <crtdbg.h>
 #endif
-
+#define SHOULD_FORK 0
 typedef struct _cairo_test_list {
     const cairo_test_t *test;
     struct _cairo_test_list *next;
-- 
1.7.9.5

-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo