From 39bfa863354698370a60a70531d4966227e3a4f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Thu, 19 Feb 2026 19:43:32 +0100 Subject: [PATCH] CmdPal: Fixes and improve main window positioning (#45585) ## Summary of the Pull Request This PR improves main window positioning: - Fixes cases where an invalid window size or position was saved. - `UpdateWindowPositionInMemory` failed to capture correct values when the window was minimized or maximized (for example, a minimized window reports coordinates like `(-32000, -32000)`). - Improves repositioning logic to use relative anchors (corners and center). When switching displays, the window should reappear in the expected position. This also reduces cases that trigger the failsafe recentering. - Fixes the dragging rectangle size after switching DPIs - the rectangle was not adapting, so it when switching from 100 % to 200 % it covered only left half of the window and had teeny-tiny height. - Suppresses system DPI handling during summon to prevent double scaling. - Makes `WindowPosition` class immutable. - Adds light-weight failsafe preventing overwriting position with invalid data. - Hotfixes a min/max state conflict with the WinUIEx window manager. ## PR Checklist - [x] Closes: #45576 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed --- .../WindowPosition.cs | 19 ++- .../Helpers/WindowPositionHelper.cs | 120 ++++++++++++++---- .../Microsoft.CmdPal.UI/MainWindow.xaml.cs | 108 +++++++++++----- .../Microsoft.CmdPal.UI/NativeMethods.txt | 6 +- 4 files changed, 185 insertions(+), 68 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/WindowPosition.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/WindowPosition.cs index 7963aec154..ac0dfddf58 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/WindowPosition.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/WindowPosition.cs @@ -11,37 +11,42 @@ public sealed class WindowPosition /// /// Gets or sets left position in device pixels. /// - public int X { get; set; } + public int X { get; init; } /// /// Gets or sets top position in device pixels. /// - public int Y { get; set; } + public int Y { get; init; } /// /// Gets or sets width in device pixels. /// - public int Width { get; set; } + public int Width { get; init; } /// /// Gets or sets height in device pixels. /// - public int Height { get; set; } + public int Height { get; init; } /// /// Gets or sets width of the screen in device pixels where the window is located. /// - public int ScreenWidth { get; set; } + public int ScreenWidth { get; init; } /// /// Gets or sets height of the screen in device pixels where the window is located. /// - public int ScreenHeight { get; set; } + public int ScreenHeight { get; init; } /// /// Gets or sets DPI (dots per inch) of the display where the window is located. /// - public int Dpi { get; set; } + public int Dpi { get; init; } + + /// + /// Gets a value indicating whether the width and height of the window are valid (greater than 0). + /// + public bool IsSizeValid => Width > 0 && Height > 0; /// /// Converts the window position properties to a structure representing the physical window rectangle. diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/WindowPositionHelper.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/WindowPositionHelper.cs index bf8af589a6..766c4bf17c 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/WindowPositionHelper.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/WindowPositionHelper.cs @@ -18,7 +18,7 @@ internal static class WindowPositionHelper private const int MinimumVisibleSize = 100; private const int DefaultDpi = 96; - public static PointInt32? CalculateCenteredPosition(DisplayArea? displayArea, SizeInt32 windowSize, int windowDpi) + public static RectInt32? CenterOnDisplay(DisplayArea? displayArea, SizeInt32 windowSize, int windowDpi) { if (displayArea is null) { @@ -32,15 +32,9 @@ internal static class WindowPositionHelper } var targetDpi = GetDpiForDisplay(displayArea); - var predictedSize = ScaleSize(windowSize, windowDpi, targetDpi); - - // Clamp to work area - var width = Math.Min(predictedSize.Width, workArea.Width); - var height = Math.Min(predictedSize.Height, workArea.Height); - - return new PointInt32( - workArea.X + ((workArea.Width - width) / 2), - workArea.Y + ((workArea.Height - height) / 2)); + var scaledSize = ScaleSize(windowSize, windowDpi, targetDpi); + var clampedSize = ClampSize(scaledSize.Width, scaledSize.Height, workArea); + return CenterRectInWorkArea(clampedSize, workArea); } /// @@ -74,6 +68,10 @@ internal static class WindowPositionHelper savedRect = savedRect with { Width = DefaultWidth, Height = DefaultHeight }; } + // Remember the original size before DPI scaling - needed to compute + // gaps relative to the old screen when repositioning across displays. + var originalSize = new SizeInt32(savedRect.Width, savedRect.Height); + if (targetDpi != savedDpi) { savedRect = ScaleRect(savedRect, savedDpi, targetDpi); @@ -81,12 +79,17 @@ internal static class WindowPositionHelper var clampedSize = ClampSize(savedRect.Width, savedRect.Height, workArea); - var shouldRecenter = hasInvalidSize || - IsOffscreen(savedRect, workArea) || - savedScreenSize.Width != workArea.Width || - savedScreenSize.Height != workArea.Height; + if (hasInvalidSize) + { + return CenterRectInWorkArea(clampedSize, workArea); + } - if (shouldRecenter) + if (savedScreenSize.Width != workArea.Width || savedScreenSize.Height != workArea.Height) + { + return RepositionRelativeToWorkArea(savedRect, savedScreenSize, originalSize, clampedSize, workArea); + } + + if (IsOffscreen(savedRect, workArea)) { return CenterRectInWorkArea(clampedSize, workArea); } @@ -126,27 +129,92 @@ internal static class WindowPositionHelper private static RectInt32 ScaleRect(RectInt32 rect, int fromDpi, int toDpi) { + if (fromDpi <= 0 || toDpi <= 0 || fromDpi == toDpi) + { + return rect; + } + + // Don't scale position, that's absolute coordinates in virtual screen space var scale = (double)toDpi / fromDpi; return new RectInt32( - (int)Math.Round(rect.X * scale), - (int)Math.Round(rect.Y * scale), + rect.X, + rect.Y, (int)Math.Round(rect.Width * scale), (int)Math.Round(rect.Height * scale)); } - private static SizeInt32 ClampSize(int width, int height, RectInt32 workArea) => - new(Math.Min(width, workArea.Width), Math.Min(height, workArea.Height)); + private static SizeInt32 ClampSize(int width, int height, RectInt32 workArea) + { + return new SizeInt32(Math.Min(width, workArea.Width), Math.Min(height, workArea.Height)); + } - private static RectInt32 CenterRectInWorkArea(SizeInt32 size, RectInt32 workArea) => - new( + private static RectInt32 RepositionRelativeToWorkArea(RectInt32 savedRect, SizeInt32 savedScreenSize, SizeInt32 originalSize, SizeInt32 clampedSize, RectInt32 workArea) + { + // Treat each axis as a 3-zone grid (start / center / end) so that + // edge-snapped windows stay snapped and centered windows stay centered. + // We don't store the old work area origin, so we use the current one as a + // best estimate (correct when the same physical display changed resolution/DPI/taskbar). + var newX = ScaleAxisByZone(savedRect.X, originalSize.Width, clampedSize.Width, workArea.X, savedScreenSize.Width, workArea.Width); + var newY = ScaleAxisByZone(savedRect.Y, originalSize.Height, clampedSize.Height, workArea.Y, savedScreenSize.Height, workArea.Height); + + newX = Math.Clamp(newX, workArea.X, Math.Max(workArea.X, workArea.X + workArea.Width - clampedSize.Width)); + newY = Math.Clamp(newY, workArea.Y, Math.Max(workArea.Y, workArea.Y + workArea.Height - clampedSize.Height)); + + return new RectInt32(newX, newY, clampedSize.Width, clampedSize.Height); + } + + /// + /// Repositions a window along one axis using a 3-zone model (start / center / end). + /// The zone is determined by which third of the old screen the window center falls in. + /// Uses (pre-DPI-scaling) for gap calculations against + /// the old screen, and (post-scaling) for placement on the new screen. + /// + private static int ScaleAxisByZone(int savedPos, int oldWindowSize, int newWindowSize, int workAreaOrigin, int oldScreenSize, int newScreenSize) + { + if (oldScreenSize <= 0 || newScreenSize <= 0) + { + return savedPos; + } + + var gapFromStart = savedPos - workAreaOrigin; + var windowCenter = gapFromStart + (oldWindowSize / 2); + + if (windowCenter >= oldScreenSize / 3 && windowCenter <= oldScreenSize * 2 / 3) + { + // Center zone - keep centered + return workAreaOrigin + ((newScreenSize - newWindowSize) / 2); + } + + var gapFromEnd = oldScreenSize - gapFromStart - oldWindowSize; + + if (gapFromStart <= gapFromEnd) + { + // Start zone - preserve proportional distance from start edge + var rel = (double)gapFromStart / oldScreenSize; + return workAreaOrigin + (int)Math.Round(rel * newScreenSize); + } + else + { + // End zone - preserve proportional distance from end edge + var rel = (double)gapFromEnd / oldScreenSize; + return workAreaOrigin + newScreenSize - newWindowSize - (int)Math.Round(rel * newScreenSize); + } + } + + private static RectInt32 CenterRectInWorkArea(SizeInt32 size, RectInt32 workArea) + { + return new RectInt32( workArea.X + ((workArea.Width - size.Width) / 2), workArea.Y + ((workArea.Height - size.Height) / 2), size.Width, size.Height); + } - private static bool IsOffscreen(RectInt32 rect, RectInt32 workArea) => - rect.X + MinimumVisibleSize > workArea.X + workArea.Width || - rect.X + rect.Width - MinimumVisibleSize < workArea.X || - rect.Y + MinimumVisibleSize > workArea.Y + workArea.Height || - rect.Y + rect.Height - MinimumVisibleSize < workArea.Y; + private static bool IsOffscreen(RectInt32 rect, RectInt32 workArea) + { + return rect.X + MinimumVisibleSize > workArea.X + workArea.Width || + rect.X + rect.Width - MinimumVisibleSize < workArea.X || + rect.Y + MinimumVisibleSize > workArea.Y + workArea.Height || + rect.Y + rect.Height - MinimumVisibleSize < workArea.Y; + } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs index a6900773ab..2a63d5a2aa 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs @@ -72,6 +72,7 @@ public sealed partial class MainWindow : WindowEx, private readonly IThemeService _themeService; private readonly WindowThemeSynchronizer _windowThemeSynchronizer; private bool _ignoreHotKeyWhenFullScreen = true; + private bool _suppressDpiChange; private bool _themeServiceInitialized; // Session tracking for telemetry @@ -127,6 +128,16 @@ public sealed partial class MainWindow : WindowEx, _keyboardListener.SetProcessCommand(new CmdPalKeyboardService.ProcessCommand(HandleSummon)); + WM_TASKBAR_RESTART = PInvoke.RegisterWindowMessage("TaskbarCreated"); + + // LOAD BEARING: If you don't stick the pointer to HotKeyPrc into a + // member (and instead like, use a local), then the pointer we marshal + // into the WindowLongPtr will be useless after we leave this function, + // and our **WindProc will explode**. + _hotkeyWndProc = HotKeyPrc; + var hotKeyPrcPointer = Marshal.GetFunctionPointerForDelegate(_hotkeyWndProc); + _originalWndProc = Marshal.GetDelegateForFunctionPointer(PInvoke.SetWindowLongPtr(_hwnd, WINDOW_LONG_PTR_INDEX.GWL_WNDPROC, hotKeyPrcPointer)); + this.SetIcon(); AppWindow.Title = RS_.GetString("AppName"); RestoreWindowPosition(); @@ -153,16 +164,6 @@ public sealed partial class MainWindow : WindowEx, SizeChanged += WindowSizeChanged; RootElement.Loaded += RootElementLoaded; - WM_TASKBAR_RESTART = PInvoke.RegisterWindowMessage("TaskbarCreated"); - - // LOAD BEARING: If you don't stick the pointer to HotKeyPrc into a - // member (and instead like, use a local), then the pointer we marshal - // into the WindowLongPtr will be useless after we leave this function, - // and our **WindProc will explode**. - _hotkeyWndProc = HotKeyPrc; - var hotKeyPrcPointer = Marshal.GetFunctionPointerForDelegate(_hotkeyWndProc); - _originalWndProc = Marshal.GetDelegateForFunctionPointer(PInvoke.SetWindowLongPtr(_hwnd, WINDOW_LONG_PTR_INDEX.GWL_WNDPROC, hotKeyPrcPointer)); - // Load our settings, and then also wire up a settings changed handler HotReloadSettings(); App.Current.Services.GetService()!.SettingsChanged += SettingsChangedHandler; @@ -213,6 +214,11 @@ public sealed partial class MainWindow : WindowEx, // Now that our content has loaded, we can update our draggable regions UpdateRegionsForCustomTitleBar(); + // Also update regions when DPI changes. SizeChanged only fires when the logical + // (DIP) size changes — a DPI change that scales the physical size while preserving + // the DIP size won't trigger it, leaving drag regions at the old physical coordinates. + RootElement.XamlRoot.Changed += XamlRoot_Changed; + // Add dev ribbon if enabled if (!BuildInfo.IsCiBuild) { @@ -221,6 +227,8 @@ public sealed partial class MainWindow : WindowEx, } } + private void XamlRoot_Changed(XamlRoot sender, XamlRootChangedEventArgs args) => UpdateRegionsForCustomTitleBar(); + private void WindowSizeChanged(object sender, WindowSizeChangedEventArgs args) => UpdateRegionsForCustomTitleBar(); private void PositionCentered() @@ -231,16 +239,14 @@ public sealed partial class MainWindow : WindowEx, private void PositionCentered(DisplayArea displayArea) { - var position = WindowPositionHelper.CalculateCenteredPosition( + var rect = WindowPositionHelper.CenterOnDisplay( displayArea, AppWindow.Size, (int)this.GetDpiForWindow()); - if (position is not null) + if (rect is not null) { - // Use Move(), not MoveAndResize(). Windows auto-resizes on DPI change via WM_DPICHANGED; - // the helper already accounts for this when calculating the centered position. - AppWindow.Move((PointInt32)position); + MoveAndResizeDpiAware(rect.Value); } } @@ -249,29 +255,62 @@ public sealed partial class MainWindow : WindowEx, var settings = App.Current.Services.GetService(); if (settings?.LastWindowPosition is not { Width: > 0, Height: > 0 } savedPosition) { + // don't try to restore if the saved position is invalid, just recenter PositionCentered(); return; } - // MoveAndResize is safe here—we're restoring a saved state at startup, - // not moving a live window between displays. var newRect = WindowPositionHelper.AdjustRectForVisibility( savedPosition.ToPhysicalWindowRectangle(), new SizeInt32(savedPosition.ScreenWidth, savedPosition.ScreenHeight), savedPosition.Dpi); - AppWindow.MoveAndResize(newRect); + MoveAndResizeDpiAware(newRect); + } + + /// + /// Moves and resizes the window while suppressing WM_DPICHANGED. + /// The caller is expected to provide a rect already scaled for the target display's DPI. + /// Without suppression, the framework would apply its own DPI scaling on top, double-scaling the window. + /// + private void MoveAndResizeDpiAware(RectInt32 rect) + { + var originalMinHeight = MinHeight; + var originalMinWidth = MinWidth; + + _suppressDpiChange = true; + + try + { + // WindowEx is uses current DPI to calculate the minimum window size + MinHeight = 0; + MinWidth = 0; + AppWindow.MoveAndResize(rect); + } + finally + { + MinHeight = originalMinHeight; + MinWidth = originalMinWidth; + _suppressDpiChange = false; + } } private void UpdateWindowPositionInMemory() { + var placement = new WINDOWPLACEMENT { length = (uint)Marshal.SizeOf() }; + if (!PInvoke.GetWindowPlacement(_hwnd, ref placement)) + { + return; + } + + var rect = placement.rcNormalPosition; var displayArea = DisplayArea.GetFromWindowId(AppWindow.Id, DisplayAreaFallback.Nearest) ?? DisplayArea.Primary; _currentWindowPosition = new WindowPosition { - X = AppWindow.Position.X, - Y = AppWindow.Position.Y, - Width = AppWindow.Size.Width, - Height = AppWindow.Size.Height, + X = rect.X, + Y = rect.Y, + Width = rect.Width, + Height = rect.Height, Dpi = (int)this.GetDpiForWindow(), ScreenWidth = displayArea.WorkArea.Width, ScreenHeight = displayArea.WorkArea.Height, @@ -480,7 +519,7 @@ public sealed partial class MainWindow : WindowEx, { var originalScreen = new SizeInt32(_currentWindowPosition.ScreenWidth, _currentWindowPosition.ScreenHeight); var newRect = WindowPositionHelper.AdjustRectForVisibility(_currentWindowPosition.ToPhysicalWindowRectangle(), originalScreen, _currentWindowPosition.Dpi); - AppWindow.MoveAndResize(newRect); + MoveAndResizeDpiAware(newRect); } else { @@ -737,18 +776,12 @@ public sealed partial class MainWindow : WindowEx, var settings = serviceProvider.GetService(); if (settings is not null) { - settings.LastWindowPosition = new WindowPosition + // a quick sanity check, so we don't overwrite correct values + if (_currentWindowPosition.IsSizeValid) { - X = _currentWindowPosition.X, - Y = _currentWindowPosition.Y, - Width = _currentWindowPosition.Width, - Height = _currentWindowPosition.Height, - Dpi = _currentWindowPosition.Dpi, - ScreenWidth = _currentWindowPosition.ScreenWidth, - ScreenHeight = _currentWindowPosition.ScreenHeight, - }; - - SettingsModel.SaveSettings(settings); + settings.LastWindowPosition = _currentWindowPosition; + SettingsModel.SaveSettings(settings); + } } var extensionService = serviceProvider.GetService()!; @@ -1108,6 +1141,13 @@ public sealed partial class MainWindow : WindowEx, // Prevent the window from maximizing when double-clicking the title bar area case PInvoke.WM_NCLBUTTONDBLCLK: return (LRESULT)IntPtr.Zero; + + // When restoring a saved position across monitors with different DPIs, + // MoveAndResize already sets the correctly-scaled size. Suppress the + // framework's automatic DPI resize to avoid double-scaling. + case PInvoke.WM_DPICHANGED when _suppressDpiChange: + return (LRESULT)IntPtr.Zero; + case PInvoke.WM_HOTKEY: { var hotkeyIndex = (int)wParam.Value; diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/NativeMethods.txt b/src/modules/cmdpal/Microsoft.CmdPal.UI/NativeMethods.txt index 513db65b1a..6bb010e890 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/NativeMethods.txt +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/NativeMethods.txt @@ -66,4 +66,8 @@ GetStockObject GetModuleHandle GetWindowThreadProcessId -AttachThreadInput \ No newline at end of file +AttachThreadInput + +GetWindowPlacement +WINDOWPLACEMENT +WM_DPICHANGED \ No newline at end of file