Title: [294752] trunk
Revision
294752
Author
mrobin...@webkit.org
Date
2022-05-24 09:57:53 -0700 (Tue, 24 May 2022)

Log Message

REGRESSION (r289032): rotate animation doesn't interpolate between 0 and 1turn without forced 50% https://bugs.webkit.org/show_bug.cgi?id=239906

Reviewed by Simon Fraser.

When using CoreAnimation non-matrix animations to animate rotations,
CoreAnimation will use the shortest direction between two rotation
angles. This means that a rotation from 0 to 360 will not rotate at all.
This is different from how CSS works, where it expects the animation to
do a full turn. In order to avoid problems with this difference, when an
animation includes larger angles (> 180 degrees), fall back to software
animation.

No new tests. It is difficult to make a test for this because
when pausing animations the software path is used.

* LayoutTests/animations/3d/full-rotation-animation-expected.html: Added.
* LayoutTests/animations/3d/full-rotation-animation.html: Added.
* Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::transformationAnimationValueAt):
(WebCore::hasBigRotationAngle):
(WebCore::GraphicsLayerCA::createTransformAnimationsFromKeyframes):
(WebCore::GraphicsLayerCA::setTransformAnimationEndpoints):
* Source/WebCore/platform/graphics/transforms/TransformOperations.h:
(WebCore::SharedPrimitivesPrefix::primitives const):
(WebCore::SharedPrimitivesPrefix::primitives): Deleted.

Canonical link: https://commits.webkit.org/250920@main

Modified Paths

Added Paths

Diff

Added: trunk/LayoutTests/animations/3d/full-rotation-animation-expected.html (0 => 294752)


--- trunk/LayoutTests/animations/3d/full-rotation-animation-expected.html	                        (rev 0)
+++ trunk/LayoutTests/animations/3d/full-rotation-animation-expected.html	2022-05-24 16:57:53 UTC (rev 294752)
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+  <style>
+    .test {
+      height: 80px;
+      width: 80px;
+      perspective: 250px;
+      border: 1px solid black;
+      background: green;
+    }
+  </style>
+</head>
+<body>
+
+<div class="test"></div>
+
+</body>
+</html>

Added: trunk/LayoutTests/animations/3d/full-rotation-animation.html (0 => 294752)


--- trunk/LayoutTests/animations/3d/full-rotation-animation.html	                        (rev 0)
+++ trunk/LayoutTests/animations/3d/full-rotation-animation.html	2022-05-24 16:57:53 UTC (rev 294752)
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+  <style>
+    .box {
+      height: 80px;
+      width: 80px;
+      background-color: red;
+
+      animation-name: animation;
+      animation-duration: 1000000000s;
+      animation-delay: -250000000s;
+      animation-timing-function: linear;
+    }
+
+    .test {
+      height: 80px;
+      width: 80px;
+      perspective: 250px;
+      border: 1px solid black;
+      background: green;
+    }
+
+     @keyframes animation {
+         0% { transform: rotateY(0); }
+         100% { transform: rotateY(-1turn); }
+    }
+</style>
+</head>
+<body>
+
+<div class="test">
+  <div class="inner box">
+  </div>
+</div>
+
+</body>
+</html>

Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (294751 => 294752)


--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp	2022-05-24 16:39:51 UTC (rev 294751)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp	2022-05-24 16:57:53 UTC (rev 294752)
@@ -3463,6 +3463,37 @@
     return true;
 }
 
+static const TransformOperations& transformationAnimationValueAt(const KeyframeValueList& valueList, unsigned i)
+{
+    return static_cast<const TransformAnimationValue&>(valueList.at(i)).value();
+}
+
+static bool hasBigRotationAngle(const KeyframeValueList& valueList, const SharedPrimitivesPrefix& prefix)
+{
+    // Hardware non-matrix animations are used for every function in the shared primitives prefix.
+    // These kind of animations have issues with large rotation angles, so for every function that
+    // will be represented as a hardware non-matrix animation, check that for each of those functions
+    // the animation that's created for it will not have two consecutive keyframes that have a large
+    // rotation angle between them.
+    const auto& primitives = prefix.primitives();
+    for (unsigned animationIndex = 0; animationIndex < primitives.size(); ++animationIndex) {
+        auto type = primitives[animationIndex];
+        if (type != TransformOperation::ROTATE && type != TransformOperation::ROTATE_3D)
+            continue;
+        for (size_t i = 1; i < valueList.size(); ++i) {
+            // Since the shared primitive at this index is a rotation, both of these transform
+            // functions should be RotateTransformOperations.
+            auto prevOperation = downcast<RotateTransformOperation>(transformationAnimationValueAt(valueList, i - 1).at(animationIndex));
+            auto operation = downcast<RotateTransformOperation>(transformationAnimationValueAt(valueList, i).at(animationIndex));
+            auto angle = std::abs((prevOperation ? prevOperation->angle() : 0.0) - (operation ? operation->angle() : 0.0));
+            if (angle > 180.0)
+                return true;
+        }
+    }
+
+    return false;
+}
+
 bool GraphicsLayerCA::createTransformAnimationsFromKeyframes(const KeyframeValueList& valueList, const Animation* animation, const String& animationName, Seconds timeOffset, const FloatSize& boxSize, bool keyframesShouldUseAnimationWideTimingFunction)
 {
     ASSERT(animatedPropertyIsTransformOrRelated(valueList.property()));
@@ -3479,8 +3510,14 @@
     // the primitives shared between any two adjacent keyframes.
     SharedPrimitivesPrefix prefix;
     for (size_t i = 0; i < valueList.size(); ++i)
-        prefix.update(static_cast<const TransformAnimationValue&>(valueList.at(i)).value());
+        prefix.update(transformationAnimationValueAt(valueList, i));
 
+    // If this animation has a big rotation between two keyframes, fall back to software animation. CoreAnimation
+    // will always take the shortest path between two rotations, which will result in incorrect animation when
+    // the keyframes specify angles larger than one half rotation.
+    if (hasBigRotationAngle(valueList, prefix))
+        return false;
+
     const auto& primitives = prefix.primitives();
     unsigned numberOfSharedPrimitives = valueList.size() > 1 ? primitives.size() : 0;
     for (unsigned animationIndex = 0; animationIndex < numberOfSharedPrimitives; ++animationIndex) {
@@ -3708,13 +3745,13 @@
     unsigned fromIndex = !forwards;
     unsigned toIndex = forwards;
     
-    auto& startValue = static_cast<const TransformAnimationValue&>(valueList.at(fromIndex));
-    auto& endValue = static_cast<const TransformAnimationValue&>(valueList.at(toIndex));
+    const auto& startValue = transformationAnimationValueAt(valueList, fromIndex);
+    const auto& endValue = transformationAnimationValueAt(valueList, toIndex);
 
     if (isMatrixAnimation) {
         TransformationMatrix fromTransform, toTransform;
-        startValue.value().apply(boxSize, fromTransform);
-        endValue.value().apply(boxSize, toTransform);
+        startValue.apply(boxSize, fromTransform);
+        endValue.apply(boxSize, toTransform);
 
         // If any matrix is singular, CA won't animate it correctly. So fall back to software animation
         if (!fromTransform.isInvertible() || !toTransform.isInvertible())
@@ -3725,27 +3762,27 @@
     } else {
         if (isTransformTypeNumber(transformOpType)) {
             float fromValue;
-            getTransformFunctionValue(startValue.value().at(functionIndex), transformOpType, boxSize, fromValue);
+            getTransformFunctionValue(startValue.at(functionIndex), transformOpType, boxSize, fromValue);
             basicAnim->setFromValue(fromValue);
             
             float toValue;
-            getTransformFunctionValue(endValue.value().at(functionIndex), transformOpType, boxSize, toValue);
+            getTransformFunctionValue(endValue.at(functionIndex), transformOpType, boxSize, toValue);
             basicAnim->setToValue(toValue);
         } else if (isTransformTypeFloatPoint3D(transformOpType)) {
             FloatPoint3D fromValue;
-            getTransformFunctionValue(startValue.value().at(functionIndex), transformOpType, boxSize, fromValue);
+            getTransformFunctionValue(startValue.at(functionIndex), transformOpType, boxSize, fromValue);
             basicAnim->setFromValue(fromValue);
             
             FloatPoint3D toValue;
-            getTransformFunctionValue(endValue.value().at(functionIndex), transformOpType, boxSize, toValue);
+            getTransformFunctionValue(endValue.at(functionIndex), transformOpType, boxSize, toValue);
             basicAnim->setToValue(toValue);
         } else {
             TransformationMatrix fromValue;
-            getTransformFunctionValue(startValue.value().at(functionIndex), transformOpType, boxSize, fromValue);
+            getTransformFunctionValue(startValue.at(functionIndex), transformOpType, boxSize, fromValue);
             basicAnim->setFromValue(fromValue);
 
             TransformationMatrix toValue;
-            getTransformFunctionValue(endValue.value().at(functionIndex), transformOpType, boxSize, toValue);
+            getTransformFunctionValue(endValue.at(functionIndex), transformOpType, boxSize, toValue);
             basicAnim->setToValue(toValue);
         }
     }

Modified: trunk/Source/WebCore/platform/graphics/transforms/TransformOperations.h (294751 => 294752)


--- trunk/Source/WebCore/platform/graphics/transforms/TransformOperations.h	2022-05-24 16:39:51 UTC (rev 294751)
+++ trunk/Source/WebCore/platform/graphics/transforms/TransformOperations.h	2022-05-24 16:57:53 UTC (rev 294752)
@@ -118,7 +118,7 @@
     virtual ~SharedPrimitivesPrefix() = default;
     void update(const TransformOperations&);
     bool hadIncompatibleTransformFunctions() { return m_indexOfFirstMismatch.has_value(); }
-    const Vector<TransformOperation::OperationType>& primitives() { return m_primitives; }
+    const Vector<TransformOperation::OperationType>& primitives() const { return m_primitives; }
 
 private:
     std::optional<size_t> m_indexOfFirstMismatch;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to