[PATCH] Remove geometry arguments from miSourceValidate()

Søren Sandmann sandmann at cs.au.dk
Wed Mar 9 10:57:25 PST 2011


From: Søren Sandmann Pedersen <ssp at redhat.com>

The only user of the geometry coordinates is the software sprite code,
which uses them to remove the pointer whenever the window beneath is
being used as a source. However, using Window pictures as a source is
extremely rare (let alone *partial* windows), so there is no harm done
in just validating all of the drawable.

Additionally, the miSourceValidate() function was buggy in at least
three respects:

(a) It added drawable->{x,y} before calling down, which is wrong since
    the misprite code already adds them in its check. (Alternatively,
    the misprite code is wrong, but there are actual users who would
    notice if that code was broken).

(b) It didn't account for the width of the interpolation filter, so if
    the Picture had a bilinear or convolution filter, the edges
    surrounding the source area would not be validated.

(c) It didn't validate alpha maps.

Finally, computing the bounding box of the transform on every
composite request was a real performance issue in pixman, so
presumably it could be one here as well.

This patch changes miSourceValidate() to simply validate all of the
underlying drawable.

Signed-off-by: Soren Sandmann <ssp at redhat.com>
---
 fb/fbpict.c     |    4 +-
 render/mipict.c |   62 +++++++++++++-----------------------------------------
 render/mipict.h |    7 +----
 3 files changed, 19 insertions(+), 54 deletions(-)

diff --git a/fb/fbpict.c b/fb/fbpict.c
index 312f3df..133f422 100644
--- a/fb/fbpict.c
+++ b/fb/fbpict.c
@@ -54,9 +54,9 @@ fbComposite (CARD8      op,
     int msk_xoff, msk_yoff;
     int dst_xoff, dst_yoff;
     
-    miCompositeSourceValidate (pSrc, xSrc - xDst, ySrc - yDst, width, height);
+    miCompositeSourceValidate (pSrc);
     if (pMask)
-	miCompositeSourceValidate (pMask, xMask - xDst, yMask - yDst, width, height);
+	miCompositeSourceValidate (pMask);
     
     src = image_from_pict (pSrc, FALSE, &src_xoff, &src_yoff);
     mask = image_from_pict (pMask, FALSE, &msk_xoff, &msk_yoff);
diff --git a/render/mipict.c b/render/mipict.c
index 3b73888..a057840 100644
--- a/render/mipict.c
+++ b/render/mipict.c
@@ -333,12 +333,8 @@ miClipPictureSrc (RegionPtr	pRegion,
     return TRUE;
 }
 
-void
-miCompositeSourceValidate (PicturePtr	pPicture,
-			   INT16	x,
-			   INT16	y,
-			   CARD16	width,
-			   CARD16	height)
+static void
+SourceValidateOnePicture (PicturePtr pPicture)
 {
     DrawablePtr	pDrawable = pPicture->pDrawable;
     ScreenPtr	pScreen;
@@ -347,50 +343,22 @@ miCompositeSourceValidate (PicturePtr	pPicture,
         return;
 
     pScreen = pDrawable->pScreen;
-    
+
     if (pScreen->SourceValidate)
     {
-	if (pPicture->transform)
-	{
-	    xPoint	    points[4];
-	    int		    i;
-	    int		    xmin, ymin, xmax, ymax;
-
-#define VectorSet(i,_x,_y) { points[i].x = _x; points[i].y = _y; }
-	    VectorSet (0, x, y);
-	    VectorSet (1, x + width, y);
-	    VectorSet (2, x, y + height);
-	    VectorSet (3, x + width, y + height);
-	    xmin = ymin = 32767;
-	    xmax = ymax = -32737;
-	    for (i = 0; i < 4; i++)
-	    {
-		PictVector  t;
-		t.vector[0] = IntToxFixed (points[i].x);
-		t.vector[1] = IntToxFixed (points[i].y);
-		t.vector[2] = xFixed1;
-		if (pixman_transform_point (pPicture->transform, &t))
-		{
-		    int	tx = xFixedToInt (t.vector[0]);
-		    int ty = xFixedToInt (t.vector[1]);
-		    if (tx < xmin) xmin = tx;
-		    if (tx > xmax) xmax = tx;
-		    if (ty < ymin) ymin = ty;
-		    if (ty > ymax) ymax = ty;
-		}
-	    }
-	    x = xmin;
-	    y = ymin;
-	    width = xmax - xmin;
-	    height = ymax - ymin;
-	}
-        x += pPicture->pDrawable->x;
-        y += pPicture->pDrawable->y;
-	(*pScreen->SourceValidate) (pDrawable, x, y, width, height,
-				    pPicture->subWindowMode);
+	pScreen->SourceValidate (
+	    pDrawable, 0, 0, pDrawable->width, pDrawable->height, pPicture->subWindowMode);
     }
 }
 
+void
+miCompositeSourceValidate (PicturePtr pPicture)
+{
+    SourceValidateOnePicture (pPicture);
+    if (pPicture->alphaMap)
+	SourceValidateOnePicture (pPicture->alphaMap);
+}
+
 /*
  * returns FALSE if the final region is empty.  Indistinguishable from
  * an allocation failure, but rendering ignores those anyways.
@@ -480,9 +448,9 @@ miComputeCompositeRegion (RegionPtr	pRegion,
     }
 
     
-    miCompositeSourceValidate (pSrc, xSrc, ySrc, width, height);
+    miCompositeSourceValidate (pSrc);
     if (pMask)
-	miCompositeSourceValidate (pMask, xMask, yMask, width, height);
+	miCompositeSourceValidate (pMask);
 
     return TRUE;
 }
diff --git a/render/mipict.h b/render/mipict.h
index d149589..a70db24 100644
--- a/render/mipict.h
+++ b/render/mipict.h
@@ -81,11 +81,8 @@ miChangePictureFilter (PicturePtr pPicture,
 		       int	  nparams);
 
 extern _X_EXPORT void
-miCompositeSourceValidate (PicturePtr	pPicture,
-			   INT16	x,
-			   INT16	y,
-			   CARD16	width,
-			   CARD16	height);
+miCompositeSourceValidate (PicturePtr pPicture);
+
 extern _X_EXPORT Bool
 miComputeCompositeRegion (RegionPtr	pRegion,
 			  PicturePtr	pSrc,
-- 
1.7.4



More information about the xorg-devel mailing list