Skip to content

Fix animation replay and playback speed correctness - #76

Open
JonathonRP wants to merge 1 commit into
mainfrom
jonathonrp-fix-animation-replay-and-speed
Open

Fix animation replay and playback speed correctness#76
JonathonRP wants to merge 1 commit into
mainfrom
jonathonrp-fix-animation-replay-and-speed

Conversation

@JonathonRP

Copy link
Copy Markdown
Owner

Ports three targeted behavioural fixes from upstream Motion (originally landed as part of the large "Animation refactor" in motiondivision/motion#3148) without the architectural rewrite. Upstream issues: #3133, #2947, #3113.

The three fixes interlock through getFinalKeyframe, so they are delivered together.

#3133 — a finished MainThreadAnimation could not be replayed

teardown() nulls startTime when an animation finishes, so on play() the !this.startTime branch fired first and rebuilt startTime from the original, now-stale createdAt. The animation was therefore instantly "already finished" again, and the state === 'finished' branch was unreachable.

play() now checks the finished state first (matching upstream JSAnimation.play()), rebasing startTime on the driver's current time.

#2947 — speed: -1 did not play in reverse

  • getFinalKeyframe gained a speed parameter and selects the first keyframe when speed < 0.
  • play() offsets startTime by calculatedDuration when replaying a finished animation in reverse.
  • The speed setter now brings currentTime up to date (updateTime(time.now())) before the new speed is applied, so elapsed time isn't rebased at the new speed. The current-time calculation was extracted from tick() into a private updateTime() for this.

#3113 — non-default speed committed the wrong terminal keyframe

Accelerated values such as opacity run on WAAPI, and the WAAPI onfinish handlers committed getFinalKeyframe(...) without a speed, so a reversed or sped-up animation snapped to the wrong end. Speed is now threaded through both AcceleratedAnimation and the mini NativeAnimation path.

Note: upstream additionally clears finishedTime when speed goes negative (NativeAnimation.ts). This port has no finishedTime field — its state reads animation.playState directly — so there is no equivalent to port; introducing one belongs with the architectural rewrite.

Tests

  • MainThreadAnimation: replaying a finished animation once the clock has moved on, and replaying it in reverse with speed = -1. Both use a new persistentSyncDriver test helper whose clock survives driver teardown, mirroring the real frameloop driver (the existing syncDriver resets to 0 on every driver instance, which masked the bug).
  • getFinalKeyframe: first keyframe for negative speeds, final keyframe for positive speeds.
  • AcceleratedAnimation: a WAAPI animation finishing in reverse commits the first keyframe; finishing at speed = 2 still commits the final keyframe.

All four new tests fail on main and pass with this change. Full suite: bun run test:run — the only failures are pre-existing timeouts (package-imports, reorder-production-ssr) that pass in isolation on this machine and are unrelated to these changes.

No architectural refactor, no unrelated changes.

Replaying a finished MainThreadAnimation rebuilt its startTime from the
stale creation time, so the animation immediately finished again. Check
the finished state first in play(), and offset startTime by the animation
duration when replaying in reverse so speed: -1 plays backwards.

Thread the playback speed through getFinalKeyframe so animations that
finish while reversed commit their first keyframe, including the WAAPI
(AcceleratedAnimation) and mini (NativeAnimation) paths.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@changeset-bot

changeset-bot Bot commented Aug 2, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6d356be

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
motion-start Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@JonathonRP, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 47 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bdf1fcc0-ea8d-448c-891f-83f830d399f1

📥 Commits

Reviewing files that changed from the base of the PR and between 0adcef2 and 6d356be.

📒 Files selected for processing (9)
  • .changeset/shaggy-otters-replay.md
  • packages/motion-start/src/animation/animators/AcceleratedAnimation.ts
  • packages/motion-start/src/animation/animators/MainThreadAnimation.ts
  • packages/motion-start/src/animation/animators/__tests__/AcceleratedAnimation.spec.ts
  • packages/motion-start/src/animation/animators/__tests__/MainThreadAnimation.spec.ts
  • packages/motion-start/src/animation/animators/__tests__/utils.ts
  • packages/motion-start/src/animation/animators/waapi/NativeAnimation.ts
  • packages/motion-start/src/animation/animators/waapi/utils/__tests__/get-final-keyframes.spec.ts
  • packages/motion-start/src/animation/animators/waapi/utils/get-final-keyframe.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Aug 2, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/motion-start@76

commit: 6d356be

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.

2 participants