[animation-trigger] Implement play behavior for compositor triggers [chromium/src : main]
David A (Gerrit)
David A added 1 comment![]( "Open in Gerrit")
time_offset = ComputeCompositorTimeOffset(start_reason);David APer our offline discussion, we will likely deprecate `time_offset`. However, I figured this patch, now no longer implementing keyframe model reversal, is still worth landing as it has little to no interaction with `time_offset`. Still it's probably worth explaining:
There are two ways a blink animation will be prompted to create a cc Animation: 1. "automatically", to fulfill a `play()` request & 2. preemptively, due to an attached trigger request.
1 & 2 diverge here if you don't have a start time. For 1., we'll use `time_offset` as before.
For 2. we'll capture the "current time" using the `KeyframeModel::pause_time_` as `Animation::StartTriggeredAnimationOnCompositorThread` will always call `cc::Animation::PauseKeyframeModels`Note that `start_delay` will still be accounted for in [CompositorAnimation::ConvertTimingForCompositor](https://source.chromium.org/chromium/chromium/src/+/88605c70f0257bdde3b373926350940dac6282d8:third_party/blink/renderer/core/animation/compositor_animations.cc;l=757-782)
Acknowledged
Related details
- Kevin Ellis
- Robert Flack
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Robert Flack (Gerrit)
Robert Flack added 2 comments![]( "Open in Gerrit")
Kevin can you do a first pass on this?
// should KeyframeModel::set_start_time.This is linking to a fixed bug. If this is resolved i would expect not to have a todo, if there is further work to be done i'd expect an open bug.
Why should it take an optional? So that we can set an unresolved start time? Is this something that we need to do?
Related details
- David A
- Kevin Ellis
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David A (Gerrit)
David A added 1 comment![]( "Open in Gerrit")
This is linking to a fixed bug. If this is resolved i would expect not to have a todo, if there is further work to be done i'd expect an open bug.
Why should it take an optional? So that we can set an unresolved start time? Is this something that we need to do?
Is this something that we need to do?
hm I don't think so actually. It was just for symmetry with `hold_time`.
I've removed the comment.
Related details
- Kevin Ellis
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kevin Ellis (Gerrit)
Kevin Ellis added 4 comments![]( "Open in Gerrit")
base::TimeDelta hold_time = ComputeCompositorHoldTime().value();Feels odd to have to mix used of hold_time_ and hold_time. If the hold time on the compositor can differ from hold_time_ then perhaps we should rename hold_time to cc_hold_time. Can you give an example of how these can get out of sync?
V8AnimationPlayState::Enum play_state,This parameter appears to be unused.
"--enable-features=CompositorTimelineTrigger"Don't we also need to enable threaded compositing?
background-color: blue;Why the color change? In tests, red is often used to suggest something has gone wrong.
Related details
- David A