[webkit-changes] [WebKit/WebKit] e81cfc: [web-animations] update effect stack membership wh...

[Antoine Quint]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: e81cfc86cc3bc48022f3e3c436c47447ade7a2df
      https://github.com/WebKit/WebKit/commit/e81cfc86cc3bc48022f3e3c436c47447ade7a2df
  Author: Antoine Quint <graouts at webkit.org>
  Date:   2025-02-06 (Thu, 06 Feb 2025)

  Changed paths:
    M Source/WebCore/animation/AnimationTimeline.cpp
    M Source/WebCore/animation/KeyframeEffect.cpp
    M Source/WebCore/animation/KeyframeEffect.h
    M Source/WebCore/animation/KeyframeEffectStack.cpp
    M Source/WebCore/animation/WebAnimation.cpp
    M Source/WebCore/animation/WebAnimation.h
    M Source/WebCore/style/Styleable.cpp

  Log Message:
  -----------
  [web-animations] update effect stack membership when animation relevance changes
https://bugs.webkit.org/show_bug.cgi?id=287087

Reviewed by Tim Nguyen.

We used to update an effect's membership in the effect stack in various places. But since
effect stack membership is only determined by the associated animation's relevance, we should
tie membership updates to animation relevancy changes. Since we already have a method called
when animation relevancy changes, `KeyframeEffect::animationRelevancyDidChange()`, we centralize
effect membership updates there and remove all other calls to `KeyframeEffectStack::removeEffect()`
and `KeyframeEffectStack::addEffect()` besides those in `KeyframeEffect::didChangeTargetStyleable()`
used to transition an effect from one effect stack to another.

This refactoring should have no observable effect but it did break the test
webanimations/accelerated-translate-animation-additional-animation-added-in-flight.html. Indeed,
this test checks that an animation target's layer remains composited as that animation's `finished`
promise is resolved upon completion, where other tests check that the layer is no longer composited
in the next rendering frame.

To make this test pass still, we needed to make a couple of additional changes. The method used
to determine whether a running animation should trigger a compositing layer in this case is
`RenderLayerCompositor::requiresCompositingForAnimation()`. That method calls into
`Styleable::hasRunningAcceleratedAnimations()`, which queries the associated effect stack for
effects that are running accelerated. However, the other changes in this patch now more reliably
update the effect stack as soon as animation relevancy changes, and in that test's case as soon
as we determine the animation has completed, which is to say when we'll resolve the `finished`
promise, we remove the animation effect from the associated effect stack and the method
`Styleable::hasRunningAcceleratedAnimations()` will return `false` immediately.

So we now add another check in `Styleable::hasRunningAcceleratedAnimations()` where we query
all known animations in case we didn't find an accelerated effect in the effect stack.

Another required changes was to add a check for whether an animation was associated with an effect
in `KeyframeEffect::canBeAccelerated()`, which ought to always have been there from the start.

This refactoring effort will help us fix a number of `timeline-scope` going forwards where we
dynamically update timelines.

* Source/WebCore/animation/AnimationTimeline.cpp:
(WebCore::AnimationTimeline::removeAnimation):
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::animationTimelineDidChange):
(WebCore::KeyframeEffect::animationRelevancyDidChange):
(WebCore::KeyframeEffect::canBeAccelerated const):
(WebCore::KeyframeEffect::wasRemovedFromEffectStack):
(WebCore::KeyframeEffect::animationTimingDidChange): We no longer need this method since its sole
purpose was to call `updateEffectStackMembership()`, but we now do so under `animationRelevancyDidChange()`
which will be called under `WebAnimation::timingDidChange()` through `updateFinishedState()`.
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/animation/KeyframeEffectStack.cpp:
(WebCore::KeyframeEffectStack::addEffect):
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::setEffectInternal):
(WebCore::WebAnimation::timingDidChange):
(WebCore::WebAnimation::persist):
(WebCore::WebAnimation::setReplaceState):
* Source/WebCore/animation/WebAnimation.h:
(WebCore::WebAnimation::setReplaceState): Deleted.
* Source/WebCore/style/Styleable.cpp:
(WebCore::Styleable::hasRunningAcceleratedAnimations const):

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



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications