[webkit-changes] [WebKit/WebKit] d80503: [scroll-animations] setting the effect of a scroll...

[Antoine Quint]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: d80503b4c350cf810ffa9dae94f4ac4be54af973
      https://github.com/WebKit/WebKit/commit/d80503b4c350cf810ffa9dae94f4ac4be54af973
  Author: Antoine Quint <graouts at webkit.org>
  Date:   2025-01-24 (Fri, 24 Jan 2025)

  Changed paths:
    A LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/setting-effect-expected.txt
    A LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/setting-effect.html
    M Source/WebCore/animation/AnimationEffect.cpp
    M Source/WebCore/animation/KeyframeEffect.cpp
    M Source/WebCore/animation/KeyframeEffect.h
    M Source/WebCore/animation/WebAnimation.cpp

  Log Message:
  -----------
  [scroll-animations] setting the effect of a scroll-driven animation fails to recompute effect timing
https://bugs.webkit.org/show_bug.cgi?id=286471
rdar://143521617

Reviewed by Anne van Kesteren.

The backing timing object of an `AnimationEffect` needs to be recomputed when certain properties of
the effect are changed. We failed to do so when the effect is associated to a new animation, instead
opting in `AnimationEffect::setAnimation()` to recompute the associated animation's relevance, which
would trigger an assertion when trying to compare timing values from the animation and the effect
which could be of conflicting types (time-based vs. percentage-based).

We now correctly set the `m_timingDidMutate` flag in `AnimationEffect::setAnimation()` and let the
relevance update be re-computed after the animation-to-effect relationships were updated in
`WebAnimation::setEffectInternal()`.

Likewise, for keyframe effects, we failed to re-compute the flag that indicates whether that effect
is associated with a scroll-driven animation when the effect was associated to a new animation. So
we factor the code setting that flag out of the `KeyframeEffect::animationTimelineDidChange()`
method into a new `KeyframeEffect::updateIsAssociatedWithProgressBasedTimeline()` method which we now
call from both `KeyframeEffect::animationTimelineDidChange()` and `KeyframeEffect::setAnimation()`.

We also introduce a new test which checks the times are recomputed correctly when a new effect is
associated with a scroll-driven animation. That test would hit assertions prior to this patch.

* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/setting-effect-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/setting-effect.html: Added.
* Source/WebCore/animation/AnimationEffect.cpp:
(WebCore::AnimationEffect::setAnimation):
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::updateIsAssociatedWithProgressBasedTimeline):
(WebCore::KeyframeEffect::animationTimelineDidChange):
(WebCore::KeyframeEffect::setAnimation):
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::setEffectInternal):

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



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