Fix secondary monitor windows not handling DPI changes#29
Fix secondary monitor windows not handling DPI changes#29shanselman wants to merge 1 commit intomasterfrom
Conversation
Secondary 'show on all monitors' windows had no DpiChanged handler, causing them to become incorrectly sized when monitor DPI changed at runtime (e.g., docking/undocking). The cached ctx.DpiScaleX/Y values also went stale, corrupting cursor hole-punch coordinates. This adds a DpiChanged event handler to each secondary window that updates the cached DPI scale, repositions/resizes the window, and rebuilds the frame geometry.
There was a problem hiding this comment.
Pull request overview
This PR fixes DPI handling for secondary monitor windows created in "show on all monitors" mode. Previously, these windows lacked a DpiChanged event handler, causing incorrect window sizing/positioning and stale cursor hole-punch coordinates when a monitor's DPI changed at runtime (e.g., during laptop docking/undocking).
Changes:
- Added
DpiChangedevent handler to secondary monitor windows that updates DPI scale values, repositions/resizes the window, and rebuilds frame geometry
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| window.Left = screen.WorkingArea.X / ctx.DpiScaleX; | ||
| window.Top = screen.WorkingArea.Y / ctx.DpiScaleY; | ||
| window.Width = screen.WorkingArea.Width / ctx.DpiScaleX; | ||
| window.Height = screen.WorkingArea.Height / ctx.DpiScaleY; |
There was a problem hiding this comment.
The DpiChanged handler captures the screen parameter in a closure, which can become stale after monitor configuration changes. When monitors are hot-plugged or DPI settings change, the Screen objects from Screen.AllScreens can be outdated.
The main window's OnDpiChanged method (lines 585-619) handles this by calling UpdateCurrentMonitorIndex() to refresh the monitor list and get a fresh screen object before using screen.WorkingArea.
Consider storing the screen's index or device name instead of the Screen object itself, then retrieve the current Screen from a refreshed Screen.AllScreens within the DpiChanged handler. This ensures the WorkingArea values are current.
| ctx.DpiScaleX = dpiArgs.NewDpi.DpiScaleX; | ||
| ctx.DpiScaleY = dpiArgs.NewDpi.DpiScaleY; | ||
|
|
||
| window.Left = screen.WorkingArea.X / ctx.DpiScaleX; | ||
| window.Top = screen.WorkingArea.Y / ctx.DpiScaleY; | ||
| window.Width = screen.WorkingArea.Width / ctx.DpiScaleX; | ||
| window.Height = screen.WorkingArea.Height / ctx.DpiScaleY; | ||
|
|
||
| UpdateMonitorGeometry(ctx); |
There was a problem hiding this comment.
The DpiChanged handler unconditionally updates window position and size without checking if the change is significant. This could potentially cause update loops or unnecessary geometry recalculations.
The main window's OnDpiChanged method (lines 609-616) includes a threshold check to only update if the difference is greater than 1 pixel, which helps avoid infinite update loops.
Consider adding a similar threshold check before updating the window properties and calling UpdateMonitorGeometry.
| ctx.DpiScaleX = dpiArgs.NewDpi.DpiScaleX; | |
| ctx.DpiScaleY = dpiArgs.NewDpi.DpiScaleY; | |
| window.Left = screen.WorkingArea.X / ctx.DpiScaleX; | |
| window.Top = screen.WorkingArea.Y / ctx.DpiScaleY; | |
| window.Width = screen.WorkingArea.Width / ctx.DpiScaleX; | |
| window.Height = screen.WorkingArea.Height / ctx.DpiScaleY; | |
| UpdateMonitorGeometry(ctx); | |
| double newDpiX = dpiArgs.NewDpi.DpiScaleX; | |
| double newDpiY = dpiArgs.NewDpi.DpiScaleY; | |
| double newLeft = screen.WorkingArea.X / newDpiX; | |
| double newTop = screen.WorkingArea.Y / newDpiY; | |
| double newWidth = screen.WorkingArea.Width / newDpiX; | |
| double newHeight = screen.WorkingArea.Height / newDpiY; | |
| // Only update if the resulting geometry change is significant (more than 1 pixel) | |
| if (Math.Abs(window.Left - newLeft) > 1.0 || | |
| Math.Abs(window.Top - newTop) > 1.0 || | |
| Math.Abs(window.Width - newWidth) > 1.0 || | |
| Math.Abs(window.Height - newHeight) > 1.0) | |
| { | |
| ctx.DpiScaleX = newDpiX; | |
| ctx.DpiScaleY = newDpiY; | |
| window.Left = newLeft; | |
| window.Top = newTop; | |
| window.Width = newWidth; | |
| window.Height = newHeight; | |
| UpdateMonitorGeometry(ctx); | |
| } |
Bug
Secondary monitor windows created by \CreateMonitorWindow\ (used in 'show on all monitors' mode) had no \DpiChanged\ event handler. This caused two problems when a monitor's DPI changed at runtime (e.g., docking/undocking a laptop):
Fix
Added a \DpiChanged\ event handler to each secondary window in \CreateMonitorWindow\ that:
This mirrors the pattern already used in the main window's DPI handling logic.