remove unnecessary SkScalar macros

src/core/SkMatrix.cpp

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkMatrix.h"
 #include "SkFloatBits.h"
 #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);
 }
 
 /*      [scale-x    skew-x      trans-x]   [X]   [X']
         [skew-y     scale-y     trans-y] * [Y] = [Y']
         [persp-0    persp-1     persp-2]   [1]   [1 ]
 */
 
 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);
 }
 
 // this guy aligns with the masks, so we can compute a mask from a varaible 0/1
 enum {
     kTranslate_Shift,
     kScale_Shift,
     kAffine_Shift,
     kPerspective_Shift,
     kRectStaysRect_Shift
 };
 
 static const int32_t kScalar1Int = 0x3f800000;
 
 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
         // type mask computation.
         return SkToU8(kORableMasks);
     }
 
     return SkToU8(kOnlyPerspectiveValid_Mask | kUnknown_Mask);
 }
 
 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);
     }
 
     if (fMat[kMTransX] != 0 || fMat[kMTransY] != 0) {
         mask |= kTranslate_Mask;
     }
 
     int m00 = SkScalarAs2sCompliment(fMat[SkMatrix::kMScaleX]);
@@ skipping 126 matching lines @@
     vec[0].set(mx, sx);
     vec[1].set(sy, my);
 
     return SkScalarNearlyZero(vec[0].dot(vec[1]), SkScalarSquare(tol)) &&
            SkScalarNearlyEqual(vec[0].lengthSqd(), vec[1].lengthSqd(),
                                SkScalarSquare(tol));
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
+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 {
         this->reset();
     }
 }
 
 bool SkMatrix::preTranslate(SkScalar dx, SkScalar dy) {
     if (this->hasPerspective()) {
         SkMatrix    m;
         m.setTranslate(dx, dy);
         return this->preConcat(m);
     }
 
     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);
     }
     return true;
 }
 
 bool SkMatrix::postTranslate(SkScalar dx, SkScalar dy) {
     if (this->hasPerspective()) {
         SkMatrix    m;
         m.setTranslate(dx, dy);
         return this->postConcat(m);
     }
 
     if (dx || dy) {
         fMat[kMTransX] += dx;
         fMat[kMTransY] += dy;
         this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
     }
     return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 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;
 
         this->setTypeMask(kScale_Mask | kTranslate_Mask | kRectStaysRect_Mask);
     }
 }
 
 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] =
         fMat[kMPersp0] = fMat[kMPersp1] = 0;
 
         this->setTypeMask(kScale_Mask | kRectStaysRect_Mask);
     }
 }
 
 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;
 }
 
 bool SkMatrix::preScale(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
     SkMatrix    m;
     m.setScale(sx, sy, px, py);
     return this->preConcat(m);
 }
 
 bool SkMatrix::preScale(SkScalar sx, SkScalar sy) {
-    if (SK_Scalar1 == sx && SK_Scalar1 == sy) {
+    if (1 == sx && 1 == sy) {
         return true;
     }
 
     // the assumption is that these multiplies are very cheap, and that
     // a full concat and/or just computing the matrix type is more expensive.
     // 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;
     m.setScale(sx, sy, px, py);
     return this->postConcat(m);
 }
 
 bool SkMatrix::postScale(SkScalar sx, SkScalar sy) {
-    if (SK_Scalar1 == sx && SK_Scalar1 == sy) {
+    if (1 == sx && 1 == sy) {
         return true;
     }
     SkMatrix    m;
     m.setScale(sx, sy);
     return this->postConcat(m);
 }
 
 // this guy perhaps can go away, if we have a fract/high-precision way to
 // scale matrices
 bool SkMatrix::postIDiv(int divx, int divy) {
@@ skipping 13 matching lines @@
     fMat[kMTransY] *= invY;
 
     this->setTypeMask(kUnknown_Mask);
     return true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////////
 
 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);
 }
 
 void SkMatrix::setSinCos(SkScalar sinV, SkScalar cosV) {
     fMat[kMScaleX]  = cosV;
     fMat[kMSkewX]   = -sinV;
     fMat[kMTransX]  = 0;
 
     fMat[kMSkewY]   = sinV;
     fMat[kMScaleY]  = cosV;
     fMat[kMTransY]  = 0;
 
     fMat[kMPersp0] = fMat[kMPersp1] = 0;
-    fMat[kMPersp2] = kMatrix22Elem;
+    fMat[kMPersp2] = 1;
 
     this->setTypeMask(kUnknown_Mask | kOnlyPerspectiveValid_Mask);
 }
 
 void SkMatrix::setRotate(SkScalar degrees, SkScalar px, SkScalar py) {
     SkScalar sinV, cosV;
     sinV = SkScalarSinCos(SkDegreesToRadians(degrees), &cosV);
     this->setSinCos(sinV, cosV, px, py);
 }
 
@@ skipping 23 matching lines @@
 
 bool SkMatrix::postRotate(SkScalar degrees) {
     SkMatrix    m;
     m.setRotate(degrees);
     return this->postConcat(m);
 }
 
 ////////////////////////////////////////////////////////////////////////////////////
 
 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);
 }
 
 bool SkMatrix::preSkew(SkScalar sx, SkScalar sy, SkScalar px, SkScalar py) {
     SkMatrix    m;
     m.setSkew(sx, sy, px, py);
     return this->preConcat(m);
 }
 
@@ skipping 22 matching lines @@
 {
     if (src.isEmpty()) {
         this->reset();
         return false;
     }
 
     if (dst.isEmpty()) {
         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) {
             if (sx > sy) {
                 xLarger = true;
                 sx = sy;
             } else {
                 sy = sx;
             }
         }
 
-        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) {
                 diff = SkScalarHalf(diff);
             }
 
             if (xLarger) {
                 tx += diff;
             } else {
                 ty += diff;
             }
         }
 
         fMat[kMScaleX] = sx;
         fMat[kMScaleY] = sy;
         fMat[kMTransX] = tx;
         fMat[kMTransY] = ty;
         fMat[kMSkewX]  = fMat[kMSkewY] =
         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) {
             mask |= kTranslate_Mask;
         }
         this->setTypeMask(mask);
     }
     // shared cleanup
-    fMat[kMPersp2] = kMatrix22Elem;
+    fMat[kMPersp2] = 1;
     return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 static inline int fixmuladdmul(float a, float b, float c, float d,
                                float* result) {
     *result = SkDoubleToFloat((double)a * b + (double)c * d);
     return true;
 }
 
 static inline bool rowcol3(const float row[], const float col[],
                            float* result) {
     *result = row[0] * col[0] + row[1] * col[3] + row[2] * col[6];
     return true;
 }
 
 static inline int negifaddoverflows(float& result, float a, float b) {
     result = a + b;
     return 0;
 }
 
 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]);
     }
 }
 
 bool SkMatrix::setConcat(const SkMatrix& a, const SkMatrix& b) {
     TypeMask aType = a.getPerspectiveTypeMaskOnly();
     TypeMask bType = b.getPerspectiveTypeMaskOnly();
 
     if (a.isTriviallyIdentity()) {
@@ skipping 65 matching lines @@
             if (!fixmuladdmul(a.fMat[kMSkewY], b.fMat[kMTransX],
                      a.fMat[kMScaleY], b.fMat[kMTransY], &tmp.fMat[kMTransY])) {
                 return false;
             }
             if (negifaddoverflows(tmp.fMat[kMTransY], tmp.fMat[kMTransY],
                                   a.fMat[kMTransY]) < 0) {
                 return false;
             }
 
             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);
         }
         *this = tmp;
     }
     return true;
 }
 
 bool SkMatrix::preConcat(const SkMatrix& mat) {
     // check for identity first, so we don't do a needless copy of ourselves
     // to ourselves inside setConcat()
     return mat.isIdentity() || this->setConcat(*this, mat);
 }
 
 bool SkMatrix::postConcat(const SkMatrix& mat) {
     // check for identity first, so we don't do a needless copy of ourselves
     // to ourselves inside setConcat()
     return mat.isIdentity() || this->setConcat(mat, *this);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 /*  Matrix inversion is very expensive, but also the place where keeping
     precision may be most important (here and matrix concat). Hence to avoid
     bitmap blitting artifacts when walking the inverse, we use doubles for
     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,
     // compare to the cube of the default nearly-zero constant (although an
     // estimate of the condition number would be better if it wasn't so expensive).
     if (SkScalarNearlyZero((float)det, SK_ScalarNearlyZero * SK_ScalarNearlyZero * SK_ScalarNearlyZero)) {
         return 0;
     }
     return 1.0 / det;
 }
 // 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;
 }
 
 bool SkMatrix::asAffine(SkScalar affine[6]) const {
     if (this->hasPerspective()) {
         return false;
     }
     if (affine) {
         affine[kAScaleX] = this->fMat[kMScaleX];
@@ skipping 24 matching lines @@
                 invY = SkScalarInvert(invY);
 
                 // Must be careful when writing to inv, since it may be the
                 // same memory as this.
 
                 inv->fMat[kMSkewX] = inv->fMat[kMSkewY] =
                 inv->fMat[kMPersp0] = inv->fMat[kMPersp1] = 0;
 
                 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 {
                 // translate only
                 inv->setTranslate(-fMat[kMTransX], -fMat[kMTransY]);
             }
         } else {    // inv is NULL, just check if we're invertible
             if (!fMat[kMScaleX] || !fMat[kMScaleY]) {
                 invertible = false;
             }
         }
         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;
     }
 
     if (inv) {
         SkMatrix tmp;
         if (inv == this) {
             inv = &tmp;
         }
 
         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[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]  = 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[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] = 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[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;
 
         }
 
         inv->setTypeMask(fTypeMask);
 
         if (inv == &tmp) {
             *(SkMatrix*)this = tmp;
         }
     }
     return true;
@@ skipping 26 matching lines @@
 }
 
 void SkMatrix::Scale_pts(const SkMatrix& m, SkPoint dst[],
                          const SkPoint src[], int count) {
     SkASSERT(m.getType() == kScale_Mask);
 
     if (count > 0) {
         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);
     }
 }
 
 void SkMatrix::ScaleTrans_pts(const SkMatrix& m, SkPoint dst[],
                               const SkPoint src[], int count) {
     SkASSERT(m.getType() == (kScale_Mask | kTranslate_Mask));
 
     if (count > 0) {
         SkScalar mx = m.fMat[kMScaleX];
         SkScalar my = m.fMat[kMScaleY];
         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);
     }
 }
 
 void SkMatrix::Rot_pts(const SkMatrix& m, SkPoint dst[],
                        const SkPoint src[], int count) {
     SkASSERT((m.getType() & (kPerspective_Mask | kTranslate_Mask)) == 0);
 
     if (count > 0) {
         SkScalar mx = m.fMat[kMScaleX];
         SkScalar my = m.fMat[kMScaleY];
         SkScalar kx = m.fMat[kMSkewX];
         SkScalar ky = m.fMat[kMSkewY];
         do {
             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);
     }
 }
 
 void SkMatrix::RotTrans_pts(const SkMatrix& m, SkPoint dst[],
                             const SkPoint src[], int count) {
     SkASSERT(!m.hasPerspective());
 
     if (count > 0) {
         SkScalar mx = m.fMat[kMScaleX];
         SkScalar my = m.fMat[kMScaleY];
         SkScalar kx = m.fMat[kMSkewX];
         SkScalar ky = m.fMat[kMSkewY];
         SkScalar tx = m.fMat[kMTransX];
         SkScalar ty = m.fMat[kMTransY];
         do {
             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);
     }
 }
 
 void SkMatrix::Persp_pts(const SkMatrix& m, SkPoint dst[],
                          const SkPoint src[], int count) {
     SkASSERT(m.hasPerspective());
 
     if (count > 0) {
         do {
             SkScalar sy = src->fY;
             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);
     }
 }
 
 const SkMatrix::MapPtsProc SkMatrix::gMapPtsProcs[] = {
     SkMatrix::Identity_pts, SkMatrix::Trans_pts,
     SkMatrix::Scale_pts,    SkMatrix::ScaleTrans_pts,
     SkMatrix::Rot_pts,      SkMatrix::RotTrans_pts,
     SkMatrix::Rot_pts,      SkMatrix::RotTrans_pts,
@@ skipping 23 matching lines @@
         if (this->isIdentity()) {
             memcpy(dst, src, 3*count*sizeof(SkScalar));
             return;
         }
         do {
             SkScalar sx = src[0];
             SkScalar sy = src[1];
             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;
             dst[2] = w;
             dst += 3;
         } while (--count);
     }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ skipping 50 matching lines @@
     // return geometric mean
     return SkScalarSqrt(d0 * d1);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 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,
                       SkPoint* pt) {
     SkASSERT((m.getType() & (kAffine_Mask | kPerspective_Mask))== kAffine_Mask);
     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,
                              SkPoint* pt) {
     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,
                         SkPoint* pt) {
     SkASSERT((m.getType() & (kScale_Mask | kAffine_Mask | kPerspective_Mask))
              == kScale_Mask);
     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,
                         SkPoint* pt) {
     SkASSERT(m.getType() == kTranslate_Mask);
 
     pt->fX = sx + m.fMat[kMTransX];
     pt->fY = sy + m.fMat[kMTransY];
 }
 
@@ skipping 19 matching lines @@
 
 ///////////////////////////////////////////////////////////////////////////////
 
 // if its nearly zero (just made up 26, perhaps it should be bigger or smaller)
 #define PerspNearlyZero(x)  SkScalarNearlyZero(x, (1.0f / (1 << 26)))
 
 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]);
                 }
                 if (stepY) {
                     *stepY = SkScalarToFixed(fMat[kMSkewY]);
                 }
             } 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);
                 }
             }
         }
         return true;
     }
     return false;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
@@ skipping 67 matching lines @@
             case 2:
                 break;
             case 3:
                 pt2.fX = poly[0].fY - poly[2].fY;
                 pt2.fY = poly[2].fX - poly[0].fX;
                 goto CALC_X;
             default:
                 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;
         }
     }
     pt->set(x, y);
     return true;
 }
 
 bool SkMatrix::Poly2Proc(const SkPoint srcPt[], SkMatrix* dst,
                          const SkPoint& scale) {
     float invScale = 1 / scale.fY;
@@ skipping 41 matching lines @@
     y1 = srcPt[2].fY - srcPt[1].fY;
     x2 = srcPt[2].fX - srcPt[3].fX;
     y2 = srcPt[2].fY - srcPt[3].fY;
 
     /* check if abs(x2) > abs(y2) */
     if ( x2 > 0 ? y2 > 0 ? x2 > y2 : x2 > -y2 : y2 > 0 ? -x2 > y2 : x2 < y2) {
         float denom = SkScalarMulDiv(x1, y2, x2) - y1;
         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) */
     if ( x1 > 0 ? y1 > 0 ? x1 > y1 : x1 > -y1 : y1 > 0 ? -x1 > y1 : x1 < y1) {
         float denom = y2 - SkScalarMulDiv(x2, y1, x1);
         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;
     dst->setTypeMask(kUnknown_Mask);
     return true;
 }
 
 typedef bool (*PolyMapProc)(const SkPoint[], SkMatrix*, const SkPoint&);
 
 /*  Taken from Rob Johnson's original sample code in QuickDraw GX
@@ skipping 48 matching lines @@
 ///////////////////////////////////////////////////////////////////////////////
 
 enum MinOrMax {
     kMin_MinOrMax,
     kMax_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) {
              return SkMinScalar(SkScalarAbs(m[SkMatrix::kMScaleX]),
                                 SkScalarAbs(m[SkMatrix::kMScaleY]));
         } else {
              return SkMaxScalar(SkScalarAbs(m[SkMatrix::kMScaleX]),
                                 SkScalarAbs(m[SkMatrix::kMScaleY]));
         }
     }
     // 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) {
             chosenRoot = SkMinScalar(a, c);
         } else {
             chosenRoot = SkMaxScalar(a, c);
         }
     } 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 {
             chosenRoot = apluscdiv2 + x;
         }
     }
     SkASSERT(chosenRoot >= 0);
     return SkScalarSqrt(chosenRoot);
 }
 
@@ skipping 142 matching lines @@
 
     double w1, w2;
     SkScalar cos1, sin1;
     SkScalar cos2, sin2;
 
     // do polar decomposition (M = Q*S)
     SkScalar cosQ, sinQ;
     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;
         Sb = B;
         Sd = D;
     } 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;
 
         // S = Q^-1*M
         // we don't calc Sc since it's symmetric
         Sa = A*cosQ + C*sinQ;
         Sb = B*cosQ + D*sinQ;
         Sd = -B*sinQ + D*cosQ;
     }
 
     // Now we need to compute eigenvalues of S (our scale factors)
     // and eigenvectors (bases for our rotation)
     // 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;
         cos2 = cosQ;
         sin2 = sinQ;
     } else {
         double diff = Sa - Sd;
         double discriminant = sqrt(diff*diff + 4.0*Sb*Sb);
         double trace = Sa + Sd;
         if (diff > 0) {
             w1 = 0.5*(trace + discriminant);
             w2 = 0.5*(trace - discriminant);
         } else {
             w1 = 0.5*(trace - discriminant);
             w2 = 0.5*(trace + discriminant);
         }
 
         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;
 
         // rotation 2 is composition of Q and U
         cos2 = cos1*cosQ - sin1*sinQ;
         sin2 = sin1*cosQ + cos1*sinQ;
 
         // rotation 1 is U^T
         sin1 = -sin1;
     }
 
     if (NULL != scale) {
         scale->fX = SkDoubleToScalar(w1);
         scale->fY = SkDoubleToScalar(w2);
     }
     if (NULL != rotation1) {
         rotation1->fX = cos1;
         rotation1->fY = sin1;
     }
     if (NULL != rotation2) {
         rotation2->fX = cos2;
         rotation2->fY = sin2;
     }
 
     return true;
 }