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 source clipping is turned off;
  when they are turned on, only client-set clips are honored.

The old rules were unclear and inconsistently implemented.

diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index a654b46..c991422 100644 (file)
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -186,27 +186,6 @@ bits_image_fetch_alpha_pixels (bits_image_t *image, uint32_t *buffer, int n_pixe
 static void
 bits_image_fetch_pixels_src_clip (bits_image_t *image, uint32_t *buffer, int n_pixels)
 {
-    if (image->common.src_clip != &(image->common.full_region) &&
-       !pixman_region32_equal (image->common.src_clip, &(image->common.full_region)))
-    {
-       int32_t *coords = (int32_t *)buffer;
-       int i;
-
-       for (i = 0; i < n_pixels; ++i)
-       {
-           int32_t x = coords[0];
-           int32_t y = coords[1];
-
-           if (!pixman_region32_contains_point (image->common.src_clip, x, y, NULL))
-           {
-               coords[0] = 0xffffffff;
-               coords[1] = 0xffffffff;
-           }
-
-           coords += 2;
-       }
-    }
-
     bits_image_fetch_alpha_pixels (image, buffer, n_pixels);
 }
 
@@ -663,7 +642,7 @@ bits_image_fetch_untransformed_repeat_none (bits_image_t *image, pixman_bool_t w
                                            uint32_t *buffer)
 {
     uint32_t w;
-    
+
     if (y < 0 || y >= image->height)
     {
        memset (buffer, 0, width * sizeof (uint32_t));
@@ -690,11 +669,11 @@ bits_image_fetch_untransformed_repeat_none (bits_image_t *image, pixman_bool_t w
        else
            image->fetch_scanline_raw_32 (image, x, y, w, buffer);
        
+       width -= w;
        buffer += w;
        x += w;
-       width -= w;
     }
-    
+
     memset (buffer, 0, width * (wide? 8 : 4));
 }
 
@@ -889,10 +868,6 @@ pixman_image_create_bits (pixman_format_code_t  format,
                                                                        */
     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);
-    
     image->common.property_changed = bits_image_property_changed;
     
     bits_image_property_changed (image);

diff --git a/pixman/pixman-compute-region.c b/pixman/pixman-compute-region.c
index 31eaee8..70ffa3f 100644 (file)
--- a/pixman/pixman-compute-region.c
+++ b/pixman/pixman-compute-region.c
@@ -79,43 +79,15 @@ 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)
+    /* Source clips are ignored, unless they are explicitly turned on
+     * and the clip in question was set by an X client
+     */
+    if (!pPicture->common.clip_sources || !pPicture->common.client_clip)
        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);
 }
 
 /*
@@ -123,6 +95,22 @@ miClipPictureSrc (pixman_region32_t *       pRegion,
  * an allocation failure, but rendering ignores those anyways.
  */
 
+static void
+print_region (pixman_region32_t *region, const char *header)
+{
+    int n_boxes;
+    pixman_box32_t *boxes = pixman_region32_rectangles (region, &n_boxes);
+    int i;
+
+    printf ("%s\n", header);
+    for (i = 0; i < n_boxes; ++i)
+    {
+       pixman_box32_t *box = &(boxes[i]);
+
+       printf ("   %d %d %d %d\n", box->x1, box->y1, box->x2, box->y2);
+    }
+}
+
 pixman_bool_t
 pixman_compute_composite_region32 (pixman_region32_t * pRegion,
                                   pixman_image_t *     pSrc,
@@ -145,7 +133,14 @@ pixman_compute_composite_region32 (pixman_region32_t *     pRegion,
     pRegion->extents.y1 = yDst;
     v = yDst + height;
     pRegion->extents.y2 = BOUND(v);
+
+    pRegion->extents.x1 = MAX (pRegion->extents.x1, 0);
+    pRegion->extents.y1 = MAX (pRegion->extents.y1, 0);
+    pRegion->extents.x2 = MIN (pRegion->extents.x2, pDst->bits.width);
+    pRegion->extents.y2 = MIN (pRegion->extents.y2, pDst->bits.height);
+    
     pRegion->data = 0;
+    
     /* Check for empty operation */
     if (pRegion->extents.x1 >= pRegion->extents.x2 ||
        pRegion->extents.y1 >= pRegion->extents.y2)
@@ -153,13 +148,17 @@ pixman_compute_composite_region32 (pixman_region32_t *    pRegion,
        pixman_region32_init (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,
@@ -169,13 +168,17 @@ pixman_compute_composite_region32 (pixman_region32_t *    pRegion,
            return FALSE;
        }
     }
+    
     /* 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 +189,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),
@@ -204,6 +207,10 @@ pixman_compute_composite_region32 (pixman_region32_t *     pRegion,
            }
        }
     }
+
+#if 0
+    print_region (pRegion, "composite region");
+#endif
     
     return TRUE;
 }

diff --git a/pixman/pixman-fast-path.c b/pixman/pixman-fast-path.c
index 33bc0d1..7aee95e 100644 (file)
--- a/pixman/pixman-fast-path.c
+++ b/pixman/pixman-fast-path.c
@@ -1190,7 +1190,6 @@ fast_path_composite (pixman_implementation_t *imp,
         && (src->common.filter == PIXMAN_FILTER_NEAREST)
         && PIXMAN_FORMAT_BPP(dest->bits.format) == 32
         && src->bits.format == dest->bits.format
-        && src->common.src_clip == &(src->common.full_region)
         && !src->common.read_func && !src->common.write_func
         && !dest->common.read_func && !dest->common.write_func)
     {

diff --git a/pixman/pixman-general.c b/pixman/pixman-general.c
index c9fad13..bdb6550 100644 (file)
--- a/pixman/pixman-general.c
+++ b/pixman/pixman-general.c
@@ -29,7 +29,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <math.h>
-#include <assert.h>
 #include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>

diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index b930c9f..9b4b73d 100644 (file)
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -97,10 +97,10 @@ _pixman_image_allocate (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->have_clip_region = FALSE;
+       common->clip_sources = FALSE;
        common->transform = NULL;
        common->repeat = PIXMAN_REPEAT_NONE;
        common->filter = PIXMAN_FILTER_NEAREST;
@@ -112,6 +112,7 @@ _pixman_image_allocate (void)
        common->read_func = NULL;
        common->write_func = NULL;
        common->classify = NULL;
+       common->client_clip = FALSE;
     }
 
     return image;
@@ -191,7 +192,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);
@@ -233,17 +233,7 @@ pixman_image_unref (pixman_image_t *image)
 void
 _pixman_image_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_bool_t
@@ -255,7 +245,8 @@ pixman_image_set_clip_region32 (pixman_image_t *image,
 
     if (region)
     {
-       result = pixman_region32_copy (&common->clip_region, region);
+       if ((result = pixman_region32_copy (&common->clip_region, region)))
+           image->common.have_clip_region = TRUE;
     }
     else
     {
@@ -279,7 +270,8 @@ pixman_image_set_clip_region (pixman_image_t    *image,
 
     if (region)
     {
-       result = pixman_region32_copy_from_region16 (&common->clip_region, region);
+       if ((result = pixman_region32_copy_from_region16 (&common->clip_region, region)))
+           image->common.have_clip_region = TRUE;
     }
     else
     {
@@ -293,15 +285,11 @@ pixman_image_set_clip_region (pixman_image_t    *image,
     return result;
 }
 
-/* Sets whether the clip region includes a clip region set by the client
- */
 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;
-
-    image_property_changed (image);
+    image->common.client_clip = client_clip;
 }
 
 PIXMAN_EXPORT pixman_bool_t
@@ -393,16 +381,9 @@ pixman_image_set_filter (pixman_image_t       *image,
 
 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_property_changed (image);
+    image->common.clip_sources = clip_sources;
 }
 
 /* Unlike all the other property setters, this function does not
@@ -617,11 +598,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-private.h b/pixman/pixman-private.h
index 70b4dbc..ae51447 100644 (file)
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -296,10 +296,12 @@ 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_bool_t              have_clip_region;       /* FALSE if there is no clip */
+    pixman_bool_t              client_clip;    /* Whether the source clip was set by a client */
+    pixman_bool_t              clip_sources;           /* Whether the clip applies when
+                                                        * the image is used as a source
+                                                        */
     pixman_transform_t        *transform;
     pixman_repeat_t            repeat;
     pixman_filter_t            filter;

diff --git a/pixman/pixman-utils.c b/pixman/pixman-utils.c
index 5023f3b..7fa89fa 100644 (file)
--- a/pixman/pixman-utils.c
+++ b/pixman/pixman-utils.c
@@ -565,6 +565,7 @@ walk_region_internal (pixman_implementation_t *imp,
            x_src = pbox->x1 - xDst + xSrc;
            x_msk = pbox->x1 - xDst + xMask;
            x_dst = pbox->x1;
+           
            if (maskRepeat)
            {
                y_msk = MOD (y_msk, pMask->bits.height);
@@ -629,7 +630,7 @@ _pixman_walk_composite_region (pixman_implementation_t *imp,
     pixman_region32_t region;
     
     pixman_region32_init (&region);
-    
+
     if (pixman_compute_composite_region32 (
            &region, pSrc, pMask, pDst, xSrc, ySrc, xMask, yMask, xDst, yDst, width, height))
     {