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

ui/gfx/transform_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // MSVC++ requires this to be set before any other includes to get M_PI.
 #define _USE_MATH_DEFINES
 
 #include "ui/gfx/transform.h"
 
 #include <cmath>
@@ skipping 759 matching lines @@
     EXPECT_FLOAT_EQ(t * 4 + 1, to.matrix().get(0, 0)) << "i: " << i;
     EXPECT_FLOAT_EQ(t * 3 + 1, to.matrix().get(1, 1)) << "i: " << i;
     EXPECT_FLOAT_EQ(t * 2 + 1, to.matrix().get(2, 2)) << "i: " << i;
   }
 }
 
 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));
   }
 }
 
 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));
   }
 }
 
 #if defined(_WIN64)
 // http://crbug.com/406574
 #define MAYBE_BlendPerspective DISABLED_BlendPerspective
 #else
 #define MAYBE_BlendPerspective BlendPerspective
@@ skipping 90 matching lines @@
 
   to = Transform();
   to.Scale3d(200.0, 100.0, 300.0);
   to.Blend(from, 1.0);
   EXPECT_ROW1_EQ(200.0f, 0.0f,  0.0f,  0.0f, to);
   EXPECT_ROW2_EQ(0.0f,  100.0f, 0.0f,  0.0f, to);
   EXPECT_ROW3_EQ(0.0f,   0.0f, 300.0f, 0.0f, to);
   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);
   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.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);
   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.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
-  // 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
 #else
 #define MAYBE_VerifyBlendForRotationAboutX VerifyBlendForRotationAboutX
 #endif
 TEST(XFormTest, MAYBE_VerifyBlendForRotationAboutX) {
   // Even though.Blending uses quaternions, axis-aligned rotations should.
   // Blend the same with quaternions or Euler angles. So we can test
   // rotation.Blending by comparing against manually specified matrices from
@@ skipping 191 matching lines @@
   // interpolations; order of operations will be tested well enough by the
   // end cases that are easier to specify.
 
   Transform from;
   Transform to;
 
   Transform expectedEndOfAnimation;
   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;
   to.Blend(from, 0.0);
   EXPECT_EQ(from, to);
 
   to = expectedEndOfAnimation;
   // We short circuit if blend is >= 1, so to check the numerics, we will
   // check that we get close to what we expect when we're nearly done
   // interpolating.
@@ skipping 684 matching lines @@
   InitializeTestMatrix(&A);
   A.RotateAbout(Vector3dF(0.0, 0.0, 0.0), 35.0);
 
   // Verify that A remains unchanged.
   EXPECT_ROW1_EQ(10.0f, 14.0f, 18.0f, 22.0f, A);
   EXPECT_ROW2_EQ(11.0f, 15.0f, 19.0f, 23.0f, A);
   EXPECT_ROW3_EQ(12.0f, 16.0f, 20.0f, 24.0f, A);
   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);
   EXPECT_ROW4_EQ(0.0f, 0.0f, 0.0f, 1.0f, A);
 
   // Verify that skewX() 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);

[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);
   EXPECT_ROW1_EQ(1.0f, 0.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, -1.0f, 1.0f, A);
 
   // Verify that PerspectiveDepth() post-multiplies the existing matrix.
   A.MakeIdentity();
@@ skipping 51 matching lines @@
   A.Scale3d(6.0, 7.0, 8.0);
   EXPECT_TRUE(A.IsInvertible());
 
   A.MakeIdentity();
   A.RotateAboutXAxis(10.0);
   A.RotateAboutYAxis(20.0);
   A.RotateAboutZAxis(30.0);
   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
   // intuitive explanation is that perspective is eqivalent to a skew of the
   // w-axis; skews are invertible.
   A.MakeIdentity();
   A.ApplyPerspectiveDepth(1.0);
   EXPECT_TRUE(A.IsInvertible());
 
   // A "pure" perspective matrix derived by similar triangles, with m44() set
@@ skipping 639 matching lines @@
   // Rounding should only affect 2d translation components.
   translation.Translate3d(0.f, 0.f, 0.5f);
   expected.Translate3d(0.f, 0.f, 0.5f);
   translation.RoundTranslationComponents();
   EXPECT_EQ(expected.ToString(), translation.ToString());
 }
 
 }  // namespace
 
 }  // namespace gfx