[cairo] Broken mask transformations with pixman

Jeff Muizelaar jeff at infidigm.net
Wed Sep 19 13:46:15 PDT 2007


It looks like mask transforms are broken with the current pixman tree
and have been for some time. The problem is that existance of a mask
transformation is being ignored when selecting a special case routine.
This causes the special case routines to be passed inappropriate xMask
and yMask parameters.

It looks like the problem came from a bad merge. Specifically:

http://gitweb.freedesktop.org/?p=xorg/xserver.git;a=commitdiff;h=c19ece1d8c32dc81740a4036a642661f54064e75

I believe the code below is the code that should have been merged. You'll
notice that the line:
+       maskTransform = pMask->transform;
was ommitted in c19ece1d8c32dc81740a4036a642661f54064e75.

Also, the associated line:
        maskTransform = pMask->transform != 0;
still exists in the xserver cvs repository and the old pixman tree in cairo.

I've attached a patch to pixman and a test case for cairo.
Running the test under valgrind should give errors about inapproiate reads even
if the input is not incorrect.

-Jeff

$ cvs diff -u -r 1.31 -r 1.32 fbpict.c
Index: fbpict.c
===================================================================
RCS file: /cvs/xserver/xserver/fb/fbpict.c,v
retrieving revision 1.31
retrieving revision 1.32
diff -u -r1.31 -r1.32
--- fbpict.c	29 Jul 2004 08:10:15 -0000	1.31
+++ fbpict.c	26 Jan 2005 10:20:15 -0000	1.32
@@ -1,5 +1,5 @@
 /*
- * $Id: fbpict.c,v 1.31 2004-07-29 08:10:15 keithp Exp $
+ * $Id: fbpict.c,v 1.32 2005-01-26 10:20:15 davidr Exp $
  *
  * Copyright © 2000 SuSE, Inc.
  *
@@ -1422,22 +1422,42 @@
     CompositeFunc   func;
     Bool	    srcRepeat = pSrc->repeat;
     Bool	    maskRepeat = FALSE;
+    Bool	    srcTransform = pSrc->transform;
+    Bool	    maskTransform = FALSE;
     Bool	    srcAlphaMap = pSrc->alphaMap != 0;
     Bool	    maskAlphaMap = FALSE;
     Bool	    dstAlphaMap = pDst->alphaMap != 0;
     int		    x_msk, y_msk, x_src, y_src, x_dst, y_dst;
     int		    w, h, w_this, h_this;
+
+    if (pSrc->filter == PictFilterConvolution)
+	srcTransform = TRUE;
     
     xDst += pDst->pDrawable->x;
     yDst += pDst->pDrawable->y;
     xSrc += pSrc->pDrawable->x;
     ySrc += pSrc->pDrawable->y;
+
+    if (srcRepeat && srcTransform &&
+	pSrc->pDrawable->width == 1 &&
+	pSrc->pDrawable->height == 1)
+	srcTransform = FALSE;
+    
     if (pMask)
     {
 	xMask += pMask->pDrawable->x;
 	yMask += pMask->pDrawable->y;
 	maskRepeat = pMask->repeat;
+	maskTransform = pMask->transform;
+	if (pMask->filter == PictFilterConvolution)
+	    maskTransform = TRUE;
+	
 	maskAlphaMap = pMask->alphaMap != 0;
+
+	if (maskRepeat && maskTransform &&
+	    pMask->pDrawable->width == 1 &&
+	    pMask->pDrawable->height == 1)
+	    maskTransform = FALSE;
     }
     
     if (!miComputeCompositeRegion (&region,
@@ -1455,7 +1475,7 @@
 	return;
 				   
     func = fbCompositeGeneral;
-    if (!pSrc->transform && !(pMask && pMask->transform))
+    if (!srcTransform && !maskTransform)
     if (!maskAlphaMap && !srcAlphaMap && !dstAlphaMap)
     switch (op) {
     case PictOpOver:
@@ -1649,6 +1669,13 @@
 	}
 	break;
     }
+    
+    /* if we are transforming, we handle repeats in IcFetch[a]_transform */
+    if (srcTransform)
+	srcRepeat = 0;
+    if (maskTransform)
+	maskRepeat = 0;
+    
     n = REGION_NUM_RECTS (&region);
     pbox = REGION_RECTS (&region);
     while (n--)
-------------- next part --------------
diff --git a/pixman/pixman-pict.c b/pixman/pixman-pict.c
index 3bc5267..6d3a7a1 100644
--- a/pixman/pixman-pict.c
+++ b/pixman/pixman-pict.c
@@ -1429,6 +1429,7 @@ pixman_image_composite (pixman_op_t      op,
     {
 	maskRepeat = pMask->common.repeat == PIXMAN_REPEAT_NORMAL;
 
+	maskTransform = pMask->transform != 0;
 	if (pMask->common.filter == PIXMAN_FILTER_CONVOLUTION)
 	    maskTransform = TRUE;
 
-------------- next part --------------
commit 833c2cf313eab72aee0d438e67b44dfb63b2bfc3
Author: Jeff Muizelaar <jeff at freiheit.infidigm.net>
Date:   Wed Sep 19 16:43:54 2007 -0400

    Add mask-non-integer-transform test
    
    Shows pixman mask transformation bug.

diff --git a/test/Makefile.am b/test/Makefile.am
index ee789d8..e5a0aaf 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -60,6 +60,7 @@ linear-gradient-reflect		\
 long-lines			\
 mask				\
 mask-ctm			\
+mask-non-integer-transform	\
 mask-surface-ctm		\
 move-to-show-surface		\
 new-sub-path			\
@@ -317,6 +318,7 @@ REFERENCE_IMAGES = \
 	mask-ctm-ref.png	\
 	mask-ctm-rgb24-ref.png	\
 	mask-ctm-svg-argb32-ref.png	\
+	mask-non-integer-transform-ref.png	\
 	mask-ref.png	\
 	mask-rgb24-ref.png	\
 	mask-surface-ctm-ref.png	\
diff --git a/test/mask-non-integer-transform.c b/test/mask-non-integer-transform.c
new file mode 100644
index 0000000..bd09922
--- /dev/null
+++ b/test/mask-non-integer-transform.c
@@ -0,0 +1,77 @@
+/*
+ * Copyright © 2007 Jeff Muizelaar
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Jeff Muizelaar not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior
+ * permission. Jeff Muizelaar makes no representations about the
+ * suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * JEFF MUIZELAAR DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS
+ * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS, IN NO EVENT SHALL JEFF MUIZELAAR BE LIABLE FOR ANY SPECIAL,
+ * INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
+ * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR
+ * IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Author: Jeff Muizelaar <jeff at infidigm.net>
+ * Derived from: mask-ctm.c
+ */
+
+
+#include "cairo-test.h"
+
+static cairo_test_draw_function_t draw;
+
+cairo_test_t test = {
+    "mask-non-integer-transform",
+    "Test that cairo_mask does the right thing for non-integer transform",
+    12, 12,
+    draw
+};
+
+static cairo_test_status_t
+draw (cairo_t *cr, int width, int height)
+{
+    cairo_surface_t *mask_surface;
+    cairo_pattern_t *mask;
+    static uint8_t data[] = {
+	0xff, 0xff, 0xff, 0, 0, 0, 0, 0,
+	0xff, 0x00, 0xff, 0, 0, 0, 0, 0,
+	0xff, 0xff, 0xff, 0, 0, 0, 0, 0
+    };
+    cairo_matrix_t matrix;
+
+    cairo_set_source_rgb (cr, 0, 0, 0);
+    cairo_paint (cr);
+
+    mask_surface = cairo_image_surface_create_for_data (data, CAIRO_FORMAT_A8, 3, 3, 8);
+    mask = cairo_pattern_create_for_surface (mask_surface);
+    cairo_surface_destroy (mask_surface);
+
+    cairo_set_source_rgb (cr, 1.0, 0, 0);
+
+    /* Non-integer pattern matrix */
+    cairo_matrix_init_identity (&matrix);
+    cairo_matrix_rotate (&matrix, 1.0);
+    cairo_matrix_translate (&matrix, -6, -6);
+    cairo_pattern_set_matrix (mask, &matrix);
+
+    cairo_mask (cr, mask);
+
+    cairo_pattern_destroy (mask);
+
+    return CAIRO_TEST_SUCCESS;
+}
+
+int
+main (void)
+{
+    return cairo_test (&test);
+}


More information about the cairo mailing list