Repository navigation
fix(core): an animation playing at its first seek shows the sought time - #5503
Conversation
The WAAPI and CSS adapters wrote currentTime and then called pause(). On a running animation pause() is pending until the next animation frame, and Chromium then sets the hold time to the time at that frame, 16.7 ms past the sought time. Pause first: writing currentTime while a pause is pending completes it with the sought time. An animation created after the runtime's discover seek is still running when a render worker makes its first seek, so the first frame of each worker chunk after the first was drawn 16.7 ms late.
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at c867a20e. Approving.
Pausing before writing currentTime does resolve the pending pause.
- In the spec, setting the current time while a pause is pending sets the hold time to the time being written and completes the pause on the spot.
- Writing first and then calling
pause()leaves the hold time to be read from the timeline when the pending pause runs, one frame later. - I checked both orders in headless Chrome 152 on a running
element.animate()and on a runningCSSAnimation, each sought to 1000 ms:- pause-then-write reads exactly
1000andpaused, both straight away and two frames later, for both animations; - write-then-pause (main's order) drifts to
1000.038after the frames. With no real frame cadence the gap is small; in a frame-paced render it is the 16.7 ms in the issue.
- pause-then-write reads exactly
Already-paused animations behave the same. pause() does nothing on a paused animation, so the write sets the hold time as before. In Chrome, a paused animation sought again to 2500 reads 2500, paused. CSS animations were already being paused on every seek, so pausing first doesn't change their play-state override.
The error path is kept. If the first pause() throws, the time is still written and pause() is retried. That is the old write-then-pause order and the same swallow sites, so the existing "still sets currentTime when pause throws" tests hold.
The tests catch the bug. I put main's waapi.ts and css.ts back with the new tests in place, and both regression tests failed with expected 1016.6666666666666 to be 1000. At the head, all of src/runtime/adapters passes, 231 tests. lottie.test.ts didn't load locally because the workspace packages aren't built, and this PR doesn't touch it. oxfmt is clean on the changed files.
CI: nothing has failed so far. 38 checks pass and the rest are still running.
I didn't do a parallel render myself. The render evidence is the author's macOS runs.
— Rames
What
The WAAPI and CSS adapters now pause an animation before they write its
currentTime. Both use one helper,holdAnimationAtinpackages/core/src/runtime/adapters/_holdAnimationAt.ts.Why
An animation that is still playing when the runtime first seeks it was drawn one animation frame (16.7 ms) late on that seek.
The adapters wrote
currentTimeand then calledpause(). On a running animationpause()only schedules a pending pause. When it completes on the next animation frame, the browser takes the hold time from the timeline at that moment, 16.7 ms after the time just written (Web Animations, pausing an animation). WritingcurrentTimewhile a pause is pending completes the pause with the written time (setting the current time), so pausing first fixes it.The runtime's discover seek hides this for animations that exist at load. An animation the page creates later, for example inside
document.fonts.ready.then(...)or a timer, is still running when a render worker makes its first seek. In a parallel render that is the first frame of each chunk after the first, so those frames were late and which frames were wrong depended on the worker count.Related work
Fixes #5502.
Different symptom on the same worker-start frame: #4435 and #2477 (the first frame shows the pre-seek page). This change does not address those.
How
holdAnimationAt(animation, timeMs, currentTimeSite, pauseSite)callspause(), then writescurrentTime. If the firstpause()throws (an infinite animation whose current time is unresolved), it still writes the time and then triespause()once more, which keeps the behaviour the existing "still sets currentTime when pause throws" tests cover.waapi.tsseekandcss.tsseekAnimationscall it with their existing swallow site ids.Test plan
New
pendingPauseAnimation.test-helpers.ts: a fake animation that follows the spec's pending pause.pause()on a running animation is pending untilnextFrame(). AcurrentTimewrite while a pause is pending completes it with that time.New
_holdAnimationAt.test.ts: the first frame of a running animation holds the sought time; a throwing first pause still writes the time and retries the pause; a successful pause runs once.waapi.test.tsandcss.test.ts: one regression test each, a running animation sought to 1 s reads 1000 ms after the next frame. Before the fix both read 1016.67 ms.bunx vitest run src/runtime(packages/core): 2029 passed.bun run --cwd packages/core typecheck, oxlint and oxfmt on the changed files,check-comment-citations,comment-ratchet,check-test-reachabilityandfallow audit --base origin/main: all pass.bun run --cwd packages/core testhas one failure,deterministicFonts-googleSubsets.test.ts"still embeds a local file when the page has no Google link" (5 s timeout). It fails the same way on an unmodified checkout of main.Render check on macOS (Apple M3 Ultra, chrome-headless-shell 152.0.7977.30, screenshot capture). The page from the issue is a box moving at 300 px/s, created 100 ms after load, 4 workers, 30 fps, 120 frames:
document.fonts.ready.then(...): frame 462 was 16.6 ms late at 4 workers before, and all 189 measured frames in motion are exact at 4 and 6 workers after.Unit tests added/updated
Manual testing performed
Documentation updated (if applicable): not applicable
Comments follow CONTRIBUTING.md "Comments": they say why, not what, and a bug fix says what the code must do and how to reproduce the bug