Repository navigation
fix(core): keep rebound root timelines clock-driven - #5165
Conversation
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at cb71a652. Approving.
Why it's right. Everywhere else, the runtime keeps the root paused and lets the transport drive it:
transport.play()callspauseTimelineIfPossible(tl)beforeclock.play()(init.ts:3854).- The first bind pauses after its
totalTimerestore (init.ts:1960).
rebindTimelineFromResolution was the one place that broke that rule by calling play() on the replacement. Deleting it makes the rebind match the bind and play paths. Nothing new is added, so on reuse and simplicity this is the smallest possible fix.
Every root-replacement site checked
state.capturedTimeline =appears atinit.ts:1922(first bind: pauses),:2089and:5158(set to null on teardown),:2297(this fix), and:3969. The last is the player'ssetTimelinesetter, andplayer.tsnever calls it.rebindTimelineFromResolutionhas a single caller (init.ts:2378, the media-metadata rebind).- The other
play()/paused(false)sites are the child and sibling activation helpers (init.ts:1615,:4268). They're unchanged, and their comments already say the transport re-pauses or re-seeks them. Children held by the root still follow itstotalTime, so they don't depend on the root's own ticker.
Checked locally
- The new test passes at this head. With only the
init.tschange reverted, it fails atexpect(rebound.paused()).toBe(true), so it guards the bug with real GSAP. - Full
init.test.ts: 221/221. Withinit.timingResolver.test.tsandaudioFxCopy.test.ts(the parked-loop coverage) added: 251/251.
Limits, agreed with the PR body: the browser evidence is Linux only. The change is a removed call in platform-independent runtime code, so I don't expect macOS, Windows or desktop-host behaviour to differ. That's reasoning, not something I ran.
Verdict: APPROVE
Reasoning: It removes the only root-rebind path that handed the timeline to GSAP's global ticker, which aligns it with how play and the first bind already work, and the regression test fails on the old code.
— Rames
Edit accuracy: accurate 2059 (base branch 2059), smooth 1521 of thoseThe gate passes. Quarantined, measured but not gated (0) |
What changed
When late media metadata selected a replacement root during playback, the runtime restored its position and called the root's
play(). That gave GSAP's global ticker permission to advance the timeline independently of the transport clock.The replacement root now stays paused after its position is restored. The transport remains playing and continues driving the root with explicit seeks. Child activation is unchanged.
Validation
Known gate red
The Studio timeline viewport gate has a known overscan regression addressed by #5151. This fix changes only runtime root ownership and its regression test.
Limits
The browser evidence is from Linux. macOS, Windows and desktop-host playback have not been exercised.
The old autonomous root also could continue ticking after the transport reached its end. Keeping the root clock-driven removes that second source of motion through the end state as well.