Make SkMatrix::get*Scale[s]() fail on NaN

BUG=skia:4718
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1532963002

Review URL: https://codereview.chromium.org/1532963002

3 files changed, 25 insertions, 10 deletions

diff --git a/include/core/SkMatrix.h b/include/core/SkMatrix.h
index 97a53505ef..0ebe3280e2 100644
--- a/include/core/SkMatrix.h
+++ b/include/core/SkMatrix.h
@@ -635,15 +635,17 @@ public:
 
     /**
      * Calculates the minimum scaling factor of the matrix as computed from the SVD of the upper
-     * left 2x2. If the matrix has perspective -1 is returned.
+     * left 2x2. If the max scale factor cannot be computed (for example overflow or perspective)
+     * -1 is returned.
      *
-     * @return minumum scale factor
+     * @return minimum scale factor
      */
     SkScalar getMinScale() const;
 
     /**
      * Calculates the maximum scaling factor of the matrix as computed from the SVD of the upper
-     * left 2x2. If the matrix has perspective -1 is returned.
+     * left 2x2. If the max scale factor cannot be computed (for example overflow or perspective)
+     * -1 is returned.
      *
      * @return maximum scale factor
      */
@@ -651,10 +653,10 @@ public:
 
     /**
      * Gets both the min and max scale factors. The min scale factor is scaleFactors[0] and the max
-     * is scaleFactors[1]. If the matrix has perspective false will be returned and scaleFactors
-     * will be unchanged.
+     * is scaleFactors[1]. If the min/max scale factors cannot be computed false is returned and the
+     * values of scaleFactors[] are undefined.
      */
-    bool getMinMaxScales(SkScalar scaleFactors[2]) const;
+    bool SK_WARN_UNUSED_RESULT getMinMaxScales(SkScalar scaleFactors[2]) const;
 
     /**
      *  Attempt to decompose this matrix into a scale-only component and whatever remains, where
diff --git a/src/core/SkMatrix.cpp b/src/core/SkMatrix.cpp
index 67ae052e06..dfeb721d45 100644
--- a/src/core/SkMatrix.cpp
+++ b/src/core/SkMatrix.cpp
@@ -1488,9 +1488,15 @@ template <MinMaxOrBoth MIN_MAX_OR_BOTH> bool get_scale_factor(SkMatrix::TypeMask
             results[1] = apluscdiv2 + x;
         }
     }
+    if (SkScalarIsNaN(results[0])) {
+        return false;
+    }
     SkASSERT(results[0] >= 0);
     results[0] = SkScalarSqrt(results[0]);
     if (kBoth_MinMaxOrBoth == MIN_MAX_OR_BOTH) {
+        if (SkScalarIsNaN(results[1])) {
+            return false;
+        }
         SkASSERT(results[1] >= 0);
         results[1] = SkScalarSqrt(results[1]);
     }
diff --git a/tests/MatrixTest.cpp b/tests/MatrixTest.cpp
index edeb649a66..f08613f22e 100644
--- a/tests/MatrixTest.cpp
+++ b/tests/MatrixTest.cpp
@@ -205,11 +205,18 @@ static void test_matrix_min_max_scale(skiatest::Reporter* reporter) {
     perspX.setPerspX(SK_Scalar1 / 1000);
     REPORTER_ASSERT(reporter, -SK_Scalar1 == perspX.getMinScale());
     REPORTER_ASSERT(reporter, -SK_Scalar1 == perspX.getMaxScale());
-    // Verify that getMinMaxScales() doesn't update the scales array on failure.
-    scales[0] = -5;
-    scales[1] = -5;
     success = perspX.getMinMaxScales(scales);
-    REPORTER_ASSERT(reporter, !success && -5 * SK_Scalar1 == scales[0] && -5 * SK_Scalar1  == scales[1]);
+    REPORTER_ASSERT(reporter, !success);
+
+    // skbug.com/4718
+    SkMatrix big;
+    big.setAll(2.39394089e+36f, 8.85347779e+36f, 9.26526204e+36f,
+               3.9159619e+36f, 1.44823453e+37f, 1.51559342e+37f,
+               0.f, 0.f, 1.f);
+    REPORTER_ASSERT(reporter, -SK_Scalar1 == perspX.getMinScale());
+    REPORTER_ASSERT(reporter, -SK_Scalar1 == perspX.getMaxScale());
+    success = big.getMinMaxScales(scales);
+    REPORTER_ASSERT(reporter, !success);
 
     SkMatrix perspY;
     perspY.reset();