feat: native macOS cursor capture using NSCursor#671
Conversation
Capture the real system cursor image during macOS recording so custom and default cursors render natively instead of being mapped to bundled SVGs, bringing macOS in line with the Windows WGC capture path. - macOS cursor helper grabs NSCursor.currentSystem as a PNG asset (SHA256 id, intrinsic scale factor, pixel hotspot); the bitmap payload is emitted once per shape and referenced by assetId thereafter - helper returns nil cursorType instead of an arrow fallback so default/custom cursors fall through to the captured bitmap while text/pointer stay beautified - MacNativeCursorRecordingSession collects deduped assets, tags samples with assetId, and reports provider "native" when bitmaps are captured
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds macOS native cursor bitmap capture and stable asset ids in Swift, emits asset metadata conditionally during sampling, and wires those assets into the TypeScript recording session which stores assets and includes them in final reports; adds a test validating rendering of untyped native cursor samples using captured bitmaps. ChangesNative Cursor Asset Capture Pipeline
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e82fc0d8cc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.ts (1)
210-214:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe Accessibility warning is stale now.
With the bitmap path, missing Accessibility only disables text/pointer affordance detection; it doesn't force arrow-only rendering anymore. This message/comment will send debugging down the wrong branch pretty fast.
Also applies to: 333-337
🤖 Prompt for 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. In `@electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.ts` around lines 210 - 214, Update the stale comment around the systemPreferences.isTrustedAccessibilityClient(true) call in macNativeCursorRecordingSession.ts (and the similar block at lines 333-337) to accurately describe the current behavior: missing Accessibility only disables text/pointer affordance detection (bitmap path still renders cursor shapes), not "arrow-only" rendering; change any nearby log messages or comments that claim it forces arrow-only rendering to reflect this precise limitation so future debugging isn't misled. Ensure references are made near the existing try/catch that calls systemPreferences.isTrustedAccessibilityClient(true) and the catch comment is replaced with the corrected explanation.
🤖 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
`@electron/native/screencapturekit/Sources/OpenScreenMacOSCursorHelper/main.swift`:
- Around line 239-268: The code is re-rasterizing, PNG-encoding, hashing and
base64-encoding the cursor on every poll; cache the last computed signature and
asset and only regenerate when the cursor image or hotspot changes. Introduce a
cached state (e.g. lastDigestId, lastHotSpot, lastImageDataUrl) alongside the
polling logic that reads cursor.image and cursor.hotSpot, compute
digest/id/png/base64 only if either the current hotSpot differs from lastHotSpot
or the image content differs (compare a lightweight marker such as image.size
and proposedRect/cgImage reference or compute digest once and reuse), then
update the cache variables (lastDigestId, lastHotSpot, lastImageDataUrl);
otherwise reuse the cached id and imageDataUrl to avoid repeated work in the
loop that currently creates png, digest, id, and imageDataUrl from cursor.image
and cursor.hotSpot.
- Around line 317-337: The current dedupe uses lastEmittedAssetId so only
adjacent identical cursor shapes are deduped; replace this with a process-wide
Set to track all emitted asset ids: introduce a Set<String> (e.g.,
emittedAssetIds) instead of lastEmittedAssetId, and in the loop call
currentCursorAsset() and check if asset.id is not in emittedAssetIds before
building assetPayload, then insert asset.id into emittedAssetIds when you emit
it; update any references to lastEmittedAssetId accordingly (remove it) so each
unique cursor shape is only serialized once.
---
Outside diff comments:
In `@electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.ts`:
- Around line 210-214: Update the stale comment around the
systemPreferences.isTrustedAccessibilityClient(true) call in
macNativeCursorRecordingSession.ts (and the similar block at lines 333-337) to
accurately describe the current behavior: missing Accessibility only disables
text/pointer affordance detection (bitmap path still renders cursor shapes), not
"arrow-only" rendering; change any nearby log messages or comments that claim it
forces arrow-only rendering to reflect this precise limitation so future
debugging isn't misled. Ensure references are made near the existing try/catch
that calls systemPreferences.isTrustedAccessibilityClient(true) and the catch
comment is replaced with the corrected explanation.
🪄 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: 8510e47c-e397-41b3-a8c4-4807fb4d8cb8
📒 Files selected for processing (3)
electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.tselectron/native/screencapturekit/Sources/OpenScreenMacOSCursorHelper/main.swiftsrc/lib/cursor/nativeCursor.test.ts
- Replace lastEmittedAssetId with a process-wide Set<String> so each unique
cursor shape is serialised at most once even across non-adjacent repeats
(e.g. arrow → text → arrow no longer resends the arrow bitmap)
- Wrap the sampling loop body in autoreleasepool{} to prevent Cocoa objects
(NSBitmapImageRep, PNG Data, base64 String) from accumulating for the
lifetime of the helper during long recordings
- Update stale Accessibility comments: missing Accessibility only disables
text/pointer affordance detection; native bitmap capture is unaffected
|
Addressed all actionable review comments in eac5cc6:
|
|
please include video recordings |
|
Due to the 10MB file upload limit, this is all I could record. If you want to see any other screen recordings, just let me know. trim.mp4 |
2 participants
Description
Implements native cursor capture on macOS, bringing it in line with the existing Windows WGC path. Custom and default cursors now render from their real system bitmaps instead of being mapped to bundled SVG approximations.
No third-party libraries. The implementation is built entirely on AppKit APIs that ship with macOS — no uiohook, no CGEventTap pixel-scraping, no overlay heuristics.
Test
My PC is Mac Mini 4, Apple M4 chip, , MacOS 26.5 (25F71)
✅ Local test passed, take video, export, cursor capture & highlight are all good.
How it works
Swift helper (
openscreen-macos-cursor-helper)NSCursor.currentSystemevery sample interval to grab the live system cursor imageassetIdreference — stdout stays small even across many cursor shape changespixelsWide / pointWidth, e.g. 2.0 on Retina), and hotspot in pixel coordinates so the renderer can convert back to points correctlycurrentCursorType()now returnsnilinstead of falling back to"arrow"— default and custom cursors fall through to the captured bitmap; only explicittext/pointeraffordances (detected via Accessibility) continue to use the bundled pretty SVGsTypeScript session (
MacNativeCursorRecordingSession)MapCursorRecordingSamplewithassetIdprovider: "native"when at least one bitmap was captured,"none"as a graceful fallback (e.g. helper not found, Accessibility denied)The rendering, persistence, and export layers (
nativeCursor.ts,frameRenderer.ts, etc.) already supported theassetId+imageDataUrlpath used by Windows — no changes needed there.Features
CGEventTap; click bounce animation works as beforeprovider: "none") without crashingSummary by CodeRabbit
New Features
Tests