Skip to content

fix(android): pace frame-rate caps like rive-android - #417

Open
mfazekas wants to merge 3 commits into
fix/android-idle-frame-loopsfrom
fix/android-frame-pacer
Open

mfazekas wants to merge 3 commits into
fix/android-idle-frame-loopsfrom
fix/android-frame-pacer

Conversation

@mfazekas

Copy link
Copy Markdown
Collaborator

Stacked on #416.

The frameRate cap compared each vsync against the last drawn frame with a 4 ms tolerance, so a cap that doesn't divide the refresh rate came out low (60 fps on a 90 Hz display rendered at 45), and every skipped vsync still cost a frame callback. This replaces it with a copy of rive-android's internal RiveFramePacer, which tracks the next due frame time, and posts the next frame callback delayed until shortly before it, as the Rive composable does.

On a 60 Hz emulator (release build, per 10 s): a 45 fps cap went from 293 to 449 draws, and a 15 fps cap from 600 to 294 frame callbacks. 30 fps on 60 Hz still wakes on every vsync, the same as upstream, because the pacer's early-wake margin lands on the vsync before the target. The Frame Rate Cap example page gets a 45 fps option to show the non-divisor case.

@mfazekas
mfazekas force-pushed the fix/android-idle-frame-loops branch from 0bfdca3 to e787746 Compare October 28, 2026 08:41
@mfazekas
mfazekas force-pushed the fix/android-frame-pacer branch from eb2c743 to 45fa68f Compare October 28, 2026 08:41
@HayesGordon
HayesGordon requested a balanced review from Copilot October 28, 2026 09:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Pending frame callbacks are not rescheduled or canceled when the cap or playback state changes, causing delayed updates after uncapping and animation jumps after pause/resume.

2 open issues
What changed in this PR

This PR refines the new Android backend’s frame-rate caps on top of #416’s idle-loop changes, targeting accurate pacing for non-divisor caps and fewer unnecessary frame callbacks.

Changes:

  • Adds a frame pacer modeled on rive-android’s internal implementation.
  • Uses delayed callbacks and resets pacing alongside the render timebase.
  • Adds a 45 fps option to demonstrate non-divisor pacing.
File Description
example/​src/​reproducers/​FrameRateCap.tsx Adds the 45 fps example option.
android/​src/​new/​java/​com/​rive/​RiveReactNativeView.kt Integrates pacing, delayed callbacks, and timing resets.
android/​src/​new/​java/​com/​rive/​RiveFramePacer.kt Tracks target render times and calculates callback delays.

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are reported for the Android frame-rate pacing changes.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

A pending capped callback can survive a settle-and-wake cycle, causing the next animation advance to include idle time and playback to jump forward.

1 open issue

Review effort: Balanced

The frameRate cap compared each vsync against the last drawn frame with a
4 ms tolerance, so a cap that does not divide the refresh rate came out
low (60 fps on a 90 Hz display rendered at 45), and every skipped vsync
still cost a frame callback.

Use a copy of rive-android's internal RiveFramePacer, which tracks the
next due frame time, and post the next frame callback delayed until close
to that time, as the Rive composable does.
@mfazekas
mfazekas force-pushed the fix/android-frame-pacer branch from 752d446 to b7f2ffa Compare October 29, 2026 06:05
@mfazekas
mfazekas requested a balanced review from Copilot October 29, 2026 06:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are identified for the Android frame-rate pacing changes.

No open issues 🎉

Review effort: Balanced

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