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 git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294752 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 24, 2022 · 43e80183650b2053fb865f81218eb3c94724f42c · 43e8018
1 parent 7f90acb
commit 43e80183650b2053fb865f81218eb3c94724f42c
Showing 4 changed files with 108 additions and 12 deletions.
diff --git a/LayoutTests/animations/3d/full-rotation-animation-expected.html b/LayoutTests/animations/3d/full-rotation-animation-expected.html
@@ -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>
diff --git a/LayoutTests/animations/3d/full-rotation-animation.html b/LayoutTests/animations/3d/full-rotation-animation.html
@@ -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>
diff --git a/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp b/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
@@ -3463,6 +3463,37 @@ bool GraphicsLayerCA::appendToUncommittedAnimations(const KeyframeValueList& val
     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,7 +3510,13 @@ bool GraphicsLayerCA::createTransformAnimationsFromKeyframes(const KeyframeValue
     // 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;
@@ -3708,13 +3745,13 @@ bool GraphicsLayerCA::setTransformAnimationEndpoints(const KeyframeValueList& va
     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 @@ bool GraphicsLayerCA::setTransformAnimationEndpoints(const KeyframeValueList& va
     } 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);
         }
     }

diff --git a/Source/WebCore/platform/graphics/transforms/TransformOperations.h b/Source/WebCore/platform/graphics/transforms/TransformOperations.h
@@ -118,7 +118,7 @@ class SharedPrimitivesPrefix {
     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;