Apply skew on both axes together rather than sequentially on Compositor thread

ui/gfx/transform_unittest.cc

Index: ui/gfx/transform_unittest.cc
diff --git a/ui/gfx/transform_unittest.cc b/ui/gfx/transform_unittest.cc
index eb0b2fdcc69980f4f704e84d3f2fc4fa1c2f3146..8c1754213c690a8d7d06cdc068be4b3151e2d630 100644
--- a/ui/gfx/transform_unittest.cc
+++ b/ui/gfx/transform_unittest.cc
@@ -777,12 +777,10 @@ TEST(XFormTest, BlendSkew) {
   Transform from;
   for (int i = 0; i < 2; ++i) {
     Transform to;
-    to.SkewX(10);
-    to.SkewY(5);
+    to.Skew(10, 5);
     double t = i;
     Transform expected;
-    expected.SkewX(t * 10);
-    expected.SkewY(t * 5);
+    expected.Skew(t * 10, t * 5);
     EXPECT_TRUE(to.Blend(from, t));
     EXPECT_TRUE(MatricesAreNearlyEqual(expected, to));
   }
@@ -792,10 +790,10 @@ TEST(XFormTest, ExtrapolateSkew) {
   Transform from;
   for (int i = -1; i < 2; ++i) {
     Transform to;
-    to.SkewX(20);
+    to.Skew(20, 20);
     double t = i;
     Transform expected;
-    expected.SkewX(t * 20);
+    expected.Skew(t * 20, t * 20);
     EXPECT_TRUE(to.Blend(from, t));
     EXPECT_TRUE(MatricesAreNearlyEqual(expected, to));
   }
@@ -906,18 +904,18 @@ TEST(XFormTest, VerifyBlendForScale) {
   EXPECT_ROW4_EQ(0.0f,   0.0f,  0.0f,  1.0f, to);
 }
 
-TEST(XFormTest, VerifyBlendForSkewX) {
+TEST(XFormTest, VerifyBlendForSkew) {
   Transform from;
-  from.SkewX(0.0);
+  from.Skew(0.0, 0.0);
 
   Transform to;
 
-  to.SkewX(45.0);
+  to.Skew(45.0, 0.0);
   to.Blend(from, 0.0);
   EXPECT_EQ(from, to);
 
   to = Transform();
-  to.SkewX(45.0);
+  to.Skew(45.0, 0.0);
   to.Blend(from, 0.5);
   EXPECT_ROW1_EQ(1.0f, 0.5f, 0.0f, 0.0f, to);
   EXPECT_ROW2_EQ(0.0f, 1.0f, 0.0f, 0.0f, to);
@@ -925,7 +923,7 @@ TEST(XFormTest, VerifyBlendForSkewX) {
   EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, to);
 
   to = Transform();
-  to.SkewX(45.0);
+  to.Skew(45.0, 0.0);
   to.Blend(from, 0.25);
   EXPECT_ROW1_EQ(1.0f, 0.25f, 0.0f, 0.0f, to);
   EXPECT_ROW2_EQ(0.0f, 1.0f,  0.0f, 0.0f, to);
@@ -933,7 +931,7 @@ TEST(XFormTest, VerifyBlendForSkewX) {
   EXPECT_ROW4_EQ(0.0f, 0.0f,  0.0f, 1.0f, to);
 
   to = Transform();
-  to.SkewX(45.0);
+  to.Skew(45.0, 0.0);
   to.Blend(from, 1.0);
   EXPECT_ROW1_EQ(1.0f, 1.0f, 0.0f, 0.0f, to);
   EXPECT_ROW2_EQ(0.0f, 1.0f, 0.0f, 0.0f, to);
@@ -941,78 +939,6 @@ TEST(XFormTest, VerifyBlendForSkewX) {
   EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, to);
 }
 
-TEST(XFormTest, VerifyBlendForSkewY) {
-  // NOTE CAREFULLY: Decomposition of skew and rotation terms of the matrix
-  // is inherently underconstrained, and so it does not always compute the
-  // originally intended skew parameters. The current implementation uses QR
-  // decomposition, which decomposes the shear into a rotation + non-uniform
-  // scale.
-  //
-  // It is unlikely that the decomposition implementation will need to change
-  // very often, so to get any test coverage, the compromise is to verify the
-  // exact matrix that the.Blend() operation produces.
-  //
-  // This problem also potentially exists for skewX, but the current QR
-  // decomposition implementation just happens to decompose those test
-  // matrices intuitively.
-  //
-  // Unfortunately, this case suffers from uncomfortably large precision
-  // error.

[ajuma, 2015/09/01 13:25:10]

This test and comment still seem to have value. Do the issues described in the
comment still apply? If so, I'd suggest adapting this to use the new Skew
method.

[nainar1, 2015/09/02 01:43:33]

Done.

-
-  Transform from;
-  from.SkewY(0.0);
-
-  Transform to;
-
-  to.SkewY(45.0);
-  to.Blend(from, 0.0);
-  EXPECT_EQ(from, to);
-
-  to = Transform();
-  to.SkewY(45.0);
-  to.Blend(from, 0.25);
-  EXPECT_ROW1_NEAR(1.0823489449280947471976333,
-                   0.0464370719145053845178239,
-                   0.0,
-                   0.0,
-                   to,
-                   LOOSE_ERROR_THRESHOLD);
-  EXPECT_ROW2_NEAR(0.2152925909665224513123150,
-                   0.9541702441750861130032035,
-                   0.0,
-                   0.0,
-                   to,
-                   LOOSE_ERROR_THRESHOLD);
-  EXPECT_ROW3_EQ(0.0f, 0.0f, 1.0f, 0.0f, to);
-  EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, to);
-
-  to = Transform();
-  to.SkewY(45.0);
-  to.Blend(from, 0.5);
-  EXPECT_ROW1_NEAR(1.1152212925809066312865525,
-                   0.0676495144007326631996335,
-                   0.0,
-                   0.0,
-                   to,
-                   LOOSE_ERROR_THRESHOLD);
-  EXPECT_ROW2_NEAR(0.4619397844342648662419037,
-                   0.9519009045724774464858342,
-                   0.0,
-                   0.0,
-                   to,
-                   LOOSE_ERROR_THRESHOLD);
-  EXPECT_ROW3_EQ(0.0f, 0.0f, 1.0f, 0.0f, to);
-  EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, to);
-
-  to = Transform();
-  to.SkewY(45.0);
-  to.Blend(from, 1.0);
-  EXPECT_ROW1_NEAR(1.0, 0.0, 0.0, 0.0, to, LOOSE_ERROR_THRESHOLD);
-  EXPECT_ROW2_NEAR(1.0, 1.0, 0.0, 0.0, to, LOOSE_ERROR_THRESHOLD);
-  EXPECT_ROW3_EQ(0.0f, 0.0f, 1.0f, 0.0f, to);
-  EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, to);
-}
-
 #if defined(_WIN64)
 // http://crbug.com/406574
 #define MAYBE_VerifyBlendForRotationAboutX DISABLED_VerifyBlendForRotationAboutX
@@ -1224,7 +1150,7 @@ TEST(XFormTest, VerifyBlendForCompositeTransform) {
   expectedEndOfAnimation.ApplyPerspectiveDepth(1.0);
   expectedEndOfAnimation.Translate3d(10.0, 20.0, 30.0);
   expectedEndOfAnimation.RotateAbout(Vector3dF(0.0, 0.0, 1.0), 25.0);
-  expectedEndOfAnimation.SkewY(45.0);
+  expectedEndOfAnimation.Skew(0.0, 45.0);
   expectedEndOfAnimation.Scale3d(6.0, 7.0, 8.0);
 
   to = expectedEndOfAnimation;
@@ -1929,9 +1855,9 @@ TEST(XFormTest, verifyRotateAboutForDegenerateAxis) {
   EXPECT_ROW4_EQ(13.0f, 17.0f, 21.0f, 25.0f, A);
 }
 
-TEST(XFormTest, verifySkewX) {
+TEST(XFormTest, verifySkew) {
   Transform A;
-  A.SkewX(45.0);
+  A.Skew(45.0, 0.0);
   EXPECT_ROW1_EQ(1.0f, 1.0f, 0.0f, 0.0f, A);
   EXPECT_ROW2_EQ(0.0f, 1.0f, 0.0f, 0.0f, A);
   EXPECT_ROW3_EQ(0.0f, 0.0f, 1.0f, 0.0f, A);
@@ -1942,33 +1868,13 @@ TEST(XFormTest, verifySkewX) {
   // of post-multiplied.
   A.MakeIdentity();
   A.Scale3d(6.0, 7.0, 8.0);
-  A.SkewX(45.0);
+  A.Skew(45.0, 0.0);
   EXPECT_ROW1_EQ(6.0f, 6.0f, 0.0f, 0.0f, A);
   EXPECT_ROW2_EQ(0.0f, 7.0f, 0.0f, 0.0f, A);
   EXPECT_ROW3_EQ(0.0f, 0.0f, 8.0f, 0.0f, A);
   EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, A);

[ajuma, 2015/09/01 13:25:10]

Please also test the case where we're skewing along both axes.

[nainar1, 2015/09/02 01:43:33]

Done.

 }
 
-TEST(XFormTest, verifySkewY) {
-  Transform A;
-  A.SkewY(45.0);
-  EXPECT_ROW1_EQ(1.0f, 0.0f, 0.0f, 0.0f, A);
-  EXPECT_ROW2_EQ(1.0f, 1.0f, 0.0f, 0.0f, A);
-  EXPECT_ROW3_EQ(0.0f, 0.0f, 1.0f, 0.0f, A);
-  EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, A);
-
-  // Verify that skewY() post-multiplies the existing matrix. Row 2, column 1 ,
-  // would incorrectly have value "6" if the matrix is pre-multiplied instead
-  // of post-multiplied.
-  A.MakeIdentity();
-  A.Scale3d(6.0, 7.0, 8.0);
-  A.SkewY(45.0);
-  EXPECT_ROW1_EQ(6.0f, 0.0f, 0.0f, 0.0f, A);
-  EXPECT_ROW2_EQ(7.0f, 7.0f, 0.0f, 0.0f, A);
-  EXPECT_ROW3_EQ(0.0f, 0.0f, 8.0f, 0.0f, A);
-  EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, A);
-}
-
 TEST(XFormTest, verifyPerspectiveDepth) {
   Transform A;
   A.ApplyPerspectiveDepth(1.0);
@@ -2040,7 +1946,7 @@ TEST(XFormTest, verifyIsInvertible) {
   EXPECT_TRUE(A.IsInvertible());
 
   A.MakeIdentity();
-  A.SkewX(45.0);
+  A.Skew(45.0, 0.0);
   EXPECT_TRUE(A.IsInvertible());
 
   // A perspective matrix (projection plane at z=0) is invertible. The