fix(cursor): make the Windows cursor sampler DPI-aware - #278
Merged
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #272.
The bug
The cursor drawn in the editor preview sits short of where it really was, by more the further it is from the top-left of the display. Only on Windows, only on a display (screen) capture, only at a display scaling other than 100%.
Root cause
cursor-sampler.exeships with nodpiAwaremanifest and never callsSetProcessDpiAwareness*:So the process is DPI-unaware, and Win32 hands it virtualized coordinates —
GetCursorInfo().ptScreenPosandGetWindowRectboth come back divided by the primary display's scale factor.b31bb71f(shipped in v1.7.0, so present in the reported 1.8.0) assumed the opposite — "the cursor-sampler reports raw x/y in physical screen pixels" — and converted the Electron display bounds to physical withdipToScreenRectbefore normalizing. Numerator and denominator then lived in different spaces:The preview cursor lands at
1/sof its true offset from the display origin. At 150% on a 2560px-wide screen that is ~850px short at the right edge. At 100% the two spaces coincide, which is why it was invisible on the dev machines and in the Windows smoke tests.Window captures were never affected: there the sampler supplies its own
GetWindowRectbounds, so both sides were virtualized together and the ratio came out right either way.The fix
Opt the helper into per-monitor-v2 awareness, rather than walking the normalization back to DIPs. The reported numbers then really are physical — which is what every consumer already assumes — and unlike DIP normalization this also holds on mixed-DPI multi-monitor setups, where virtualization always uses the primary display's scale whatever monitor the cursor is on.
Second hunk:
payload.x/ybeing physical now, the asset's display lookup goes throughscreenToDipPoint—screen.getDisplayNearestPointworks in DIPs and would otherwise pick the wrong monitor.Verification
GetProcessDpiAwarenesscursor-sampler.exeas shipped in 1.8.0UNAWAREPER_MONITOR_AWAREtsc --noEmitandbiome checkpass.253,611==253,611) — no regression on unscaled displays.Cross-OS
ScreenCaptureRecorder.swift:127), the same space asscreen.getCursorScreenPoint()No test is added: the whole failure lives in a Win32 process attribute, and CI is Linux-only, so a guard here would never run.