Feature/sync brightness wmi#49101
Conversation
…d debounced hardware broadcasting
|
@microsoft-github-policy-service agree |
|
looks good overall. But just as I said in #47290 (comment) we have more consider on this feature. The CLI part is ready #48632 and we have discussed about KBM. So, we we cannot merge this PR. Sorry about that. |
|
I see what youre saying, that you can achieve some of this with the CLI. i can separate that part, so its agnostic to whenever its used by CLI or anything else, so brightness is unified across all software with WMI. with this build its not a shortcut to adjust brightness, its the hardware keys that control brightness on all monitors. |
There was a problem hiding this comment.
Pull request overview
Adds PowerDisplay brightness synchronization capabilities by introducing a Settings UI toggle to sync external monitor brightness when the internal (laptop) display brightness changes, implemented via WMI brightness change event monitoring in the PowerDisplay module.
Changes:
- Added
sync_brightness_with_internal_displaypersisted setting and exposed it in Settings UI (ViewModel + XAML + localized string). - Added WMI brightness event subscription in
WmiControllerand surfaced it throughMonitorManagerintoMainViewModelto drive external monitor brightness updates. - Extended monitor VM surface area to expose
InstanceNamefor matching WMI events to discovered monitors.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/settings-ui/Settings.UI/ViewModels/PowerDisplayViewModel.cs | Exposes new toggle in Settings UI VM and signals settings updates. |
| src/settings-ui/Settings.UI/Strings/en-us/Resources.resw | Adds localized string for the new toggle label. |
| src/settings-ui/Settings.UI/SettingsXAML/Views/PowerDisplayPage.xaml | Adds the new checkbox to the PowerDisplay settings page. |
| src/settings-ui/Settings.UI.Library/PowerDisplayProperties.cs | Adds persisted boolean property + JSON key for internal-display sync. |
| src/modules/powerdisplay/PowerDisplay/ViewModels/MonitorViewModel.cs | Exposes InstanceName for WMI-to-monitor matching. |
| src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.Settings.cs | Persists the new setting to settings.json when it changes. |
| src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.LinkedBrightness.cs | Handles WMI brightness change events and propagates brightness to monitors. |
| src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs | Hooks/unhooks WMI brightness change notifications from MonitorManager. |
| src/modules/powerdisplay/PowerDisplay/Helpers/WmiBrightnessChangedEventArgs.cs | New event args type for WMI brightness change propagation. |
| src/modules/powerdisplay/PowerDisplay/Helpers/MonitorManager.cs | Bridges WmiController brightness events to PowerDisplay view models. |
| src/modules/powerdisplay/PowerDisplay.Lib/Drivers/WMI/WmiController.cs | Subscribes to WmiMonitorBrightnessEvent and raises brightness change callbacks. |
| var lookupId = MonitorIdentity.FromInstanceName(instanceName); | ||
| if (string.IsNullOrEmpty(lookupId)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var matchingMonitor = Monitors.FirstOrDefault(m => MonitorIdComparer.Instance.Equals(m.Id, lookupId)); | ||
| if (matchingMonitor == null) | ||
| { | ||
| matchingMonitor = Monitors.FirstOrDefault(m => string.Equals(m.InstanceName, instanceName, StringComparison.OrdinalIgnoreCase)); | ||
| } | ||
|
|
| private void MonitorManager_WmiBrightnessChanged(object? sender, WmiBrightnessChangedEventArgs e) | ||
| { | ||
| _dispatcherQueue.TryEnqueue(() => | ||
| { | ||
| HandleWmiBrightnessChanged(e.InstanceName, e.Brightness); | ||
| }); | ||
| } |
| Logger.LogInfo($"[WmiBrightness] Internal monitor brightness changed to {brightness}%. Syncing displays."); | ||
| matchingMonitor.UpdateBrightnessDisplay(brightness); |
| [JsonPropertyName("sync_brightness_with_internal_display")] | ||
| public bool SyncBrightnessWithInternalDisplay { get; set; } | ||
|
|
| try | ||
| { | ||
| var instanceName = eventObj.GetPropertyValue<string>("InstanceName"); | ||
| var brightness = eventObj.GetPropertyValue<byte>("Brightness"); | ||
| BrightnessChanged?.Invoke(instanceName, brightness); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Logger.LogWarning($"Error handling WMI brightness event: {ex.Message}"); | ||
| } | ||
| }); |
moooyo
left a comment
moooyo
left a comment
There was a problem hiding this comment.
We need to discuss this feature further.
Summary of the Pull Request
This PR adds two highly requested features to the PowerDisplay utility:
Detailed Changes
1. Linked Brightness Controls
LinkedBrightness) that broadcasts value changes to all linked monitors.IsExcludedFromSync/excluded_from_sync_monitor_ids). Excluded monitors keep their own independent sliders and do not follow the master level.SliderCommitScheduler) to prevent high-frequency DDC/CI commands from locking up or lagging external monitors during drags.2. Laptop Screen Sync (Windows Slider Integration)
WmiMonitorBrightnessEventinWmiControllerto detect changes to the built-in display's brightness.sync_brightness_with_internal_display).PR Checklist
PowerDisplay.Lib.UnitTests). All 138/138 unit tests pass successfully.