fix(cua-driver)(macos): stop idle overlay frame ticks - #1865
Conversation
|
@redwine99 is attempting to deploy a commit to the Cua Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR optimizes the macOS overlay render loop to eliminate ~50% idle CPU usage by blocking when no cursor animation or movement is active, only compositing pixmaps when commands arrive or frame ticks could visibly change the cursor, and conditionally ticking/repinning only when necessary. ChangesRender Loop Idle Blocking & Conditional Ticking
Initialization, Safety & Supporting Changes
Test Expansion & New Quiescence Validation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/cua-driver/rust/crates/platform-macos/src/cursor/overlay.rs`:
- Around line 585-599: The frame delta dt is being computed after a blocking
rx.recv() so the first frame after waking includes the entire idle duration
(clamped to 50ms) and causes wake-up animations to jump; modify the logic in the
loop around first_msg/frame_tick_needed so that when we woke due to a received
message (i.e., first_msg.is_some() or had_msg is true after rx.recv()) you reset
last_tick or set dt = 0.0 before using it for ticking/animation updates
(affecting the code paths that use dt in the later tick-on-had_msg section),
ensuring the first post-idle frame starts from the initial animation state
rather than partway through.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7dae37a7-1958-4488-8187-f11fd4568e37
📒 Files selected for processing (1)
libs/cua-driver/rust/crates/platform-macos/src/cursor/overlay.rs
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
I installed the patched cua-driver binary into the existing /Applications/CuaDriver.app bundle on macOS and verified the fix against the real app-bundle daemon path: /Applications/CuaDriver.app/Contents/MacOS/cua-driver serve The installed binary matched the patched release build by SHA-256, and idle CPU remained near zero: avg_cpu=0.01 samples=8 values=0.0,0.0,0.0,0.0,0.0,0.1,0.0,0.0 I also verified the on-demand daemon path used by the MCP server. Hermes was configured to run: mcp_servers: where /Users/test/.local/bin/cua-driver resolves to the app-bundle binary. One macOS-specific note from testing a locally replaced app-bundle binary: after replacing and ad-hoc re-signing the bundle, Screen Recording permission had to be reset/re-added for /Applications/CuaDriver.app, and all existing cua-driver mcp / cua-driver serve processes had to be stopped so TCC would apply the new code identity. After that, Finder screenshot capture worked again, confirming the patched daemon still functions end-to-end. |
Let the agent-cursor overlay render loop block on incoming commands when no cursor animation, fade, click pulse, or focus rect can change pixels. This keeps an idle `cua-driver serve` daemon from compositing a fullscreen pixmap at roughly 60fps while preserving frame ticks during active cursor motion. Add focused tests for the quiescent sentinel-cursor state and for active cursor state that should continue requesting frame ticks. The committed file also includes the Rust formatting changes from running `cargo fmt`, so nearby existing code may be reflowed in addition to the logic change. Following CONTRIBUTING.md, ran formatting and validation before preparing this PR: `cargo fmt`, `cargo build -p cua-driver`, and `cargo test -p platform-macos --lib`. I also ran the repository pre-commit configuration against the changed Rust file; all configured hooks skipped because they target TS/JS/JSON/Markdown/YAML or Python files, not Rust.
Avoid charging the render loop's blocking recv() idle duration to the first animation tick after a command arrives. This keeps wake-up frames at the initial animation state instead of advancing by the clamped idle delta. Validated with `cargo build -p cua-driver`, `cargo test -p platform-macos --lib`, and the local idle CPU sampling helper.
8b068ff to
d347430
Compare
|
Rebased this branch onto current Local validation on the rebased branch:
I also checked formatting for the changed macOS overlay file only: Note: a full |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@f-trycua Could you take a look when you have a chance? The PR is currently mergeable. |
f-trycua
left a comment
f-trycua
left a comment
There was a problem hiding this comment.
Reviewed: tightly-scoped macOS overlay idle-CPU fix — stops idle frame ticks when nothing is animating. CodeRabbit green, mergeable, no conflicts. LGTM.
…#1808) (#1933) * fix(cua-driver-rs)(windows): stop idle overlay CPU + orphan mcp child (#1808) The Windows agent-cursor overlay render timer ran at ~125 Hz unconditionally: every WM_TIMER tick allocated a full virtual-screen tiny-skia pixmap, swizzled it RGBA->BGRA pixel-by-pixel, and blitted it via UpdateLayeredWindow — even when no cursor was animating and the pointer was static. An idle `cua-driver mcp` therefore pinned 60-85% of a CPU core (issue #1808), and long-lived instances accumulated CPU-hours. Part A (idle CPU): mirror the macOS fix (#1865). Add a `needs_frame_tick` predicate (in-flight path / spring / click pulse / unfinished idle-fade) and gate the composite+blit+z-order behind it. The render timer is now re-armed between an ACTIVE cadence (~125 Hz, smooth animation) and a slow IDLE heartbeat (250 ms) once every cursor goes quiescent. `send_command` / `remove_cursor` call `wake_overlay()` to flip back to ACTIVE within ~8 ms via a cross-thread SetTimer, so the first move after idle is not delayed. A final settle frame is still emitted as animations finish, so the layered window is left in its resting/cleared state before the loop parks. No full-screen pixmap allocation, no RGBA->BGRA copy, no UpdateLayeredWindow while idle. Part B (orphan on disconnect): the overlay runs on a detached STA thread with its own Win32 message loop, so returning from `async_main` after the stdio MCP server loop ended (stdin EOF) was not guaranteed to tear it down promptly. The in-process Windows/Linux `mcp` path now `std::process::exit`es once `server::run` returns, mirroring the macOS arm, so the overlay thread dies with the process the moment the client disconnects. Adds headless unit tests for the quiescent-sentinel state, the active-animation state, and the click-pulse-then-quiescent transition. Verified `platform-windows` cross-compiles cleanly for x86_64-pc-windows-msvc and all platform-windows lib tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(cua-driver-rs): keep Cargo.lock matching main (avoid nix cargoHash churn) The overlay fix needs no new dependencies; an incidental cargo build had re-synced the workspace member versions (0.5.3 -> 0.5.6) in Cargo.lock, which fetchCargoVendor hashes, breaking the Nix cargoHash and turning every Linux nix job red. Restore Cargo.lock to main's committed state so the hash stays valid. (The Cargo.toml/Cargo.lock version drift on main is a separate pre-existing issue, not this PR's concern.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Summary
Fixes #1808
Root cause
The macOS overlay render thread used a 60fps loop even when
cua-driver servehad only the off-screen default/sentinel cursor and no active cursor work. That kept allocating/compositing a fullscreen pixmap while the daemon was otherwise idle.Verification
Following
CONTRIBUTING.md, I ran formatting and validation before preparing this PR:Result:
I also ran the repository pre-commit configuration against the changed Rust file. The configured hooks skipped because they currently target TS/JS/JSON/Markdown/YAML or Python files, not Rust.
Idle CPU check
Using a local sampling helper against
cua-driver serve --no-permissions-gate:Before this fix:
After this fix:
Notes
The diff includes some nearby Rust formatting/reflow from
cargo fmtin addition to the render-loop logic change.Summary by CodeRabbit
Bug Fixes
Chores