Mirrors/PowerToys
1
0
Fork 0
mirror of https://github.com/microsoft/PowerToys.git synced 2026-08-29 01:59:34 +02:00
Code Issues Packages Projects Releases Wiki Activity

CmdPal: Fix Command Palette Dock breaking on monitor topology changes (#49814)

Browse Source 
Docking or undocking a laptop, or flipping display modes with Win+P, can
leave the Dock empty, missing, or misconfigured. Fixes #48516.

The root of it: the Dock's per-monitor config leans on a stable hardware
ID for each monitor. Right after a `WM_DISPLAYCHANGE`, before Windows
has settled the new topology, that lookup can come back empty or fall
back to a volatile GDI name. The reconciler then reads that as "hey, a
new monitor showed up" and creates a fresh, disabled, empty config for a
monitor that never actually left. On top of that, a burst of
`WM_DISPLAYCHANGE` messages during a mode switch each triggered an
immediate write to settings, so one bad intermediate snapshot could get
baked in permanently. And since only the Dock window itself was
listening for `WM_DISPLAYCHANGE`, the Settings page's monitor list could
go stale whenever no Dock window happened to be alive.

## The plan

- Retry the stable-ID lookup a few times before giving up and falling
back to the volatile name.
- Debounce monitor-change handling so a flurry of `WM_DISPLAYCHANGE`
events settles down before we reconcile and persist, instead of writing
every half-finished intermediate state.
- Have the main window forward `WM_DISPLAYCHANGE` too, so the monitor
cache stays fresh even when the Dock is off or has no windows up.
- Teach the reconciler to reassociate a secondary monitor's config with
its new ID when there's exactly one unmatched monitor and one unmatched
config, the Win+P round trip case, instead of treating it as new
hardware.
- Added tests covering the transient ID fallback, the ambiguous
multi-monitor case, and the Win+P reassociation.

Scaling behavior when the Dock lands on a monitor with a different DPI
is a separate issue (#48466) and isn't touched here.

---------

Copilot-Session: d2bc281b-062c-4e6e-9356-bdb7a2ef9e1e
This commit is contained in:
Michael Jolley
2026-08-14 16:52:18 -05:00
committed by GitHub
parent 523409ed06
commit bcb2ed6dc7
5 changed files with 213 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
src/modules/cmdpal/Microsoft.CmdPal.UI/Dock/DockWindow.xaml.cs
View File

@@ -614,6 +614,30 @@ public sealed partial class DockWindow : WindowEx,
}
}
internal void RefreshForMonitorChange()
{
if (_isDisposed)
{
return;
}
RefreshTargetMonitor();
if (_appBarData.hWnd != IntPtr.Zero)
{
// The Shell caches the monitor coordinates from the original
// ABM_NEW registration, so after a topology change the stale
// AppBar rect cannot be repositioned correctly. Destroy and
// recreate to re-register with the new monitor geometry.
DestroyAppBar(_hwnd);
CreateAppBar(_hwnd);
}
else
{
UpdateWindowPosition();
}
}
private void RefreshSideOverride()
{
if (_targetMonitor is null)
@@ -1295,45 +1319,6 @@ public sealed partial class DockWindow : WindowEx,
DispatcherQueue.TryEnqueue(HandleWorkAreaChanged);
}
}
else if (msg == PInvoke.WM_DISPLAYCHANGE)
{
Logger.LogDebug("WM_DISPLAYCHANGE");
// Invalidate the monitor cache so DockWindowManager can reconcile
_monitorService.NotifyMonitorsChanged();
// Use dispatcher to ensure we're on the UI thread.
// Refresh _targetMonitor before re-positioning: the MonitorInfo
// captured at construction is an immutable record, so its Bounds
// are stale after a topology change (e.g. an external display was
// disconnected, shifting our monitor's virtual-screen origin).
// Without this, UpdateAppBarDataForEdge would compute the AppBar
// rect against the old coordinates and produce a wildly incorrect
// size/position.
DispatcherQueue.TryEnqueue(() =>
{
if (_isDisposed)
{
return;
}
RefreshTargetMonitor();
if (_appBarData.hWnd != IntPtr.Zero)
{
// The Shell caches the monitor coordinates from the original
// ABM_NEW registration, so after a topology change the stale
// AppBar rect cannot be repositioned correctly. Destroy and
// recreate to re-register with the new monitor geometry.
DestroyAppBar(_hwnd);
CreateAppBar(_hwnd);
}
else
{
UpdateWindowPosition();
}
});
}
else if (msg == PInvoke.WM_MOUSEMOVE)
{
HandleMouseMoveForAutoHide();

38
src/modules/cmdpal/Microsoft.CmdPal.UI/Dock/DockWindowManager.cs
View File

@@ -2,6 +2,7 @@
// The Microsoft Corporation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
using CommunityToolkit.WinUI;
using Microsoft.CmdPal.UI.ViewModels;
using Microsoft.CmdPal.UI.ViewModels.Dock;
using Microsoft.CmdPal.UI.ViewModels.Models;
@@ -26,6 +27,15 @@ public sealed partial class DockWindowManager : IDisposable
private bool _disposed;
private int _syncing;
/// <summary>
/// Debounces rapid-fire monitor-change notifications (several WM_DISPLAYCHANGE messages
/// during a Win+P switch or dock/undock). Without it, each intermediate topology
/// snapshot gets reconciled and persisted, which can permanently corrupt dock configs
/// even though things settle fine on their own a moment later.
/// </summary>
private static readonly TimeSpan MonitorsChangedDebounceInterval = TimeSpan.FromMilliseconds(400);
private readonly DispatcherQueueTimer _monitorsChangedDebounceTimer;
private bool? _lastSyncedEnableDock;
private DockSettings? _lastSyncedDockSettings;
@@ -37,6 +47,7 @@ public sealed partial class DockWindowManager : IDisposable
_monitorService = monitorService;
_settingsService = settingsService;
_dispatcherQueue = dispatcherQueue;
_monitorsChangedDebounceTimer = _dispatcherQueue.CreateTimer();
_monitorService.MonitorsChanged += OnMonitorsChanged;
_settingsService.SettingsChanged += OnSettingsChanged;
@@ -74,7 +85,7 @@ public sealed partial class DockWindowManager : IDisposable
/// <summary>
/// Synchronizes running dock windows to match the current settings and connected monitors.
/// </summary>
public void SyncDocksToSettings()
public void SyncDocksToSettings(bool refreshDockWindows = false)
{
if (Interlocked.CompareExchange(ref _syncing, 1, 0) != 0)
{
@@ -83,7 +94,7 @@ public sealed partial class DockWindowManager : IDisposable
try
{
SyncDocksToSettingsCore();
SyncDocksToSettingsCore(refreshDockWindows);
}
finally
{
@@ -91,7 +102,7 @@ public sealed partial class DockWindowManager : IDisposable
}
}
private void SyncDocksToSettingsCore()
private void SyncDocksToSettingsCore(bool refreshDockWindows)
{
var settings = _settingsService.Settings;
if (!settings.EnableDock)
@@ -167,6 +178,16 @@ public sealed partial class DockWindowManager : IDisposable
dock.ViewModel.Dispose();
}
}
if (!refreshDockWindows)
{
return;
}
foreach (var (_, (window, _)) in _docks)
{
window.RefreshForMonitorChange();
}
}
public void Dispose()
@@ -180,6 +201,8 @@ public sealed partial class DockWindowManager : IDisposable
_monitorService.MonitorsChanged -= OnMonitorsChanged;
_settingsService.SettingsChanged -= OnSettingsChanged;
_monitorsChangedDebounceTimer.Stop();
HideDocks();
}
@@ -211,10 +234,15 @@ public sealed partial class DockWindowManager : IDisposable
{
_dispatcherQueue.TryEnqueue(() =>
{
if (!_disposed)
if (_disposed)
{
SyncDocksToSettings();
return;
}
_monitorsChangedDebounceTimer.Debounce(
() => SyncDocksToSettings(refreshDockWindows: true),
interval: MonitorsChangedDebounceInterval,
immediate: false);
});
}

10
src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs
View File

@@ -73,6 +73,7 @@ public sealed partial class MainWindow : WindowEx,
private readonly LocalKeyboardListener _localKeyboardListener;
private readonly HiddenOwnerWindowBehavior _hiddenOwnerBehavior = new();
private readonly ICmdPalProtocolActivation _protocolActivation;
private readonly ViewModels.Models.IMonitorService _monitorService;
private readonly IThemeService _themeService;
private readonly WindowThemeSynchronizer _windowThemeSynchronizer;
private readonly List<long> _breakthroughTimestamps = [];
@@ -135,6 +136,7 @@ public sealed partial class MainWindow : WindowEx,
public MainWindow()
{
_protocolActivation = App.Current.Services.GetRequiredService<ICmdPalProtocolActivation>();
_monitorService = App.Current.Services.GetRequiredService<ViewModels.Models.IMonitorService>();
InitializeComponent();
@@ -1768,6 +1770,14 @@ public sealed partial class MainWindow : WindowEx,
return (LRESULT)IntPtr.Zero;
}
// Unlike DockWindow instances, MainWindow always exists, so it's the one
// reliable place to catch topology changes. Without this, the Settings page's
// monitor list goes stale whenever no dock window is around to see WM_DISPLAYCHANGE.
case PInvoke.WM_DISPLAYCHANGE:
Logger.LogDebug("MainWindow WM_DISPLAYCHANGE");
_monitorService.NotifyMonitorsChanged();
break;
default:
if (uMsg == WM_TASKBAR_RESTART)
{

61
src/modules/cmdpal/Microsoft.CmdPal.UI/Services/MonitorService.cs
View File

@@ -30,6 +30,20 @@ public sealed class MonitorService : IMonitorService
/// <inheritdoc/>
public IReadOnlyList<MonitorInfo> GetMonitors()
{
// Check the cache first without paying for a retry-with-sleep under the lock.
lock (_lock)
{
if (_cachedSnapshot is not null)
{
return _cachedSnapshot;
}
}
// BuildDisplayInfoMapWithRetry sleeps between attempts, so it runs unlocked. Another
// thread might race us and rebuild the map too, but that's cheaper than blocking
// every caller for up to 100ms.
var displayInfo = BuildDisplayInfoMapWithRetry();
lock (_lock)
{
if (_cachedSnapshot is not null)
@@ -37,7 +51,7 @@ public sealed class MonitorService : IMonitorService
return _cachedSnapshot;
}
_cachedMonitors = EnumerateMonitors();
_cachedMonitors = EnumerateMonitors(displayInfo);
_cachedSnapshot = _cachedMonitors.AsReadOnly();
return _cachedSnapshot;
}
@@ -104,10 +118,17 @@ public sealed class MonitorService : IMonitorService
MonitorsChanged?.Invoke(this, EventArgs.Empty);
}
private static unsafe List<MonitorInfo> EnumerateMonitors()
/// <summary>
/// Number of immediate attempts to build the stable-ID display info map before giving up.
/// Right after WM_DISPLAYCHANGE, the Display Configuration API can transiently fail or
/// return an incomplete topology while Windows is still settling. Immediate retries
/// avoid blocking the UI thread while giving the API another chance to return stable data.
/// </summary>
private const int DisplayInfoMapRetryCount = 3;
private static unsafe List<MonitorInfo> EnumerateMonitors(Dictionary<string, (string FriendlyName, string DevicePath)> displayInfo)
{
var monitors = new List<MonitorInfo>();
var displayInfo = BuildDisplayInfoMap();
PInvoke.EnumDisplayMonitors(
HDC.Null,
@@ -173,14 +194,41 @@ public sealed class MonitorService : IMonitorService
return monitors;
}
/// <summary>
/// Calls <see cref="BuildDisplayInfoMap"/>, retrying a few times if it comes back
/// incomplete. Right after WM_DISPLAYCHANGE the API can transiently fail or only resolve
/// some active sources, and a partial map would leave those monitors on their volatile
/// GDI name, tricking <see cref="Settings.MonitorConfigReconciler"/> into treating a
/// still-connected monitor as brand new.
/// </summary>
private static Dictionary<string, (string FriendlyName, string DevicePath)> BuildDisplayInfoMapWithRetry()
{
var map = new Dictionary<string, (string FriendlyName, string DevicePath)>(StringComparer.OrdinalIgnoreCase);
for (var attempt = 0; attempt < DisplayInfoMapRetryCount; attempt++)
{
map = BuildDisplayInfoMap(out var expectedSourceCount);
if (map.Count >= expectedSourceCount && expectedSourceCount > 0)
{
return map;
}
}
return map;
}
/// <summary>
/// Builds a map from GDI device name (e.g. <c>\\.\DISPLAY1</c>) to display metadata
/// (friendly name and stable device path) using the Display Configuration APIs.
/// Returns an empty dictionary on failure so callers can fall back gracefully.
/// <paramref name="expectedSourceCount"/> is the number of distinct GDI source device
/// names among the active paths, not the raw path count: in Duplicate/clone mode several
/// paths share one source, so comparing against the path count would never be satisfied.
/// </summary>
private static unsafe Dictionary<string, (string FriendlyName, string DevicePath)> BuildDisplayInfoMap()
private static unsafe Dictionary<string, (string FriendlyName, string DevicePath)> BuildDisplayInfoMap(out uint expectedSourceCount)
{
var map = new Dictionary<string, (string FriendlyName, string DevicePath)>(StringComparer.OrdinalIgnoreCase);
expectedSourceCount = 0;
try
{
@@ -209,9 +257,12 @@ public sealed class MonitorService : IMonitorService
return map;
}
var expectedSources = new HashSet<(LUID AdapterId, uint Id)>();
for (var i = 0; i < pathCount; i++)
{
var path = paths[i];
expectedSources.Add((path.sourceInfo.adapterId, path.sourceInfo.id));
// Get the GDI device name from the source info
var sourceName = default(DISPLAYCONFIG_SOURCE_DEVICE_NAME);
@@ -250,6 +301,8 @@ public sealed class MonitorService : IMonitorService
map.TryAdd(gdiName, (friendly ?? string.Empty, devicePath ?? string.Empty));
}
}
expectedSourceCount = (uint)expectedSources.Count;
}
catch (Exception ex) when (ex is not OutOfMemoryException)
{

101
src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/DockMultiMonitorTests.cs
View File

@@ -5,6 +5,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Text.Json;
using Microsoft.CmdPal.UI.ViewModels.Dock;
using Microsoft.CmdPal.UI.ViewModels.Models;
@@ -271,27 +272,103 @@ public class DockMultiMonitorTests
}
[TestMethod]
public void Reconciler_FuzzyMatch_DoesNotMatchNonPrimaryMonitors()
public void Reconciler_UnmatchedSecondary_DoesNotConsumeExistingConfig()
{
// Config has stale stable ID for a non-primary monitor
// A secondary ID change is ambiguous, so preserve the old config and create a
// separate default rather than moving the old settings onto the new monitor.
var configs = ImmutableList.Create(
new DockMonitorConfig { MonitorDeviceId = PrimaryMonitor.StableId, Enabled = true, IsPrimary = true },
new DockMonitorConfig { MonitorDeviceId = @"\\?\DISPLAY#STALE#4&eee&0&UID333#{guidStale}", Enabled = true, IsPrimary = false, IsCustomized = true, LastSeen = DateTime.UtcNow });
new DockMonitorConfig
{
MonitorDeviceId = @"\\?\DISPLAY#STALE#4&eee&0&UID333#{guidStale}",
Enabled = true,
IsPrimary = false,
IsCustomized = true,
StartBands = ImmutableList.Create(new DockBandSettings { ProviderId = "p", CommandId = "c" }),
LastSeen = DateTime.UtcNow,
});
// Current monitors have primary + a different secondary
// Current monitors have primary + a secondary whose stable ID changed
var monitors = new List<MonitorInfo> { PrimaryMonitor, SecondaryMonitor };
var result = MonitorConfigReconciler.Reconcile(configs, monitors);
// Primary keeps its config, new secondary gets a fresh customized config,
// stale secondary is retained at end for future reconnection
Assert.AreEqual(3, result.Count);
Assert.AreEqual(PrimaryMonitor.StableId, result[0].MonitorDeviceId);
Assert.AreEqual(SecondaryMonitor.StableId, result[1].MonitorDeviceId);
Assert.IsTrue(result[1].IsCustomized, "New secondary should get an empty-bands customized config.");
Assert.AreEqual(0, result[1].StartBands?.Count ?? 0, "New secondary should start with empty bands.");
Assert.AreEqual(@"\\?\DISPLAY#STALE#4&eee&0&UID333#{guidStale}", result[2].MonitorDeviceId, "Stale config should be preserved.");
Assert.IsTrue(result[2].IsCustomized, "Stale config should retain its customizations.");
var newConfig = result.FirstOrDefault(c => c.MonitorDeviceId == SecondaryMonitor.StableId);
var retainedConfig = result.FirstOrDefault(c => c.MonitorDeviceId.Contains("STALE", StringComparison.OrdinalIgnoreCase));
Assert.IsNotNull(newConfig);
Assert.IsNotNull(retainedConfig);
Assert.IsFalse(newConfig!.Enabled, "An unmatched secondary monitor should get a disabled default.");
Assert.AreEqual(1, retainedConfig!.StartBands?.Count ?? 0, "The old config should remain available for reconnection.");
}
[TestMethod]
public void Reconciler_MultipleUnmatchedSecondaryMonitors_PreservesConfigs()
{
// Two unmatched secondary monitors and one unmatched secondary config: ambiguous,
// so reconciliation must not guess and should fall back to creating fresh configs.
var thirdMonitor = SecondaryMonitor with
{
DeviceId = @"\\.\DISPLAY3",
StableId = @"\\?\DISPLAY#THIRD#4&ccc&0&UID333#{guid3}",
DisplayName = "Display 3",
};
var configs = ImmutableList.Create(
new DockMonitorConfig { MonitorDeviceId = PrimaryMonitor.StableId, Enabled = true, IsPrimary = true },
new DockMonitorConfig
{
MonitorDeviceId = @"\\?\DISPLAY#STALE#4&eee&0&UID333#{guidStale}",
Enabled = true,
IsPrimary = false,
IsCustomized = true,
LastSeen = DateTime.UtcNow,
});
var monitors = new List<MonitorInfo> { PrimaryMonitor, SecondaryMonitor, thirdMonitor };
var result = MonitorConfigReconciler.Reconcile(configs, monitors);
// Both new secondary monitors get fresh configs; the stale one is retained for reconnection.
Assert.AreEqual(4, result.Count);
Assert.IsTrue(result.Any(c => c.MonitorDeviceId == SecondaryMonitor.StableId));
Assert.IsTrue(result.Any(c => c.MonitorDeviceId == thirdMonitor.StableId));
Assert.IsTrue(result.Any(c => c.MonitorDeviceId == @"\\?\DISPLAY#STALE#4&eee&0&UID333#{guidStale}"));
}
[TestMethod]
public void Reconciler_TransientStableIdFallbackToDeviceId_PreservesExistingConfig()
{
// Simulates the StableId momentarily falling back to the volatile GDI DeviceId
// right after WM_DISPLAYCHANGE (Display Configuration API not yet settled).
// The fallback is ambiguous, so keep the stable-ID config instead of moving it.
var degradedSecondary = SecondaryMonitor with { StableId = SecondaryMonitor.DeviceId };
var configs = ImmutableList.Create(
new DockMonitorConfig { MonitorDeviceId = PrimaryMonitor.StableId, Enabled = true, IsPrimary = true },
new DockMonitorConfig
{
MonitorDeviceId = SecondaryMonitor.StableId,
Enabled = true,
IsPrimary = false,
IsCustomized = true,
StartBands = ImmutableList.Create(new DockBandSettings { ProviderId = "p", CommandId = "c" }),
LastSeen = DateTime.UtcNow,
});
var monitors = new List<MonitorInfo> { PrimaryMonitor, degradedSecondary };
var result = MonitorConfigReconciler.Reconcile(configs, monitors);
Assert.AreEqual(3, result.Count);
var retainedConfig = result.FirstOrDefault(c => c.MonitorDeviceId == SecondaryMonitor.StableId);
var fallbackConfig = result.FirstOrDefault(c => c.MonitorDeviceId == degradedSecondary.StableId);
Assert.IsNotNull(retainedConfig, "The existing stable-ID config should remain available.");
Assert.IsNotNull(fallbackConfig, "The ambiguous fallback monitor should receive a default config.");
Assert.IsTrue(retainedConfig!.Enabled);
Assert.IsTrue(retainedConfig.IsCustomized);
Assert.AreEqual(1, retainedConfig.StartBands?.Count ?? 0, "Pinned bands should remain with the original config.");
Assert.IsFalse(fallbackConfig!.Enabled, "An unmatched secondary monitor should start disabled.");
}
// --- JSON serialization round-trip ---