1289915 - Allow scroll-driven animations to run without being paused

Status: RESOLVED DUPLICATE of bug 1321428

(Core :: DOM: Animation, defect, P3)

Description

[Botond Ballo [:botond]]

In the initial scroll-driven animations prototype by Mantaroh, the animation had to be paused to be driven by scrolling, because if it was playing it would also be driven by time.

I have some patches that fix this. They are a requirement for prototyping OMTA support for scroll-driven animations (bug 1289909), because the OMTA codepath will not pick up an animation that's paused.

The patches are a bit rough around the edges; they currently cause a crash during shutdown, and they cause the refresh driver to tick while a scroll-driven animation is in the Playing state even if the page isn't currently being scrolled. However, they accomplish the main goal of the scroll position being propagated through the scroll timeline's current time.

With these patches, the demo page here [1] (which is a modified version of Mantaroh's original demo [2]) can be scrolled to drive the animation of a transform.

The last patch explicitly disables OMTA for scroll-driven animations until that's implemented properly; otherwise, the current OMTA code would cause it to be driven by time on the compositor thread.

[1] https://people.mozilla.org/~bballo/scroll-driven-animations/test.html
[2] https://people.mozilla.org/~mantaroh/scroll-animation/test.html

Comment 1

[Botond Ballo [:botond]]

Attachment: Initial prototype by Mantaroh

Review commit: https://reviewboard.mozilla.org/r/67492/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67492/

Comment 2

[Botond Ballo [:botond]]

Attachment: Support vertical scrolling

Review commit: https://reviewboard.mozilla.org/r/67494/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67494/

Comment 3

[Botond Ballo [:botond]]

Attachment: Add Max() and Min() methods to TimeDuration

Review commit: https://reviewboard.mozilla.org/r/67496/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67496/

Comment 4

[Botond Ballo [:botond]]

Attachment: Derive a scroll timeline's current time from the scroll progress

Review commit: https://reviewboard.mozilla.org/r/67498/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67498/

Comment 5

[Botond Ballo [:botond]]

Attachment: Have ScrollObserverImpl notify the ScrollTimeline when the scroll values along the relevant axis change

Review commit: https://reviewboard.mozilla.org/r/67500/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67500/

Comment 6

[Botond Ballo [:botond]]

Attachment: Determine a scroll timeline's 'timeline time' based on scroll position

Review commit: https://reviewboard.mozilla.org/r/67502/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67502/

Comment 7

[Botond Ballo [:botond]]

Attachment: Access the scroll values directly in ScrollTimeline

Review commit: https://reviewboard.mozilla.org/r/67504/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67504/

Comment 8

[Botond Ballo [:botond]]

Attachment: Initialize the scroll values in the ScrollTimeline constructor

Review commit: https://reviewboard.mozilla.org/r/67506/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67506/

Comment 9

[Botond Ballo [:botond]]

Attachment: Disable OMTA for animations with a ScrollTimeline until it's implemented properly

Review commit: https://reviewboard.mozilla.org/r/67508/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67508/

Comment 10

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #0)
> they currently cause a crash during shutdown

More precisely, a crash when the current document is unloaded.

The reason for this is that no one calls ScrollTimeline::NotifyRefreshDriverDestroying(). I'm not exactly sure who's supposed to be calling it; Mantaroh, perhaps you might have an idea.

Comment 11

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #0)
> they cause the refresh driver to tick while a
> scroll-driven animation is in the Playing state even if the page isn't
> currently being scrolled.

This could be avoided if, instead of registering itself as a refresh observer, the scroll timeline just called Animation::Tick() from the scroll observer. That would also solve the above problem of needing to call NotifyRefreshDriverDestroying().

I also discovered that there is an existing interface, nsIScrollPositionListener, which could be reused instead of ScrollHandler/ScrollObserver.

Comment 12

[Botond Ballo [:botond]]

Attachment: Don't observe refreshes

Review commit: https://reviewboard.mozilla.org/r/68100/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68100/

Comment 13

[Botond Ballo [:botond]]

Attachment: Use nsIScrollPositionListener

Review commit: https://reviewboard.mozilla.org/r/68102/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68102/

Comment 14

[Botond Ballo [:botond]]

Comment on attachment 8775323 [details]
Initial prototype by Mantaroh

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67492/diff/1-2/

Comment 15

[Botond Ballo [:botond]]

Comment on attachment 8775324 [details]
Support vertical scrolling

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67494/diff/1-2/

Comment 16

[Botond Ballo [:botond]]

Comment on attachment 8775325 [details]
Add Max() and Min() methods to TimeDuration

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67496/diff/1-2/

Comment 17

[Botond Ballo [:botond]]

Comment on attachment 8775326 [details]
Derive a scroll timeline's current time from the scroll progress

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67498/diff/1-2/

Comment 18

[Botond Ballo [:botond]]

Comment on attachment 8775331 [details]
Disable OMTA for animations with a ScrollTimeline until it's implemented properly

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67508/diff/1-2/

Comment 19

[Botond Ballo [:botond]]

Implemented the suggestions described in comment 11. Now it no longer crashes on shutdown, and it doesn't tick the refresh driver unnecessarily.

Comment 20

[Botond Ballo [:botond]]

The changes in these patches will land as part of bug 1321428.