Animate skew in angle space rather than tangent space for interop and to avoid issues with infinite tangents. (Bug 606722) r=bzbarsky a2.0=blocking
authorL. David Baron <dbaron@dbaron.org>
Tue, 16 Nov 2010 09:56:03 -0800
changeset 57583 860d55aec420a96d0535d39c9a68b50099fb697c
parent 57582 0b5df68704ad59ace22682b40b1b30843af9e05d
child 57584 f0458767cf4b2d8f37eabb8feb9475fbccf43f18
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersbzbarsky
bugs606722
milestone2.0b8pre
Animate skew in angle space rather than tangent space for interop and to avoid issues with infinite tangents. (Bug 606722) r=bzbarsky a2.0=blocking
layout/style/nsStyleAnimation.cpp
layout/style/test/test_transitions_per_property.html
--- a/layout/style/nsStyleAnimation.cpp
+++ b/layout/style/nsStyleAnimation.cpp
@@ -618,25 +618,17 @@ nsStyleAnimation::ComputeDistance(nsCSSP
             CalcValue c1 = ExtractCalcValue(v1),
                       c2 = ExtractCalcValue(v2);
             double diff = c1.mLength - c2.mLength;
             squareDistance += diff * diff;
             diff = c1.mPercent - c2.mPercent;
             squareDistance += diff * diff;
           } else {
             NS_ABORT_IF_FALSE(v1.GetUnit() == v2.GetUnit(), "unit mismatch");
-            double diff;
-            if (tfunc == eCSSKeyword_skewx ||
-                tfunc == eCSSKeyword_skewy ||
-                tfunc == eCSSKeyword_skew) {
-              NS_ABORT_IF_FALSE(v1.GetUnit() == eCSSUnit_Radian, "unexpected unit");
-              diff = tan(v2.GetFloatValue()) - tan(v1.GetFloatValue());
-            } else {
-              diff = v2.GetFloatValue() - v1.GetFloatValue();
-            }
+            double diff = v2.GetFloatValue() - v1.GetFloatValue();
             squareDistance += diff * diff;
           }
         }
       }
       NS_ABORT_IF_FALSE(!list1 && !list2,
                         "list lengths should match after AddWeighted");
 
       aDistance = sqrt(squareDistance);
@@ -876,31 +868,16 @@ AddTransformScale(const nsCSSValue &aVal
   NS_ABORT_IF_FALSE(aValue2.GetUnit() == eCSSUnit_Number, "unexpected unit");
 
   float v1 = aValue1.GetFloatValue() - 1.0f,
         v2 = aValue2.GetFloatValue() - 1.0f;
   float result = v1 * aCoeff1 + v2 * aCoeff2;
   aResult.SetFloatValue(result + 1.0f, eCSSUnit_Number);
 }
 
-// FIXME: The spec still says skew should animate in angle space,
-// although I think we at least sort of agreed that it should animate
-// in tangent space.  So here I animate in in tangent space.
-// Animating in angle space would mean just using AddCSSValueAngle.
-static void
-AddTransformSkew(const nsCSSValue &aValue1, double aCoeff1,
-                 const nsCSSValue &aValue2, double aCoeff2,
-                 nsCSSValue &aResult)
-{
-  aResult.SetFloatValue(atan(aCoeff1 * tan(aValue1.GetAngleValueInRadians()) +
-                             aCoeff2 * tan(aValue2.GetAngleValueInRadians())),
-                        eCSSUnit_Radian);
-}
-
-
 static already_AddRefed<nsCSSValue::Array>
 AppendTransformFunction(nsCSSKeyword aTransformFunction,
                         nsCSSValueList**& aListTail)
 {
   PRUint32 nargs;
   if (aTransformFunction == eCSSKeyword_matrix) {
     nargs = 6;
   } else if (aTransformFunction == eCSSKeyword_translate ||
@@ -1151,20 +1128,17 @@ AddTransformMatrix(const nsStyleTransfor
 
   float rotate1, XYshear1, scaleX1, scaleY1;
   DecomposeMatrix(aMatrix1, rotate1, XYshear1, scaleX1, scaleY1);
   float rotate2, XYshear2, scaleX2, scaleY2;
   DecomposeMatrix(aMatrix2, rotate2, XYshear2, scaleX2, scaleY2);
 
   float rotate = rotate1 * aCoeff1 + rotate2 * aCoeff2;
 
-  // FIXME: The spec still says skew should animate in angle space,
-  // although I think we at least sort of agreed that it should animate
-  // in tangent space.  So here I animate in in tangent space.
-  float skewX = atanf(XYshear1 * aCoeff1 + XYshear2 * aCoeff2);
+  float skewX = atanf(XYshear1) * aCoeff1 + atanf(XYshear2) * aCoeff2;
 
   // Handle scale, and the two matrix components where identity is 1, by
   // subtracting 1, multiplying by the coefficients, and then adding 1
   // back.  This gets the right AddWeighted behavior and gets us the
   // interpolation-against-identity behavior for free.
   float scaleX =
     ((scaleX1 - 1.0f) * aCoeff1 + (scaleX2 - 1.0f) * aCoeff2) + 1.0f;
   float scaleY =
@@ -1275,47 +1249,44 @@ AddTransformLists(const nsCSSValueList* 
         NS_ABORT_IF_FALSE(a1->Count() == 2, "unexpected count");
         NS_ABORT_IF_FALSE(a2->Count() == 2, "unexpected count");
 
         AddTransformScale(a1->Item(1), aCoeff1, a2->Item(1), aCoeff2,
                           arr->Item(1));
 
         break;
       }
+      // It would probably be nicer to animate skew in tangent space
+      // rather than angle space.  However, it's easy to specify
+      // skews with infinite tangents, and behavior changes pretty
+      // drastically when crossing such skews (since the direction of
+      // animation flips), so interop is probably more important here.
       case eCSSKeyword_skew: {
         NS_ABORT_IF_FALSE(a1->Count() == 2 || a1->Count() == 3,
                           "unexpected count");
         NS_ABORT_IF_FALSE(a2->Count() == 2 || a2->Count() == 3,
                           "unexpected count");
 
         nsCSSValue zero(0.0f, eCSSUnit_Radian);
         // Add Y component of skew.
-        AddTransformSkew(a1->Count() == 3 ? a1->Item(2) : zero,
+        AddCSSValueAngle(a1->Count() == 3 ? a1->Item(2) : zero,
                          aCoeff1,
                          a2->Count() == 3 ? a2->Item(2) : zero,
                          aCoeff2,
                          arr->Item(2));
 
         // Add X component of skew (which can be merged with case below
         // in non-DEBUG).
-        AddTransformSkew(a1->Item(1), aCoeff1, a2->Item(1), aCoeff2,
+        AddCSSValueAngle(a1->Item(1), aCoeff1, a2->Item(1), aCoeff2,
                          arr->Item(1));
 
         break;
       }
       case eCSSKeyword_skewx:
-      case eCSSKeyword_skewy: {
-        NS_ABORT_IF_FALSE(a1->Count() == 2, "unexpected count");
-        NS_ABORT_IF_FALSE(a2->Count() == 2, "unexpected count");
-
-        AddTransformSkew(a1->Item(1), aCoeff1, a2->Item(1), aCoeff2,
-                         arr->Item(1));
-
-        break;
-      }
+      case eCSSKeyword_skewy:
       case eCSSKeyword_rotate: {
         NS_ABORT_IF_FALSE(a1->Count() == 2, "unexpected count");
         NS_ABORT_IF_FALSE(a2->Count() == 2, "unexpected count");
 
         AddCSSValueAngle(a1->Item(1), aCoeff1, a2->Item(1), aCoeff2,
                          arr->Item(1));
 
         break;
--- a/layout/style/test/test_transitions_per_property.html
+++ b/layout/style/test/test_transitions_per_property.html
@@ -954,44 +954,45 @@ function test_transform_transition(prop)
       expected_uncomputed: 'scaleX(2.5)',
       expected: 'matrix(2.5, 0, 0, 1, 0px, 0px)' },
     { start: 'scaleY(5)', end: 'none',
       expected_uncomputed: 'scaleY(4)',
       expected: 'matrix(1, 0, 0, 4, 0px, 0px)' },
 
     // skew
     { start: 'skewX(45deg)', end: 'none',
-      expected: 'matrix(1, 0, 0.75, 1, 0px, 0px)' },
+      expected_uncomputed: 'skewX(33.75deg)' },
     { start: 'skewY(45deg)', end: 'none',
-      expected: 'matrix(1, 0.75, 0, 1, 0px, 0px)' },
+      expected_uncomputed: 'skewY(33.75deg)' },
     { start: 'skew(45deg)', end: 'none',
-      expected: 'matrix(1, 0, 0.75, 1, 0px, 0px)' },
+      expected_uncomputed: 'skew(33.75deg)' },
     { start: 'skew(45deg, 45deg)', end: 'none',
-      expected: 'matrix(1, 0.75, 0.75, 1, 0px, 0px)' },
+      expected_uncomputed: 'skew(33.75deg, 33.75deg)' },
     { start: 'skewX(45deg)', end: 'skewX(-45deg)',
-      expected: 'matrix(1, 0, 0.5, 1, 0px, 0px)' },
+      expected_uncomputed: 'skewX(22.5deg)' },
     { start: 'skewX(0)', end: 'skewX(-45deg)',
-      expected: 'matrix(1, 0, -0.25, 1, 0px, 0px)' },
+      expected_uncomputed: 'skewX(-11.25deg)' },
     { start: 'skewY(45deg)', end: 'skewY(-45deg)',
-      expected: 'matrix(1, 0.5, 0, 1, 0px, 0px)' },
+      expected_uncomputed: 'skewY(22.5deg)' },
 
     // matrix : skewX
     { start: 'matrix(1, 0, 3, 1, 0px, 0px)', end: 'none',
-      expected_uncomputed: 'matrix(1, 0, 2.25, 1, 0px, 0px)' },
+      force_compute: true,
+      expected_uncomputed: 'matrix(1, 0, ' + Math.tan(Math.atan(3) * 0.75) + ', 1, 0px, 0px)' },
     { start: 'skewX(0)', end: 'skewX(-45deg) translate(0)',
-      expected: 'matrix(1, 0, -0.25, 1, 0px, 0px)' },
+      expected_uncomputed: 'skewX(-11.25deg)' },
     // matrix : rotate
     { start: 'rotate(-30deg)', end: 'matrix(0, 1, -1, 0, 0px, 0px)',
       expected_uncomputed: 'matrix(1, 0, 0, 1, 0px, 0px)' },
     { start: 'rotate(-30deg) translateX(0)',
       end: 'translateX(0) rotate(-90deg)',
       expected: c('rotate(-45deg)') },
     // matrix decomposition of skewY
     { start: 'skewY(60deg)', end: 'skewY(-60deg) translateX(0)',
-      expected: c('rotate(30deg) skewX(' + Math.atan(Math.tan(Math.PI / 3) / 2) + 'rad) scale(2, 0.5)') },
+      expected: c('rotate(30deg) skewX(30deg) scale(2, 0.5)') },
 
     // matrix decomposition
 
     // Four pairs of the same matrix expressed different ways.
     { start: 'matrix(-1, 0, 0, -1, 0pt, 0pt)', /* rotate(180deg) */
       end: 'matrix(1, 0, 0, 1, 0, 0)',
       expected: c('rotate(135deg)') },
     { start: 'scale(-1)', end: 'none',
@@ -1036,74 +1037,72 @@ function test_transform_transition(prop)
     // the animations in
     // http://dbaron.org/css/test/2010/transition-negative-determinant
     // don't flip when they finish, and then wrote tests corresponding
     // to the current code's behavior.
     // ... start with four with positive determinants
     { start: 'none',
       end: 'matrix(1, 0, 1.5, 1, 0pt, 0pt)',
               /* skewX(atan(1.5)) */
-      expected: c('matrix(1, 0, 0.375, 1, 0, 0)') },
+      expected_uncomputed: 'matrix(1, 0, ' + Math.tan(Math.atan(1.5) * 0.25).toFixed(6) + ', 1, 0px, 0px)' },
     { start: 'none',
       end: 'matrix(-1, 0, 2, -1, 0pt, 0pt)',
               /* rotate(180deg) skewX(atan(-2)) */
-      expected: c('rotate(45deg) matrix(1, 0, -0.5, 1, 0, 0)') },
+      expected: c('rotate(45deg) matrix(1, 0, ' + Math.tan(Math.atan(-2) * 0.25) + ', 1, 0px, 0px)') },
     { start: 'none',
       end: 'matrix(0, -1, 1, -3, 0pt, 0pt)',
               /* rotate(-90deg) skewX(atan(3)) */
-      expected: c('rotate(-22.5deg) matrix(1, 0, 0.75, 1, 0, 0)') },
+      expected: c('rotate(-22.5deg) matrix(1, 0, ' + Math.tan(Math.atan(3) * 0.25) + ', 1, 0px, 0px)') },
     { start: 'none',
       end: 'matrix(0, 1, -1, 4, 0pt, 0pt)',
               /* rotate(90deg) skewX(atan(4)) */
-      expected: c('rotate(22.5deg) matrix(1, 0, 1, 1, 0, 0)') },
+      expected: c('rotate(22.5deg) matrix(1, 0, ' + Math.tan(Math.atan(4) * 0.25) + ', 1, 0px, 0px)') },
     // and then four with negative determinants
     { start: 'none',
       end: 'matrix(1, 0, 1, -1, 0pt, 0pt)',
               /* rotate(-180deg) skewX(atan(-1)) scaleX(-1) */
-      expected: c('rotate(-45deg) matrix(1, 0, -0.25, 1, 0, 0) scaleX(0.5)'),
-      round_error_ok: true },
+      expected: c('rotate(-45deg) matrix(1, 0, ' + Math.tan(Math.atan(-1) * 0.25) + ', 1, 0px, 0px) scaleX(0.5)') },
     { start: 'none',
       end: 'matrix(-1, 0, -1, 1, 0pt, 0pt)',
               /* skewX(atan(-1)) scaleX(-1) */
-      expected: c('matrix(1, 0, -0.25, 1, 0, 0) scaleX(0.5)') },
+      expected: c('matrix(1, 0, ' + Math.tan(Math.atan(-1) * 0.25) + ', 1, 0px, 0px) scaleX(0.5)') },
     { start: 'none',
       end: 'matrix(0, 1, 1, -2, 0pt, 0pt)',
               /* rotate(-90deg) skewX(atan(2)) scaleX(-1) */
-      expected: c('rotate(-22.5deg) matrix(1, 0, 0.5, 1, 0, 0) scaleX(0.5)') },
+      expected: c('rotate(-22.5deg) matrix(1, 0, ' + Math.tan(Math.atan(2) * 0.25) + ', 1, 0px, 0px) scaleX(0.5)') },
     { start: 'none',
       end: 'matrix(0, -1, -1, 0.5, 0pt, 0pt)',
               /* rotate(90deg) skewX(atan(0.5)) scaleX(-1) */
-      expected: c('rotate(22.5deg) matrix(1, 0, 0.125, 1, 0, 0) scaleX(0.5)') },
+      expected: c('rotate(22.5deg) matrix(1, 0, ' + Math.tan(Math.atan(0.5) * 0.25) + ', 1, 0px, 0px) scaleX(0.5)') },
 
     // lists vs. matrix decomposition
     { start: 'translate(10px) skewY(45deg)',
       end: 'translate(30px) skewY(-45deg)',
-      expected: 'matrix(1, 0.5, 0, 1, 15px, 0px)' },
+      expected_uncomputed: 'translate(15px) skewY(22.5deg)' },
     { start: 'skewY(45deg) rotate(90deg)',
       end: 'skewY(-45deg) rotate(90deg)',
-      expected: c('matrix(1, 0.5, 0, 1, 0px, 0px) rotate(90deg)') },
+      expected_uncomputed: 'skewY(22.5deg) rotate(90deg)' },
     { start: 'skewY(45deg) rotate(90deg) translate(0)',
       end: 'skewY(-45deg) rotate(90deg)',
-      expected: c('matrix(1, 0.5, 0, 1, 0px, 0px) rotate(90deg)') },
+      expected: c('rotate(90deg) skewX(-22.5deg)') },
     { start: 'skewX(45deg) rotate(90deg)',
       end: 'skewX(-45deg) rotate(90deg)',
-      expected: c('matrix(1, 0, 0.5, 1, 0px, 0px) rotate(90deg)') },
+      expected_uncomputed: 'skewX(22.5deg) rotate(90deg)' },
     { start: 'skewX(-60deg) rotate(90deg) translate(0)',
       end: 'skewX(60deg) rotate(90deg)',
-      expected: c('rotate(120deg) skewX(' +
-                   Math.atan(Math.tan(Math.PI / 3) / 2) +
-                  'rad) scale(2, 0.5)') },
+      expected: c('rotate(120deg) skewX(30deg) scale(2, 0.5)'),
+      round_error_ok: true },
   ];
 
   var matrix_re = /^matrix\(([^,]*), ([^,]*), ([^,]*), ([^,]*), ([^,]*)px, ([^,]*)px\)$/;
   for (var i in tests) {
     var test = tests[i];
     if (!("expected" in test)) {
       var v = test.expected_uncomputed;
-      if (v.match(matrix_re)) {
+      if (v.match(matrix_re) && !test.force_compute) {
         test.expected = v;
       } else {
         test.expected = c(v);
       }
     }
   }
 
   for (var i in tests) {