Title: [125702] trunk/Source
Revision
125702
Author
voll...@chromium.org
Date
2012-08-15 13:50:10 -0700 (Wed, 15 Aug 2012)

Log Message

[chromium] Must account for empty transformation lists when checking for big rotations.
https://bugs.webkit.org/show_bug.cgi?id=93975

Reviewed by James Robinson.

Source/WebCore:

AnimationTranslationUtil.cpp is supposed to reject large rotations
(>= 180 degrees between keyframes). The current code assumes that if
the lists of transforms at two consecutive keyframes do not match
(i.e., are different types), then do not need to reject. The rationale
is that we will revert to matrix blending -- we will collapse the lists
of transform operations to matrices at each keyframe and blend those.
Unfortunately, this is not true if a list is empty. It can be the case
that we transition from no transform to a rotation about the z axis of
360 degrees. In this case, the first list of transform operations will
be empty and the second will have a single rotation of 360 degrees. An
empty list should be treated as a rotation of zero degrees.

Unit tested in: GraphicsLayerChromiumTest.createTransformAnimationWithBigRotationAndEmptyTransformOperationList

* platform/graphics/chromium/AnimationTranslationUtil.cpp:
(WebCore::causesRotationOfAtLeast180Degrees):

Source/WebKit/chromium:

Adds a test to check that creating an animation with a big (> 180
degree) rotation, where the transform operations list in the 1st
keyframe is empty fails as expected.

* tests/AnimationTranslationUtilTest.cpp:
(WebKit::TEST):
(WebKit):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (125701 => 125702)


--- trunk/Source/WebCore/ChangeLog	2012-08-15 20:40:04 UTC (rev 125701)
+++ trunk/Source/WebCore/ChangeLog	2012-08-15 20:50:10 UTC (rev 125702)
@@ -1,3 +1,27 @@
+2012-08-15  Ian Vollick  <voll...@chromium.org>
+
+        [chromium] Must account for empty transformation lists when checking for big rotations.
+        https://bugs.webkit.org/show_bug.cgi?id=93975
+
+        Reviewed by James Robinson.
+
+        AnimationTranslationUtil.cpp is supposed to reject large rotations 
+        (>= 180 degrees between keyframes). The current code assumes that if 
+        the lists of transforms at two consecutive keyframes do not match 
+        (i.e., are different types), then do not need to reject. The rationale
+        is that we will revert to matrix blending -- we will collapse the lists
+        of transform operations to matrices at each keyframe and blend those. 
+        Unfortunately, this is not true if a list is empty. It can be the case 
+        that we transition from no transform to a rotation about the z axis of 
+        360 degrees. In this case, the first list of transform operations will 
+        be empty and the second will have a single rotation of 360 degrees. An 
+        empty list should be treated as a rotation of zero degrees.
+
+        Unit tested in: GraphicsLayerChromiumTest.createTransformAnimationWithBigRotationAndEmptyTransformOperationList
+
+        * platform/graphics/chromium/AnimationTranslationUtil.cpp:
+        (WebCore::causesRotationOfAtLeast180Degrees):
+
 2012-08-15  Beth Dakin  <bda...@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=93693

Modified: trunk/Source/WebCore/platform/graphics/chromium/AnimationTranslationUtil.cpp (125701 => 125702)


--- trunk/Source/WebCore/platform/graphics/chromium/AnimationTranslationUtil.cpp	2012-08-15 20:40:04 UTC (rev 125701)
+++ trunk/Source/WebCore/platform/graphics/chromium/AnimationTranslationUtil.cpp	2012-08-15 20:50:10 UTC (rev 125702)
@@ -153,19 +153,31 @@
     const TransformOperations& operations = *value->value();
     const TransformOperations& lastOperations = *lastValue->value();
 
-    // We'll be doing matrix interpolation in this case. No risk of incorrect
-    // rotations here.
-    if (!operations.operationsMatch(lastOperations))
+    // We'll be doing matrix interpolation in this case. No risk of incorrect rotations here.
+    if (operations.size() && lastOperations.size() && !operations.operationsMatch(lastOperations))
         return false;
 
-    for (size_t i = 0; i < operations.size(); ++i) {
-        if (!isRotationType(operations.operations()[i]->getOperationType()))
-            continue;
+    size_t numOperations = max(operations.size(), lastOperations.size());
+    for (size_t i = 0; i < numOperations; ++i) {
+        float angle = 0;
+        if (i < operations.size()) {
+            if (!isRotationType(operations.operations()[i]->getOperationType()))
+                continue;
 
-        RotateTransformOperation* rotation = static_cast<RotateTransformOperation*>(operations.operations()[i].get());
-        RotateTransformOperation* lastRotation = static_cast<RotateTransformOperation*>(lastOperations.operations()[i].get());
+            RotateTransformOperation* rotation = static_cast<RotateTransformOperation*>(operations.operations()[i].get());
+            angle = rotation->angle();
+        }
 
-        if (fabs(rotation->angle() - lastRotation->angle()) >= 180)
+        float lastAngle = 0;
+        if (i < lastOperations.size()) {
+            if (!isRotationType(lastOperations.operations()[i]->getOperationType()))
+                continue;
+
+            RotateTransformOperation* lastRotation = static_cast<RotateTransformOperation*>(lastOperations.operations()[i].get());
+            lastAngle = lastRotation->angle();
+        }
+
+        if (fabs(angle - lastAngle) >= 180)
             return true;
     }
 

Modified: trunk/Source/WebKit/chromium/ChangeLog (125701 => 125702)


--- trunk/Source/WebKit/chromium/ChangeLog	2012-08-15 20:40:04 UTC (rev 125701)
+++ trunk/Source/WebKit/chromium/ChangeLog	2012-08-15 20:50:10 UTC (rev 125702)
@@ -1,3 +1,18 @@
+2012-08-15  Ian Vollick  <voll...@chromium.org>
+
+        [chromium] Must account for empty transformation lists when checking for big rotations.
+        https://bugs.webkit.org/show_bug.cgi?id=93975
+
+        Reviewed by James Robinson.
+
+        Adds a test to check that creating an animation with a big (> 180
+        degree) rotation, where the transform operations list in the 1st
+        keyframe is empty fails as expected.
+
+        * tests/AnimationTranslationUtilTest.cpp:
+        (WebKit::TEST):
+        (WebKit):
+
 2012-08-15  Joshua Bell  <jsb...@chromium.org>
 
         [chromium] IndexedDB: Delete unused WebKit API cursor accessors

Modified: trunk/Source/WebKit/chromium/tests/AnimationTranslationUtilTest.cpp (125701 => 125702)


--- trunk/Source/WebKit/chromium/tests/AnimationTranslationUtilTest.cpp	2012-08-15 20:40:04 UTC (rev 125701)
+++ trunk/Source/WebKit/chromium/tests/AnimationTranslationUtilTest.cpp	2012-08-15 20:50:10 UTC (rev 125702)
@@ -99,6 +99,24 @@
     EXPECT_FALSE(animationCanBeTranslated(values, animation.get()));
 }
 
+TEST(AnimationTranslationUtilTest, createTransformAnimationWithBigRotationAndEmptyTransformOperationList)
+{
+    const double duration = 1;
+    WebCore::KeyframeValueList values(AnimatedPropertyWebkitTransform);
+
+    TransformOperations operations1;
+    values.insert(new TransformAnimationValue(0, &operations1));
+
+    TransformOperations operations2;
+    operations2.operations().append(RotateTransformOperation::create(270, TransformOperation::ROTATE));
+    values.insert(new TransformAnimationValue(duration, &operations2));
+
+    RefPtr<Animation> animation = Animation::create();
+    animation->setDuration(duration);
+
+    EXPECT_FALSE(animationCanBeTranslated(values, animation.get()));
+}
+
 TEST(AnimationTranslationUtilTest, createTransformAnimationWithRotationInvolvingNegativeAngles)
 {
     const double duration = 1;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to