remove unnecessary SkScalar macros

src/core/SkMatrix.cpp

Index: src/core/SkMatrix.cpp
diff --git a/src/core/SkMatrix.cpp b/src/core/SkMatrix.cpp
index f32f485771ff087398e9ed21f0db7e420220b84e..cdd5142422adc360974a358f0912d4086127d364 100644
--- a/src/core/SkMatrix.cpp
+++ b/src/core/SkMatrix.cpp
@@ -10,7 +10,20 @@
 #include "SkOnce.h"
 #include "SkString.h"
 
-#define kMatrix22Elem   SK_Scalar1
+// In a few places, we performed the following
+//      a * b + c * d + e
+// as
+//      a * b + (c * d + e)
+//
+// sdot and scross are indended to capture these compound operations into a
+// function, with an eye toward considering upscaling the intermediates to
+// doubles for more precision (as we do in concat and invert).
+//
+// However, these few lines that performed the last add before the "dot", cause
+// tiny image differences, so we guard that change until we see the impact on
+// chrome's layouttests.
+//
+#define SK_LEGACY_MATRIX_MATH_ORDER
 
 static inline float SkDoubleToFloat(double x) {
     return static_cast<float>(x);
@@ -22,11 +35,10 @@ static inline float SkDoubleToFloat(double x) {
 */
 
 void SkMatrix::reset() {
-    fMat[kMScaleX] = fMat[kMScaleY] = SK_Scalar1;
+    fMat[kMScaleX] = fMat[kMScaleY] = fMat[kMPersp2] = 1;
     fMat[kMSkewX]  = fMat[kMSkewY] =
     fMat[kMTransX] = fMat[kMTransY] =
     fMat[kMPersp0] = fMat[kMPersp1] = 0;
-    fMat[kMPersp2] = kMatrix22Elem;
 
     this->setTypeMask(kIdentity_Mask | kRectStaysRect_Mask);
 }
@@ -46,8 +58,7 @@ uint8_t SkMatrix::computePerspectiveTypeMask() const {
     // Benchmarking suggests that replacing this set of SkScalarAs2sCompliment
     // is a win, but replacing those below is not. We don't yet understand
     // that result.
-    if (fMat[kMPersp0] != 0 || fMat[kMPersp1] != 0 ||
-        fMat[kMPersp2] != kMatrix22Elem) {
+    if (fMat[kMPersp0] != 0 || fMat[kMPersp1] != 0 || fMat[kMPersp2] != 1) {
         // If this is a perspective transform, we return true for all other
         // transform flags - this does not disable any optimizations, respects
         // the rule that the type mask must be conservative, and speeds up
@@ -61,8 +72,7 @@ uint8_t SkMatrix::computePerspectiveTypeMask() const {
 uint8_t SkMatrix::computeTypeMask() const {
     unsigned mask = 0;
 
-    if (fMat[kMPersp0] != 0 || fMat[kMPersp1] != 0 ||
-        fMat[kMPersp2] != kMatrix22Elem) {
+    if (fMat[kMPersp0] != 0 || fMat[kMPersp1] != 0 || fMat[kMPersp2] != 1) {
         // Once it is determined that that this is a perspective transform,
         // all other flags are moot as far as optimizations are concerned.
         return SkToU8(kORableMasks);
@@ -209,15 +219,27 @@ bool SkMatrix::preservesRightAngles(SkScalar tol) const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+static inline SkScalar sdot(SkScalar a, SkScalar b, SkScalar c, SkScalar d) {
+    return a * b + c * d;
+}
+
+static inline SkScalar sdot3(SkScalar a, SkScalar b, SkScalar c, SkScalar d,

[mtklein, 2014/01/30 19:14:55]

Don't think calling this sdot would hurt if you're into that sort of thing.

[reed1, 2014/01/30 20:53:48]

Done.

+                             SkScalar e, SkScalar f) {
+    return a * b + c * d + e * f;
+}
+
+static inline SkScalar scross(SkScalar a, SkScalar b, SkScalar c, SkScalar d) {
+    return a * b - c * d;
+}
+
 void SkMatrix::setTranslate(SkScalar dx, SkScalar dy) {
     if (dx || dy) {
         fMat[kMTransX] = dx;
         fMat[kMTransY] = dy;
 
-        fMat[kMScaleX] = fMat[kMScaleY] = SK_Scalar1;
+        fMat[kMScaleX] = fMat[kMScaleY] = fMat[kMPersp2] = 1;
         fMat[kMSkewX]  = fMat[kMSkewY] =
         fMat[kMPersp0] = fMat[kMPersp1] = 0;
-        fMat[kMPersp2] = kMatrix22Elem;
 
         this->setTypeMask(kTranslate_Mask | kRectStaysRect_Mask);
     } else {
@@ -233,10 +255,8 @@ bool SkMatrix::preTranslate(SkScalar dx, SkScalar dy) {
     }
 
     if (dx || dy) {
-        fMat[kMTransX] += SkScalarMul(fMat[kMScaleX], dx) +
-                          SkScalarMul(fMat[kMSkewX], dy);
-        fMat[kMTransY] += SkScalarMul(fMat[kMSkewY], dx) +
-                          SkScalarMul(fMat[kMScaleY], dy);
+        fMat[kMTransX] += sdot(fMat[kMScaleX], dx, fMat[kMSkewX], dy);
+        fMat[kMTransY] += sdot(fMat[kMSkewY], dx, fMat[kMScaleY], dy);
 
         this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
     }
@@ -261,14 +281,14 @@ bool SkMatrix::postTranslate(SkScalar dx, SkScalar dy) {
 ///////////////////////////////////////////////////////////////////////////////
 
 void SkMatrix::setScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
-    if (SK_Scalar1 == sx && SK_Scalar1 == sy) {
+    if (1 == sx && 1 == sy) {
         this->reset();
     } else {
         fMat[kMScaleX] = sx;
         fMat[kMScaleY] = sy;
-        fMat[kMTransX] = px - SkScalarMul(sx, px);
-        fMat[kMTransY] = py - SkScalarMul(sy, py);
-        fMat[kMPersp2] = kMatrix22Elem;
+        fMat[kMTransX] = px - sx * px;
+        fMat[kMTransY] = py - sy * py;
+        fMat[kMPersp2] = 1;
 
         fMat[kMSkewX]  = fMat[kMSkewY] =
         fMat[kMPersp0] = fMat[kMPersp1] = 0;
@@ -278,12 +298,12 @@ void SkMatrix::setScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
 }
 
 void SkMatrix::setScale(SkScalar sx, SkScalar sy) {
-    if (SK_Scalar1 == sx && SK_Scalar1 == sy) {
+    if (1 == sx && 1 == sy) {
         this->reset();
     } else {
         fMat[kMScaleX] = sx;
         fMat[kMScaleY] = sy;
-        fMat[kMPersp2] = kMatrix22Elem;
+        fMat[kMPersp2] = 1;
 
         fMat[kMTransX] = fMat[kMTransY] =
         fMat[kMSkewX]  = fMat[kMSkewY] =
@@ -297,7 +317,7 @@ bool SkMatrix::setIDiv(int divx, int divy) {
     if (!divx || !divy) {
         return false;
     }
-    this->setScale(SK_Scalar1 / divx, SK_Scalar1 / divy);
+    this->setScale(SkScalarInvert(divx), SkScalarInvert(divy));

[caryclark, 2014/01/30 21:19:10]

This makes me think that SkScalarInvert is special some of the time, on some
platform, or may be in the future. Is that this case?

Otherwise,

this->setScale(1 / divx, 1 / divy);

would be more readable for me.

[reed1, 2014/01/30 21:38:52]

1. I always imaging that we could somehow be faster if we knew that we were just
inverting, and not a general divide... but that never seems to be the case.

2. In this case, 1 / divx will do the wrong thing, since divx is an int, not a
scalar, and we will always get 0 or 1. SkScalarInvert performs the casts to
ensure we treat the parameter as a float.

Frankly, setIDiv was added specifically to help precision when we had a
fixed-point backend (and inverting a fixed threw away too many bits). Since that
is no longer a convert, I bet I can just remove this method.

     return true;
 }
 
@@ -308,7 +328,7 @@ bool SkMatrix::preScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
 }
 
 bool SkMatrix::preScale(SkScalar sx, SkScalar sy) {
-    if (SK_Scalar1 == sx && SK_Scalar1 == sy) {
+    if (1 == sx && 1 == sy) {
         return true;
     }
 
@@ -317,20 +337,20 @@ bool SkMatrix::preScale(SkScalar sx, SkScalar sy) {
     // Also, the fixed-point case checks for overflow, but the float doesn't,
     // so we can get away with these blind multiplies.
 
-    fMat[kMScaleX] = SkScalarMul(fMat[kMScaleX], sx);
-    fMat[kMSkewY] = SkScalarMul(fMat[kMSkewY],   sx);
-    fMat[kMPersp0] = SkScalarMul(fMat[kMPersp0], sx);
+    fMat[kMScaleX] *= sx;
+    fMat[kMSkewY]  *= sx;
+    fMat[kMPersp0] *= sx;
 
-    fMat[kMSkewX] = SkScalarMul(fMat[kMSkewX],   sy);
-    fMat[kMScaleY] = SkScalarMul(fMat[kMScaleY], sy);
-    fMat[kMPersp1] = SkScalarMul(fMat[kMPersp1], sy);
+    fMat[kMSkewX]  *= sy;
+    fMat[kMScaleY] *= sy;
+    fMat[kMPersp1] *= sy;
 
     this->orTypeMask(kScale_Mask);
     return true;
 }
 
 bool SkMatrix::postScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
-    if (SK_Scalar1 == sx && SK_Scalar1 == sy) {
+    if (1 == sx && 1 == sy) {
         return true;
     }
     SkMatrix    m;
@@ -339,7 +359,7 @@ bool SkMatrix::postScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
 }
 
 bool SkMatrix::postScale(SkScalar sx, SkScalar sy) {
-    if (SK_Scalar1 == sx && SK_Scalar1 == sy) {
+    if (1 == sx && 1 == sy) {
         return true;
     }
     SkMatrix    m;
@@ -373,18 +393,18 @@ bool SkMatrix::postIDiv(int divx, int divy) {
 
 void SkMatrix::setSinCos(SkScalar sinV, SkScalar cosV,
                          SkScalar px, SkScalar py) {
-    const SkScalar oneMinusCosV = SK_Scalar1 - cosV;
+    const SkScalar oneMinusCosV = 1 - cosV;
 
     fMat[kMScaleX]  = cosV;
     fMat[kMSkewX]   = -sinV;
-    fMat[kMTransX]  = SkScalarMul(sinV, py) + SkScalarMul(oneMinusCosV, px);
+    fMat[kMTransX]  = sdot(sinV, py, oneMinusCosV, px);
 
     fMat[kMSkewY]   = sinV;
     fMat[kMScaleY]  = cosV;
-    fMat[kMTransY]  = SkScalarMul(-sinV, px) + SkScalarMul(oneMinusCosV, py);
+    fMat[kMTransY]  = sdot(-sinV, px, oneMinusCosV, py);
 
     fMat[kMPersp0] = fMat[kMPersp1] = 0;
-    fMat[kMPersp2] = kMatrix22Elem;
+    fMat[kMPersp2] = 1;
 
     this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
 }
@@ -399,7 +419,7 @@ void SkMatrix::setSinCos(SkScalar sinV, SkScalar cosV) {
     fMat[kMTransY]  = 0;
 
     fMat[kMPersp0] = fMat[kMPersp1] = 0;
-    fMat[kMPersp2] = kMatrix22Elem;
+    fMat[kMPersp2] = 1;
 
     this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
 }
@@ -443,31 +463,31 @@ bool SkMatrix::postRotate(SkScalar degrees) {
 ////////////////////////////////////////////////////////////////////////////////////
 
 void SkMatrix::setSkew(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
-    fMat[kMScaleX]  = SK_Scalar1;
+    fMat[kMScaleX]  = 1;
     fMat[kMSkewX]   = sx;
-    fMat[kMTransX]  = SkScalarMul(-sx, py);
+    fMat[kMTransX]  = -sx * py;
 
     fMat[kMSkewY]   = sy;
-    fMat[kMScaleY]  = SK_Scalar1;
-    fMat[kMTransY]  = SkScalarMul(-sy, px);
+    fMat[kMScaleY]  = 1;
+    fMat[kMTransY]  = -sy * px;
 
     fMat[kMPersp0] = fMat[kMPersp1] = 0;
-    fMat[kMPersp2] = kMatrix22Elem;

[mtklein, 2014/01/30 19:14:55]

Is there no value in keeping a name on this to indicate it's special?  (This is
my one. There are many like it, but this one is mine.)

[reed1, 2014/01/30 20:53:48]

The perspective column used to be special, when we supported fixed-point... it
was stored in 2.30 (fract) instead of fixed (16.16). Since that backend is now
completely gone, I don't think the column is special anymore.

"And when everyones special... no one will be." -- Syndrome

+    fMat[kMPersp2] = 1;
 
     this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
 }
 
 void SkMatrix::setSkew(SkScalar sx, SkScalar sy) {
-    fMat[kMScaleX]  = SK_Scalar1;
+    fMat[kMScaleX]  = 1;
     fMat[kMSkewX]   = sx;
     fMat[kMTransX]  = 0;
 
     fMat[kMSkewY]   = sy;
-    fMat[kMScaleY]  = SK_Scalar1;
+    fMat[kMScaleY]  = 1;
     fMat[kMTransY]  = 0;
 
     fMat[kMPersp0] = fMat[kMPersp1] = 0;
-    fMat[kMPersp2] = kMatrix22Elem;
+    fMat[kMPersp2] = 1;
 
     this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
 }
@@ -510,8 +530,8 @@ bool SkMatrix::setRectToRect(const SkRect& src, const SkRect& dst,
         sk_bzero(fMat, 8 * sizeof(SkScalar));
         this->setTypeMask(kScale_Mask | kRectStaysRect_Mask);
     } else {
-        SkScalar    tx, sx = SkScalarDiv(dst.width(), src.width());
-        SkScalar    ty, sy = SkScalarDiv(dst.height(), src.height());
+        SkScalar    tx, sx = dst.width() / src.width();
+        SkScalar    ty, sy = dst.height() / src.height();
         bool        xLarger = false;
 
         if (align != kFill_ScaleToFit) {
@@ -523,15 +543,15 @@ bool SkMatrix::setRectToRect(const SkRect& src, const SkRect& dst,
             }
         }
 
-        tx = dst.fLeft - SkScalarMul(src.fLeft, sx);
-        ty = dst.fTop - SkScalarMul(src.fTop, sy);
+        tx = dst.fLeft - src.fLeft * sx;
+        ty = dst.fTop - src.fTop * sy;
         if (align == kCenter_ScaleToFit || align == kEnd_ScaleToFit) {
             SkScalar diff;
 
             if (xLarger) {
-                diff = dst.width() - SkScalarMul(src.width(), sy);
+                diff = dst.width() - src.width() * sy;
             } else {
-                diff = dst.height() - SkScalarMul(src.height(), sy);
+                diff = dst.height() - src.height() * sy;
             }
 
             if (align == kCenter_ScaleToFit) {
@@ -553,7 +573,7 @@ bool SkMatrix::setRectToRect(const SkRect& src, const SkRect& dst,
         fMat[kMPersp0] = fMat[kMPersp1] = 0;
 
         unsigned mask = kRectStaysRect_Mask;
-        if (sx != SK_Scalar1 || sy != SK_Scalar1) {
+        if (sx != 1 || sy != 1) {
             mask |= kScale_Mask;
         }
         if (tx || ty) {
@@ -562,7 +582,7 @@ bool SkMatrix::setRectToRect(const SkRect& src, const SkRect& dst,
         this->setTypeMask(mask);
     }
     // shared cleanup
-    fMat[kMPersp2] = kMatrix22Elem;
+    fMat[kMPersp2] = 1;
     return true;
 }
 
@@ -586,7 +606,7 @@ static inline int negifaddoverflows(float& result, float a, float b) {
 }
 
 static void normalize_perspective(SkScalar mat[9]) {
-    if (SkScalarAbs(mat[SkMatrix::kMPersp2]) > kMatrix22Elem) {
+    if (SkScalarAbs(mat[SkMatrix::kMPersp2]) > 1) {
         for (int i = 0; i < 9; i++)
             mat[i] = SkScalarHalf(mat[i]);
     }
@@ -672,7 +692,7 @@ bool SkMatrix::setConcat(const SkMatrix& a, const SkMatrix& b) {
             }
 
             tmp.fMat[kMPersp0] = tmp.fMat[kMPersp1] = 0;
-            tmp.fMat[kMPersp2] = kMatrix22Elem;
+            tmp.fMat[kMPersp2] = 1;
             //SkDebugf("Concat mat non-persp type: %d\n", tmp.getType());
             //SkASSERT(!(tmp.getType() & kPerspective_Mask));
             tmp.setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
@@ -702,19 +722,19 @@ bool SkMatrix::postConcat(const SkMatrix& mat) {
     the intermediate math, even though we know that is more expensive.
  */
 
-typedef double SkDetScalar;
-#define SkPerspMul(a, b)            SkScalarMul(a, b)
-#define SkScalarMulShift(a, b, s)   SkDoubleToFloat((a) * (b))
-static double sk_inv_determinant(const float mat[9], int isPerspective,
-                                int* /* (only used in Fixed case) */) {
+static inline double dcross(double a, double b, double c, double d) {
+    return a * b - c * d;
+}
+
+static double sk_inv_determinant(const float mat[9], int isPerspective) {
     double det;
 
     if (isPerspective) {
-        det =   mat[SkMatrix::kMScaleX] * ((double)mat[SkMatrix::kMScaleY] * mat[SkMatrix::kMPersp2] - (double)mat[SkMatrix::kMTransY] * mat[SkMatrix::kMPersp1]) +
-                mat[SkMatrix::kMSkewX] * ((double)mat[SkMatrix::kMTransY] * mat[SkMatrix::kMPersp0] - (double)mat[SkMatrix::kMSkewY] * mat[SkMatrix::kMPersp2]) +
-                mat[SkMatrix::kMTransX] * ((double)mat[SkMatrix::kMSkewY] * mat[SkMatrix::kMPersp1] - (double)mat[SkMatrix::kMScaleY] * mat[SkMatrix::kMPersp0]);
+        det = mat[SkMatrix::kMScaleX] * dcross(mat[SkMatrix::kMScaleY], mat[SkMatrix::kMPersp2], mat[SkMatrix::kMTransY], mat[SkMatrix::kMPersp1]) +

[mtklein, 2014/01/30 19:14:55]

These lines are pretty hard to parse (they were before too).  Maybe drop the
dcrosses to a new line each?

[reed1, 2014/01/30 20:53:48]

Done.

[caryclark, 2014/01/30 21:19:10]

Or, if this was a member of SkMatrix, it would be equally efficient as

        const float* m = fMat;
        det = m[kMScaleX] * dcross(m[kMScaleY], m[kMPersp2], m[kMTransY],
m[kMPersp1]) +
        ...

and fit in 100 chars.

[reed1, 2014/01/30 21:38:52]

My rewrite address the 100 chars, but I agree that these could be namespaced
into the class, so we could drop the prefix (and make it more readable). How
about I do that next CL.

+              mat[SkMatrix::kMSkewX]  * dcross(mat[SkMatrix::kMTransY], mat[SkMatrix::kMPersp0], mat[SkMatrix::kMSkewY],  mat[SkMatrix::kMPersp2]) +
+              mat[SkMatrix::kMTransX] * dcross(mat[SkMatrix::kMSkewY],  mat[SkMatrix::kMPersp1], mat[SkMatrix::kMScaleY], mat[SkMatrix::kMPersp0]);
     } else {
-        det =   (double)mat[SkMatrix::kMScaleX] * mat[SkMatrix::kMScaleY] - (double)mat[SkMatrix::kMSkewX] * mat[SkMatrix::kMSkewY];
+        det =  dcross(mat[SkMatrix::kMScaleX], mat[SkMatrix::kMScaleY], mat[SkMatrix::kMSkewX], mat[SkMatrix::kMSkewY]);
     }
 
     // Since the determinant is on the order of the cube of the matrix members,
@@ -728,16 +748,16 @@ static double sk_inv_determinant(const float mat[9], int isPerspective,
 // we declar a,b,c,d to all be doubles, because we want to perform
 // double-precision muls and subtract, even though the original values are
 // from the matrix, which are floats.
-static float inline mul_diff_scale(double a, double b, double c, double d,
+static SkScalar inline mul_diff_scale(double a, double b, double c, double d,

[mtklein, 2014/01/30 19:14:55]

mul_diff_scale might be better named dcross_scale?

[reed1, 2014/01/30 20:53:48]

That guy is now unused. deleting.

                                    double scale) {
-    return SkDoubleToFloat((a * b - c * d) * scale);
+    return SkDoubleToScalar(dcross(a, b, c, d) * scale);
 }
 
 void SkMatrix::SetAffineIdentity(SkScalar affine[6]) {
-    affine[kAScaleX] = SK_Scalar1;
+    affine[kAScaleX] = 1;
     affine[kASkewY] = 0;
     affine[kASkewX] = 0;
-    affine[kAScaleY] = SK_Scalar1;
+    affine[kAScaleY] = 1;
     affine[kATransX] = 0;
     affine[kATransY] = 0;
 }
@@ -782,9 +802,9 @@ bool SkMatrix::invertNonIdentity(SkMatrix* inv) const {
 
                 inv->fMat[kMScaleX] = invX;
                 inv->fMat[kMScaleY] = invY;
-                inv->fMat[kMPersp2] = kMatrix22Elem;
-                inv->fMat[kMTransX] = -SkScalarMul(fMat[kMTransX], invX);
-                inv->fMat[kMTransY] = -SkScalarMul(fMat[kMTransY], invY);
+                inv->fMat[kMPersp2] = 1;
+                inv->fMat[kMTransX] = -fMat[kMTransX] * invX;
+                inv->fMat[kMTransY] = -fMat[kMTransY] * invY;
 
                 inv->setTypeMask(mask | kRectStaysRect_Mask);
             } else {
@@ -799,9 +819,8 @@ bool SkMatrix::invertNonIdentity(SkMatrix* inv) const {
         return invertible;
     }
 
-    int         isPersp = mask & kPerspective_Mask;
-    int         shift;
-    SkDetScalar scale = sk_inv_determinant(fMat, isPersp, &shift);
+    int    isPersp = mask & kPerspective_Mask;
+    double scale = sk_inv_determinant(fMat, isPersp);
 
     if (scale == 0) { // underflow
         return false;
@@ -814,32 +833,29 @@ bool SkMatrix::invertNonIdentity(SkMatrix* inv) const {
         }
 
         if (isPersp) {
-            shift = 61 - shift;
-            inv->fMat[kMScaleX] = SkScalarMulShift(SkPerspMul(fMat[kMScaleY], fMat[kMPersp2]) - SkPerspMul(fMat[kMTransY], fMat[kMPersp1]), scale, shift);
-            inv->fMat[kMSkewX]  = SkScalarMulShift(SkPerspMul(fMat[kMTransX], fMat[kMPersp1]) - SkPerspMul(fMat[kMSkewX],  fMat[kMPersp2]), scale, shift);
-            inv->fMat[kMTransX] = SkScalarMulShift(SkScalarMul(fMat[kMSkewX], fMat[kMTransY]) - SkScalarMul(fMat[kMTransX], fMat[kMScaleY]), scale, shift);
-
-            inv->fMat[kMSkewY]  = SkScalarMulShift(SkPerspMul(fMat[kMTransY], fMat[kMPersp0]) - SkPerspMul(fMat[kMSkewY],   fMat[kMPersp2]), scale, shift);
-            inv->fMat[kMScaleY] = SkScalarMulShift(SkPerspMul(fMat[kMScaleX], fMat[kMPersp2]) - SkPerspMul(fMat[kMTransX],  fMat[kMPersp0]), scale, shift);
-            inv->fMat[kMTransY] = SkScalarMulShift(SkScalarMul(fMat[kMTransX], fMat[kMSkewY]) - SkScalarMul(fMat[kMScaleX], fMat[kMTransY]), scale, shift);
-
-            inv->fMat[kMPersp0] = SkScalarMulShift(SkScalarMul(fMat[kMSkewY], fMat[kMPersp1]) - SkScalarMul(fMat[kMScaleY], fMat[kMPersp0]), scale, shift);
-            inv->fMat[kMPersp1] = SkScalarMulShift(SkScalarMul(fMat[kMSkewX], fMat[kMPersp0]) - SkScalarMul(fMat[kMScaleX], fMat[kMPersp1]), scale, shift);
-            inv->fMat[kMPersp2] = SkScalarMulShift(SkScalarMul(fMat[kMScaleX], fMat[kMScaleY]) - SkScalarMul(fMat[kMSkewX], fMat[kMSkewY]), scale, shift);
+            inv->fMat[kMScaleX] = SkDoubleToScalar(scross(fMat[kMScaleY], fMat[kMPersp2], fMat[kMTransY], fMat[kMPersp1]) * scale);

[mtklein, 2014/01/30 19:14:55]

This part is also pretty hard to parse.  could be an

SkScalar scross_scale(SkScalar,SkScalar,SkScalar,SkScalar, double)

would make it clearer?

+            inv->fMat[kMSkewX]  = SkDoubleToScalar(scross(fMat[kMTransX], fMat[kMPersp1], fMat[kMSkewX],  fMat[kMPersp2]) * scale);
+            inv->fMat[kMTransX] = SkDoubleToScalar(scross(fMat[kMSkewX],  fMat[kMTransY], fMat[kMTransX], fMat[kMScaleY]) * scale);
+
+            inv->fMat[kMSkewY]  = SkDoubleToScalar(scross(fMat[kMTransY], fMat[kMPersp0], fMat[kMSkewY],  fMat[kMPersp2]) * scale);
+            inv->fMat[kMScaleY] = SkDoubleToScalar(scross(fMat[kMScaleX], fMat[kMPersp2], fMat[kMTransX], fMat[kMPersp0]) * scale);
+            inv->fMat[kMTransY] = SkDoubleToScalar(scross(fMat[kMTransX], fMat[kMSkewY],  fMat[kMScaleX], fMat[kMTransY]) * scale);
+
+            inv->fMat[kMPersp0] = SkDoubleToScalar(scross(fMat[kMSkewY],  fMat[kMPersp1], fMat[kMScaleY], fMat[kMPersp0]) * scale);
+            inv->fMat[kMPersp1] = SkDoubleToScalar(scross(fMat[kMSkewX],  fMat[kMPersp0], fMat[kMScaleX], fMat[kMPersp1]) * scale);
+            inv->fMat[kMPersp2] = SkDoubleToScalar(scross(fMat[kMScaleX], fMat[kMScaleY], fMat[kMSkewX],  fMat[kMSkewY])  * scale);
         } else {   // not perspective
-            inv->fMat[kMScaleX] = SkDoubleToFloat(fMat[kMScaleY] * scale);
-            inv->fMat[kMSkewX] = SkDoubleToFloat(-fMat[kMSkewX] * scale);
-            inv->fMat[kMTransX] = mul_diff_scale(fMat[kMSkewX], fMat[kMTransY],
-                                     fMat[kMScaleY], fMat[kMTransX], scale);
+            inv->fMat[kMScaleX] = SkDoubleToScalar(fMat[kMScaleY] * scale);
+            inv->fMat[kMSkewX]  = SkDoubleToScalar(-fMat[kMSkewX] * scale);
+            inv->fMat[kMTransX] = SkDoubleToScalar(dcross(fMat[kMSkewX], fMat[kMTransY], fMat[kMScaleY], fMat[kMTransX]) * scale);

[mtklein, 2014/01/30 19:14:55]

Why switch away from mul_diff_scale here?

[reed1, 2014/01/30 20:53:48]

Done.

 
-            inv->fMat[kMSkewY] = SkDoubleToFloat(-fMat[kMSkewY] * scale);
-            inv->fMat[kMScaleY] = SkDoubleToFloat(fMat[kMScaleX] * scale);
-            inv->fMat[kMTransY] = mul_diff_scale(fMat[kMSkewY], fMat[kMTransX],
-                                        fMat[kMScaleX], fMat[kMTransY], scale);
+            inv->fMat[kMSkewY]  = SkDoubleToScalar(-fMat[kMSkewY] * scale);
+            inv->fMat[kMScaleY] = SkDoubleToScalar(fMat[kMScaleX] * scale);
+            inv->fMat[kMTransY] = SkDoubleToScalar(dcross(fMat[kMSkewY], fMat[kMTransX], fMat[kMScaleX], fMat[kMTransY]) * scale);

[mtklein, 2014/01/30 19:14:55]

Same?

[reed1, 2014/01/30 20:53:48]

Done.

 
             inv->fMat[kMPersp0] = 0;
             inv->fMat[kMPersp1] = 0;
-            inv->fMat[kMPersp2] = kMatrix22Elem;
+            inv->fMat[kMPersp2] = 1;
 
         }
 
@@ -886,8 +902,8 @@ void SkMatrix::Scale_pts(const SkMatrix& m, SkPoint dst[],
         SkScalar mx = m.fMat[kMScaleX];
         SkScalar my = m.fMat[kMScaleY];
         do {
-            dst->fY = SkScalarMul(src->fY, my);
-            dst->fX = SkScalarMul(src->fX, mx);
+            dst->fY = src->fY * my;
+            dst->fX = src->fX * mx;
             src += 1;
             dst += 1;
         } while (--count);
@@ -904,8 +920,8 @@ void SkMatrix::ScaleTrans_pts(const SkMatrix& m, SkPoint dst[],
         SkScalar tx = m.fMat[kMTransX];
         SkScalar ty = m.fMat[kMTransY];
         do {
-            dst->fY = SkScalarMulAdd(src->fY, my, ty);
-            dst->fX = SkScalarMulAdd(src->fX, mx, tx);
+            dst->fY = src->fY * my + ty;
+            dst->fX = src->fX * mx + tx;
             src += 1;
             dst += 1;
         } while (--count);
@@ -925,8 +941,8 @@ void SkMatrix::Rot_pts(const SkMatrix& m, SkPoint dst[],
             SkScalar sy = src->fY;
             SkScalar sx = src->fX;
             src += 1;
-            dst->fY = SkScalarMul(sx, ky) + SkScalarMul(sy, my);
-            dst->fX = SkScalarMul(sx, mx) + SkScalarMul(sy, kx);
+            dst->fY = sdot(sx, ky, sy, my);
+            dst->fX = sdot(sx, mx, sy, kx);
             dst += 1;
         } while (--count);
     }
@@ -947,8 +963,13 @@ void SkMatrix::RotTrans_pts(const SkMatrix& m, SkPoint dst[],
             SkScalar sy = src->fY;
             SkScalar sx = src->fX;
             src += 1;
-            dst->fY = SkScalarMul(sx, ky) + SkScalarMulAdd(sy, my, ty);
-            dst->fX = SkScalarMul(sx, mx) + SkScalarMulAdd(sy, kx, tx);
+#ifdef SK_LEGACY_MATRIX_MATH_ORDER
+            dst->fY = sx * ky + (sy * my + ty);
+            dst->fX = sx * mx + (sy * kx + tx);
+#else
+            dst->fY = sdot(sx, ky, sy, my) + ty;
+            dst->fX = sdot(sx, mx, sy, kx) + tx;
+#endif
             dst += 1;
         } while (--count);
     }
@@ -964,18 +985,19 @@ void SkMatrix::Persp_pts(const SkMatrix& m, SkPoint dst[],
             SkScalar sx = src->fX;
             src += 1;
 
-            SkScalar x = SkScalarMul(sx, m.fMat[kMScaleX]) +
-                         SkScalarMul(sy, m.fMat[kMSkewX]) + m.fMat[kMTransX];
-            SkScalar y = SkScalarMul(sx, m.fMat[kMSkewY]) +
-                         SkScalarMul(sy, m.fMat[kMScaleY]) + m.fMat[kMTransY];
-            SkScalar z = SkScalarMul(sx, m.fMat[kMPersp0]) +
-                        SkScalarMulAdd(sy, m.fMat[kMPersp1], m.fMat[kMPersp2]);
+            SkScalar x = sdot(sx, m.fMat[kMScaleX], sy, m.fMat[kMSkewX])  + m.fMat[kMTransX];
+            SkScalar y = sdot(sx, m.fMat[kMSkewY],  sy, m.fMat[kMScaleY]) + m.fMat[kMTransY];
+#ifdef SK_LEGACY_MATRIX_MATH_ORDER
+            SkScalar z = sx * m.fMat[kMPersp0] + (sy * m.fMat[kMPersp1] + m.fMat[kMPersp2]);
+#else
+            SkScalar z = sdot(sx, m.fMat[kMPersp0], sy, m.fMat[kMPersp1]) + m.fMat[kMPersp2];
+#endif
             if (z) {
                 z = SkScalarFastInvert(z);
             }
 
-            dst->fY = SkScalarMul(y, z);
-            dst->fX = SkScalarMul(x, z);
+            dst->fY = y * z;
+            dst->fX = x * z;
             dst += 1;
         } while (--count);
     }
@@ -1019,15 +1041,9 @@ void SkMatrix::mapHomogeneousPoints(SkScalar dst[], const SkScalar src[], int co
             SkScalar sw = src[2];
             src += 3;
 
-            SkScalar x = SkScalarMul(sx, fMat[kMScaleX]) +
-                         SkScalarMul(sy, fMat[kMSkewX]) +
-                         SkScalarMul(sw, fMat[kMTransX]);
-            SkScalar y = SkScalarMul(sx, fMat[kMSkewY]) +
-                         SkScalarMul(sy, fMat[kMScaleY]) +
-                         SkScalarMul(sw, fMat[kMTransY]);
-            SkScalar w = SkScalarMul(sx, fMat[kMPersp0]) +
-                         SkScalarMul(sy, fMat[kMPersp1]) +
-                         SkScalarMul(sw, fMat[kMPersp2]);
+            SkScalar x = sdot3(sx, fMat[kMScaleX], sy, fMat[kMSkewX],  sw, fMat[kMTransX]);
+            SkScalar y = sdot3(sx, fMat[kMSkewY],  sy, fMat[kMScaleY], sw, fMat[kMTransY]);
+            SkScalar w = sdot3(sx, fMat[kMPersp0], sy, fMat[kMPersp1], sw, fMat[kMPersp2]);
 
             dst[0] = x;
             dst[1] = y;
@@ -1098,27 +1114,27 @@ void SkMatrix::Persp_xy(const SkMatrix& m, SkScalar sx, SkScalar sy,
                         SkPoint* pt) {
     SkASSERT(m.hasPerspective());
 
-    SkScalar x = SkScalarMul(sx, m.fMat[kMScaleX]) +
-                 SkScalarMul(sy, m.fMat[kMSkewX]) + m.fMat[kMTransX];
-    SkScalar y = SkScalarMul(sx, m.fMat[kMSkewY]) +
-                 SkScalarMul(sy, m.fMat[kMScaleY]) + m.fMat[kMTransY];
-    SkScalar z = SkScalarMul(sx, m.fMat[kMPersp0]) +
-                 SkScalarMul(sy, m.fMat[kMPersp1]) + m.fMat[kMPersp2];
+    SkScalar x = sdot(sx, m.fMat[kMScaleX], sy, m.fMat[kMSkewX])  + m.fMat[kMTransX];
+    SkScalar y = sdot(sx, m.fMat[kMSkewY],  sy, m.fMat[kMScaleY]) + m.fMat[kMTransY];
+    SkScalar z = sdot(sx, m.fMat[kMPersp0], sy, m.fMat[kMPersp1]) + m.fMat[kMPersp2];
     if (z) {
         z = SkScalarFastInvert(z);
     }
-    pt->fX = SkScalarMul(x, z);
-    pt->fY = SkScalarMul(y, z);
+    pt->fX = x * z;
+    pt->fY = y * z;
 }
 
 void SkMatrix::RotTrans_xy(const SkMatrix& m, SkScalar sx, SkScalar sy,
                            SkPoint* pt) {
     SkASSERT((m.getType() & (kAffine_Mask | kPerspective_Mask)) == kAffine_Mask);
 
-    pt->fX = SkScalarMul(sx, m.fMat[kMScaleX]) +
-             SkScalarMulAdd(sy, m.fMat[kMSkewX], m.fMat[kMTransX]);
-    pt->fY = SkScalarMul(sx, m.fMat[kMSkewY]) +
-             SkScalarMulAdd(sy, m.fMat[kMScaleY], m.fMat[kMTransY]);
+#ifdef SK_LEGACY_MATRIX_MATH_ORDER
+    pt->fX = sx * m.fMat[kMScaleX] + (sy * m.fMat[kMSkewX]  +  m.fMat[kMTransX]);
+    pt->fY = sx * m.fMat[kMSkewY]  + (sy * m.fMat[kMScaleY] + m.fMat[kMTransY]);
+#else
+    pt->fX = sdot(sx, m.fMat[kMScaleX], sy, m.fMat[kMSkewX])  + m.fMat[kMTransX];
+    pt->fY = sdot(sx, m.fMat[kMSkewY],  sy, m.fMat[kMScaleY]) + m.fMat[kMTransY];
+#endif
 }
 
 void SkMatrix::Rot_xy(const SkMatrix& m, SkScalar sx, SkScalar sy,
@@ -1127,10 +1143,13 @@ void SkMatrix::Rot_xy(const SkMatrix& m, SkScalar sx, SkScalar sy,
     SkASSERT(0 == m.fMat[kMTransX]);
     SkASSERT(0 == m.fMat[kMTransY]);
 
-    pt->fX = SkScalarMul(sx, m.fMat[kMScaleX]) +
-             SkScalarMulAdd(sy, m.fMat[kMSkewX], m.fMat[kMTransX]);
-    pt->fY = SkScalarMul(sx, m.fMat[kMSkewY]) +
-             SkScalarMulAdd(sy, m.fMat[kMScaleY], m.fMat[kMTransY]);
+#ifdef SK_LEGACY_MATRIX_MATH_ORDER
+    pt->fX = sx * m.fMat[kMScaleX] + (sy * m.fMat[kMSkewX]  + m.fMat[kMTransX]);
+    pt->fY = sx * m.fMat[kMSkewY]  + (sy * m.fMat[kMScaleY] + m.fMat[kMTransY]);
+#else
+    pt->fX = sdot(sx, m.fMat[kMScaleX], sy, m.fMat[kMSkewX])  + m.fMat[kMTransX];
+    pt->fY = sdot(sx, m.fMat[kMSkewY],  sy, m.fMat[kMScaleY]) + m.fMat[kMTransY];
+#endif
 }
 
 void SkMatrix::ScaleTrans_xy(const SkMatrix& m, SkScalar sx, SkScalar sy,
@@ -1138,8 +1157,8 @@ void SkMatrix::ScaleTrans_xy(const SkMatrix& m, SkScalar sx, SkScalar sy,
     SkASSERT((m.getType() & (kScale_Mask | kAffine_Mask | kPerspective_Mask))
              == kScale_Mask);
 
-    pt->fX = SkScalarMulAdd(sx, m.fMat[kMScaleX], m.fMat[kMTransX]);
-    pt->fY = SkScalarMulAdd(sy, m.fMat[kMScaleY], m.fMat[kMTransY]);
+    pt->fX = sx * m.fMat[kMScaleX] + m.fMat[kMTransX];
+    pt->fY = sy * m.fMat[kMScaleY] + m.fMat[kMTransY];
 }
 
 void SkMatrix::Scale_xy(const SkMatrix& m, SkScalar sx, SkScalar sy,
@@ -1149,8 +1168,8 @@ void SkMatrix::Scale_xy(const SkMatrix& m, SkScalar sx, SkScalar sy,
     SkASSERT(0 == m.fMat[kMTransX]);
     SkASSERT(0 == m.fMat[kMTransY]);
 
-    pt->fX = SkScalarMul(sx, m.fMat[kMScaleX]);
-    pt->fY = SkScalarMul(sy, m.fMat[kMScaleY]);
+    pt->fX = sx * m.fMat[kMScaleX];
+    pt->fY = sy * m.fMat[kMScaleY];
 }
 
 void SkMatrix::Trans_xy(const SkMatrix& m, SkScalar sx, SkScalar sy,
@@ -1190,7 +1209,7 @@ bool SkMatrix::fixedStepInX(SkScalar y, SkFixed* stepX, SkFixed* stepY) const {
     if (PerspNearlyZero(fMat[kMPersp0])) {
         if (stepX || stepY) {
             if (PerspNearlyZero(fMat[kMPersp1]) &&
-                    PerspNearlyZero(fMat[kMPersp2] - kMatrix22Elem)) {
+                    PerspNearlyZero(fMat[kMPersp2] - 1)) {
                 if (stepX) {
                     *stepX = SkScalarToFixed(fMat[kMScaleX]);
                 }
@@ -1200,10 +1219,10 @@ bool SkMatrix::fixedStepInX(SkScalar y, SkFixed* stepX, SkFixed* stepY) const {
             } else {
                 SkScalar z = y * fMat[kMPersp1] + fMat[kMPersp2];
                 if (stepX) {
-                    *stepX = SkScalarToFixed(SkScalarDiv(fMat[kMScaleX], z));
+                    *stepX = SkScalarToFixed(fMat[kMScaleX] / z);
                 }
                 if (stepY) {
-                    *stepY = SkScalarToFixed(SkScalarDiv(fMat[kMSkewY], z));
+                    *stepY = SkScalarToFixed(fMat[kMSkewY] / z);
                 }
             }
         }
@@ -1291,8 +1310,7 @@ static inline bool poly_to_point(SkPoint* pt, const SkPoint poly[], int count) {
                 pt2.fX = poly[0].fY - poly[3].fY;
                 pt2.fY = poly[3].fX - poly[0].fX;
             CALC_X:
-                x = SkScalarDiv(SkScalarMul(pt1.fX, pt2.fX) +
-                                SkScalarMul(pt1.fY, pt2.fY), y);
+                x = sdot(pt1.fX, pt2.fX, pt1.fY, pt2.fY) / y;
                 break;
         }
     }
@@ -1354,13 +1372,13 @@ bool SkMatrix::Poly4Proc(const SkPoint srcPt[], SkMatrix* dst,
         if (checkForZero(denom)) {
             return false;
         }
-        a1 = SkScalarDiv(SkScalarMulDiv(x0 - x1, y2, x2) - y0 + y1, denom);
+        a1 = (SkScalarMulDiv(x0 - x1, y2, x2) - y0 + y1) / denom;
     } else {
         float denom = x1 - SkScalarMulDiv(y1, x2, y2);
         if (checkForZero(denom)) {
             return false;
         }
-        a1 = SkScalarDiv(x0 - x1 - SkScalarMulDiv(y0 - y1, x2, y2), denom);
+        a1 = (x0 - x1 - SkScalarMulDiv(y0 - y1, x2, y2)) / denom;
     }
 
     /* check if abs(x1) > abs(y1) */
@@ -1369,27 +1387,25 @@ bool SkMatrix::Poly4Proc(const SkPoint srcPt[], SkMatrix* dst,
         if (checkForZero(denom)) {
             return false;
         }
-        a2 = SkScalarDiv(y0 - y2 - SkScalarMulDiv(x0 - x2, y1, x1), denom);
+        a2 = (y0 - y2 - SkScalarMulDiv(x0 - x2, y1, x1)) / denom;
     } else {
         float denom = SkScalarMulDiv(y2, x1, y1) - x2;
         if (checkForZero(denom)) {
             return false;
         }
-        a2 = SkScalarDiv(SkScalarMulDiv(y0 - y2, x1, y1) - x0 + x2, denom);
+        a2 = (SkScalarMulDiv(y0 - y2, x1, y1) - x0 + x2) / denom;
     }
 
-    float invScale = 1 / scale.fX;
-    dst->fMat[kMScaleX] = SkScalarMul(SkScalarMul(a2, srcPt[3].fX) +
-                                      srcPt[3].fX - srcPt[0].fX, invScale);
-    dst->fMat[kMSkewY] = SkScalarMul(SkScalarMul(a2, srcPt[3].fY) +
-                                     srcPt[3].fY - srcPt[0].fY, invScale);
-    dst->fMat[kMPersp0] = SkScalarMul(a2, invScale);
-    invScale = 1 / scale.fY;
-    dst->fMat[kMSkewX] = SkScalarMul(SkScalarMul(a1, srcPt[1].fX) +
-                                     srcPt[1].fX - srcPt[0].fX, invScale);
-    dst->fMat[kMScaleY] = SkScalarMul(SkScalarMul(a1, srcPt[1].fY) +
-                                      srcPt[1].fY - srcPt[0].fY, invScale);
-    dst->fMat[kMPersp1] = SkScalarMul(a1, invScale);
+    float invScale = SkScalarInvert(scale.fX);
+    dst->fMat[kMScaleX] = (a2 * srcPt[3].fX + srcPt[3].fX - srcPt[0].fX) * invScale;
+    dst->fMat[kMSkewY]  = (a2 * srcPt[3].fY + srcPt[3].fY - srcPt[0].fY) * invScale;
+    dst->fMat[kMPersp0] = a2 * invScale;
+
+    invScale = SkScalarInvert(scale.fY);
+    dst->fMat[kMSkewX]  = (a1 * srcPt[1].fX + srcPt[1].fX - srcPt[0].fX) * invScale;
+    dst->fMat[kMScaleY] = (a1 * srcPt[1].fY + srcPt[1].fY - srcPt[0].fY) * invScale;
+    dst->fMat[kMPersp1] = a1 * invScale;
+
     dst->fMat[kMTransX] = srcPt[0].fX;
     dst->fMat[kMTransY] = srcPt[0].fY;
     dst->fMat[kMPersp2] = 1;
@@ -1458,10 +1474,10 @@ enum MinOrMax {
 template <MinOrMax MIN_OR_MAX> SkScalar get_stretch_factor(SkMatrix::TypeMask typeMask,
                                                            const SkScalar m[9]) {
     if (typeMask & SkMatrix::kPerspective_Mask) {
-        return -SK_Scalar1;
+        return -1;
     }
     if (SkMatrix::kIdentity_Mask == typeMask) {
-        return SK_Scalar1;
+        return 1;
     }
     if (!(typeMask & SkMatrix::kAffine_Mask)) {
         if (kMin_MinOrMax == MIN_OR_MAX) {
@@ -1475,19 +1491,16 @@ template <MinOrMax MIN_OR_MAX> SkScalar get_stretch_factor(SkMatrix::TypeMask ty
     // ignore the translation part of the matrix, just look at 2x2 portion.
     // compute singular values, take largest or smallest abs value.
     // [a b; b c] = A^T*A
-    SkScalar a = SkScalarMul(m[SkMatrix::kMScaleX], m[SkMatrix::kMScaleX]) +
-                 SkScalarMul(m[SkMatrix::kMSkewY],  m[SkMatrix::kMSkewY]);
-    SkScalar b = SkScalarMul(m[SkMatrix::kMScaleX], m[SkMatrix::kMSkewX]) +
-                 SkScalarMul(m[SkMatrix::kMScaleY], m[SkMatrix::kMSkewY]);
-    SkScalar c = SkScalarMul(m[SkMatrix::kMSkewX],  m[SkMatrix::kMSkewX]) +
-                 SkScalarMul(m[SkMatrix::kMScaleY], m[SkMatrix::kMScaleY]);
+    SkScalar a = sdot(m[SkMatrix::kMScaleX], m[SkMatrix::kMScaleX], m[SkMatrix::kMSkewY],  m[SkMatrix::kMSkewY]);

[mtklein, 2014/01/30 19:14:55]

Clearer back on two lines each?

[reed1, 2014/01/30 20:53:48]

Done.

+    SkScalar b = sdot(m[SkMatrix::kMScaleX], m[SkMatrix::kMSkewX],  m[SkMatrix::kMScaleY], m[SkMatrix::kMSkewY]);
+    SkScalar c = sdot(m[SkMatrix::kMSkewX],  m[SkMatrix::kMSkewX],  m[SkMatrix::kMScaleY], m[SkMatrix::kMScaleY]);
     // eigenvalues of A^T*A are the squared singular values of A.
     // characteristic equation is det((A^T*A) - l*I) = 0
     // l^2 - (a + c)l + (ac-b^2)
     // solve using quadratic equation (divisor is non-zero since l^2 has 1 coeff
     // and roots are guaranteed to be pos and real).
     SkScalar chosenRoot;
-    SkScalar bSqd = SkScalarMul(b,b);
+    SkScalar bSqd = b * b;
     // if upper left 2x2 is orthogonal save some math
     if (bSqd <= SK_ScalarNearlyZero*SK_ScalarNearlyZero) {
         if (kMin_MinOrMax == MIN_OR_MAX) {
@@ -1498,7 +1511,7 @@ template <MinOrMax MIN_OR_MAX> SkScalar get_stretch_factor(SkMatrix::TypeMask ty
     } else {
         SkScalar aminusc = a - c;
         SkScalar apluscdiv2 = SkScalarHalf(a + c);
-        SkScalar x = SkScalarHalf(SkScalarSqrt(SkScalarMul(aminusc, aminusc) + 4 * bSqd));
+        SkScalar x = SkScalarHalf(SkScalarSqrt(aminusc * aminusc + 4 * bSqd));
         if (kMin_MinOrMax == MIN_OR_MAX) {
             chosenRoot = apluscdiv2 - x;
         } else {
@@ -1661,7 +1674,7 @@ bool SkDecomposeUpper2x2(const SkMatrix& matrix,
     double Sa, Sb, Sd;
     // if M is already symmetric (i.e., M = I*S)
     if (SkScalarNearlyEqual(B, C)) {
-        cosQ = SK_Scalar1;
+        cosQ = 1;
         sinQ = 0;
 
         Sa = A;
@@ -1670,7 +1683,7 @@ bool SkDecomposeUpper2x2(const SkMatrix& matrix,
     } else {
         cosQ = A + D;
         sinQ = C - B;
-        SkScalar reciplen = SK_Scalar1/SkScalarSqrt(cosQ*cosQ + sinQ*sinQ);
+        SkScalar reciplen = SkScalarInvert(SkScalarSqrt(cosQ*cosQ + sinQ*sinQ));
         cosQ *= reciplen;
         sinQ *= reciplen;
 
@@ -1686,7 +1699,7 @@ bool SkDecomposeUpper2x2(const SkMatrix& matrix,
     // From this, should be able to reconstruct S as U*W*U^T
     if (SkScalarNearlyZero(SkDoubleToScalar(Sb))) {
         // already diagonalized
-        cos1 = SK_Scalar1;
+        cos1 = 1;
         sin1 = 0;
         w1 = Sa;
         w2 = Sd;
@@ -1705,7 +1718,7 @@ bool SkDecomposeUpper2x2(const SkMatrix& matrix,
         }
 
         cos1 = SkDoubleToScalar(Sb); sin1 = SkDoubleToScalar(w1 - Sa);
-        SkScalar reciplen = SK_Scalar1/SkScalarSqrt(cos1*cos1 + sin1*sin1);
+        SkScalar reciplen = SkScalarInvert(SkScalarSqrt(cos1*cos1 + sin1*sin1));
         cos1 *= reciplen;
         sin1 *= reciplen;