Improve computeMatrices singular matrix handling.

The existing singular matrix detection in computeMatrices is sufficient
but not necessary. Since compute matrices is already doing a QR
decomposition, use that to determine singularity instead. This is both
faster and more accurate than the previous method for the common case.

BUG=https://bugzilla.mozilla.org/show_bug.cgi?id=1305085

Change-Id: Iccef8368527b45e4eb565eddbebbbcf41ca66a2c
Reviewed-on: https://skia-review.googlesource.com/20054
Reviewed-by: Lee Salzman <lsalzman@mozilla.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>

2 files changed, 58 insertions, 27 deletions

diff --git a/src/core/SkScalerContext.cpp b/src/core/SkScalerContext.cpp
index f310011dcf..b2860e8f5c 100644
--- a/src/core/SkScalerContext.cpp
+++ b/src/core/SkScalerContext.cpp
@@ -703,37 +703,11 @@ bool SkScalerContextRec::computeMatrices(PreMatrixScale preMatrixScale, SkVector
         *A_out = A;
     }
 
-    // If the 'total' matrix is singular, set the 'scale' to something finite and zero the matrices.
-    // All underlying ports have issues with zero text size, so use the matricies to zero.
-
-    // Map the vectors [0,1], [1,0], [1,1] and [1,-1] (the EM) through the 'total' matrix.
-    // If the length of one of these vectors is less than 1/256 then an EM filling square will
-    // never affect any pixels.
-    SkVector diag[4] = { { A.getScaleX()               ,                 A.getSkewY() },
-                         {                 A.getSkewX(), A.getScaleY()                },
-                         { A.getScaleX() + A.getSkewX(), A.getScaleY() + A.getSkewY() },
-                         { A.getScaleX() - A.getSkewX(), A.getScaleY() - A.getSkewY() }, };
-    if (diag[0].lengthSqd() <= SK_ScalarNearlyZero * SK_ScalarNearlyZero ||
-        diag[1].lengthSqd() <= SK_ScalarNearlyZero * SK_ScalarNearlyZero ||
-        diag[2].lengthSqd() <= SK_ScalarNearlyZero * SK_ScalarNearlyZero ||
-        diag[3].lengthSqd() <= SK_ScalarNearlyZero * SK_ScalarNearlyZero)
-    {
-        s->fX = SK_Scalar1;
-        s->fY = SK_Scalar1;
-        sA->setScale(0, 0);
-        if (GsA) {
-            GsA->setScale(0, 0);
-        }
-        if (G_inv) {
-            G_inv->reset();
-        }
-        return false;
-    }
-
     // GA is the matrix A with rotation removed.
     SkMatrix GA;
     bool skewedOrFlipped = A.getSkewX() || A.getSkewY() || A.getScaleX() < 0 || A.getScaleY() < 0;
     if (skewedOrFlipped) {
+        // QR by Givens rotations. G is Q^T and GA is R. G is rotational (no reflections).
         // h is where A maps the horizontal baseline.
         SkPoint h = SkPoint::Make(SK_Scalar1, 0);
         A.mapPoints(&h, 1);
@@ -759,6 +733,25 @@ bool SkScalerContextRec::computeMatrices(PreMatrixScale preMatrixScale, SkVector
         }
     }
 
+    // If the 'total' matrix is singular, set the 'scale' to something finite and zero the matrices.
+    // All underlying ports have issues with zero text size, so use the matricies to zero.
+    // If one of the scale factors is less than 1/256 then an EM filling square will
+    // never affect any pixels.
+    if (SkScalarAbs(GA.get(SkMatrix::kMScaleX)) <= SK_ScalarNearlyZero ||
+        SkScalarAbs(GA.get(SkMatrix::kMScaleY)) <= SK_ScalarNearlyZero)
+    {
+        s->fX = SK_Scalar1;
+        s->fY = SK_Scalar1;
+        sA->setScale(0, 0);
+        if (GsA) {
+            GsA->setScale(0, 0);
+        }
+        if (G_inv) {
+            G_inv->reset();
+        }
+        return false;
+    }
+
     // At this point, given GA, create s.
     switch (preMatrixScale) {
         case kFull_PreMatrixScale:
diff --git a/tests/DrawTextTest.cpp b/tests/DrawTextTest.cpp
index 2f8fe056eb..0134d0585d 100644
--- a/tests/DrawTextTest.cpp
+++ b/tests/DrawTextTest.cpp
@@ -128,3 +128,41 @@ DEF_TEST(DrawText_weirdCoordinates, r) {
         canvas->drawString("a", 0.0f, -y, SkPaint());
     }
 }
+
+// Test drawing text with some unusual matricies.
+// We measure success by not crashing or asserting.
+DEF_TEST(DrawText_weirdMatricies, r) {
+    auto surface = SkSurface::MakeRasterN32Premul(100,100);
+    auto canvas = surface->getCanvas();
+
+    SkPaint paint;
+    paint.setAntiAlias(true);
+    paint.setLCDRenderText(true);
+
+    struct {
+        SkScalar textSize;
+        SkScalar matrix[9];
+    } testCases[] = {
+        // 2x2 singular
+        {10, { 0,  0,  0,  0,  0,  0,  0,  0,  1}},
+        {10, { 0,  0,  0,  0,  1,  0,  0,  0,  1}},
+        {10, { 0,  0,  0,  1,  0,  0,  0,  0,  1}},
+        {10, { 0,  0,  0,  1,  1,  0,  0,  0,  1}},
+        {10, { 0,  1,  0,  0,  1,  0,  0,  0,  1}},
+        {10, { 1,  0,  0,  0,  0,  0,  0,  0,  1}},
+        {10, { 1,  0,  0,  1,  0,  0,  0,  0,  1}},
+        {10, { 1,  1,  0,  0,  0,  0,  0,  0,  1}},
+        {10, { 1,  1,  0,  1,  1,  0,  0,  0,  1}},
+        // See https://bugzilla.mozilla.org/show_bug.cgi?id=1305085 .
+        { 1, {10, 20,  0, 20, 40,  0,  0,  0,  1}},
+    };
+
+    for (const auto& testCase : testCases) {
+        paint.setTextSize(testCase.textSize);
+        const SkScalar(&m)[9] = testCase.matrix;
+        SkMatrix mat;
+        mat.setAll(m[0], m[1], m[2], m[3], m[4], m[5], m[6], m[7], m[8]);
+        canvas->setMatrix(mat);
+        canvas->drawString("Hamburgefons", 10, 10, paint);
+    }
+}