Apply skew on both axes together rather than sequentially on Compositor

thread.

Currently skew is not implemented correctly on the Compositor thread.
Instead of calling skew(angle_x, angle_y), the implementation calls
skewX(angle_x) followed by skewY(angle_y). This leads to a different
result. This patch applied skew on both axes together rather than
calling skewX(angle_x) and skewX(angle_Y) sequentially. FF and IE
correctly call skew(angle_x, angle_y).

BUG=268468

Review URL: https://codereview.chromium.org/1325803002

Cr-Commit-Position: refs/heads/master@{#347089}

10 files changed, 58 insertions, 79 deletions

diff --git a/cc/animation/transform_operation.cc b/cc/animation/transform_operation.cc
index 7421924..465429d 100644
--- a/cc/animation/transform_operation.cc
+++ b/cc/animation/transform_operation.cc
@@ -157,8 +157,8 @@ bool TransformOperation::BlendTransformOperations(
     SkMScalar from_y = IsOperationIdentity(from) ? 0 : from->skew.y;
     SkMScalar to_x = IsOperationIdentity(to) ? 0 : to->skew.x;
     SkMScalar to_y = IsOperationIdentity(to) ? 0 : to->skew.y;
-    result->SkewX(BlendSkMScalars(from_x, to_x, progress));
-    result->SkewY(BlendSkMScalars(from_y, to_y, progress));
+    result->Skew(BlendSkMScalars(from_x, to_x, progress),
+                 BlendSkMScalars(from_y, to_y, progress));
     break;
   }
   case TransformOperation::TRANSFORM_OPERATION_PERSPECTIVE: {
diff --git a/cc/animation/transform_operations.cc b/cc/animation/transform_operations.cc
index fb1c17d..29a3d90 100644
--- a/cc/animation/transform_operations.cc
+++ b/cc/animation/transform_operations.cc
@@ -224,8 +224,7 @@ void TransformOperations::AppendScale(SkMScalar x, SkMScalar y, SkMScalar z) {
 
 void TransformOperations::AppendSkew(SkMScalar x, SkMScalar y) {
   TransformOperation to_add;
-  to_add.matrix.SkewX(x);
-  to_add.matrix.SkewY(y);
+  to_add.matrix.Skew(x, y);
   to_add.type = TransformOperation::TRANSFORM_OPERATION_SKEW;
   to_add.skew.x = x;
   to_add.skew.y = y;
diff --git a/cc/animation/transform_operations_unittest.cc b/cc/animation/transform_operations_unittest.cc
index c5fb3c6..5a195227 100644
--- a/cc/animation/transform_operations_unittest.cc
+++ b/cc/animation/transform_operations_unittest.cc
@@ -209,8 +209,7 @@ TEST(TransformOperationTest, ApplySkew) {
   TransformOperations operations;
   operations.AppendSkew(x, y);
   gfx::Transform expected;
-  expected.SkewX(x);
-  expected.SkewY(y);
+  expected.Skew(x, y);
   EXPECT_TRANSFORMATION_MATRIX_EQ(expected, operations.Apply());
 }
 
@@ -552,8 +551,7 @@ TEST(TransformOperationTest, BlendSkewFromEmpty) {
   SkMScalar progress = 0.5f;
 
   gfx::Transform expected;
-  expected.SkewX(1);
-  expected.SkewY(1);
+  expected.Skew(1, 1);
 
   EXPECT_TRANSFORMATION_MATRIX_EQ(expected,
                                   operations.Blend(empty_operation, progress));
@@ -561,8 +559,7 @@ TEST(TransformOperationTest, BlendSkewFromEmpty) {
   progress = -0.5f;
 
   expected.MakeIdentity();
-  expected.SkewX(-1);
-  expected.SkewY(-1);
+  expected.Skew(-1, -1);
 
   EXPECT_TRANSFORMATION_MATRIX_EQ(expected,
                                   operations.Blend(empty_operation, progress));
@@ -570,8 +567,7 @@ TEST(TransformOperationTest, BlendSkewFromEmpty) {
   progress = 1.5f;
 
   expected.MakeIdentity();
-  expected.SkewX(3);
-  expected.SkewY(3);
+  expected.Skew(3, 3);
 
   EXPECT_TRANSFORMATION_MATRIX_EQ(expected,
                                   operations.Blend(empty_operation, progress));
@@ -658,8 +654,7 @@ TEST(TransformOperationTest, BlendSkewToEmpty) {
   SkMScalar progress = 0.5f;
 
   gfx::Transform expected;
-  expected.SkewX(1);
-  expected.SkewY(1);
+  expected.Skew(1, 1);
 
   EXPECT_TRANSFORMATION_MATRIX_EQ(expected,
                                   empty_operation.Blend(operations, progress));
diff --git a/cc/base/float_quad_unittest.cc b/cc/base/float_quad_unittest.cc
index 6d9ce02..64c448c 100644
--- a/cc/base/float_quad_unittest.cc
+++ b/cc/base/float_quad_unittest.cc
@@ -18,8 +18,8 @@ TEST(FloatQuadTest, IsRectilinearTest) {
   rectilinear_trans[1].Rotate(90.f);
   rectilinear_trans[2].Rotate(180.f);
   rectilinear_trans[3].Rotate(270.f);
-  rectilinear_trans[4].SkewX(0.00000000001f);
-  rectilinear_trans[5].SkewY(0.00000000001f);
+  rectilinear_trans[4].Skew(0.00000000001f, 0.0f);
+  rectilinear_trans[5].Skew(0.0f, 0.00000000001f);
   rectilinear_trans[6].Scale(0.00001f, 0.00001f);
   rectilinear_trans[6].Rotate(180.f);
   rectilinear_trans[7].Scale(100000.f, 100000.f);
@@ -46,8 +46,8 @@ TEST(FloatQuadTest, IsRectilinearTest) {
   non_rectilinear_trans[5].Rotate(180.00001f);
   non_rectilinear_trans[6].Rotate(269.9999f);
   non_rectilinear_trans[7].Rotate(270.0001f);
-  non_rectilinear_trans[8].SkewX(0.00001f);
-  non_rectilinear_trans[9].SkewY(0.00001f);
+  non_rectilinear_trans[8].Skew(0.00001f, 0.0f);
+  non_rectilinear_trans[9].Skew(0.0f, 0.00001f);
 
   for (int i = 0; i < kNumNonRectilinear; ++i) {
     bool clipped = false;
diff --git a/cc/trees/layer_tree_host_common_unittest.cc b/cc/trees/layer_tree_host_common_unittest.cc
index 3a63e76..0eaeb63 100644
--- a/cc/trees/layer_tree_host_common_unittest.cc
+++ b/cc/trees/layer_tree_host_common_unittest.cc
@@ -4280,7 +4280,7 @@ TEST_P(LCDTextTest, CanUseLCDText) {
 
   // Case 6: Skew.
   gfx::Transform skew;
-  skew.SkewX(10.0);
+  skew.Skew(10.0, 0.0);
   child_->SetTransform(skew);
   child_->layer_tree_impl()->property_trees()->needs_rebuild = true;
   ExecuteCalculateDrawProperties(root_, 1.f, 1.f, NULL, can_use_lcd_text_,
diff --git a/mojo/converters/surfaces/tests/surface_unittest.cc b/mojo/converters/surfaces/tests/surface_unittest.cc
index a23f6cb..0dcfeea 100644
--- a/mojo/converters/surfaces/tests/surface_unittest.cc
+++ b/mojo/converters/surfaces/tests/surface_unittest.cc
@@ -208,7 +208,7 @@ TEST(SurfaceLibTest, RenderPass) {
   gfx::Rect output_rect(4, 9, 13, 71);
   gfx::Rect damage_rect(9, 17, 41, 45);
   gfx::Transform transform_to_root_target;
-  transform_to_root_target.SkewY(43.0);
+  transform_to_root_target.Skew(0.0, 43.0);
   bool has_transparent_background = false;
   pass->SetAll(pass_id,
                output_rect,
diff --git a/ui/gfx/transform.cc b/ui/gfx/transform.cc
index 33946cb..c42c961 100644
--- a/ui/gfx/transform.cc
+++ b/ui/gfx/transform.cc
@@ -184,21 +184,13 @@ void Transform::Translate3d(SkMScalar x, SkMScalar y, SkMScalar z) {
   matrix_.preTranslate(x, y, z);
 }
 
-void Transform::SkewX(double angle_x) {
+void Transform::Skew(double angle_x, double angle_y) {
   if (matrix_.isIdentity()) {
     matrix_.set(0, 1, TanDegrees(angle_x));
-  } else {
-    SkMatrix44 skew(SkMatrix44::kIdentity_Constructor);
-    skew.set(0, 1, TanDegrees(angle_x));
-    matrix_.preConcat(skew);
-  }
-}
-
-void Transform::SkewY(double angle_y) {
-  if (matrix_.isIdentity()) {
     matrix_.set(1, 0, TanDegrees(angle_y));
   } else {
     SkMatrix44 skew(SkMatrix44::kIdentity_Constructor);
+    skew.set(0, 1, TanDegrees(angle_x));
     skew.set(1, 0, TanDegrees(angle_y));
     matrix_.preConcat(skew);
   }
diff --git a/ui/gfx/transform.h b/ui/gfx/transform.h
index 55ab938..20740ad 100644
--- a/ui/gfx/transform.h
+++ b/ui/gfx/transform.h
@@ -101,8 +101,7 @@ class GFX_EXPORT Transform {
 
   // Applies the current transformation on a skew and assigns the result
   // to |this|.
-  void SkewX(double angle_x);
-  void SkewY(double angle_y);
+  void Skew(double angle_x, double angle_y);
 
   // Applies the current transformation on a perspective transform and assigns
   // the result to |this|.
diff --git a/ui/gfx/transform_unittest.cc b/ui/gfx/transform_unittest.cc
index eb0b2fd..81333a3 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, 0);
     double t = i;
     Transform expected;
-    expected.SkewX(t * 20);
+    expected.Skew(t * 20, t * 0);
     EXPECT_TRUE(to.Blend(from, t));
     EXPECT_TRUE(MatricesAreNearlyEqual(expected, to));
   }
@@ -906,18 +904,19 @@ TEST(XFormTest, VerifyBlendForScale) {
   EXPECT_ROW4_EQ(0.0f,   0.0f,  0.0f,  1.0f, to);
 }
 
-TEST(XFormTest, VerifyBlendForSkewX) {
+TEST(XFormTest, VerifyBlendForSkew) {
+  // Along X axis only
   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 +924,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,15 +932,13 @@ 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);
   EXPECT_ROW3_EQ(0.0f, 0.0f, 1.0f, 0.0f, to);
   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
@@ -952,24 +949,24 @@ TEST(XFormTest, VerifyBlendForSkewY) {
   // 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.
+  // This problem also potentially exists for skew along the X axis, but the
+  // current QR decomposition implementation just happens to decompose those
+  // test matrices intuitively.
   //
   // Unfortunately, this case suffers from uncomfortably large precision
   // error.
 
-  Transform from;
-  from.SkewY(0.0);
+  from = Transform();
+  from.Skew(0.0, 0.0);
 
-  Transform to;
+  to = Transform();
 
-  to.SkewY(45.0);
+  to.Skew(0.0, 45.0);
   to.Blend(from, 0.0);
   EXPECT_EQ(from, to);
 
   to = Transform();
-  to.SkewY(45.0);
+  to.Skew(0.0, 45.0);
   to.Blend(from, 0.25);
   EXPECT_ROW1_NEAR(1.0823489449280947471976333,
                    0.0464370719145053845178239,
@@ -987,7 +984,7 @@ TEST(XFormTest, VerifyBlendForSkewY) {
   EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, to);
 
   to = Transform();
-  to.SkewY(45.0);
+  to.Skew(0.0, 45.0);
   to.Blend(from, 0.5);
   EXPECT_ROW1_NEAR(1.1152212925809066312865525,
                    0.0676495144007326631996335,
@@ -1005,7 +1002,7 @@ TEST(XFormTest, VerifyBlendForSkewY) {
   EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, to);
 
   to = Transform();
-  to.SkewY(45.0);
+  to.Skew(0.0, 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);
@@ -1224,7 +1221,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,44 +1926,41 @@ TEST(XFormTest, verifyRotateAboutForDegenerateAxis) {
   EXPECT_ROW4_EQ(13.0f, 17.0f, 21.0f, 25.0f, A);
 }
 
-TEST(XFormTest, verifySkewX) {
+TEST(XFormTest, verifySkew) {
+  // Test a skew along X axis only
   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);
   EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, A);
 
-  // Verify that skewX() post-multiplies the existing matrix. Row 1, column 2,
+  // Test a skew along Y axis only
+  A.MakeIdentity();
+  A.Skew(0.0, 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 skew() post-multiplies the existing matrix. Row 1, column 2,
   // would incorrectly have value "7" if the matrix is pre-multiplied instead
   // 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);
-}
 
-TEST(XFormTest, verifySkewY) {
-  Transform A;
-  A.SkewY(45.0);
-  EXPECT_ROW1_EQ(1.0f, 0.0f, 0.0f, 0.0f, A);
+  // Test a skew along X and Y axes both
+  A.MakeIdentity();
+  A.Skew(45.0, 45.0);
+  EXPECT_ROW1_EQ(1.0f, 1.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) {
@@ -2040,7 +2034,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
diff --git a/ui/gfx/transform_util_unittest.cc b/ui/gfx/transform_util_unittest.cc
index 02bf46a..ce5af13 100644
--- a/ui/gfx/transform_util_unittest.cc
+++ b/ui/gfx/transform_util_unittest.cc
@@ -170,7 +170,7 @@ TEST(TransformUtilTest, NoSnapSkewedCompositeTransform) {
                     SkDoubleToMScalar(2.0));
   transform.Translate3d(SkDoubleToMScalar(30.5), SkDoubleToMScalar(20.0),
                         SkDoubleToMScalar(10.1));
-  transform.SkewX(20.0);
+  transform.Skew(20.0, 0.0);
   Rect viewport(1920, 1200);
   bool snapped = SnapTransform(&result, transform, viewport);
   EXPECT_FALSE(snapped) << "Skewed viewport should not snap.";