gpui_windows: Fix window placement on secondary monitor with different DPI scaling (#62859)
# Objective Fixes #48927. On Windows, when monitors have different DPI scaling factors, a new window opened on a secondary monitor can be positioned partially off-screen, with the title bar unreachable. ## Solution `WindowsWindow::new` calls `CreateWindowExW` with `CW_USEDEFAULT` for the window's initial position, so Windows picks a starting spot (typically based on where the previous window was, which is often the primary monitor) before the window is explicitly moved to its intended monitor via `SetWindowPlacement`. `WindowsWindowState::new` immediately reads the window's scale factor via `GetDpiForWindow(hwnd)` at that point — i.e. the DPI of wherever Windows initially placed it, not necessarily the target monitor. `retrieve_window_placement` then used that scale factor to convert the target monitor's logical bounds into physical pixels. When the two monitors have different scaling (e.g. 150% vs 100%, as in the linked issue), this produces the wrong physical rect, leaving the window off-screen. The target `WindowsDisplay` already computes its own correct `scale_factor` from the actual monitor (`get_scale_factor_for_monitor`) — this is also what `check_given_bounds` already uses. This PR exposes that value via `WindowsDisplay::scale_factor()` and uses it in `retrieve_window_placement` instead of the transient window's scale factor, since the bounds being converted are expressed in logical pixels for that target display. ## Testing - Verified the logic by tracing through `WindowsWindowState::new` → `retrieve_window_placement` → `calculate_window_rect` and confirming the target monitor's own DPI (not the temporarily-assigned window DPI) is now used to compute the physical placement rect, matching the approach `WindowsDisplay::check_given_bounds` already uses for the same target/temporary-monitor mismatch. - I wasn't able to build/run Zed locally in this environment (missing Windows SDK components in the local toolchain, unrelated to this change) to reproduce the original multi-monitor/mixed-DPI repro steps first-hand. I'd appreciate a maintainer or anyone who can reproduce #48927 double-checking on real mixed-DPI hardware. - Platforms: Windows only (`gpui_windows` crate); no other platform code touched. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments (no new unsafe blocks added) - [ ] The content adheres to Zed's UI standards (N/A, no UI change) - [ ] Tests cover the new/changed behavior (no existing test harness for Windows-specific placement logic; happy to add one if pointed at the right pattern) - [x] Performance impact has been considered and is acceptable (no measurable impact; same number of DPI queries, just reading from the already-computed `WindowsDisplay` instead of the window) Release Notes: - Fixed: new windows on Windows could open partially off-screen when placed on a secondary monitor with a different DPI scaling factor than the monitor Windows initially placed them on. --------- Co-authored-by: Lukas Wirth <lukas@zed.dev> Co-authored-by: zed-zippy[bot] <234243425+zed-zippy[bot]@users.noreply.github.com>
This commit is contained in:
2 files changed
+16
-9
No files matched your search
@@ -92,6 +92,12 @@ impl WindowsDisplay {
|
||||
WindowsDisplay::new(Self::display_id_for_monitor(monitor))
|
||||
}
|
||||
|
||||
/// The DPI scale factor of this monitor, independent of whatever monitor
|
||||
/// a not-yet-positioned window currently happens to be on.
|
||||
pub(crate) fn scale_factor(&self) -> f32 {
|
||||
self.scale_factor
|
||||
}
|
||||
|
||||
/// Check if the center point of given bounds is inside this monitor
|
||||
pub fn check_given_bounds(&self, bounds: Bounds<Pixels>) -> bool {
|
||||
let center = bounds.center();
|
||||
|
||||
@@ -552,13 +552,8 @@ impl WindowsWindow {
|
||||
set_non_rude_hwnd(hwnd, true);
|
||||
configure_dwm_dark_mode(hwnd, appearance);
|
||||
this.state.border_offset.update(hwnd)?;
|
||||
let placement = retrieve_window_placement(
|
||||
hwnd,
|
||||
display,
|
||||
params.bounds,
|
||||
this.state.scale_factor.get(),
|
||||
&this.state.border_offset,
|
||||
)?;
|
||||
let placement =
|
||||
retrieve_window_placement(hwnd, display, params.bounds, &this.state.border_offset)?;
|
||||
if params.show {
|
||||
let mut placement = placement;
|
||||
if !params.focus {
|
||||
@@ -1528,7 +1523,6 @@ fn retrieve_window_placement(
|
||||
hwnd: HWND,
|
||||
display: WindowsDisplay,
|
||||
initial_bounds: Bounds<Pixels>,
|
||||
scale_factor: f32,
|
||||
border_offset: &WindowBorderOffset,
|
||||
) -> Result<WINDOWPLACEMENT> {
|
||||
let mut placement = WINDOWPLACEMENT {
|
||||
@@ -1542,7 +1536,14 @@ fn retrieve_window_placement(
|
||||
} else {
|
||||
display.default_bounds()
|
||||
};
|
||||
let bounds = bounds.to_device_pixels(scale_factor);
|
||||
// `bounds` is expressed in logical pixels for `display`, so it must be converted
|
||||
// to device pixels using that display's own scale factor. The window's current
|
||||
// scale factor can't be used here: `CreateWindowExW` was called with
|
||||
// `CW_USEDEFAULT`, so at this point the window may still be sitting on whichever
|
||||
// monitor Windows picked by default, which can have a different DPI than `display`
|
||||
// and would otherwise throw off the physical position (e.g. leaving the window
|
||||
// partially off-screen when moved to a monitor with a different scale factor).
|
||||
let bounds = bounds.to_device_pixels(display.scale_factor());
|
||||
placement.rcNormalPosition = calculate_window_rect(bounds, border_offset);
|
||||
Ok(placement)
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user