1817051 - Animations with a scroll progress timeline should be sampled once during an event loop

Status: ASSIGNED

(Core :: CSS Transitions and Animations, enhancement)

Description

[Boris Chiou [:boris]]

Per spec:

To avoid such layout cycles, animations with a scroll progress timeline are sampled once per frame, after scrolling in response to input events has taken place, but before requestAnimationFrame() callbacks are run. If the sampling of such an animation causes a change to a scroll offset, the animation will not be re-sampled to reflect the new offset until the next frame.

Looks like we may sample the animation multiple times. The related wpt:

1. https://searchfox.org/mozilla-central/source/testing/web-platform/tests/scroll-animations/css/scroll-timeline-sampling.html
2. https://searchfox.org/mozilla-central/source/testing/web-platform/tests/scroll-animations/css/scroll-timeline-multi-pass.tentative.html

Comment 1

[Hiroyuki Ikezoe (:hiro)]

(In reply to Boris Chiou [:boris] from comment #0)

Looks like we may sample the animation multiple times. The related wpt:
https://searchfox.org/mozilla-central/source/testing/web-platform/tests/scroll-animations/css/scroll-timeline-sampling.html

This test doesn't precisely test what the spec defined. It tests an animated computed style value after requestAnimationFrame() call backs. And to be precise, the test shouldn't use getComputedStyle(), it shouldn't invoke any forcibly flushing style things. And with those, I don't think we will pass the test.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Note for Boris.

(In reply to Boris Chiou [:boris] from comment #0)

Per spec:
after scrolling in response to input events has taken place,

This part is done here, i.e., reflecting APZ scroll offsets is done as an early runner of our refresh driver.

but before requestAnimationFrame() callbacks are run.

And this part is done here as you know.

Comment 3

[Boris Chiou [:boris]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)

Note for Boris.

(In reply to Boris Chiou [:boris] from comment #0)

Per spec:
after scrolling in response to input events has taken place,

This part is done here, i.e., reflecting APZ scroll offsets is done as an early runner of our refresh driver.

but before requestAnimationFrame() callbacks are run.

And this part is done here as you know.

Great. This makes us easier to sample the current time of scroll timelines in step 11.

Comment 4

[Boris Chiou [:boris]]

Attachment: Bug 1817051 - Part 1: Add AnimationTimelinesController.

We wrap the list of timelines attached to the document in a controller.
We will add the implementation of scroll timelines in the following
patches.

Comment 5

[Boris Chiou [:boris]]

Attachment: Bug 1817051 - Part 2: Add scroll timeline into AnimationTimelinesController.

In order to sample scroll timelines together with document timelines, we
add scroll timelines into AnimationTimelinesController as well.

We will sampling for scroll timelines in the following patches.

Comment 6

[Boris Chiou [:boris]]

Attachment: Bug 1817051 - Part 4: Update stale timelines.

Per spec, we need an extra steps in HTML event loop to update the stale
timelines after we update the animation frame callbacks and resize
observer callbacks.

Comment 7

[Boris Chiou [:boris]]

Attachment: Bug 1817051 - Part 4: Start to sample scroll timeline in the HTML loop.

Also, we drop ProgressTimelineScheduler since we don't need it anymore.

The update in the test is necessary (just like what we do for wpt) for
make sure getComputedStyle() works after we sample the scroll timelines,
because the updating of stale timelines has no effect on forced style and
layout calculations triggered by getComputedStyle() or similar.
In other words, initially stale timelines are visible as such through those
APIs.

Note: there are two new test failures because the timing of caching the
current time is later than the moment we trigger the animation and
scrollTo API. I will try to fix them in the following patch. In this
patch we focus on switching the sampling way and to make sure we don't
cause lots of regressions.

Comment 8

[Boris Chiou [:boris]]

Attachment: Bug 1817051 - Part 5: Fix the cases when the element is in a content-visibility:auto subtree.

We trigger the pending scroll-driven animations after we flush the
layout, and so we need to do an extra sampling of the scroll timelines
to make sure thoses animations can get the correct current time
(otherwise we cannot finish it pending state). Therefore, We have to sample
the scroll timelines when the pending animation tracker is trying to
tigger the animations.

Also, there is a potential timing issue when we call scrollTo APIs and
create the animations in the meantime, in a content-visibility:auto
subtree. The newly-created animation should use the up-to-date current
time for styling before it becomes not relevant to the user (see the
test case in this patch for more details), so it's unfortunate we have
to try to sample the scroll timelines just right after we finish the
scrollTo APIs to make sure the computed style of the element is correct.

Besides, we have to update the OMTA test in file_animations_omta_scroll.html
beacause we may clear the OMTA style in the HTML event loop (i.e. when
advancing the timing). This may be slightly better than our previous
version because using the base style for delay phase to override the
out-of-date OMTA style is because the main thread doesn't update the
OMTA in time.

Comment 9

[Boris Chiou [:boris]]

Land part 1 - part 3 first because they may fix Bug 1878215 as well.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/85c927d79ed9
https://hg.mozilla.org/integration/autoland/rev/9745dd376697
Part 1: Add AnimationTimelinesController. r=layout-reviewers,hiro,dom-core,smaug
https://github.com/mozilla-firefox/firefox/commit/4bdbdb5d7622
https://hg.mozilla.org/integration/autoland/rev/dc480315935b
Part 2: Add scroll timeline into AnimationTimelinesController. r=hiro,layout-reviewers
https://github.com/mozilla-firefox/firefox/commit/42e172e27aea
https://hg.mozilla.org/integration/autoland/rev/1cbc83b410ab
Part 3: Start to sample scroll timeline in the HTML loop. r=firefox-style-system-reviewers,layout-reviewers,hiro,emilio

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d0cad31feab6
https://hg.mozilla.org/integration/autoland/rev/5da9551e15f3
Revert "Bug 1817051 - Part 3: Start to sample scroll timeline in the HTML loop. r=firefox-style-system-reviewers,layout-reviewers,hiro,emilio" on request for causing timeout

Comment 12

[Cristina Horotan [:chorotan]]

Backed out on request by causing a timeout

Backout link

Push with failures

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Also non-unified build bustage:
https://treeherder.mozilla.org/logviewer?job_id=539896566&repo=autoland&task=dqeYHgc-R_2h1xxrxPYi8Q.14&lineNumber=109305