Added check for ill-conditioned invert

src/core/SkMatrix.cpp

Index: src/core/SkMatrix.cpp
diff --git a/src/core/SkMatrix.cpp b/src/core/SkMatrix.cpp
index 9c9c4375f9e33e36ae2810b379b1c4cd48e632bd..6966745b9c5dccf23041858c9ac6905fe02f9564 100644
--- a/src/core/SkMatrix.cpp
+++ b/src/core/SkMatrix.cpp
@@ -752,6 +752,16 @@ static double sk_inv_determinant(const float mat[9], int isPerspective) {
     return 1.0 / det;
 }
 
+bool SkMatrix::isFinite() const {
+    for (int i = 0; i < 9; ++i) {
+        if (!SkScalarIsFinite(fMat[i])) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 void SkMatrix::SetAffineIdentity(SkScalar affine[6]) {
     affine[kAScaleX] = 1;
     affine[kASkewY] = 0;
@@ -776,6 +786,37 @@ bool SkMatrix::asAffine(SkScalar affine[6]) const {
     return true;
 }
 
+void SkMatrix::ComputeInv(const SkScalar* src, SkScalar* dst, SkScalar scale, bool isPersp) {
+    SkASSERT(src != dst);
+    SkASSERT(src && dst);
+
+    if (isPersp) {
+        dst[kMScaleX] = scross_dscale(src[kMScaleY], src[kMPersp2], src[kMTransY], src[kMPersp1], scale);
+        dst[kMSkewX]  = scross_dscale(src[kMTransX], src[kMPersp1], src[kMSkewX],  src[kMPersp2], scale);
+        dst[kMTransX] = scross_dscale(src[kMSkewX],  src[kMTransY], src[kMTransX], src[kMScaleY], scale);
+
+        dst[kMSkewY]  = scross_dscale(src[kMTransY], src[kMPersp0], src[kMSkewY],  src[kMPersp2], scale);
+        dst[kMScaleY] = scross_dscale(src[kMScaleX], src[kMPersp2], src[kMTransX], src[kMPersp0], scale);
+        dst[kMTransY] = scross_dscale(src[kMTransX], src[kMSkewY],  src[kMScaleX], src[kMTransY], scale);
+
+        dst[kMPersp0] = scross_dscale(src[kMSkewY],  src[kMPersp1], src[kMScaleY], src[kMPersp0], scale);
+        dst[kMPersp1] = scross_dscale(src[kMSkewX],  src[kMPersp0], src[kMScaleX], src[kMPersp1], scale);
+        dst[kMPersp2] = scross_dscale(src[kMScaleX], src[kMScaleY], src[kMSkewX],  src[kMSkewY],  scale);
+    } else {   // not perspective
+        dst[kMScaleX] = SkDoubleToScalar(src[kMScaleY] * scale);
+        dst[kMSkewX]  = SkDoubleToScalar(-src[kMSkewX] * scale);
+        dst[kMTransX] = dcross_dscale(src[kMSkewX], src[kMTransY], src[kMScaleY], src[kMTransX], scale);
+
+        dst[kMSkewY]  = SkDoubleToScalar(-src[kMSkewY] * scale);
+        dst[kMScaleY] = SkDoubleToScalar(src[kMScaleX] * scale);
+        dst[kMTransY] = dcross_dscale(src[kMSkewY], src[kMTransX], src[kMScaleX], src[kMTransY], scale);
+
+        dst[kMPersp0] = 0;
+        dst[kMPersp1] = 0;
+        dst[kMPersp2] = 1;
+    }
+}
+
 bool SkMatrix::invertNonIdentity(SkMatrix* inv) const {
     SkASSERT(!this->isIdentity());
 
@@ -825,44 +866,34 @@ bool SkMatrix::invertNonIdentity(SkMatrix* inv) const {
         return false;
     }
 
-    if (inv) {
-        SkMatrix tmp;
-        if (inv == this) {
-            inv = &tmp;
-        }
+    // If the scale is close to the float min check the validity of the 
+    // inverse result regardless of if the actual result is requested
+    bool checkInv = abs(scale) <= SK_ScalarSmallest;
 
-        if (isPersp) {
-            inv->fMat[kMScaleX] = scross_dscale(fMat[kMScaleY], fMat[kMPersp2], fMat[kMTransY], fMat[kMPersp1], scale);
-            inv->fMat[kMSkewX]  = scross_dscale(fMat[kMTransX], fMat[kMPersp1], fMat[kMSkewX],  fMat[kMPersp2], scale);
-            inv->fMat[kMTransX] = scross_dscale(fMat[kMSkewX],  fMat[kMTransY], fMat[kMTransX], fMat[kMScaleY], scale);
-
-            inv->fMat[kMSkewY]  = scross_dscale(fMat[kMTransY], fMat[kMPersp0], fMat[kMSkewY],  fMat[kMPersp2], scale);
-            inv->fMat[kMScaleY] = scross_dscale(fMat[kMScaleX], fMat[kMPersp2], fMat[kMTransX], fMat[kMPersp0], scale);
-            inv->fMat[kMTransY] = scross_dscale(fMat[kMTransX], fMat[kMSkewY],  fMat[kMScaleX], fMat[kMTransY], scale);
-
-            inv->fMat[kMPersp0] = scross_dscale(fMat[kMSkewY],  fMat[kMPersp1], fMat[kMScaleY], fMat[kMPersp0], scale);
-            inv->fMat[kMPersp1] = scross_dscale(fMat[kMSkewX],  fMat[kMPersp0], fMat[kMScaleX], fMat[kMPersp1], scale);
-            inv->fMat[kMPersp2] = scross_dscale(fMat[kMScaleX], fMat[kMScaleY], fMat[kMSkewX],  fMat[kMSkewY],  scale);
-        } else {   // not perspective
-            inv->fMat[kMScaleX] = SkDoubleToScalar(fMat[kMScaleY] * scale);
-            inv->fMat[kMSkewX]  = SkDoubleToScalar(-fMat[kMSkewX] * scale);
-            inv->fMat[kMTransX] = dcross_dscale(fMat[kMSkewX], fMat[kMTransY], fMat[kMScaleY], fMat[kMTransX], scale);
-
-            inv->fMat[kMSkewY]  = SkDoubleToScalar(-fMat[kMSkewY] * scale);
-            inv->fMat[kMScaleY] = SkDoubleToScalar(fMat[kMScaleX] * scale);
-            inv->fMat[kMTransY] = dcross_dscale(fMat[kMSkewY], fMat[kMTransX], fMat[kMScaleX], fMat[kMTransY], scale);
-
-            inv->fMat[kMPersp0] = 0;
-            inv->fMat[kMPersp1] = 0;
-            inv->fMat[kMPersp2] = 1;
-        }
+    if (!inv && !checkInv) {
+        return true;        // no reason to actually compute the inverse
+    }
 
-        inv->setTypeMask(fTypeMask);
+    bool applyingInPlace = (inv == this);
 
-        if (inv == &tmp) {
-            *(SkMatrix*)this = tmp;
-        }
+    SkMatrix* tmp = inv;
+
+    SkMatrix storage;
+    if (applyingInPlace || NULL == tmp) {
+        tmp = &storage;     // we either need to avoid trampling memory or have no memory
     }
+
+    ComputeInv(fMat, tmp->fMat, scale, isPersp);
+    if (!tmp->isFinite()) {
+        return false;
+    }
+
+    tmp->setTypeMask(fTypeMask);
+
+    if (applyingInPlace) {
+        *(SkMatrix*)this = storage; // need to copy answer back

[reed1, 2015/06/17 19:27:56]

why the cast? why not 

if (applyingInPlace) {
    *inv = storage;
}

[robertphillips, 2015/06/17 19:44:46]

Done. It was left over from the prior code.

+    }
+
     return true;
 }