mirror of
https://github.com/microsoft/PowerToys.git
synced 2025-12-16 19:57:57 +01:00
Improve thread safety and simplify monitor management
Refactored `DisplayChangeWatcher` to enhance thread safety by introducing `_initialEnumerationComplete` and ensuring state changes and event handling are dispatched to the UI thread. Removed `MonitorListChangedEventArgs` and the `MonitorsChanged` event from `MonitorManager`, simplifying monitor management logic. Updated `MainViewModel` to remove its dependency on `MonitorsChanged`. Cleaned up `TrayIconService` by removing unused `_showWindowAction` and simplifying resource fallback logic. Fixed minor wording inconsistencies in XML documentation. These changes improve maintainability, thread safety, and performance.
This commit is contained in:
@@ -25,6 +25,7 @@ public sealed partial class DisplayChangeWatcher : IDisposable
|
||||
private CancellationTokenSource? _debounceCts;
|
||||
private bool _isRunning;
|
||||
private bool _disposed;
|
||||
private bool _initialEnumerationComplete;
|
||||
|
||||
/// <summary>
|
||||
/// Event triggered when display configuration changes (after debounce period).
|
||||
@@ -74,9 +75,12 @@ public sealed partial class DisplayChangeWatcher : IDisposable
|
||||
_deviceWatcher.EnumerationCompleted += OnEnumerationCompleted;
|
||||
_deviceWatcher.Stopped += OnWatcherStopped;
|
||||
|
||||
// Reset state before starting (must be before Start() to avoid race)
|
||||
_initialEnumerationComplete = false;
|
||||
_isRunning = true;
|
||||
|
||||
// Start watching
|
||||
_deviceWatcher.Start();
|
||||
_isRunning = true;
|
||||
|
||||
Logger.LogInfo("[DisplayChangeWatcher] Started watching for display changes");
|
||||
}
|
||||
@@ -115,14 +119,36 @@ public sealed partial class DisplayChangeWatcher : IDisposable
|
||||
|
||||
private void OnDeviceAdded(DeviceWatcher sender, DeviceInformation args)
|
||||
{
|
||||
// Dispatch to UI thread to ensure thread-safe state access
|
||||
_dispatcherQueue.TryEnqueue(() =>
|
||||
{
|
||||
// Ignore events during initial enumeration or after disposal
|
||||
if (_disposed || !_initialEnumerationComplete)
|
||||
{
|
||||
Logger.LogDebug($"[DisplayChangeWatcher] Ignoring add: {args.Name} (disposed={_disposed}, enumComplete={_initialEnumerationComplete})");
|
||||
return;
|
||||
}
|
||||
|
||||
Logger.LogInfo($"[DisplayChangeWatcher] Display added: {args.Name} ({args.Id})");
|
||||
ScheduleDisplayChanged();
|
||||
});
|
||||
}
|
||||
|
||||
private void OnDeviceRemoved(DeviceWatcher sender, DeviceInformationUpdate args)
|
||||
{
|
||||
// Dispatch to UI thread to ensure thread-safe state access
|
||||
_dispatcherQueue.TryEnqueue(() =>
|
||||
{
|
||||
// Ignore events during initial enumeration or after disposal
|
||||
if (_disposed || !_initialEnumerationComplete)
|
||||
{
|
||||
Logger.LogDebug($"[DisplayChangeWatcher] Ignoring remove: {args.Id} (disposed={_disposed}, enumComplete={_initialEnumerationComplete})");
|
||||
return;
|
||||
}
|
||||
|
||||
Logger.LogInfo($"[DisplayChangeWatcher] Display removed: {args.Id}");
|
||||
ScheduleDisplayChanged();
|
||||
});
|
||||
}
|
||||
|
||||
private void OnDeviceUpdated(DeviceWatcher sender, DeviceInformationUpdate args)
|
||||
@@ -136,15 +162,23 @@ public sealed partial class DisplayChangeWatcher : IDisposable
|
||||
|
||||
private void OnEnumerationCompleted(DeviceWatcher sender, object args)
|
||||
{
|
||||
Logger.LogInfo("[DisplayChangeWatcher] Initial enumeration completed");
|
||||
|
||||
// Don't trigger refresh on initial enumeration - MainViewModel handles initial discovery
|
||||
// Dispatch to UI thread to ensure thread-safe state access
|
||||
_dispatcherQueue.TryEnqueue(() =>
|
||||
{
|
||||
_initialEnumerationComplete = true;
|
||||
Logger.LogInfo("[DisplayChangeWatcher] Initial enumeration completed, now responding to display changes");
|
||||
});
|
||||
}
|
||||
|
||||
private void OnWatcherStopped(DeviceWatcher sender, object args)
|
||||
{
|
||||
// Dispatch to UI thread to ensure thread-safe state access
|
||||
_dispatcherQueue.TryEnqueue(() =>
|
||||
{
|
||||
_isRunning = false;
|
||||
_initialEnumerationComplete = false;
|
||||
Logger.LogInfo("[DisplayChangeWatcher] Watcher stopped");
|
||||
});
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -1,32 +0,0 @@
|
||||
// Copyright (c) Microsoft Corporation
|
||||
// The Microsoft Corporation licenses this file to you under the MIT license.
|
||||
// See the LICENSE file in the project root for more information.
|
||||
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using PowerDisplay.Common.Models;
|
||||
|
||||
namespace PowerDisplay.Helpers
|
||||
{
|
||||
/// <summary>
|
||||
/// Monitor list changed event arguments
|
||||
/// </summary>
|
||||
public class MonitorListChangedEventArgs : EventArgs
|
||||
{
|
||||
public IReadOnlyList<Monitor> AddedMonitors { get; }
|
||||
|
||||
public IReadOnlyList<Monitor> RemovedMonitors { get; }
|
||||
|
||||
public IReadOnlyList<Monitor> AllMonitors { get; }
|
||||
|
||||
public MonitorListChangedEventArgs(
|
||||
IReadOnlyList<Monitor> addedMonitors,
|
||||
IReadOnlyList<Monitor> removedMonitors,
|
||||
IReadOnlyList<Monitor> allMonitors)
|
||||
{
|
||||
AddedMonitors = addedMonitors;
|
||||
RemovedMonitors = removedMonitors;
|
||||
AllMonitors = allMonitors;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -34,8 +34,6 @@ namespace PowerDisplay.Helpers
|
||||
|
||||
public IReadOnlyList<Monitor> Monitors => _monitors.AsReadOnly();
|
||||
|
||||
public event EventHandler<MonitorListChangedEventArgs>? MonitorsChanged;
|
||||
|
||||
public MonitorManager()
|
||||
{
|
||||
// Initialize controllers
|
||||
@@ -313,16 +311,6 @@ namespace PowerDisplay.Helpers
|
||||
}
|
||||
|
||||
// Trigger change events
|
||||
var addedMonitors = newMonitors.Where(m => !oldMonitors.Any(o => o.Id == m.Id)).ToList();
|
||||
var removedMonitors = oldMonitors.Where(o => !newMonitors.Any(m => m.Id == o.Id)).ToList();
|
||||
|
||||
if (addedMonitors.Count > 0 || removedMonitors.Count > 0)
|
||||
{
|
||||
MonitorsChanged?.Invoke(this, new MonitorListChangedEventArgs(
|
||||
addedMonitors.AsReadOnly(),
|
||||
removedMonitors.AsReadOnly(),
|
||||
_monitors.AsReadOnly()));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -24,7 +24,7 @@ namespace PowerDisplay.Helpers
|
||||
/// <param name="hwnd">Handle to the window.</param>
|
||||
/// <param name="msg">The message.</param>
|
||||
/// <param name="wParam">Additional message information.</param>
|
||||
/// <param name="lParam">Additional message information.</param>
|
||||
/// <param name="lParam">Additional message.</param>
|
||||
/// <returns>The result of the message processing.</returns>
|
||||
internal delegate nint WndProcDelegate(nint hwnd, uint msg, nuint wParam, nint lParam);
|
||||
|
||||
@@ -36,7 +36,6 @@ namespace PowerDisplay.Helpers
|
||||
private const uint WM_TRAY_ICON = PInvoke.WM_USER + 1;
|
||||
|
||||
private readonly ISettingsUtils _settingsUtils;
|
||||
private readonly Action _showWindowAction;
|
||||
private readonly Action _toggleWindowAction;
|
||||
private readonly Action _exitAction;
|
||||
private readonly Action _openSettingsAction;
|
||||
@@ -58,7 +57,6 @@ namespace PowerDisplay.Helpers
|
||||
Action openSettingsAction)
|
||||
{
|
||||
_settingsUtils = settingsUtils;
|
||||
_showWindowAction = showWindowAction;
|
||||
_toggleWindowAction = toggleWindowAction;
|
||||
_exitAction = exitAction;
|
||||
_openSettingsAction = openSettingsAction;
|
||||
@@ -175,14 +173,7 @@ namespace PowerDisplay.Helpers
|
||||
}
|
||||
catch
|
||||
{
|
||||
// Fallback if resource not found
|
||||
return key switch
|
||||
{
|
||||
"AppName" => "PowerDisplay",
|
||||
"TrayMenu_Settings" => "Settings",
|
||||
"TrayMenu_Exit" => "Exit",
|
||||
_ => key,
|
||||
};
|
||||
return "unknown";
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -200,55 +200,6 @@ public partial class MainViewModel
|
||||
}
|
||||
}
|
||||
|
||||
private void OnMonitorsChanged(object? sender, MonitorListChangedEventArgs e)
|
||||
{
|
||||
_dispatcherQueue.TryEnqueue(() =>
|
||||
{
|
||||
// Load settings to check for hidden monitors
|
||||
var settings = _settingsUtils.GetSettingsOrDefault<PowerDisplaySettings>(PowerDisplaySettings.ModuleName);
|
||||
var hiddenMonitorIds = GetHiddenMonitorIds(settings);
|
||||
|
||||
// Handle monitors being added or removed
|
||||
if (e.AddedMonitors.Count > 0)
|
||||
{
|
||||
foreach (var monitor in e.AddedMonitors)
|
||||
{
|
||||
// Skip monitors that are marked as hidden
|
||||
if (hiddenMonitorIds.Contains(monitor.Id))
|
||||
{
|
||||
Logger.LogInfo($"[OnMonitorsChanged] Skipping hidden monitor (added): {monitor.Name} ({monitor.Id})");
|
||||
continue;
|
||||
}
|
||||
|
||||
var existingVm = GetMonitorViewModel(monitor.Id);
|
||||
if (existingVm == null)
|
||||
{
|
||||
var vm = new MonitorViewModel(monitor, _monitorManager, this);
|
||||
Monitors.Add(vm);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (e.RemovedMonitors.Count > 0)
|
||||
{
|
||||
foreach (var monitor in e.RemovedMonitors)
|
||||
{
|
||||
var vm = GetMonitorViewModel(monitor.Id);
|
||||
if (vm != null)
|
||||
{
|
||||
Monitors.Remove(vm);
|
||||
vm.Dispose();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
StatusText = $"Monitor list updated ({Monitors.Count} total)";
|
||||
|
||||
// Note: SaveMonitorsToSettings() is called by UpdateMonitorList() after full scan completes
|
||||
// to avoid double-firing the refresh event during re-scan operations
|
||||
});
|
||||
}
|
||||
|
||||
private MonitorViewModel? GetMonitorViewModel(string monitorId)
|
||||
=> Monitors.FirstOrDefault(vm => vm.Id == monitorId);
|
||||
|
||||
|
||||
@@ -68,9 +68,6 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable
|
||||
// Initialize the monitor manager
|
||||
_monitorManager = new MonitorManager();
|
||||
|
||||
// Subscribe to events
|
||||
_monitorManager.MonitorsChanged += OnMonitorsChanged;
|
||||
|
||||
// Initialize and start LightSwitch integration listener
|
||||
_lightSwitchListener = new LightSwitchListener();
|
||||
_lightSwitchListener.ThemeChanged += OnLightSwitchThemeChanged;
|
||||
|
||||
Reference in New Issue
Block a user