[cairo] Simplify clipping in pixman

Soeren Sandmann sandmann at daimi.au.dk
Sun May 10 22:08:29 PDT 2009


In anticipation of Render getting its clip rules clarified to mean
that source clipping happens for all pictures, and that it happens
after transformation (ie., the clip region is not affected by
repeating and transforming), here is a patch that implements them in
pixman.

^From f7724a23170413f4d33e635e4b156a3bc07067c7 Mon Sep 17 00:00:00 2001
^From: =?utf-8?q?S=C3=B8ren=20Sandmann=20Pedersen?= <sandmann at redhat.com>
^Date: Mon, 11 May 2009 00:36:17 -0400
^Subject: [PATCH] Simplify clipping rule.

The new rule is:

- Output is clipped to the destination clip region.

- If a source image has the clip_sources property set, then there
  is an additional step, after repeating and transforming, but before
  compositing, where pixels that are not in the source clip are
  rejected. Rejected means no compositing takes place (not that the
  pixel is treated as 0). By default no source clipping is taking
  place.

The old rules were unclear and inconsistently implemented.
---
 pixman/pixman-compute-region.c |   66 +++++++++++++---------------------------
 pixman/pixman-image.c          |   52 +++++++++++--------------------
 pixman/pixman-pict.c           |    1 -
 pixman/pixman-private.h        |    7 ++--
 pixman/pixman-transformed.c    |   48 ++++++++++-------------------
 5 files changed, 60 insertions(+), 114 deletions(-)

diff --git a/pixman/pixman-compute-region.c b/pixman/pixman-compute-region.c
index a93cee0..077bb7d 100644
--- a/pixman/pixman-compute-region.c
+++ b/pixman/pixman-compute-region.c
@@ -79,43 +79,12 @@ miClipPictureSrc (pixman_region32_t *	pRegion,
 		  int		dx,
 		  int		dy)
 {
-    /* XXX what to do with clipping from transformed pictures? */
-    if (pPicture->common.transform || pPicture->type != BITS)
+    if (!pPicture->common.clip_sources)
 	return TRUE;
 
-    if (pPicture->common.repeat)
-    {
-	/* If the clip region was set by a client, then it should be intersected
-	 * with the composite region since it's interpreted as happening
-	 * after the repeat algorithm.
-	 *
-	 * If the clip region was not set by a client, then it was imposed by
-	 * boundaries of the pixmap, or by sibling or child windows, which means
-	 * it should in theory be repeated along. FIXME: we ignore that case.
-	 * It is only relevant for windows that are (a) clipped by siblings/children
-	 * and (b) used as source. However this case is not useful anyway due
-	 * to lack of GraphicsExpose events.
-	 */
-	if (pPicture->common.has_client_clip)
-	{
-	    pixman_region32_translate ( pRegion, dx, dy);
-	    
-	    if (!pixman_region32_intersect (pRegion, pRegion, 
-					    pPicture->common.src_clip))
-		return FALSE;
-	    
-	    pixman_region32_translate ( pRegion, -dx, -dy);
-	}
-	    
-	return TRUE;
-    }
-    else
-    {
-	return miClipPictureReg (pRegion,
-				 pPicture->common.src_clip,
-				 dx,
-				 dy);
-    }
+    return miClipPictureReg (pRegion,
+			     &pPicture->common.clip_region,
+			     dx, dy);
 }
 
 /*
@@ -154,12 +123,16 @@ pixman_compute_composite_region32 (pixman_region32_t *	pRegion,
 	return FALSE;
     }
     /* clip against dst */
-    if (!miClipPictureReg (pRegion, &pDst->common.clip_region, 0, 0))
+    if (pDst->common.have_clip_region)
     {
-	pixman_region32_fini (pRegion);
-	return FALSE;
+	if (!miClipPictureReg (pRegion, &pDst->common.clip_region, 0, 0))
+	{
+	    pixman_region32_fini (pRegion);
+	    return FALSE;
+	}
     }
-    if (pDst->common.alpha_map)
+    
+    if (pDst->common.alpha_map && pDst->common.alpha_map->common.have_clip_region)
     {
 	if (!miClipPictureReg (pRegion, &pDst->common.alpha_map->common.clip_region,
 			       -pDst->common.alpha_origin.x,
@@ -170,12 +143,15 @@ pixman_compute_composite_region32 (pixman_region32_t *	pRegion,
 	}
     }
     /* clip against src */
-    if (!miClipPictureSrc (pRegion, pSrc, xDst - xSrc, yDst - ySrc))
+    if (pSrc->common.have_clip_region)
     {
-	pixman_region32_fini (pRegion);
-	return FALSE;
+	if (!miClipPictureSrc (pRegion, pSrc, xDst - xSrc, yDst - ySrc))
+	{
+	    pixman_region32_fini (pRegion);
+	    return FALSE;
+	}
     }
-    if (pSrc->common.alpha_map)
+    if (pSrc->common.alpha_map && pSrc->common.alpha_map->common.have_clip_region)
     {
 	if (!miClipPictureSrc (pRegion, (pixman_image_t *)pSrc->common.alpha_map,
 			       xDst - (xSrc + pSrc->common.alpha_origin.x),
@@ -186,14 +162,14 @@ pixman_compute_composite_region32 (pixman_region32_t *	pRegion,
 	}
     }
     /* clip against mask */
-    if (pMask)
+    if (pMask && pMask->common.have_clip_region)
     {
 	if (!miClipPictureSrc (pRegion, pMask, xDst - xMask, yDst - yMask))
 	{
 	    pixman_region32_fini (pRegion);
 	    return FALSE;
 	}	
-	if (pMask->common.alpha_map)
+	if (pMask->common.alpha_map && pMask->common.alpha_map->common.have_clip_region)
 	{
 	    if (!miClipPictureSrc (pRegion, (pixman_image_t *)pMask->common.alpha_map,
 				   xDst - (xMask + pMask->common.alpha_origin.x),
diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index bd52f25..5ae55dd 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -81,10 +81,8 @@ allocate_image (void)
     {
 	image_common_t *common = &image->common;
 
-	pixman_region32_init (&common->full_region);
 	pixman_region32_init (&common->clip_region);
-	common->src_clip = &common->full_region;
-	common->has_client_clip = FALSE;
+	common->clip_sources = FALSE;
 	common->transform = NULL;
 	common->repeat = PIXMAN_REPEAT_NONE;
 	common->filter = PIXMAN_FILTER_NEAREST;
@@ -120,7 +118,6 @@ pixman_image_unref (pixman_image_t *image)
     if (common->ref_count == 0)
     {
 	pixman_region32_fini (&common->clip_region);
-	pixman_region32_fini (&common->full_region);
 
 	if (common->transform)
 	    free (common->transform);
@@ -323,17 +320,7 @@ create_bits (pixman_format_code_t format,
 static void
 reset_clip_region (pixman_image_t *image)
 {
-    pixman_region32_fini (&image->common.clip_region);
-
-    if (image->type == BITS)
-    {
-	pixman_region32_init_rect (&image->common.clip_region, 0, 0,
-				   image->bits.width, image->bits.height);
-    }
-    else
-    {
-	pixman_region32_init (&image->common.clip_region);
-    }
+    image->common.have_clip_region = FALSE;
 }
 
 PIXMAN_EXPORT pixman_image_t *
@@ -374,14 +361,10 @@ pixman_image_create_bits (pixman_format_code_t  format,
     image->bits.free_me = free_me;
 
     image->bits.rowstride = rowstride_bytes / (int) sizeof (uint32_t); /* we store it in number
-								  * of uint32_t's
-								  */
+									* of uint32_t's
+									*/
     image->bits.indexed = NULL;
 
-    pixman_region32_fini (&image->common.full_region);
-    pixman_region32_init_rect (&image->common.full_region, 0, 0,
-			       image->bits.width, image->bits.height);
-
     reset_clip_region (image);
     return image;
 }
@@ -394,6 +377,8 @@ pixman_image_set_clip_region32 (pixman_image_t *image,
 
     if (region)
     {
+	image->common.have_clip_region = TRUE;
+	
 	return pixman_region32_copy (&common->clip_region, region);
     }
     else
@@ -429,7 +414,7 @@ PIXMAN_EXPORT void
 pixman_image_set_has_client_clip (pixman_image_t *image,
 				  pixman_bool_t	  client_clip)
 {
-    image->common.has_client_clip = client_clip;
+    /* This function is obsolete - it had to do with source and hierarchy clipping */
 }
 
 PIXMAN_EXPORT pixman_bool_t
@@ -506,16 +491,14 @@ pixman_image_set_filter (pixman_image_t       *image,
     return TRUE;
 }
 
+/* Whether there is an extra step in the pipeline where clips are applied
+ * to source images. Don't use it unless you are X.
+ */
 PIXMAN_EXPORT void
 pixman_image_set_source_clipping (pixman_image_t  *image,
-				  pixman_bool_t    source_clipping)
+				  pixman_bool_t    clip_sources)
 {
-    image_common_t *common = &image->common;
-
-    if (source_clipping)
-	common->src_clip = &common->clip_region;
-    else
-	common->src_clip = &common->full_region;
+    image->common.clip_sources = clip_sources;
 }
 
 /* Unlike all the other property setters, this function does not
@@ -712,11 +695,14 @@ pixman_image_fill_rectangles (pixman_op_t		    op,
 		pixman_box32_t *boxes;
 
 		pixman_region32_init_rect (&fill_region, rects[i].x, rects[i].y, rects[i].width, rects[i].height);
-		if (!pixman_region32_intersect (&fill_region,
-						&fill_region,
-						&dest->common.clip_region))
-		    return FALSE;
 
+		if (dest->common.have_clip_region)
+		{
+		    if (!pixman_region32_intersect (&fill_region,
+						    &fill_region,
+						    &dest->common.clip_region))
+			return FALSE;
+		}
 
 		boxes = pixman_region32_rectangles (&fill_region, &n_boxes);
 		for (j = 0; j < n_boxes; ++j)
diff --git a/pixman/pixman-pict.c b/pixman/pixman-pict.c
index 548d38d..4f6ef91 100644
--- a/pixman/pixman-pict.c
+++ b/pixman/pixman-pict.c
@@ -1989,7 +1989,6 @@ pixman_image_composite (pixman_op_t      op,
         && (pSrc->common.filter == PIXMAN_FILTER_NEAREST)
         && PIXMAN_FORMAT_BPP(pDst->bits.format) == 32
         && pSrc->bits.format == pDst->bits.format
-        && pSrc->common.src_clip == &(pSrc->common.full_region)
         && !pSrc->common.read_func && !pSrc->common.write_func
         && !pDst->common.read_func && !pDst->common.write_func)
     {
diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
index debd723..ee0bb0b 100644
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -292,10 +292,9 @@ struct image_common
 {
     image_type_t		type;
     int32_t			ref_count;
-    pixman_region32_t		full_region;
-    pixman_region32_t		clip_region;
-    pixman_region32_t	       *src_clip;
-    pixman_bool_t               has_client_clip;
+    pixman_region32_t	        clip_region;
+    pixman_bool_t               have_clip_region;      /* if FALSE, there is no clip */
+    pixman_bool_t		clip_sources;	/* Whether clips apply to source images */
     pixman_transform_t	       *transform;
     pixman_repeat_t		repeat;
     pixman_filter_t		filter;
diff --git a/pixman/pixman-transformed.c b/pixman/pixman-transformed.c
index 6d695c5..ba45cc6 100644
--- a/pixman/pixman-transformed.c
+++ b/pixman/pixman-transformed.c
@@ -61,17 +61,9 @@ typedef FASTCALL uint32_t (*fetchFromRegionProc)(bits_image_t *pict, int x, int
  */
 static force_inline uint32_t
 do_fetch (bits_image_t *pict, int x, int y, fetchPixelProc32 fetch,
-	  pixman_bool_t src_clip,
 	  pixman_bool_t inside_bounds)
 {
-    if (src_clip)
-    {
-	if (pixman_region32_contains_point (pict->common.src_clip, x, y,NULL))
-	    return fetch (pict, x, y);
-	else
-	    return 0;
-    }
-    else if (inside_bounds)
+    if (inside_bounds)
     {
 	return fetch (pict, x, y);
     }
@@ -92,7 +84,6 @@ fetch_nearest (bits_image_t		*pict,
 	       fetchPixelProc32		 fetch,
 	       pixman_bool_t		 affine,
 	       pixman_repeat_t		 repeat,
-	       pixman_bool_t             has_src_clip,
 	       const pixman_vector_t    *v)
 {
     if (!v->vector[2])
@@ -147,7 +138,7 @@ fetch_nearest (bits_image_t		*pict,
 	    return 0;
 	}
 
-	return do_fetch (pict, x, y, fetch, has_src_clip, inside_bounds);
+	return do_fetch (pict, x, y, fetch, inside_bounds);
     }
 }
 
@@ -156,7 +147,6 @@ fetch_bilinear (bits_image_t		*pict,
 		fetchPixelProc32	 fetch,
 		pixman_bool_t		 affine,
 		pixman_repeat_t		 repeat,
-		pixman_bool_t		 has_src_clip,
 		const pixman_vector_t   *v)
 {
     if (!v->vector[2])
@@ -235,10 +225,10 @@ fetch_bilinear (bits_image_t		*pict,
 	    return 0;
 	}
 	
-	tl = do_fetch(pict, x1, y1, fetch, has_src_clip, inside_bounds);
-	tr = do_fetch(pict, x2, y1, fetch, has_src_clip, inside_bounds);
-	bl = do_fetch(pict, x1, y2, fetch, has_src_clip, inside_bounds);
-	br = do_fetch(pict, x2, y2, fetch, has_src_clip, inside_bounds);
+	tl = do_fetch(pict, x1, y1, fetch, inside_bounds);
+	tr = do_fetch(pict, x2, y1, fetch, inside_bounds);
+	bl = do_fetch(pict, x1, y2, fetch, inside_bounds);
+	br = do_fetch(pict, x2, y2, fetch, inside_bounds);
 	
 	ft = FbGet8(tl,0) * idistx + FbGet8(tr,0) * distx;
 	fb = FbGet8(bl,0) * idistx + FbGet8(br,0) * distx;
@@ -317,6 +307,8 @@ fbFetchTransformed_Convolution(bits_image_t * pict, int width, uint32_t *buffer,
                     for (x = x1; x < x2; x++) {
                         if (*p) {
                             int tx;
+			    uint32_t c;
+			    
                             switch (pict->common.repeat) {
                                 case PIXMAN_REPEAT_NORMAL:
                                     tx = MOD (x, pict->width);
@@ -332,14 +324,13 @@ fbFetchTransformed_Convolution(bits_image_t * pict, int width, uint32_t *buffer,
                                 default:
                                     tx = x;
                             }
-                            if (pixman_region32_contains_point (pict->common.src_clip, tx, ty, NULL)) {
-                                uint32_t c = fetch(pict, tx, ty);
-
-                                srtot += Red(c) * *p;
-                                sgtot += Green(c) * *p;
-                                sbtot += Blue(c) * *p;
-                                satot += Alpha(c) * *p;
-                            }
+			    
+			    c = fetch(pict, tx, ty);
+			    
+			    srtot += Red(c) * *p;
+			    sgtot += Green(c) * *p;
+			    sbtot += Blue(c) * *p;
+			    satot += Alpha(c) * *p;
                         }
                         p++;
                     }
@@ -419,7 +410,6 @@ ACCESS(fbFetchTransformed)(bits_image_t * pict, int x, int y, int width,
     if (pict->common.filter == PIXMAN_FILTER_NEAREST || pict->common.filter == PIXMAN_FILTER_FAST)
     {
 	fetchPixelProc32   fetch;
-	pixman_bool_t src_clip;
 	int i;
 
 	/* Round down to closest integer, ensuring that 0.5 rounds to 0, not 1 */
@@ -427,12 +417,10 @@ ACCESS(fbFetchTransformed)(bits_image_t * pict, int x, int y, int width,
 
 	fetch = ACCESS(pixman_fetchPixelProcForPicture32)(pict);
 	
-	src_clip = pict->common.src_clip != &(pict->common.full_region);
-	
 	for ( i = 0; i < width; ++i)
 	{
 	    if (!mask || mask[i] & maskBits)
-		*(buffer + i) = fetch_nearest (pict, fetch, affine, pict->common.repeat, src_clip, &v);
+		*(buffer + i) = fetch_nearest (pict, fetch, affine, pict->common.repeat, &v);
 	    
 	    v.vector[0] += unit.vector[0];
 	    v.vector[1] += unit.vector[1];
@@ -443,7 +431,6 @@ ACCESS(fbFetchTransformed)(bits_image_t * pict, int x, int y, int width,
 	       pict->common.filter == PIXMAN_FILTER_GOOD	||
 	       pict->common.filter == PIXMAN_FILTER_BEST)
     {
-	pixman_bool_t src_clip;
 	fetchPixelProc32   fetch;
 	int i;
 
@@ -451,12 +438,11 @@ ACCESS(fbFetchTransformed)(bits_image_t * pict, int x, int y, int width,
 	adjust (&v, &unit, -(pixman_fixed_1 / 2));
 
 	fetch = ACCESS(pixman_fetchPixelProcForPicture32)(pict);
-	src_clip = pict->common.src_clip != &(pict->common.full_region);
 	
 	for (i = 0; i < width; ++i)
 	{
 	    if (!mask || mask[i] & maskBits)
-		*(buffer + i) = fetch_bilinear (pict, fetch, affine, pict->common.repeat, src_clip, &v);
+		*(buffer + i) = fetch_bilinear (pict, fetch, affine, pict->common.repeat, &v);
 	    
 	    v.vector[0] += unit.vector[0];
 	    v.vector[1] += unit.vector[1];
-- 
1.6.2.2



More information about the cairo mailing list