Changeset 294752 in webkit

Timestamp:

May 24, 2022, 9:57:53 AM (4 years ago)

Author:

Martin Robinson

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

Location:

trunk

Files:

• LayoutTests/animations/3d/full-rotation-animation-expected.html (added)
• LayoutTests/animations/3d/full-rotation-animation.html (added)
• Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (modified) (4 diffs)
• Source/WebCore/platform/graphics/transforms/TransformOperations.h (modified) (1 diff)

trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp

@@ -3464 +3464 @@
 }
 
+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)
 {
@@ -3480 +3511 @@
     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();
@@ -3709 +3746 @@
     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
@@ -3726 +3763 @@
         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);
         }

trunk/Source/WebCore/platform/graphics/transforms/TransformOperations.h

@@ -119 +119 @@
     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: