[web-animations] dynamically toggle acceleration of offset animations…

… depending on ability of animations in the effect stack to be accelerated https://bugs.webkit.org/show_bug.cgi?id=236712 <rdar://89030146> Reviewed by Dean Jackson. We implement CSS Motion Path by using it as another input for the matrix computed in RenderStyle::applyCSSTransform(). This means that if we were to run an accelerated animation for a transform-related property, we'd clobber a motion path set either on the underlying style or via another animation on this element. For transforms and motion paths to co-exist, we opt out of acceleration for all effects in a stack associated with an element that has a motion set using the underlying style or containing an effect animating one of the CSS Motion Path properties. This is done simply by adding those checks to KeyframeEffect::preventsAcceleration(). But we also need to determine when the underlying style may have changed to either newly contain a motion path or no longer contain one. To do this, we message from ElementAnimationRareData::setLastStyleChangeEventStyle() down to all effects on the associated stack and check in KeyframeEffect::lastStyleChangeEventStyleDidChange() whether the motion path state changed. Test: webanimations/accelerated-animations-and-motion-path.html * Source/WebCore/animation/ElementAnimationRareData.cpp: (WebCore::ElementAnimationRareData::setLastStyleChangeEventStyle): * Source/WebCore/animation/ElementAnimationRareData.h: (WebCore::ElementAnimationRareData::setLastStyleChangeEventStyle): Deleted. * Source/WebCore/animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::preventsAcceleration const): (WebCore::KeyframeEffect::lastStyleChangeEventStyleDidChange): * Source/WebCore/animation/KeyframeEffect.h: * Source/WebCore/animation/KeyframeEffectStack.cpp: (WebCore::KeyframeEffectStack::lastStyleChangeEventStyleDidChange): * Source/WebCore/animation/KeyframeEffectStack.h: * LayoutTests/webanimations/accelerated-animations-and-motion-path-expected.txt: Added. * LayoutTests/webanimations/accelerated-animations-and-motion-path.html: Added. Canonical link: https://commits.webkit.org/250737@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294478 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · May 19, 2022 · 2b84a94d94784d1d3cc4cffd1e1a337ec2feb599 · 2b84a94
1 parent bdb8c16
commit 2b84a94d94784d1d3cc4cffd1e1a337ec2feb599
Showing 8 changed files with 182 additions and 1 deletion.
diff --git a/LayoutTests/webanimations/accelerated-animations-and-motion-path-expected.txt b/LayoutTests/webanimations/accelerated-animations-and-motion-path-expected.txt
@@ -0,0 +1,7 @@
+
+PASS Setting 'offset' on the underlying style prevents a 'transform' animation from being accelerated.
+PASS Toggling 'offset' dynamically on the underlying style toggles the ability for a 'transform' animation to be accelerated.
+PASS Animating both 'offset' and 'transform' on the same animation prevents acceleration.
+PASS Animating both 'offset' and 'transform' on different animations prevents acceleration.
+PASS Setting keyframes on an animation to include 'offset' on top of 'transform' prevents acceleration.
+
diff --git a/LayoutTests/webanimations/accelerated-animations-and-motion-path.html b/LayoutTests/webanimations/accelerated-animations-and-motion-path.html
@@ -0,0 +1,126 @@
+<!DOCTYPE html>
+<body>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<style>
+
+div {
+    position: absolute;
+    left: 0;
+    top: 0;
+    width: 100px;
+    height: 100px;
+    background-color: black;
+}
+
+</style>
+<script>
+
+const createDiv = test => {
+    const div = document.createElement("div");
+    test.add_cleanup(() => div.remove());
+    return document.body.appendChild(div);
+}
+
+const animationReadyToAnimateAccelerated = async animation => {
+    await animation.ready;
+    await new Promise(requestAnimationFrame);
+    await new Promise(requestAnimationFrame);
+    await new Promise(requestAnimationFrame);
+}
+
+const duration = 1000 * 1000; // 1000s.
+
+promise_test(async test => {
+    const target = createDiv(test);
+
+    // Set offset on the underlying style.
+    target.style.offset = "path('M10,10 H10')";
+
+    // Add a transform animation that could be accelerated by itself.
+    const animation = target.animate(
+        { transform: "translateX(100px)" },
+        { duration }
+    );
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
+}, "Setting 'offset' on the underlying style prevents a 'transform' animation from being accelerated.");
+
+promise_test(async test => {
+    const target = createDiv(test);
+
+    // Add a transform animation that can be accelerated by itself.
+    const animation = target.animate(
+        { transform: "translateX(100px)" },
+        { duration }
+    );
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+
+    // Set offset on the underlying style.
+    target.style.offset = "path('M10,10 H10')";
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
+
+    // Remove offset on the underlying style.
+    target.style.removeProperty("offset");
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+}, "Toggling 'offset' dynamically on the underlying style toggles the ability for a 'transform' animation to be accelerated.");
+
+promise_test(async test => {
+    const target = createDiv(test);
+
+    const animation = target.animate(
+        { transform: "translateX(100px)", cssOffset: ["path('M10,10 H10')", "path('M10,10 H20')"] },
+        { duration }
+    );
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
+}, "Animating both 'offset' and 'transform' on the same animation prevents acceleration.");
+
+promise_test(async test => {
+    const target = createDiv(test);
+    const animations = [];
+
+    // First add a transform animation that could be accelerated by itself.
+    animations.push(target.animate(
+        { transform: "translateX(100px)" },
+        { duration }
+    ));
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+
+    // Then, add an offset animation that prevents the whole stack from being accelerated.
+    animations.push(target.animate(
+        { cssOffset: ["path('M10,10 H10')", "path('M10,10 H20')"] },
+        { duration }
+    ));
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
+
+    // Canceling the offset animation will no longer prevent the stack from being accelerated. 
+    animations.at(-1).cancel();
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+}, "Animating both 'offset' and 'transform' on different animations prevents acceleration.");
+
+promise_test(async test => {
+    const target = createDiv(test);
+
+    const animation = target.animate(
+        { transform: "translateX(100px)" },
+        { duration }
+    );
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+
+    animation.effect.setKeyframes(
+        { transform: "translateX(100px)", cssOffset: ["path('M10,10 H10')", "path('M10,10 H20')"] }
+    );
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
+}, "Setting keyframes on an animation to include 'offset' on top of 'transform' prevents acceleration.");
+
+</script>
+</body>
diff --git a/Source/WebCore/animation/ElementAnimationRareData.cpp b/Source/WebCore/animation/ElementAnimationRareData.cpp
@@ -54,4 +54,15 @@ void ElementAnimationRareData::setAnimationsCreatedByMarkup(CSSAnimationCollecti
     m_animationsCreatedByMarkup = WTFMove(animations);
 }
 
+void ElementAnimationRareData::setLastStyleChangeEventStyle(std::unique_ptr<const RenderStyle>&& style)
+{
+    if (m_keyframeEffectStack && m_lastStyleChangeEventStyle != style) {
+        auto previousStyleChangeEventStyle = std::exchange(m_lastStyleChangeEventStyle, WTFMove(style));
+        m_keyframeEffectStack->lastStyleChangeEventStyleDidChange(previousStyleChangeEventStyle.get(), m_lastStyleChangeEventStyle.get());
+        return;
+    }
+
+    m_lastStyleChangeEventStyle = WTFMove(style);
+}
+
 } // namespace WebCore
diff --git a/Source/WebCore/animation/ElementAnimationRareData.h b/Source/WebCore/animation/ElementAnimationRareData.h
@@ -53,7 +53,7 @@ class ElementAnimationRareData {
     PropertyToTransitionMap& completedTransitionsByProperty() { return m_completedTransitionsByProperty; }
     PropertyToTransitionMap& runningTransitionsByProperty() { return m_runningTransitionsByProperty; }
     const RenderStyle* lastStyleChangeEventStyle() const { return m_lastStyleChangeEventStyle.get(); }
-    void setLastStyleChangeEventStyle(std::unique_ptr<const RenderStyle>&& style) { m_lastStyleChangeEventStyle = WTFMove(style); }
+    void setLastStyleChangeEventStyle(std::unique_ptr<const RenderStyle>&&);
     void cssAnimationsDidUpdate() { m_hasPendingKeyframesUpdate = false; }
     void keyframesRuleDidChange() { m_hasPendingKeyframesUpdate = true; }
     bool hasPendingKeyframesUpdate() const { return m_hasPendingKeyframesUpdate; }

diff --git a/Source/WebCore/animation/KeyframeEffect.cpp b/Source/WebCore/animation/KeyframeEffect.cpp
@@ -1603,6 +1603,23 @@ bool KeyframeEffect::canBeAccelerated() const
 
 bool KeyframeEffect::preventsAcceleration() const
 {
+    // We cannot run accelerated transform animations if a motion path is applied
+    // to an element, either through the underlying style, or through a keyframe.
+    if (auto target = targetStyleable()) {
+        if (auto* lastStyleChangeEventStyle = target->lastStyleChangeEventStyle()) {
+            if (lastStyleChangeEventStyle->offsetPath())
+                return true;
+        }
+    }
+
+    if (animatesProperty(CSSPropertyOffsetAnchor)
+        || animatesProperty(CSSPropertyOffsetDistance)
+        || animatesProperty(CSSPropertyOffsetPath)
+        || animatesProperty(CSSPropertyOffsetPosition)
+        || animatesProperty(CSSPropertyOffsetRotate)) {
+        return true;
+    }
+
     if (m_acceleratedPropertiesState == AcceleratedProperties::None)
         return false;
 
@@ -2282,4 +2299,14 @@ KeyframeEffect::CanBeAcceleratedMutationScope::~CanBeAcceleratedMutationScope()
         m_effect->abilityToBeAcceleratedDidChange();
 }
 
+void KeyframeEffect::lastStyleChangeEventStyleDidChange(const RenderStyle* previousStyle, const RenderStyle* currentStyle)
+{
+    auto hasMotionPath = [](const RenderStyle* style) {
+        return style && style->offsetPath();
+    };
+
+    if (hasMotionPath(previousStyle) != hasMotionPath(currentStyle))
+        abilityToBeAcceleratedDidChange();
+}
+
 } // namespace WebCore
diff --git a/Source/WebCore/animation/KeyframeEffect.h b/Source/WebCore/animation/KeyframeEffect.h
@@ -179,6 +179,8 @@ class KeyframeEffect : public AnimationEffect
     void effectStackNoLongerPreventsAcceleration();
     void effectStackNoLongerAllowsAcceleration();
 
+    void lastStyleChangeEventStyleDidChange(const RenderStyle* previousStyle, const RenderStyle* currentStyle);
+
     static String CSSPropertyIDToIDLAttributeName(CSSPropertyID);
 
 private:

diff --git a/Source/WebCore/animation/KeyframeEffectStack.cpp b/Source/WebCore/animation/KeyframeEffectStack.cpp
@@ -238,4 +238,10 @@ void KeyframeEffectStack::stopAcceleratedAnimations()
         effect->effectStackNoLongerAllowsAcceleration();
 }
 
+void KeyframeEffectStack::lastStyleChangeEventStyleDidChange(const RenderStyle* previousStyle, const RenderStyle* currentStyle)
+{
+    for (auto& effect : m_effects)
+        effect->lastStyleChangeEventStyleDidChange(previousStyle, currentStyle);
+}
+
 } // namespace WebCore
diff --git a/Source/WebCore/animation/KeyframeEffectStack.h b/Source/WebCore/animation/KeyframeEffectStack.h
@@ -65,6 +65,8 @@ class KeyframeEffectStack {
     bool containsInvalidCSSAnimationName(const String&) const;
     void addInvalidCSSAnimationName(const String&);
 
+    void lastStyleChangeEventStyleDidChange(const RenderStyle* previousStyle, const RenderStyle* currentStyle);
+
 private:
     void ensureEffectsAreSorted();