Revise SVD code to remove arctangents.

Also added bench for timing matrix decomposition.

R=reed@google.com

Author: jvanverth@google.com

Review URL: https://chromiumcodereview.appspot.com/23596006

git-svn-id: http://skia.googlecode.com/svn/trunk@11066 2bbb7eff-a529-9590-31e7-b0007b416f81

5 files changed, 202 insertions, 205 deletions

diff --git a/bench/MatrixBench.cpp b/bench/MatrixBench.cpp
index cae5622414..8fe9e34e4b 100644
--- a/bench/MatrixBench.cpp
+++ b/bench/MatrixBench.cpp
@@ -7,6 +7,7 @@
  */
 #include "SkBenchmark.h"
 #include "SkMatrix.h"
+#include "SkMatrixUtils.h"
 #include "SkRandom.h"
 #include "SkString.h"
 
@@ -345,6 +346,34 @@ class ScaleTransDoubleMatrixBench : public MatrixBench {
     typedef MatrixBench INHERITED;
 };
 
+class DecomposeMatrixBench : public MatrixBench {
+public:
+    DecomposeMatrixBench(void* param) : INHERITED(param, "decompose") {}
+
+protected:
+    virtual void onPreDraw() {
+        for (int i = 0; i < 10; ++i) {
+            SkScalar rot0 = (fRandom.nextBool()) ? fRandom.nextRangeF(-180, 180) : 0.0f;
+            SkScalar sx = fRandom.nextRangeF(-3000.f, 3000.f);
+            SkScalar sy = (fRandom.nextBool()) ? fRandom.nextRangeF(-3000.f, 3000.f) : sx;
+            SkScalar rot1 = fRandom.nextRangeF(-180, 180);
+            fMatrix[i].setRotate(rot0);
+            fMatrix[i].postScale(sx, sy);
+            fMatrix[i].postRotate(rot1);
+        }
+    }
+    virtual void performTest() {
+        SkPoint rotation1, scale, rotation2;
+        for (int i = 0; i < 10; ++i) {
+            (void) SkDecomposeUpper2x2(fMatrix[i], &rotation1, &scale, &rotation2);
+        }
+    }
+private:
+    SkMatrix fMatrix[10];
+    SkMWCRandom fRandom;
+    typedef MatrixBench INHERITED;
+};
+
 class InvertMapRectMatrixBench : public MatrixBench {
 public:
     InvertMapRectMatrixBench(void* param, const char* name, int flags)
@@ -408,6 +437,8 @@ DEF_BENCH( return new FloatConcatMatrixBench(p); )
 DEF_BENCH( return new FloatDoubleConcatMatrixBench(p); )
 DEF_BENCH( return new DoubleConcatMatrixBench(p); )
 DEF_BENCH( return new GetTypeMatrixBench(p); )
+DEF_BENCH( return new DecomposeMatrixBench(p); )
+
 DEF_BENCH( return new InvertMapRectMatrixBench(p, "invert_maprect_identity", 0); )
 
 DEF_BENCH(return new InvertMapRectMatrixBench(p,
diff --git a/src/core/SkMatrix.cpp b/src/core/SkMatrix.cpp
index e5d48f022f..e3e43ca690 100644
--- a/src/core/SkMatrix.cpp
+++ b/src/core/SkMatrix.cpp
@@ -2007,13 +2007,18 @@ bool SkTreatAsSprite(const SkMatrix& mat, int width, int height,
     return isrc == idst;
 }
 
+// A square matrix M can be decomposed (via polar decomposition) into two matrices --
+// an orthogonal matrix Q and a symmetric matrix S. In turn we can decompose S into U*W*U^T,
+// where U is another orthogonal matrix and W is a scale matrix. These can be recombined
+// to give M = (Q*U)*W*U^T, i.e., the product of two orthogonal matrices and a scale matrix.
+//
+// The one wrinkle is that traditionally Q may contain a reflection -- the
+// calculation has been rejiggered to put that reflection into W.
 bool SkDecomposeUpper2x2(const SkMatrix& matrix,
-                         SkScalar* rotation0,
-                         SkScalar* xScale, SkScalar* yScale,
-                         SkScalar* rotation1) {
+                         SkPoint* rotation1,
+                         SkPoint* scale,
+                         SkPoint* rotation2) {
 
-    // borrowed from Jim Blinn's article "Consider the Lowly 2x2 Matrix"
-    // Note: he uses row vectors, so we have to do some swapping of terms
     SkScalar A = matrix[SkMatrix::kMScaleX];
     SkScalar B = matrix[SkMatrix::kMSkewX];
     SkScalar C = matrix[SkMatrix::kMSkewY];
@@ -2023,70 +2028,82 @@ bool SkDecomposeUpper2x2(const SkMatrix& matrix,
         return false;
     }
 
-    SkScalar E = SK_ScalarHalf*(A + D);
-    SkScalar F = SK_ScalarHalf*(A - D);
-    SkScalar G = SK_ScalarHalf*(C + B);
-    SkScalar H = SK_ScalarHalf*(C - B);
+    double w1, w2;
+    SkScalar cos1, sin1;
+    SkScalar cos2, sin2;
 
-    SkScalar sqrt0 = SkScalarSqrt(E*E + H*H);
-    SkScalar sqrt1 = SkScalarSqrt(F*F + G*G);
+    // 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;
+        sinQ = 0;
 
-    SkScalar xs, ys, r0, r1;
-
-    xs = sqrt0 + sqrt1;
-    ys = sqrt0 - sqrt1;
-    // can't have zero yScale, must be degenerate
-    SkASSERT(!SkScalarNearlyZero(ys));
-
-    // uniformly scaled rotation
-    if (SkScalarNearlyZero(F) && SkScalarNearlyZero(G)) {
-        SkASSERT(!SkScalarNearlyZero(E) || !SkScalarNearlyZero(H));
-        r0 = SkScalarATan2(H, E);
-        r1 = 0;
-    // uniformly scaled reflection
-    } else if (SkScalarNearlyZero(E) && SkScalarNearlyZero(H)) {
-        SkASSERT(!SkScalarNearlyZero(F) || !SkScalarNearlyZero(G));
-        r0 = -SkScalarATan2(G, F);
-        r1 = 0;
+        Sa = A;
+        Sb = B;
+        Sd = D;
     } else {
-        SkASSERT(!SkScalarNearlyZero(E) || !SkScalarNearlyZero(H));
-        SkASSERT(!SkScalarNearlyZero(F) || !SkScalarNearlyZero(G));
-
-        SkScalar arctan0 = SkScalarATan2(H, E);
-        SkScalar arctan1 = SkScalarATan2(G, F);
-        r0 = SK_ScalarHalf*(arctan0 - arctan1);
-        r1 = SK_ScalarHalf*(arctan0 + arctan1);
-
-        // simplify the results
-        const SkScalar kHalfPI = SK_ScalarHalf*SK_ScalarPI;
-        if (SkScalarNearlyEqual(SkScalarAbs(r0), kHalfPI)) {
-            SkScalar tmp = xs;
-            xs = ys;
-            ys = tmp;
-
-            r1 += r0;
-            r0 = 0;
-        } else if (SkScalarNearlyEqual(SkScalarAbs(r1), kHalfPI)) {
-            SkScalar tmp = xs;
-            xs = ys;
-            ys = tmp;
-
-            r0 += r1;
-            r1 = 0;
+        cosQ = A + D;
+        sinQ = C - B;
+        SkScalar reciplen = SK_Scalar1/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(Sb)) {
+        // already diagonalized
+        cos1 = SK_Scalar1;
+        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);
         }
-    }
-
-    if (NULL != xScale) {
-        *xScale = xs;
-    }
-    if (NULL != yScale) {
-        *yScale = ys;
-    }
-    if (NULL != rotation0) {
-        *rotation0 = r0;
+        
+        cos1 = Sb; sin1 = w1 - Sa;
+        SkScalar reciplen = SK_Scalar1/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 = w1;
+        scale->fY = w2;
     }
     if (NULL != rotation1) {
-        *rotation1 = r1;
+        rotation1->fX = cos1;
+        rotation1->fY = sin1;
+    }
+    if (NULL != rotation2) {
+        rotation2->fX = cos2;
+        rotation2->fY = sin2;
     }
 
     return true;
diff --git a/src/core/SkMatrixUtils.h b/src/core/SkMatrixUtils.h
index 37341f289e..3fc1440e15 100644
--- a/src/core/SkMatrixUtils.h
+++ b/src/core/SkMatrixUtils.h
@@ -40,15 +40,15 @@ static inline bool SkTreatAsSpriteFilter(const SkMatrix& matrix,
     return SkTreatAsSprite(matrix, width, height, kSkSubPixelBitsForBilerp);
 }
 
-/** Decomposes the upper-left 2x2 of the matrix into a rotation, followed by a non-uniform scale,
-    followed by another rotation. Returns true if successful.
-    If the scale factors are uniform, then rotation1 will be 0.
-    If there is a reflection, yScale will be negative.
-    Returns false if the matrix is degenerate.
+/** Decomposes the upper-left 2x2 of the matrix into a rotation (represented by
+    the cosine and sine of the rotation angle), followed by a non-uniform scale,
+    followed by another rotation. If there is a reflection, one of the scale 
+    factors will be negative.
+    Returns true if successful. Returns false if the matrix is degenerate.
     */
 bool SkDecomposeUpper2x2(const SkMatrix& matrix,
-                         SkScalar* rotation0,
-                         SkScalar* xScale, SkScalar* yScale,
-                         SkScalar* rotation1);
+                         SkPoint* rotation1,
+                         SkPoint* scale,
+                         SkPoint* rotation2);
 
 #endif
diff --git a/tests/Matrix44Test.cpp b/tests/Matrix44Test.cpp
index 67af60df90..6c29a55966 100644
--- a/tests/Matrix44Test.cpp
+++ b/tests/Matrix44Test.cpp
@@ -513,10 +513,10 @@ static void TestMatrix44(skiatest::Reporter* reporter) {
 
     // test mixed-valued matrix inverse
     mat.reset();
-    mat.setScale(1.0e-10, 3.0, 1.0e+10);
+    mat.setScale(1.0e-11, 3.0, 1.0e+11);
     rot.setRotateDegreesAbout(0, 0, -1, 90);
     mat.postConcat(rot);
-    mat.postTranslate(1.0e+10, 3.0, 1.0e-10);
+    mat.postTranslate(1.0e+11, 3.0, 1.0e-11);
     REPORTER_ASSERT(reporter, mat.invert(NULL));
     mat.invert(&inverse);
     iden1.setConcat(mat, inverse);
diff --git a/tests/MatrixTest.cpp b/tests/MatrixTest.cpp
index 8785730322..c225565cc4 100644
--- a/tests/MatrixTest.cpp
+++ b/tests/MatrixTest.cpp
@@ -350,11 +350,13 @@ static void test_matrix_is_similarity(skiatest::Reporter* reporter) {
 static bool scalar_nearly_equal_relative(SkScalar a, SkScalar b,
                                          SkScalar tolerance = SK_ScalarNearlyZero) {
     // from Bruce Dawson
+    // absolute check
     SkScalar diff = SkScalarAbs(a - b);
     if (diff < tolerance) {
         return true;
     }
 
+    // relative check
     a = SkScalarAbs(a);
     b = SkScalarAbs(b);
     SkScalar largest = (b > a) ? b : a;
@@ -366,9 +368,32 @@ static bool scalar_nearly_equal_relative(SkScalar a, SkScalar b,
     return false;
 }
 
+static bool check_matrix_recomposition(const SkMatrix& mat,
+                                       const SkPoint& rotation1,
+                                       const SkPoint& scale,
+                                       const SkPoint& rotation2) {
+    SkScalar c1 = rotation1.fX;
+    SkScalar s1 = rotation1.fY;
+    SkScalar scaleX = scale.fX;
+    SkScalar scaleY = scale.fY;
+    SkScalar c2 = rotation2.fX;
+    SkScalar s2 = rotation2.fY;
+    
+    // We do a relative check here because large scale factors cause problems with an absolute check
+    bool result = scalar_nearly_equal_relative(mat[SkMatrix::kMScaleX],
+                                               scaleX*c1*c2 - scaleY*s1*s2) &&
+                  scalar_nearly_equal_relative(mat[SkMatrix::kMSkewX],
+                                               -scaleX*s1*c2 - scaleY*c1*s2) &&
+                  scalar_nearly_equal_relative(mat[SkMatrix::kMSkewY],
+                                               scaleX*c1*s2 + scaleY*s1*c2) &&
+                  scalar_nearly_equal_relative(mat[SkMatrix::kMScaleY],
+                                               -scaleX*s1*s2 + scaleY*c1*c2);
+    return result;
+}
+
 static void test_matrix_decomposition(skiatest::Reporter* reporter) {
     SkMatrix mat;
-    SkScalar rotation0, scaleX, scaleY, rotation1;
+    SkPoint rotation1, scale, rotation2;
 
     const float kRotation0 = 15.5f;
     const float kRotation1 = -50.f;
@@ -377,150 +402,108 @@ static void test_matrix_decomposition(skiatest::Reporter* reporter) {
 
     // identity
     mat.reset();
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, SK_Scalar1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, SK_Scalar1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
     // make sure it doesn't crash if we pass in NULLs
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, NULL, NULL, NULL, NULL));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, NULL, NULL, NULL));
 
     // rotation only
     mat.setRotate(kRotation0);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(rotation0, SkDegreesToRadians(kRotation0)));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, SK_Scalar1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, SK_Scalar1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // uniform scale only
     mat.setScale(kScale0, kScale0);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // anisotropic scale only
     mat.setScale(kScale1, kScale0);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, kScale1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // rotation then uniform scale
     mat.setRotate(kRotation1);
     mat.postScale(kScale0, kScale0);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(rotation0, SkDegreesToRadians(kRotation1)));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // uniform scale then rotation
     mat.setScale(kScale0, kScale0);
     mat.postRotate(kRotation1);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(rotation0, SkDegreesToRadians(kRotation1)));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // rotation then uniform scale+reflection
     mat.setRotate(kRotation0);
     mat.postScale(kScale1, -kScale1);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(rotation0, SkDegreesToRadians(kRotation0)));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, kScale1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, -kScale1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // uniform scale+reflection, then rotate
     mat.setScale(kScale0, -kScale0);
     mat.postRotate(kRotation1);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(rotation0, SkDegreesToRadians(-kRotation1)));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, -kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // rotation then anisotropic scale
     mat.setRotate(kRotation1);
     mat.postScale(kScale1, kScale0);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(rotation0, SkDegreesToRadians(kRotation1)));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, kScale1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
+    // rotation then anisotropic scale
+    mat.setRotate(90);
+    mat.postScale(kScale1, kScale0);
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
+    
     // anisotropic scale then rotation
     mat.setScale(kScale1, kScale0);
     mat.postRotate(kRotation0);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, kScale1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(rotation1, SkDegreesToRadians(kRotation0)));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
+    
+    // anisotropic scale then rotation
+    mat.setScale(kScale1, kScale0);
+    mat.postRotate(90);
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // rotation, uniform scale, then different rotation
     mat.setRotate(kRotation1);
     mat.postScale(kScale0, kScale0);
     mat.postRotate(kRotation0);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(rotation0,
-                                                  SkDegreesToRadians(kRotation0 + kRotation1)));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleX, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyEqual(scaleY, kScale0));
-    REPORTER_ASSERT(reporter, SkScalarNearlyZero(rotation1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // rotation, anisotropic scale, then different rotation
     mat.setRotate(kRotation0);
     mat.postScale(kScale1, kScale0);
     mat.postRotate(kRotation1);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    // Because of the shear/skew we won't get the same results, so we need to multiply it out.
-    // Generating the matrices requires doing a radian-to-degree calculation, then degree-to-radian
-    // calculation (in setRotate()), which adds error, so this just computes the matrix elements
-    // directly.
-    SkScalar c0;
-    SkScalar s0 = SkScalarSinCos(rotation0, &c0);
-    SkScalar c1;
-    SkScalar s1 = SkScalarSinCos(rotation1, &c1);
-    // We do a relative check here because large scale factors cause problems with an absolute check
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMScaleX],
-                                                           scaleX*c0*c1 - scaleY*s0*s1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMSkewX],
-                                                           -scaleX*s0*c1 - scaleY*c0*s1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMSkewY],
-                                                           scaleX*c0*s1 + scaleY*s0*c1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMScaleY],
-                                                           -scaleX*s0*s1 + scaleY*c0*c1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
+    
+    // rotation, anisotropic scale + reflection, then different rotation
+    mat.setRotate(kRotation0);
+    mat.postScale(-kScale1, kScale0);
+    mat.postRotate(kRotation1);
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // try some random matrices
     SkMWCRandom rand;
     for (int m = 0; m < 1000; ++m) {
-        SkScalar rot0 = rand.nextRangeF(-SK_ScalarPI, SK_ScalarPI);
+        SkScalar rot0 = rand.nextRangeF(-180, 180);
         SkScalar sx = rand.nextRangeF(-3000.f, 3000.f);
         SkScalar sy = rand.nextRangeF(-3000.f, 3000.f);
-        SkScalar rot1 = rand.nextRangeF(-SK_ScalarPI, SK_ScalarPI);
+        SkScalar rot1 = rand.nextRangeF(-180, 180);
         mat.setRotate(rot0);
         mat.postScale(sx, sy);
         mat.postRotate(rot1);
 
-        if (SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1)) {
-            SkScalar c0;
-            SkScalar s0 = SkScalarSinCos(rotation0, &c0);
-            SkScalar c1;
-            SkScalar s1 = SkScalarSinCos(rotation1, &c1);
-            REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMScaleX],
-                                                                   scaleX*c0*c1 - scaleY*s0*s1));
-            REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMSkewX],
-                                                                   -scaleX*s0*c1 - scaleY*c0*s1));
-            REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMSkewY],
-                                                                   scaleX*c0*s1 + scaleY*s0*c1));
-            REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMScaleY],
-                                                                   -scaleX*s0*s1 + scaleY*c0*c1));
+        if (SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2)) {
+            REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
         } else {
             // if the matrix is degenerate, the basis vectors should be near-parallel or near-zero
             SkScalar perpdot = mat[SkMatrix::kMScaleX]*mat[SkMatrix::kMScaleY] -
@@ -531,65 +514,31 @@ static void test_matrix_decomposition(skiatest::Reporter* reporter) {
 
     // translation shouldn't affect this
     mat.postTranslate(-1000.f, 1000.f);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    s0 = SkScalarSinCos(rotation0, &c0);
-    s1 = SkScalarSinCos(rotation1, &c1);
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMScaleX],
-                                                           scaleX*c0*c1 - scaleY*s0*s1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMSkewX],
-                                                           -scaleX*s0*c1 - scaleY*c0*s1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMSkewY],
-                                                           scaleX*c0*s1 + scaleY*s0*c1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMScaleY],
-                                                           -scaleX*s0*s1 + scaleY*c0*c1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // perspective shouldn't affect this
     mat[SkMatrix::kMPersp0] = 12.f;
     mat[SkMatrix::kMPersp1] = 4.f;
     mat[SkMatrix::kMPersp2] = 1872.f;
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    s0 = SkScalarSinCos(rotation0, &c0);
-    s1 = SkScalarSinCos(rotation1, &c1);
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMScaleX],
-                                                           scaleX*c0*c1 - scaleY*s0*s1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMSkewX],
-                                                           -scaleX*s0*c1 - scaleY*c0*s1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMSkewY],
-                                                           scaleX*c0*s1 + scaleY*s0*c1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMScaleY],
-                                                           -scaleX*s0*s1 + scaleY*c0*c1));
-
-    // rotation, anisotropic scale + reflection, then different rotation
-    mat.setRotate(kRotation0);
-    mat.postScale(-kScale1, kScale0);
-    mat.postRotate(kRotation1);
-    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
-    s0 = SkScalarSinCos(rotation0, &c0);
-    s1 = SkScalarSinCos(rotation1, &c1);
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMScaleX],
-                                                           scaleX*c0*c1 - scaleY*s0*s1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMSkewX],
-                                                           -scaleX*s0*c1 - scaleY*c0*s1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMSkewY],
-                                                           scaleX*c0*s1 + scaleY*s0*c1));
-    REPORTER_ASSERT(reporter, scalar_nearly_equal_relative(mat[SkMatrix::kMScaleY],
-                                                           -scaleX*s0*s1 + scaleY*c0*c1));
+    REPORTER_ASSERT(reporter, SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
+    REPORTER_ASSERT(reporter, check_matrix_recomposition(mat, rotation1, scale, rotation2));
 
     // degenerate matrices
     // mostly zero entries
     mat.reset();
     mat[SkMatrix::kMScaleX] = 0.f;
-    REPORTER_ASSERT(reporter, !SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
+    REPORTER_ASSERT(reporter, !SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
     mat.reset();
     mat[SkMatrix::kMScaleY] = 0.f;
-    REPORTER_ASSERT(reporter, !SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
+    REPORTER_ASSERT(reporter, !SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
     mat.reset();
     // linearly dependent entries
     mat[SkMatrix::kMScaleX] = 1.f;
     mat[SkMatrix::kMSkewX] = 2.f;
     mat[SkMatrix::kMSkewY] = 4.f;
     mat[SkMatrix::kMScaleY] = 8.f;
-    REPORTER_ASSERT(reporter, !SkDecomposeUpper2x2(mat, &rotation0, &scaleX, &scaleY, &rotation1));
+    REPORTER_ASSERT(reporter, !SkDecomposeUpper2x2(mat, &rotation1, &scale, &rotation2));
 }
 
 // For test_matrix_homogeneous, below.