fix(cua-driver)(windows): correct VIRTUALDESK normalization fence-post - #1980
Merged
Merged
Conversation
…t for negative-offset multi-monitor (#1979) `send_click_synthesized` / `send_drag_synthesized` mapped screen pixels to `MOUSEEVENTF_VIRTUALDESK` absolute coords via (sx - vd_x) * 65535 / vd_w which is off-by-one at the right/bottom edge of the virtual desktop: the last pixel of a 3840-wide virt rect mapped to dx=65517 instead of 65535, and the seam between two stitched monitors landed exactly on dx=32767 (the first pixel of the primary in a "secondary-to-left-of-primary" layout was rounded back onto the secondary). Windows reverses the normalization with `dx * (vw - 1) / 65535 + vx`, so the forward map must divide by `(vw - 1)`, not `vw`. The fix extracts the math into a new pure module `platform_windows::virtualdesk` so it can be unit-tested on any host (no Win32 runtime needed), and adds 6 tests covering the 8 canonical multi-monitor layouts from issue #1979 — the reporter's exact `(-1795, 383)` pixel on a `(-1920, 0, 3840, 1080)` virt rect now round-trips with ±1 px drift and stays on the secondary half of the normalized band. The seam-monotonicity test fails under the old formula and passes under the new one. Scope: pure-math fix only. Public API of `send_click_synthesized` / `send_drag_synthesized` unchanged. Two call sites in `mouse.rs` updated to route through the helper. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qxy4BYpesmB4keyJf8DkXC
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
|
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 (3)
📝 WalkthroughWalkthroughA new Changesvirtualdesk coordinate normalization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
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 docstrings
🧪 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 |
Contributor
Linux visual regression artifactsMatrix jobs now run independently. Download visual artifacts from this workflow run.
|
This was referenced Jun 23, 2026
f-trycua
added a commit
that referenced
this pull request
Jun 23, 2026
…egative client-area coords (#1979) (#1983) Audit pass on the second half of #1979: the PostMessage click path (`input::mouse::post_click_screen` → `make_lparam` → `PostMessageW`) was the next suspect for the multi-monitor wrong-screen symptom after the SendInput VIRTUALDESK normalization fix in #1980. Traced the full call chain from `tools::impl_::ClickTool` / `DoubleClickTool` (e.g. `impl_.rs:3655`) → `input::mouse::post_click_screen` (`mouse.rs:83`) → `deepest_child` → `ScreenToClient` → `make_lparam` (`mouse.rs:226`) → `PostMessageW(WM_LBUTTONDOWN, ..., lparam)`. For every coordinate that fits in `i16` range — including the reporter's exact `(-1198, 292)` and `(-1795, 383)` examples after the post-`ScreenToClient` conversion to window-local coords — the existing implicit `i16` truncation in `make_lparam` round-trips correctly through the receiver's `GET_X_LPARAM` / `GET_Y_LPARAM` sign-extension. The bit-pattern lives in the low 16 bits of each half-word and `(short)LOWORD(lp)` recovers the original signed `i32` value. Verified empirically; no bug in the packing. But this is exactly the kind of pure-math invariant that's load-bearing and easy to silently regress (a future "simplification" dropping the implicit `i16` truncation would still compile and still work for positive coords). Extract the bit-math into a new `crate::lparam` pure module mirroring `crate::virtualdesk`, with cross-platform tests that lock in: - round-trip for the reporter's `(-1198, 292)` and `(-1795, 383)` - full `i16` range round-trip at the fence-posts - sign-stability: every negative `x` decodes to the same negative `x` (no sign-flip into the primary-monitor half) - screen→window-local→pack monotonicity across the secondary monitor - end-to-end reconstruction of the reporter's `(-1795, 383)` scenario - explicit `Err` on out-of-i16-range inputs (vs the previous silent truncation) — the new `pack_xy` surface fails fast so a future >32k-px virtual-desktop click can't reach a PostMessage handler interpreting the wrong bit pattern `make_lparam` now delegates to `lparam::pack_xy` and clamps + warns on the (currently unreachable) out-of-range path. Public API of `make_lparam` unchanged — no caller signature touched. Tests run on the macOS host via `cargo test -p platform-windows`: 6 new `lparam` tests + the existing 26 pass (32 total). Scope: regression coverage for the PostMessage half of #1979 only. The visible-wrong-screen symptom the reporter saw must originate elsewhere (top candidates: stale element-cache center after a window move, or a receiver app that reads `GetCursorPos` instead of the lParam — neither fixable in `make_lparam`). Filed as a follow-up note for further audit. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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
1 participant
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.
Closes part of #1979.
Bug
The
MOUSEEVENTF_VIRTUALDESKforward mapping insend_click_synthesized/send_drag_synthesizeddivided byvd_winstead ofvd_w − 1(classic fence-post error). With the reporter's(-1920, 0, 3840, 1080)virtual-desktop layout this means:dx=65517instead ofdx=65535dx=32767instead ofdx=32768Both off-by-(less-than-1-pixel) under normal use, but at the seam Windows rounds dx=32767 back onto the secondary monitor's last pixel instead of the primary's first — so clicks at exact monitor boundaries land on the wrong screen.
Fix
Single arithmetic correction:
(p − vd_origin) * 65535 / vd_size→(p − vd_origin) * 65535 / (vd_size − 1)for both X and Y axes, in both the click and drag paths. Hoisted into a newcrates/platform-windows/src/virtualdesk.rsmodule with the formula isolated as a pure function so it's testable without a Windows runtime.Tests
26 passing (20 pre-existing + 6 new in
virtualdesk.rs). The 6 new tests cover the 8 canonical multi-monitor layouts:Each layout asserts:
[0, 65535]pixels_to_the_left_of_seam_stay_on_secondary_half— the actual regression: with(-1920, 0, 3840, 1080)virt rect, screenx=0(the seam) must producedx ≥ 32768, NOTdx=32767. Fails under the old/ vd_wformula, passes under the new/ (vd_w − 1).corners_map_to_extreme_normalized_values— BR corner must map todx=65535. Was65517before,65535now.Honest caveat
This is a real, provable fence-post bug — but the reporter's specific symptom ("completely wrong monitor") in their log is happening on the PostMessage (
post_click_screen) code path, which doesn't go through VIRTUALDESK at all. So this PR hardens theSendInput/ foreground-dispatch path but is not sufficient to fully close #1979. The next thing to audit isget_element_center→deepest_child→ packed-lparam HWND resolution for elements on negative-X monitors — likely a separate bug worth its own issue. Leaving #1979 open after merge, with a comment pointing at the next code path.Verification
cargo test -p platform-windows→ 26 passedcargo check -p cua-drivercleancargo check -p platform-windows --target x86_64-pc-windows-msvccleanSummary by CodeRabbit