Skip to content

fix(core): an animation playing at its first seek shows the sought time - #5503

Merged
miguel-heygen merged 1 commit into
heygen-com:mainfrom
cathrynlavery:fix/worker-first-frame-time
Oct 12, 2026
Merged

miguel-heygen merged 1 commit into
heygen-com:mainfrom
cathrynlavery:fix/worker-first-frame-time

Conversation

@cathrynlavery

Copy link
Copy Markdown
Contributor

What

The WAAPI and CSS adapters now pause an animation before they write its currentTime. Both use one helper, holdAnimationAt in packages/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 currentTime and then called pause(). On a running animation pause() 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). Writing currentTime while 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) calls pause(), then writes currentTime. If the first pause() throws (an infinite animation whose current time is unresolved), it still writes the time and then tries pause() once more, which keeps the behaviour the existing "still sets currentTime when pause throws" tests cover.
  • waapi.ts seek and css.ts seekAnimations call it with their existing swallow site ids.
  • No change to baselines, discovery or the render pipeline.

Test plan

  • New pendingPauseAnimation.test-helpers.ts: a fake animation that follows the spec's pending pause. pause() on a running animation is pending until nextFrame(). A currentTime write 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.ts and css.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-reachability and fallow audit --base origin/main: all pass.

  • bun run --cwd packages/core test has 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:

    • 0.8.150 as published, three runs: chunk-start frames late by 16.7 ms in every run (60 and 90; 30, 60 and 90; 60 and 90).
    • The same 0.8.150 runtime with this change applied: three runs, every frame within 4 ms.
    • A larger composition that builds its animations inside 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

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 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 running CSSAnimation, each sought to 1000 ms:
    • pause-then-write reads exactly 1000 and paused, both straight away and two frames later, for both animations;
    • write-then-pause (main's order) drifts to 1000.038 after 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.

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

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.

Parallel render: first frame of each worker chunk is 16.7 ms late for a WAAPI animation created after load

3 participants