diff --git a/src/modules/powerdisplay/PowerDisplay/Commands/RelayCommand.cs b/src/modules/powerdisplay/PowerDisplay/Commands/RelayCommand.cs index 6f4f12dd41..66abb1a297 100644 --- a/src/modules/powerdisplay/PowerDisplay/Commands/RelayCommand.cs +++ b/src/modules/powerdisplay/PowerDisplay/Commands/RelayCommand.cs @@ -4,6 +4,7 @@ using System; using System.Windows.Input; +using ManagedCommon; namespace PowerDisplay.Commands { @@ -23,9 +24,35 @@ namespace PowerDisplay.Commands public event EventHandler? CanExecuteChanged; - public bool CanExecute(object? parameter) => _canExecute?.Invoke() ?? true; + public bool CanExecute(object? parameter) + { + if (_canExecute == null) + { + return true; + } - public void Execute(object? parameter) => _execute(); + try + { + return _canExecute.Invoke(); + } + catch (Exception ex) + { + Logger.LogError($"CanExecute failed: {ex.Message}"); + return false; + } + } + + public void Execute(object? parameter) + { + try + { + _execute(); + } + catch (Exception ex) + { + Logger.LogError($"Command execution failed: {ex.Message}"); + } + } public void RaiseCanExecuteChanged() => CanExecuteChanged?.Invoke(this, EventArgs.Empty); } @@ -47,9 +74,35 @@ namespace PowerDisplay.Commands public event EventHandler? CanExecuteChanged; - public bool CanExecute(object? parameter) => _canExecute?.Invoke((T?)parameter) ?? true; + public bool CanExecute(object? parameter) + { + if (_canExecute == null) + { + return true; + } - public void Execute(object? parameter) => _execute((T?)parameter); + try + { + return _canExecute.Invoke((T?)parameter); + } + catch (Exception ex) + { + Logger.LogError($"CanExecute failed: {ex.Message}"); + return false; + } + } + + public void Execute(object? parameter) + { + try + { + _execute((T?)parameter); + } + catch (Exception ex) + { + Logger.LogError($"Command execution failed: {ex.Message}"); + } + } public void RaiseCanExecuteChanged() => CanExecuteChanged?.Invoke(this, EventArgs.Empty); } diff --git a/src/modules/powerdisplay/PowerDisplay/Configuration/AppConstants.cs b/src/modules/powerdisplay/PowerDisplay/Configuration/AppConstants.cs index 17418bd536..30467dd4b4 100644 --- a/src/modules/powerdisplay/PowerDisplay/Configuration/AppConstants.cs +++ b/src/modules/powerdisplay/PowerDisplay/Configuration/AppConstants.cs @@ -60,6 +60,11 @@ namespace PowerDisplay.Configuration public const int AnimationDelayMs = 100; public const int LayoutUpdateDelayMs = 50; public const int MonitorDiscoveryDelayMs = 200; + + /// + /// Debounce delay for slider controls in milliseconds + /// + public const int SliderDebounceDelayMs = 300; } /// diff --git a/src/modules/powerdisplay/PowerDisplay/Core/MonitorManager.cs b/src/modules/powerdisplay/PowerDisplay/Core/MonitorManager.cs index 30da5c2d9e..1906b7ab86 100644 --- a/src/modules/powerdisplay/PowerDisplay/Core/MonitorManager.cs +++ b/src/modules/powerdisplay/PowerDisplay/Core/MonitorManager.cs @@ -175,7 +175,8 @@ namespace PowerDisplay.Core }); var initializedMonitors = await Task.WhenAll(initTasks); - newMonitors.AddRange(initializedMonitors.Where(m => m != null)); + var validMonitors = initializedMonitors.Where(m => m != null).Cast(); + newMonitors.AddRange(validMonitors); } // Update monitor list diff --git a/src/modules/powerdisplay/PowerDisplay/Helpers/MonitorStateManager.cs b/src/modules/powerdisplay/PowerDisplay/Helpers/MonitorStateManager.cs index 0e9a5a98b2..a96456bdcb 100644 --- a/src/modules/powerdisplay/PowerDisplay/Helpers/MonitorStateManager.cs +++ b/src/modules/powerdisplay/PowerDisplay/Helpers/MonitorStateManager.cs @@ -140,8 +140,6 @@ namespace PowerDisplay.Helpers // Reset timer to debounce rapid updates (e.g., during slider drag) _saveTimer.Change(SaveDebounceMs, Timeout.Infinite); - - Logger.LogTrace($"[State] Updated {property}={value} for monitor HardwareId='{hardwareId}', save scheduled"); } catch (Exception ex) { diff --git a/src/modules/powerdisplay/PowerDisplay/Helpers/SimpleDebouncer.cs b/src/modules/powerdisplay/PowerDisplay/Helpers/SimpleDebouncer.cs index 6e91f435d7..3b925674dc 100644 --- a/src/modules/powerdisplay/PowerDisplay/Helpers/SimpleDebouncer.cs +++ b/src/modules/powerdisplay/PowerDisplay/Helpers/SimpleDebouncer.cs @@ -34,43 +34,9 @@ namespace PowerDisplay.Helpers /// Debounce an async action. Cancels previous invocation if still pending. /// /// Async action to execute after delay - public async void Debounce(Func action) + public void Debounce(Func action) { - if (_disposed) - { - return; - } - - CancellationTokenSource cts; - - lock (_lock) - { - // Cancel previous invocation - _cts?.Cancel(); - _cts = new CancellationTokenSource(); - cts = _cts; - } - - try - { - // Wait for quiet period - await Task.Delay(_delayMs, cts.Token); - - // Execute action if not cancelled - if (!cts.Token.IsCancellationRequested) - { - await action(); - } - } - catch (OperationCanceledException) - { - // Expected when debouncing - a newer call cancelled this one - Logger.LogTrace("Debounced action cancelled (replaced by newer call)"); - } - catch (Exception ex) - { - Logger.LogError($"Debounced action failed: {ex.Message}"); - } + _ = DebounceAsync(action); } /// @@ -78,13 +44,68 @@ namespace PowerDisplay.Helpers /// public void Debounce(Action action) { - Debounce(() => + _ = DebounceAsync(() => { action(); return Task.CompletedTask; }); } + private async Task DebounceAsync(Func action) + { + if (_disposed) + { + return; + } + + CancellationTokenSource cts; + CancellationTokenSource? oldCts = null; + + lock (_lock) + { + // Store old CTS to dispose later + oldCts = _cts; + + // Create new CTS + _cts = new CancellationTokenSource(); + cts = _cts; + } + + // Dispose old CTS outside the lock to avoid blocking + if (oldCts != null) + { + try + { + oldCts.Cancel(); + oldCts.Dispose(); + } + catch + { + // Ignore disposal errors + } + } + + try + { + // Wait for quiet period + await Task.Delay(_delayMs, cts.Token).ConfigureAwait(false); + + // Execute action if not cancelled + if (!cts.Token.IsCancellationRequested) + { + await action().ConfigureAwait(false); + } + } + catch (OperationCanceledException) + { + // Expected when debouncing - a newer call cancelled this one + } + catch (Exception ex) + { + Logger.LogError($"Debounced action failed: {ex.Message}"); + } + } + public void Dispose() { if (_disposed) diff --git a/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/App.xaml.cs b/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/App.xaml.cs index 9a17f1985c..52e3aeeeb9 100644 --- a/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/App.xaml.cs +++ b/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/App.xaml.cs @@ -114,7 +114,8 @@ namespace PowerDisplay { Logger.LogError($"[EVENT] _mainWindow type mismatch, actual type: {_mainWindow?.GetType().Name}"); } - }); + }, + CancellationToken.None); NativeEventWaiter.WaitForEventLoop( TogglePowerDisplayEvent, @@ -130,7 +131,8 @@ namespace PowerDisplay { Logger.LogError($"[EVENT] _mainWindow type mismatch for toggle"); } - }); + }, + CancellationToken.None); NativeEventWaiter.WaitForEventLoop( TerminatePowerDisplayEvent, @@ -138,7 +140,8 @@ namespace PowerDisplay { Logger.LogInfo("Received terminate event - exiting immediately"); Environment.Exit(0); - }); + }, + CancellationToken.None); // Note: PowerDisplay.exe should NOT listen to RefreshMonitorsEvent // That event is sent BY PowerDisplay TO Settings UI for one-way notification @@ -155,7 +158,8 @@ namespace PowerDisplay mainWindow.ViewModel.ApplySettingsFromUI(); } }); - }); + }, + CancellationToken.None); NativeEventWaiter.WaitForEventLoop( ApplyColorTemperatureEvent, @@ -169,7 +173,8 @@ namespace PowerDisplay mainWindow.ViewModel.ApplyColorTemperatureFromSettings(); } }); - }); + }, + CancellationToken.None); // Monitor Runner process (backup exit mechanism) if (_powerToysRunnerPid > 0) diff --git a/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs b/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs index 1ed5649b66..7b5894ce28 100644 --- a/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs +++ b/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs @@ -772,8 +772,6 @@ public partial class MainViewModel : INotifyPropertyChanged, IDisposable // This is thread-safe - _stateManager has internal locking // No UI thread operations, no ObservableCollection access _stateManager.UpdateMonitorParameter(hardwareId, property, value); - - Logger.LogTrace($"[State] Queued setting change for HardwareId '{hardwareId}': {property}={value}"); } catch (Exception ex) { diff --git a/src/modules/powerdisplay/PowerDisplay/ViewModels/MonitorViewModel.cs b/src/modules/powerdisplay/PowerDisplay/ViewModels/MonitorViewModel.cs index fdc6ff37de..787b7a82fa 100644 --- a/src/modules/powerdisplay/PowerDisplay/ViewModels/MonitorViewModel.cs +++ b/src/modules/powerdisplay/PowerDisplay/ViewModels/MonitorViewModel.cs @@ -10,6 +10,7 @@ using System.Windows.Input; using ManagedCommon; using Microsoft.UI.Xaml; using PowerDisplay.Commands; +using PowerDisplay.Configuration; using PowerDisplay.Core; using PowerDisplay.Core.Models; using PowerDisplay.Helpers; @@ -27,9 +28,9 @@ public partial class MonitorViewModel : INotifyPropertyChanged, IDisposable private readonly MainViewModel? _mainViewModel; // Simple debouncers for each property (KISS principle - simpler than complex queue) - private readonly SimpleDebouncer _brightnessDebouncer = new(300); - private readonly SimpleDebouncer _contrastDebouncer = new(300); - private readonly SimpleDebouncer _volumeDebouncer = new(300); + private readonly SimpleDebouncer _brightnessDebouncer = new(AppConstants.UI.SliderDebounceDelayMs); + private readonly SimpleDebouncer _contrastDebouncer = new(AppConstants.UI.SliderDebounceDelayMs); + private readonly SimpleDebouncer _volumeDebouncer = new(AppConstants.UI.SliderDebounceDelayMs); private int _brightness; private int _contrast; @@ -196,7 +197,6 @@ public partial class MonitorViewModel : INotifyPropertyChanged, IDisposable if (result.IsSuccess) { _mainViewModel?.SaveMonitorSettingDirect(_monitor.HardwareId, "Brightness", brightness); - Logger.LogTrace($"[{HardwareId}] Brightness applied and saved"); } else { @@ -223,7 +223,6 @@ public partial class MonitorViewModel : INotifyPropertyChanged, IDisposable if (result.IsSuccess) { _mainViewModel?.SaveMonitorSettingDirect(_monitor.HardwareId, "Contrast", contrast); - Logger.LogTrace($"[{HardwareId}] Contrast applied and saved"); } else { @@ -250,7 +249,6 @@ public partial class MonitorViewModel : INotifyPropertyChanged, IDisposable if (result.IsSuccess) { _mainViewModel?.SaveMonitorSettingDirect(_monitor.HardwareId, "Volume", volume); - Logger.LogTrace($"[{HardwareId}] Volume applied and saved"); } else { diff --git a/src/settings-ui/Settings.UI/ViewModels/PowerDisplayViewModel.cs b/src/settings-ui/Settings.UI/ViewModels/PowerDisplayViewModel.cs index cf066eefc1..aac842c09b 100644 --- a/src/settings-ui/Settings.UI/ViewModels/PowerDisplayViewModel.cs +++ b/src/settings-ui/Settings.UI/ViewModels/PowerDisplayViewModel.cs @@ -73,8 +73,7 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels { Logger.LogInfo("Received refresh monitors event from PowerDisplay.exe"); ReloadMonitorsFromSettings(); - }, - _cancellationTokenSource.Token); + }); } private void InitializeEnabledValue()