[web-animations-2] Add steps for transitioning to/from a scroll timeline.

[kevers-google]

#5422

Adds the following steps:

• When transitioning to a scroll timeline, start time is updated to synchronize current time with the scroll position.
• When transitioning from a scroll timeline to a document timeline, current time is preserved.

[kevers-google]

See also web-platform-tests/wpt#24832

[birtles]

I think this seems right but I'd like it if @ogerchikov could review.

[birtles]

This seems to be missing an "If" ?

[kevers-google]

Done.

[birtles]

I think we typically prefix these conditions with "If"

[kevers-google]

Done.

[ogerchikov]

I assume it's 'previous play state'?

[kevers-google]

Correct. Fixing here and elsewhere.

[ogerchikov]

Here we are changing current time of paused animation if the new timeline is active and leaving unchanged otherwise.
I think it's unexpected for the paused animation to change current time.
I suggest leaving the old hold_time unchanged when setting new timeline to paused animation.

[kevers-google]

If we don't preserve current time, we cause a jump in the progress of the animation when transition from a scroll timeline to a document timeline. Here we are setting current time to preserve the current progress in the animation.

[kevers-google]

Just to followup on my last comment. We are preserving current time if we can, and leaving it along otherwise. Transitioning between scroll timelines is handled in the previous branch. AFAIK, the only way for the document timeline to be inactive is if the animation clock has not ticked yet. If the new timeline is null, it will simply preserve the previous scroll timeline's value of hold time.

Handling in this manner, no distinction needs to be made regarding whether the animation was running at the time of the timeline change.

[ogerchikov]

start_time value will be resolved here if the animation is not paused. In this case hold_time set here will be cleared in the following statement (l. 879)

[kevers-google]

Oops, this should ahve read [=current time=] and not [=hold time=]. Making the correction.

[ogerchikov]

I suggest adding a variable 'has finite timeline' to improve readability.

[kevers-google]

I thought about hat but 'had finite timeline' and 'has finite timeline' look too similar and could be confused at a glance. Perhaps "from finite timeline' and 'to finite timeline'.

[ogerchikov]

I am wondering if it makes more sense to look at previous start/hold time instead of play states to determine what should be new start/hold time. The reason is that applying this logic on play/pause pending animations may introduce new states (see this state chart)

[kevers-google]

Switching the logic to rely on start and hold time, would also require factoring in the pending state. Once pending state is introduced into the equation we essentially end up replicating the functionality of calculating the play state. The new WPT tests associated with the Blink change test changing the timeline on both pending and non-pending animations.

[kevers-google]

Taking a step back to cover the options at a higher level, which should hopefully help us to reach consensus on which path to adopt. Starting with some high level design considerations:

• Try to avoid unexpected jumps in the animation.
• Keep a running scroll-linked animation in sync with the scroll position.
• Ideally, limit the amount of special case logic.
• Seamlessly handle pending play state changes.
• Equivalent behavior between the following use cases:

const animation = new Animation(..., scrollTimeline);
animation.play();

and

const animation = element.animate();
animation.timeline = scrollTimeline;

Option1:

• Store the previous value for current time before changing the timeline.
• Unconditionally, set current time to previous current time after changing the timeline.
• pros:
  • Prevent jumps when switching timeilnes.
• cons:
  • Potentially Introduces a phase-shift when switching to a scroll timeline. In other words, the value for current time may be out of sync with the scroll position.
  • Breaks one of the WPT tests, for transitioning between two document timelines with differing originTimes.

I believe the phase shift is unexpected behavior especially when using scroll timelines via CSS rules and the animation-timeline property. Nonetheless, including it here for discussion.

Option 2:

• Special handling for transitioning to/from a scroll timeline.
• Set start time to beginning or end depending on the sign of the playback rate.
• pros:
  • No phase shift. Current time is in sync with the scroll position.
    *cons:
  • Introduces special casing, which makes the spec harder to conceptualize.
  • How do we handle paused or pause-pending animations? The tricky edge case here is when the new scroll timeline is inactive. Preserving current time might be our best option here, but we'll then need to handle synchronization once the animation becomes unpaused.

Option 3:

• Special handling for transitions to/from a scroll timeline.
• Set the start time when transitioning to a running/finished scroll timeline.
• Otherwise set current time to previous current time like in option 1.
• Update the play procedure to unconditionally update the start time if using a scroll timeline. Currently, only done if current time is unresolved.
• pros:
  • Avoids unnecessary jumps when an animation is paused.
  • Believe this aligns with our expected behavior for the animation-timeline property.
  • This also addresses the problem of pause + scroll + unpause. Currently, this causes a phase shift. Naturally, this conclusion is based on an assumption that we don't want to allow a phase shift.
• cons:
  • Now tweaking two procedures instead of one.

There may be other options to consider and would welcome any feedback. Option 3 (my new preference) is not what is currently implemented in the pull request, but would like some feedback before proceeding with the next update.

[birtles]

Thanks for writing this up. Sorry for my ignorance, but what is the "phase shift" we are concerned about here?

Also, would it be correct that option 3 also introduces special casing for scroll timelines? Or is it a different kind of special casing to that of option 2?

[kevers-google]

Using the term "phase shift" to indicate the progress getting out of sync with the scroll position, being either ahead or behind the scroll position by a fixed amount if the start time is not adjusted.

Yes both option 2 and 3 have special casing for scroll timelines, which I should have listed under cons for option 3. In fact, for option 3, it's in 2 methods (setTimeline and play). The main difference between options 2 and 3 in terms of observable behavior is in the handling of pause/unpause. In option 2, we adjust current time to align with the scroll position when changing the timeline for either a playing or paused animation. With option 3, setting the timeline of a paused animation does not trigger a jump to sync with the scroll position. Instead, it is deferred until we resume playing the animation. A potential gotcha with option 2 is when setting the timeline of a paused animation when the new scroll timeline is inactive. In this case, we don't know what the hold time should be set to in order to align with the scroll position once the timeline becomes active once again. Deferring the decision until play resolves the issue, since even if the timeline remains inactive we know whether start should be set to 0 or effect end. This awkward edge case with option 2 is the main reason for my shift towards option 3 as a preference.

[ogerchikov]

Thanks @kevers-google for listing the options.

I believe the main motivation for making this change right now is supporting scroll timeline via CSS. Do you know for which CSS scenarios updating timeline is required and what is broken if we just follow existing procedure? Perhaps we should concentrate on those since when setting timeline via Javascript user can set desired start/current time.

[kevers-google]

One of the great things with the web animations API is that, for the most part, animations constructed via CSS behave consistently with animations created via JavaScript. Sure there is more control from JavaScript, but there is a common baseline. In fact, we can freely mix the two approaches by applying CSS style changes from JavaScript, or by fetching CSS animations via getAnimations. The primary goal with the proposed algorithm is to maintain a consistent and expected behavior between CSS and JavaScript animations.

By "existing procedure" I assume that you mean the one that is currently published in web-animations-1 and not the one in this pull request.

case 1: Transitioning from a document timeline to a scroll timeline

@Keyframes fade-anim { ... }
@scroll-timeline timeline { ... }
.fade { animation: fade-anim ... }
.scrolling-animation { animation-timeline: timeline }

In this example, if an animation is created by adding the fade class and then later the animation is attached to a scrollbar via the scrolling-animation class we can wind up with some weird inconsistencies.

case 1a: animation is play-pending when scroll timeline is added.
The initial play procedure sets the hold time to zero; however, a scroll linked animation initializes start time and not hold time in the play procedure. This leaves us in an inconsistent state unless we rethink the play procedure to avoid special casing for scroll timelines.

case 1b: animation is pause-pending when the scroll timeline is added
The pause procedure initializes the hold time to zero; however, the same procedure initializes start time if run with a scroll timeline. Again this leaves us in an inconsistent state.

case 1c: animation is running when scroll timeline is added.
The animation jumps to an unexpected position when the scroll timeline is set.

case 1d: animation is paused when scroll timeline is added.
No jump when the timeline is set; however, once the animation resumes, it continues from the current position which is likely to be out of sync with the scroll position. It is unrealistic to expect that the scroll position will be at zero when the timelne is added.

case 1e: animation is idle when the scroll timeline is added.
This case works as expected.

The question may arise, "well why can't the developer simply avoid these cases by setting the timeline in the same frame where the animation starts?" One reason might be that the scroll-timeline attachment is deferred until content is loaded to avoid jank. The developer could inadvertently change the flow by querying style during the process of attaching the timeline (e.g. querying to determine the scroll range). From the implementation side, it should not matter if the timeline is passed into the animation constructor or immediately before/after play/pause.

case 2: CSS theme change triggering a change from one CSS scroll timeline to another.

Since the two timelines can have different values for the start and end properties, the scroll position can translate to different values of current time. The existing procedure does not accommodate this, and current time may become out of sync with the scroll position.

[ogerchikov]

case 1: Transitioning from a document timeline to a scroll timeline

I believe it is the most common case and, I assume, the desired behavior is:

• Animation doesn't animate on document timeline before scroll timeline is attached.
• Once the animation is attached to scroll timeline and running, animation progress is synced with scroll position.

In this case initial CSS is:

@Keyframes fade-anim { ... }
.fade { animation: fade-anim, paused; }

element.classList.add("fade ");// play_state = 'paused', hold_time = 0

Attaching the animation to scroll timeline:

@scroll-timeline timeline { ... }
.scrolling-animation { animation-timeline: timeline; animation-play-state: running; }
  OR
.scrolling-animation { animation-play-state: running; animation-timeline: timeline;  }

element.classList.add("scrolling-animation");

For both declarations of .scrolling-animation class we want to achieve identical behavior.

• Setting scroll timeline first, unpausing animation second:

anim.timeline = scroll_timeline; // hold_time remains 0
anim.play(); // set start_time to 0 or end_effect => animation progress is in sync with scroll 

• Unpausing first, setting scroll timeline second

anim.play(); // hold_time = 0, play pending state
anim.timeline = scroll_timeline;  // animation is running => set start_time to 0 or end_effect  => animation progress is in sync with scroll 

Note, to achieve the above we need to change how unpause currently works for scroll animations to always keep it in sync with the scroller. Keeping current time on unpause can be controlled by setting current/start time on running animation via Javascript.

To summarize, Option 4, additional to described her, is:

• Modify unpause procedure to resume from position synchronized with a scroller.
• Add additional step after 4 to handle transition to scroll timeline as follows:
  • If animation is in 'running' state set start time to 0 or end effect.

[kevers-google]

Your option 4 is essentially a subset of what I was proposing with option 3 a few comments ago:

• Play (unpause) sets the start time to 0 or effect end, which has the effect of resuming from the scroll position.
• setting the timeline sets the start time of a running or finished animation to 0 or effect end depending in the sign of the playback rate if using a scroll timeline.

The extra step in option 3 is to preserve current time if transitioning from a scroll timeline to a document timeline. Otherwise resetting the animation-timeline could have unexpected results.

If we are converging on consensus for this approach I can quickly update the pull request and Blink CL.

[kevers-google]

Options 3 and 4 break the following tests:

scroll-animations/play-animation.html

• Resuming an animation from paused calculates start time

scroll-animations/reverse-animation.html

• Reversing an animation maintains the same current time

scroll-animations/updating-the-finished-state.html

• Updating the finished state when there is a pending task

Each of the tests allow for the scroll position to get out of sync with current time for a scroll animation.

[kevers-google]

Option 0: No special handling for scroll timelines (i.e. abandon this pull request).
https://chromium-review.googlesource.com/c/chromium/src/+/2324101

With this change setting the timeline on a play-pending animation gives different results than setting the animation prior to playing the animation. Will see what this breaks for animation-timeline. We can likely resolve some of the issues with pending animation by ensuring correct ordering of calls when the CSS animation in created.

[andruud]

(I'll add my thoughts to this, not sure if they'll help):

At a higher level, when considering how @scroll-timeline should work when timelines are switched around, we should keep the following guideline from css-animations-1 in mind:

Changes to the values of animation properties while the animation is running apply as if the animation had those values from when it began.

This makes a lot of sense given how CSS works in general. We should probably try to not deviate from this without a good reason. (Some things deviate already, e.g. animation-play-state).

This means that the behavior we should aim for "by default" is:

• Switching from DocumentTimeline/none -> ScrollTimeline: the result is whatever it would have been if the ScrollTimeline had been there from the start, i.e. the animation state is synchronized to the scroll position.
• Switching from ScrollTimeline/none -> DocumentTimeline: the result is whatever it would have been if the DocumentTimeline had been there from the start, i.e. if you switch to the DocumentTimeline after five seconds, you get the same result as if DocumentTimeline was there initially, and you had waited five seconds. This may or may not be practical, but it's the behavior that follows from css-animations-1.

Pausing makes it more complicated. @ogerchikov seems to say that changing the current time of a paused animation is weird, and that we shouldn't do that, but instead change how the unpause procedure works. From the author's perspective, this means the timeline switch would not "take effect" until the animation is unpaused. I think that's fine ... paused animations are paused regardless of timeline. Makes sense to me.

@kevers-google asked if I could test "Option 0" with some practical examples and report what doesn't work. I think the main problem is the start time is preserved when switching timelines, which doesn't work at all for ScrollTimelines:

• Start with a DocumentTimeline that's initially running.
• When the pending-play commits (sorry for possibly Blink-specific language), the animation gets some start time greater than zero.
• Switch to a ScrollTimeline.
• Result: There is no effect value on scroll position zero, because that corresponds to a time that's less than the startTime.

Here's a similar issue, (that probably has the same root cause):

• Start with a DocumentTimeline that's initially paused.
• Switch to a ScrollTimeline, with a source that's at max scroll.
• Unpause.
• Result: There is no effect value for any scroll position except max scroll.

[kevers-google]

Updated the Blink CL: https://chromium-review.googlesource.com/c/chromium/src/+/2324101 with the following rules:

• when setting a scroll timeline

  • if play state was playing or finished
    • set start time to 0 or effect end
  • if play state was paused
    • set current time to previous current time (this is to avoid a jump with a pause-pending animation with a start time
      and a pending pause microtask).
    • set a flag to reset current time on resuming the animation.

• when transition from a scroll timeline to a document timeline

  • set current time to previous current time

• when playing an animation

  • if the flag to reset current time on resume is set
    • set current time to unresolved
    • set clear flag to false.

This implementation is very close to option 4 outlined above and should address the concerns with animation jumps during pause. No change in existing WPTs. I could be convinced either way on preserving current time when transitioning from a scroll timeline to a document timeline; however, I believe having this rule would better align with user expectations.

@ogerchikov please take a look at the CL (particularly setting-timeline.tentative.html) and let me know what you think.

[birtles]

(Just a drive-by comment that the reasoning and options you've generated sound great. Thank you so much @kevers-google @andruud and @ogerchikov!)

[ogerchikov]

Looks good assuming reset current time on resume flag is reset at the right places, as per the CL.

[kevers-google]

The revised plan is to pull this into web-animations-2 rather than web-animations-1 as there are no backwards compatibility concerns. Will put up a separate pull request shortly with a link back to this one to capture the discussion, and with a few more tweaks to address Olga's comments regarding where we need to reset the resume flag.

[kevers-google]

Moved the change to web-animations-2. Sorry for the delay.

Now includes changes for resetting the "reset current time on resume" flag as required. Previous points regarding changes required for the play procedure were marked as issues to avoid them getting lost with the change or obscuring readability of the procedure. Once this lands, we should be in a good state for integrating changes for hold phase.

[ogerchikov]

Looks good. Thanks for updating the spec!

[flackr]

Overall looks good to me, though we may want to use a better word than finite for non-monotonic timelines.

[flackr]

finite implies that we have a maximum value and range, but the key disinction seems to be that it's not monotonic. Maybe we should say non-monotonic or unordered or flip the flag and call it from monotonic timeline?

[kevers-google]

Consistent with naming convention in play and pause. If we change "from/to finite timeline" here, should we not also change the "finite timeline" flag in play and pause? Not opposed to the name change, but wondering if we should handle that separately.

[flackr]

Ah this is preexisting terminology. I propose we merge as is and change this separately.