[PATCH] Avoid overflow in PictureTransformPoint. Fix PictureTransformIsIdentity.

Keith Packard keithp at keithp.com
Fri Nov 14 14:12:50 PST 2008


PictureTransformPoint computes homogeneous coordinates internally, but fails
to handle intermediate values larger than 16.16. Use 64 bit intermediate
values while computing the final result at 16.16 and only complain if that
result is too large.

PictureTransformIsIdentity was completely wrong -- it was not checking for
identity transforms at all.
---
 render/matrix.c     |   58 ++++++++++++++++++--------------------------------
 render/picturestr.h |    5 +---
 2 files changed, 22 insertions(+), 41 deletions(-)

diff --git a/render/matrix.c b/render/matrix.c
index a4cde4f..603281e 100644
--- a/render/matrix.c
+++ b/render/matrix.c
@@ -50,6 +50,8 @@ PictureTransformInitIdentity (PictTransformPtr matrix)
 	matrix->matrix[i][i] = F(1);
 }
 
+typedef xFixed_32_32	xFixed_34_30;
+
 _X_EXPORT Bool
 PictureTransformPoint3d (PictTransformPtr transform,
                          PictVectorPtr	vector)
@@ -78,7 +80,6 @@ PictureTransformPoint3d (PictTransformPtr transform,
     return TRUE;
 }
 
-
 _X_EXPORT Bool
 PictureTransformPoint (PictTransformPtr transform,
 		       PictVectorPtr	vector)
@@ -86,30 +87,27 @@ PictureTransformPoint (PictTransformPtr transform,
     PictVector	    result;
     int		    i, j;
     xFixed_32_32    partial;
-    xFixed_48_16    v;
+    xFixed_34_30    v[3];
+    xFixed_48_16    quo;
 
     for (j = 0; j < 3; j++)
     {
-	v = 0;
+	v[j] = 0;
 	for (i = 0; i < 3; i++)
 	{
-	    partial = ((xFixed_48_16) transform->matrix[j][i] * 
-		       (xFixed_48_16) vector->vector[i]);
-	    v += partial >> 16;
+	    partial = ((xFixed_32_32) transform->matrix[j][i] * 
+		       (xFixed_32_32) vector->vector[i]);
+	    v[j] += partial >> 2;
 	}
-	if (v > MAX_FIXED_48_16 || v < MIN_FIXED_48_16)
-	    return FALSE;
-	result.vector[j] = (xFixed) v;
     }
-    if (!result.vector[2])
+    if (!v[2])
 	return FALSE;
     for (j = 0; j < 2; j++)
     {
-	partial = (xFixed_48_16) result.vector[j] << 16;
-	v = partial / result.vector[2];
-	if (v > MAX_FIXED_48_16 || v < MIN_FIXED_48_16)
+	quo = v[j] / (v[2] >> 16);
+	if (quo > MAX_FIXED_48_16 || quo < MIN_FIXED_48_16)
 	    return FALSE;
-	vector->vector[j] = (xFixed) v;
+	vector->vector[j] = (xFixed) quo;
     }
     vector->vector[2] = xFixed1;
     return TRUE;
@@ -296,29 +294,15 @@ within_epsilon (xFixed a, xFixed b, xFixed epsilon)
 _X_EXPORT Bool
 PictureTransformIsIdentity(PictTransform *t)
 {
-    return (IsUnit (t->matrix[0][0]) &&
-	    IsUnit (t->matrix[0][1]) &&
-            IsInt  (t->matrix[0][2]) &&
-	    IsUnit (t->matrix[1][0]) &&
-	    IsUnit (t->matrix[1][1]) &&
-            IsInt  (t->matrix[1][2]) &&
-            IsZero (t->matrix[2][0]) &&
-            IsZero (t->matrix[2][1]) &&
-	    IsOne  (t->matrix[2][2]));
-}
-
-_X_EXPORT Bool
-PictureTransformIsUnit(PictTransform *t)
-{
-    return (IsSame (t->matrix[0][0],t->matrix[1][1]) &&
+    return (IsSame (t->matrix[0][0], t->matrix[1][1]) &&
 	    IsSame (t->matrix[0][0], t->matrix[2][2]) &&
-            !IsZero (t->matrix[0][0]) &&
+	    !IsZero (t->matrix[0][0]) &&
 	    IsZero (t->matrix[0][1]) &&
-            IsZero (t->matrix[0][2]) &&
-            IsZero (t->matrix[1][0]) &&
-            IsZero (t->matrix[1][2]) &&
-            IsZero (t->matrix[2][0]) &&
-            IsZero (t->matrix[2][1]));
+	    IsZero (t->matrix[0][2]) &&
+	    IsZero (t->matrix[1][0]) &&
+	    IsZero (t->matrix[1][2]) &&
+	    IsZero (t->matrix[2][0]) &&
+	    IsZero (t->matrix[2][1]));
 }
 
 _X_EXPORT Bool
@@ -334,11 +318,11 @@ PictureTransformIsScale(PictTransform *t)
 	    
 	     IsZero (t->matrix[2][0]) &&
 	     IsZero (t->matrix[2][1]) &&
-	     IsOne (t->matrix[2][2]));
+	    !IsZero (t->matrix[2][2]));
 }
 
 _X_EXPORT Bool
-PictureTransformIsTranslate(PictTransform *t)
+PictureTransformIsIntTranslate(PictTransform *t)
 {
     return ( IsOne  (t->matrix[0][0]) &&
 	     IsZero (t->matrix[0][1]) &&
diff --git a/render/picturestr.h b/render/picturestr.h
index b18f316..4a7b6e7 100644
--- a/render/picturestr.h
+++ b/render/picturestr.h
@@ -734,13 +734,10 @@ Bool
 PictureTransformIsIdentity(PictTransform *t);
 
 Bool
-PictureTransformIsUnit(PictTransform *t);
-
-Bool
 PictureTransformIsScale(PictTransform *t);
 
 Bool
-PictureTransformIsTranslate (PictTransform *t);
+PictureTransformIsIntTranslate (PictTransform *t);
 
 Bool
 PictureTransformIsInverse(PictTransform *t, PictTransform *i);
-- 
1.5.6.5




More information about the xorg mailing list