-
Notifications
You must be signed in to change notification settings - Fork 716
[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
Labels
scroll-animations-1
Current Work
Comments
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
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.
The text was updated successfully, but these errors were encountered: