Skip to content
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

[web-animations-1] Swap start time position when playback rate is flipped on non-monotonic timeline #8146

Merged
merged 5 commits into from
Mar 29, 2023

Conversation

flackr
Copy link
Contributor

@flackr flackr commented Nov 29, 2022

When updating the playback rate of a non-monotonically increasing timeline we want to preserve the effective start time behavior of playing the animation. If the playback rate flips directions this requires flipping the start time to the other end of the effect. See discussion on #2075.

When updating the playback rate of a non-monotonically increasing
timeline we want to preserve the effective start time behavior of
playing the animation. If the playback rate flips directions this
requires flipping the start time to the other end of the effect. See
discussion on w3c#2075.
Copy link
Contributor

@kevers-google kevers-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kevers-google
Copy link
Contributor

Still a minor edge case if updating the playback rate while paused. One possible solution here would be to preserve the current time while paused, but set a flag to update the start time once we resume playing the animation. We have something similar for setTimeline ("reset current time on resume" flag in level 2 of the spec).

Copy link
Contributor

@kevers-google kevers-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still some followup work for paused animations and animation.reverse, but OK with those changes being in a separate PR.

@flackr
Copy link
Contributor Author

flackr commented Nov 29, 2022

Still a minor edge case if updating the playback rate while paused. One possible solution here would be to preserve the current time while paused, but set a flag to update the start time once we resume playing the animation. We have something similar for setTimeline ("reset current time on resume" flag in level 2 of the spec).

It seems odd to me that we don't use hold time to hold the current time for all paused animations (I'm sure there were reasons), but this does seem to be a a different issue.

@kevers-google
Copy link
Contributor

It seems odd to me that we don't use hold time to hold the current time for all paused animations (I'm sure there were reasons), but this does seem to be a a different issue.

We do use hold time to preserve the current time for all paused animation, the issue is when we unpause we will update start time to preserve current time. If the animation is scroll-linked when we will no longer have a start time aligned at 0 or end time. Agreed, it is a different issue however.

@flackr
Copy link
Contributor Author

flackr commented Mar 10, 2023

@birtles please take a look when you have a chance to see if you think this is reasonable to address the playback rate sign change case.

@birtles
Copy link
Contributor

birtles commented Mar 11, 2023

Sorry, just trying to catch up here. We're trying to flip the start time to the other end of the effect, so if it's currently zero, we set it to end - startTime = end - 0 = end. If it's currently, at the end we set it to end - startTime = end - end = 0.

That seems good. It might be worth adding an informative note to the spec explaining why we do this?

Also, can end ever be infinity in this case? e.g. for an infinitely repeating animation?

@kevers-google
Copy link
Contributor

For an infinity repeated animation on a scroll-timeline, the intrinsic iteration duration collapses to 0, and the active duration becomes zero (https://www.w3.org/TR/web-animations-1/#calculating-the-active-duration).

@flackr
Copy link
Contributor Author

flackr commented Mar 17, 2023

Sorry, just trying to catch up here. We're trying to flip the start time to the other end of the effect, so if it's currently zero, we set it to end - startTime = end - 0 = end. If it's currently, at the end we set it to end - startTime = end - end = 0.

Yes, exactly.

That seems good. It might be worth adding an informative note to the spec explaining why we do this?

Done.

Also, can end ever be infinity in this case? e.g. for an infinitely repeating animation?

This is a good question. Normally it would throw an error when you try to play the animation (see 4.4.8 Playing an animation Step 4) however I think it might be possible if you update the animation timing after creation. I've avoided using the infinity in this case in the latest patch.

@birtles
Copy link
Contributor

birtles commented Mar 24, 2023

@flackr Can we merge this now?

@flackr flackr merged commit e06c3a7 into w3c:main Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants