Guard SkMatrix::get*Scale[s]() against negative nearly-zero values.

src/core/SkMatrix.cpp

Index: src/core/SkMatrix.cpp
diff --git a/src/core/SkMatrix.cpp b/src/core/SkMatrix.cpp
index 492f3f6ac447fd3575dbcb775b6a7abf4531644b..fc698530ec9d2b01a31778d70a5198b5ea61e993 100644
--- a/src/core/SkMatrix.cpp
+++ b/src/core/SkMatrix.cpp
@@ -1568,15 +1568,21 @@ template <MinMaxOrBoth MIN_MAX_OR_BOTH> bool get_scale_factor(SkMatrix::TypeMask
             results[1] = apluscdiv2 + x;
         }
     }
-    if (SkScalarIsNaN(results[0])) {
+    if (!SkScalarIsFinite(results[0])) {
         return false;
     }
+    if (results[0] < 0 && results[0] > -SK_ScalarNearlyZero) {

[jvanverth1, 2016/07/15 13:49:05]

I missed something here -- why was this changed to only clamp negative nearly
zero values? This seems asymmetric.

[reed1, 2016/07/17 00:15:17]

Agreed. This reads oddly -- we clamp nearly-zero-negative values, and then
assert that we're never negative. Why not add whatever assert you want ahead of
time, and then clamp all negative values?

+        results[0] = 0;
+    }
     SkASSERT(results[0] >= 0);
     results[0] = SkScalarSqrt(results[0]);
     if (kBoth_MinMaxOrBoth == MIN_MAX_OR_BOTH) {
-        if (SkScalarIsNaN(results[1])) {
+        if (!SkScalarIsFinite(results[1])) {
             return false;
         }
+        if (results[1] < 0 && results[1] > -SK_ScalarNearlyZero) {
+            results[1] = 0;
+        }
         SkASSERT(results[1] >= 0);
         results[1] = SkScalarSqrt(results[1]);
     }