Fix FancyZones editor overlay on mixed-DPI multi-monitor setups - #44440
Conversation
Use DPI-unaware context when positioning overlay windows to match the coordinate space from the C++ backend. Co-Authored-By: Claude <noreply@anthropic.com>
|
@microsoft-github-policy-service agree |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR fixes FancyZones editor overlay window positioning issues on multi-monitor setups with different DPI scaling. The core problem was that the C++ backend uses DPI-unaware coordinates while the WPF editor interprets them with DPI scaling, causing misalignment on non-primary monitors.
Key Changes:
- Removed inconsistent DPI conversion that was only applied to monitor dimensions
- Added DPI-unaware window positioning function in C# to match coordinate space from C++ backend
- Updated window positioning logic to use DPI-unaware context after HWND creation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| EditorParameters.cpp | Removed DPIAware::Convert for monitor dimensions, ensuring all monitor properties use consistent DPI-unaware coordinates from the dpiUnawareThread |
| NativeMethods.cs | Added SetWindowPositionDpiUnaware() utility that temporarily switches to DPI-unaware context for window positioning via P/Invoke |
| Monitor.cs | Updated constructor and Scale() method to use DPI-unaware positioning, added SourceInitialized event handler to reposition windows after HWND creation |
| return; | ||
| } | ||
| monitorInfoUnaware.cbSize = sizeof(monitorInfoUnaware); | ||
| GetMonitorInfo(monitor, &monitorInfoUnaware); |
There was a problem hiding this comment.
The return value of GetMonitorInfo should be checked. If it fails, the monitorInfoUnaware structure will contain uninitialized data, leading to incorrect monitor dimensions being passed to the editor. Consider checking the return value and continuing to the next monitor if it fails, similar to how GetScreenDPIForMonitor is checked on line 187.
| IntPtr oldContext = SetThreadDpiAwarenessContext(DPI_AWARENESS_CONTEXT_UNAWARE); | ||
| try | ||
| { | ||
| SetWindowPos(helper, IntPtr.Zero, x, y, width, height, SWP_NOZORDER | SWP_NOACTIVATE); |
There was a problem hiding this comment.
The return value of SetWindowPos should be checked. If it fails, the window positioning will silently fail and the overlay may still be mispositioned. Consider checking the return value and logging a warning or error if the operation fails, similar to how error handling is done elsewhere in the codebase (e.g., checking SetLastError when the function returns false).
|
I committed a fix that scales monitor dimensions by the DPI factor so the editor shows the actual physical resolution. After fix |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Gordon Lam (yeelam-gordon)
added
the
Product-FancyZones
|
|
||
| // After HWND is created, reposition using DPI-unaware context | ||
| // This matches the C++ backend which uses a DPI-unaware thread | ||
| Window.SourceInitialized += OnWindowSourceInitialized; |
There was a problem hiding this comment.
The SourceInitialized event handler is never unregistered. While this may not cause issues in practice because the Window lifecycle is managed by the application, consider unregistering the handler after it executes once (since repositioning only needs to happen once after HWND creation) or when the Window is closed to follow proper event handler cleanup patterns.
| Window.SourceInitialized += OnWindowSourceInitialized; | |
| EventHandler handler = null; | |
| handler = (sender, args) => | |
| { | |
| Window.SourceInitialized -= handler; | |
| OnWindowSourceInitialized(sender, args); | |
| }; | |
| Window.SourceInitialized += handler; |
| // Convert virtual coordinates to physical resolution by applying DPI scale | ||
| double scale = DPI / 96.0; | ||
| int physicalWidth = (int)Math.Round(ScreenBoundsWidth * scale); | ||
| int physicalHeight = (int)Math.Round(ScreenBoundsHeight * scale); | ||
| return physicalWidth + " × " + physicalHeight; |
There was a problem hiding this comment.
When DPI is 0 (which occurs in span zones mode as set in EditorParameters.cpp line 151), the dimension calculation will result in 0x0 display. The scale will be 0/96.0 = 0, causing physicalWidth and physicalHeight to both be 0. Consider handling the DPI=0 case separately, similar to how it's marked as "unused" in the C++ code, or fall back to displaying the virtual coordinates without scaling.
Summary of the Pull Request
Use DPI-unaware context when positioning overlay windows to match the coordinate space from the C++ backend.
The FancyZones editor overlay windows were incorrectly positioned on secondary monitors when using different DPI scaling (e.g., 125%/150%/125%). Zones appeared shifted or clipped because they extended past monitor edges.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Root Cause
The C++ backend uses a DPI-unaware thread to get virtual screen coordinates, but the WPF editor (PerMonitorV2 DPI-aware) interpreted these coordinates with DPI scaling applied, causing misalignment on non-primary monitors.
Fix
DPIAware::Convertthat was only applied to dimensions)SetWindowPositionDpiUnaware()usingSetThreadDpiAwarenessContextto temporarily switch DPI awarenessValidation Steps Performed
Manually tested on 3-monitor setup with 125%/150%/125% DPI scaling - overlays now correctly cover each monitor's work area.
🤖 This fix was developed with Claude Code after 6 hours of debugging DPI coordinate systems together.