Skip to content

[scroll-animations-1] it is not clear when to update stale timelines #12120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
graouts opened this issue Apr 25, 2025 · 0 comments
Open

[scroll-animations-1] it is not clear when to update stale timelines #12120

graouts opened this issue Apr 25, 2025 · 0 comments
Labels

Comments

@graouts
Copy link
Contributor

graouts commented Apr 25, 2025

The HTML Processing Model: Event loop section of the Scroll-driven Animations spec discusses how scroll and view timelines may become stale in the process of updating the rendering since timeline sources or subjects may change bounds during that process.

However, the spec does not reference the HTML spec with hard links and rather references sections which are by now outdated. For instance (at the time of writing this issue), the step to update animations and send events is step 11 and the current spec references step 10. As such it is not clear exactly when in the process of updating the page rendering stale timelines should be updated. Using informative notes, it appears it is after resize observers have been evaluated and styles have been recalculated and layout updated.

We should probably update the HTML spec itself to add a step that clearly references the process of updating stale timelines.

Here are related spec issues and spec change:

Cc @flackr who made the spec change as well as @kevers-google and @andruud who reviewed it.

@graouts graouts added the scroll-animations-1 Current Work label Apr 25, 2025
webkit-commit-queue pushed a commit to graouts/WebKit that referenced this issue Apr 25, 2025
…l-timeline-invalidation.html has failures

https://bugs.webkit.org/show_bug.cgi?id=284546

Reviewed by Antti Koivisto.

The Scroll-driven Animations spec [0] calls for the re-computation of styles if during a page rendering
update a scroll timeline becomes "stale", ie. if its current time changes during the update. This
will happen for instance if the bounds of the scroll timeline source changes during the update as
part of an animation, a requestAnimationFrame callback or a resize observer. There is a provision
that there can only be a single additional style update caused by this.

Because the spec references the HTML spec by section number and not hyperlinks, it is not perfectly
clear when exactly potentially-stale timelines should be considered for an update, but it seems from
informative notes that this happens _after_ resize observers have been serviced, which also implies
that the style and layout update has also happened. A spec issue was filed to clarify this.

As such, we call from `Page::updateRendering()` into `AnimationTimelinesController::updateStaleScrollTimelines()`
to iterate all scroll and view timelines updated during the immediate prior call to `updateAnimationsAndSendEvents()`
and call the new `ScrollTimeline::updateCurrentTimeIfStale()` method which will recompute the basic
data used to determine a scroll timeline's current time and, if the source bounds have changed, will
invalidate the targets of all associated animations and update styles once more.

This process is non-reentrant, ensuring we only update stale timelines once.

The test that was written to test this functionality did not strictly adhere to what the spec seems
to specify (see [1]) so we update it as well to check that a timeline's current time is only updated
after much of the page rendering steps have been performed, since it previously would check it within
a `requestAnimationFrame()` callback.

[0] https://drafts.csswg.org/scroll-animations-1/#event-loop
[1] w3c/csswg-drafts#12120

* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/scroll-timeline-invalidation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/scroll-timeline-invalidation.html:
* Source/WebCore/animation/AnimationTimelinesController.cpp:
(WebCore::AnimationTimelinesController::updateAnimationsAndSendEvents):
(WebCore::AnimationTimelinesController::updateStaleScrollTimelines):
* Source/WebCore/animation/AnimationTimelinesController.h:
* Source/WebCore/animation/ScrollTimeline.cpp:
(WebCore::ScrollTimeline::updateCurrentTimeIfStale):
* Source/WebCore/animation/ScrollTimeline.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::updateStaleScrollTimelines):
* Source/WebCore/dom/Document.h:
* Source/WebCore/page/Page.cpp:
(WebCore::Page::updateRendering):

Canonical link: https://commits.webkit.org/294121@main
Jarred-Sumner pushed a commit to oven-sh/WebKit that referenced this issue Apr 27, 2025
…l-timeline-invalidation.html has failures

https://bugs.webkit.org/show_bug.cgi?id=284546

Reviewed by Antti Koivisto.

The Scroll-driven Animations spec [0] calls for the re-computation of styles if during a page rendering
update a scroll timeline becomes "stale", ie. if its current time changes during the update. This
will happen for instance if the bounds of the scroll timeline source changes during the update as
part of an animation, a requestAnimationFrame callback or a resize observer. There is a provision
that there can only be a single additional style update caused by this.

Because the spec references the HTML spec by section number and not hyperlinks, it is not perfectly
clear when exactly potentially-stale timelines should be considered for an update, but it seems from
informative notes that this happens _after_ resize observers have been serviced, which also implies
that the style and layout update has also happened. A spec issue was filed to clarify this.

As such, we call from `Page::updateRendering()` into `AnimationTimelinesController::updateStaleScrollTimelines()`
to iterate all scroll and view timelines updated during the immediate prior call to `updateAnimationsAndSendEvents()`
and call the new `ScrollTimeline::updateCurrentTimeIfStale()` method which will recompute the basic
data used to determine a scroll timeline's current time and, if the source bounds have changed, will
invalidate the targets of all associated animations and update styles once more.

This process is non-reentrant, ensuring we only update stale timelines once.

The test that was written to test this functionality did not strictly adhere to what the spec seems
to specify (see [1]) so we update it as well to check that a timeline's current time is only updated
after much of the page rendering steps have been performed, since it previously would check it within
a `requestAnimationFrame()` callback.

[0] https://drafts.csswg.org/scroll-animations-1/#event-loop
[1] w3c/csswg-drafts#12120

* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/scroll-timeline-invalidation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/scroll-timeline-invalidation.html:
* Source/WebCore/animation/AnimationTimelinesController.cpp:
(WebCore::AnimationTimelinesController::updateAnimationsAndSendEvents):
(WebCore::AnimationTimelinesController::updateStaleScrollTimelines):
* Source/WebCore/animation/AnimationTimelinesController.h:
* Source/WebCore/animation/ScrollTimeline.cpp:
(WebCore::ScrollTimeline::updateCurrentTimeIfStale):
* Source/WebCore/animation/ScrollTimeline.h:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::updateStaleScrollTimelines):
* Source/WebCore/dom/Document.h:
* Source/WebCore/page/Page.cpp:
(WebCore::Page::updateRendering):

Canonical link: https://commits.webkit.org/294121@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant