From 2b8e6247fcc998b1df58db4c15296e323315bd5c Mon Sep 17 00:00:00 2001 From: moooyo <42196638+moooyo@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:33:21 +0800 Subject: [PATCH] [PowerDisplay] Add stable profile IDs (#49175) ## Summary of the Pull Request Gives every saved PowerDisplay profile a stable, auto-incrementing integer ID and makes the app address profiles by that ID instead of by name. Duplicate profile names are allowed, renames preserve identity, and LightSwitch stores stable profile references. > Split out of the PowerDisplay CLI branch (#48632). CLI-specific contracts and commands remain in that stacked PR. ## PR Checklist - [x] **Closes:** N/A - split from #48632. - [x] **Communication:** Discussed with core contributors. - [x] **Tests:** Added and passing in `PowerDisplay.Lib.UnitTests`. - [x] **Localization:** The composed profile label uses a shared localized format resource. - [x] **New binaries:** None. - [x] **Documentation updated:** `doc/devdocs/modules/powerdisplay/design.md`. ## Implementation ### Profile model and persistence - `PowerDisplayProfile.Id` is the stable JSON `id`; `0` means unassigned. - `PowerDisplayProfiles.NextId` is monotonic and IDs are never reused. - `SetProfile` assigns IDs to new profiles and replaces existing profiles by ID. - Duplicate names are supported; name lookup remains only for migration of legacy references. - `ProfileStore` serializes cross-process load/modify/save operations with a named mutex and atomically replaces `profiles.json`. - Production callers use asynchronous `ProfileHelper` APIs. ### Migration and application - Initial PowerDisplay discovery assigns missing profile IDs and migrates legacy monitor IDs. - LightSwitch legacy name references are reconciled to IDs and written back to the current typed settings schema. - Native LightSwitch publishes pure light/dark theme events; PowerDisplay exclusively validates profile enablement and stable IDs. - Settings UI and Named Pipe ApplyProfile actions send invariant positive profile IDs. - PowerDisplay validates the ID, loads the current profile, and applies its monitor settings. ### Settings UI - Create, edit, apply, and delete operations use stable IDs. - LightSwitch selectors store profile IDs and keep legacy name fields only for migration. - Profile lists use a localized name-and-ID label so duplicate names remain distinguishable. ## Accepted Trade-offs - Profile ID migration remains dependent on the initial monitor discovery; a failed or delayed discovery can temporarily hide legacy ID-less profiles. - The one-time PowerDisplay LightSwitch migration rewrites the complete current typed settings object and does not add a new cross-process settings transaction. ## Validation - Built the affected x64 Debug projects with the repository build scripts. - `PowerDisplay.Lib.UnitTests`: 186 passed, 0 failed. --------- Co-authored-by: Yu Leng (from Dev Box) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/devdocs/modules/powerdisplay/design.md | 114 ++++--- .../LightSwitchSettings.cpp | 40 --- .../LightSwitchService/LightSwitchSettings.h | 5 - .../LightSwitchStateManager.cpp | 63 ++-- .../LightSwitchStateManager.h | 4 +- .../LightSwitchProfileIdTests.cs | 123 ++++++++ .../LightSwitchProfileReferenceHelperTests.cs | 200 ++++++++++++ .../LightSwitchProfileSettingsUpdaterTests.cs | 74 +++++ .../PowerDisplayProfilesTests.cs | 241 ++++++++++++++ .../ProfileMigrationTests.cs | 82 +++++ .../ProfileStoreTests.cs | 294 ++++++++++++++++++ .../Services/ProfileMigration.cs | 77 +++++ .../Services/ProfileService.cs | 23 -- .../PowerDisplay.Models.csproj | 3 + .../PowerDisplayProfile.cs | 10 + .../PowerDisplayProfiles.cs | 90 ++++-- .../ProfileDisplayNameFormatter.cs | 61 ++++ .../PowerDisplay.Models/ProfileHelper.cs | 182 ++--------- .../PowerDisplay.Models/ProfileStore.cs | 231 ++++++++++++++ .../Properties/Resources.resx | 19 ++ .../PowerDisplay/PowerDisplayXAML/App.xaml.cs | 17 +- .../PowerDisplayXAML/MainWindow.xaml | 56 ++-- .../PowerDisplayXAML/MainWindow.xaml.cs | 2 +- .../Services/LightSwitchService.cs | 86 +++-- .../PowerDisplay/Strings/en-us/Resources.resw | 6 + .../ViewModels/MainViewModel.Monitors.cs | 21 +- .../ViewModels/MainViewModel.Settings.cs | 222 ++++++------- .../PowerDisplay/ViewModels/MainViewModel.cs | 75 ++++- .../PowerDisplayModuleInterface/dllmain.cpp | 10 +- .../LightSwitchProfileReferenceHelper.cs | 127 ++++++++ .../LightSwitchProfileSettingsUpdater.cs | 32 ++ .../LightSwitchProperties.cs | 18 ++ .../LightSwitchSettings.cs | 2 + .../ViewModelTests/LightSwitch.cs | 70 +++++ .../ProfileEditorViewModelTests.cs | 41 +++ .../SettingsXAML/Views/LightSwitchPage.xaml | 12 +- .../Views/LightSwitchPage.xaml.cs | 4 +- .../SettingsXAML/Views/PowerDisplayPage.xaml | 4 +- .../Views/PowerDisplayPage.xaml.cs | 18 +- .../Views/ProfileEditorDialog.xaml.cs | 7 +- .../ViewModels/LightSwitchViewModel.cs | 208 ++++++------- .../ViewModels/PowerDisplayViewModel.cs | 247 +++++++++------ .../ViewModels/ProfileEditorViewModel.cs | 9 +- 43 files changed, 2458 insertions(+), 772 deletions(-) create mode 100644 src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileIdTests.cs create mode 100644 src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileReferenceHelperTests.cs create mode 100644 src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileSettingsUpdaterTests.cs create mode 100644 src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/PowerDisplayProfilesTests.cs create mode 100644 src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/ProfileMigrationTests.cs create mode 100644 src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/ProfileStoreTests.cs create mode 100644 src/modules/powerdisplay/PowerDisplay.Lib/Services/ProfileMigration.cs delete mode 100644 src/modules/powerdisplay/PowerDisplay.Lib/Services/ProfileService.cs create mode 100644 src/modules/powerdisplay/PowerDisplay.Models/ProfileDisplayNameFormatter.cs create mode 100644 src/modules/powerdisplay/PowerDisplay.Models/ProfileStore.cs create mode 100644 src/modules/powerdisplay/PowerDisplay.Models/Properties/Resources.resx create mode 100644 src/settings-ui/Settings.UI.Library/LightSwitchProfileReferenceHelper.cs create mode 100644 src/settings-ui/Settings.UI.Library/LightSwitchProfileSettingsUpdater.cs create mode 100644 src/settings-ui/Settings.UI.UnitTests/ViewModelTests/LightSwitch.cs create mode 100644 src/settings-ui/Settings.UI.UnitTests/ViewModelTests/ProfileEditorViewModelTests.cs diff --git a/doc/devdocs/modules/powerdisplay/design.md b/doc/devdocs/modules/powerdisplay/design.md index ae2eb26479..0c3c0abeaf 100644 --- a/doc/devdocs/modules/powerdisplay/design.md +++ b/doc/devdocs/modules/powerdisplay/design.md @@ -193,26 +193,18 @@ src/modules/powerdisplay/ │ │ └── PInvoke.cs # P/Invoke declarations │ ├── Interfaces/ │ │ ├── IMonitorController.cs # Controller abstraction -│ │ ├── IMonitorData.cs # Monitor data interface -│ │ └── IProfileService.cs # Profile service interface +│ │ └── IMonitorData.cs # Monitor data interface │ ├── Models/ │ │ ├── Monitor.cs # Runtime monitor data │ │ ├── MonitorCapabilities.cs # Monitor capability flags │ │ ├── MonitorOperationResult.cs # Operation result │ │ ├── MonitorStateEntry.cs # Persisted monitor state │ │ ├── MonitorStateFile.cs # State file schema -│ │ ├── PowerDisplayProfile.cs # Profile definition -│ │ ├── PowerDisplayProfiles.cs # Profile collection -│ │ ├── ProfileMonitorSetting.cs # Per-monitor profile settings -│ │ ├── ColorPresetItem.cs # Color preset UI item │ │ ├── VcpCapabilities.cs # Parsed VCP capabilities │ │ └── VcpFeatureValue.cs # VCP feature value (current/min/max) -│ ├── Serialization/ -│ │ └── ProfileSerializationContext.cs # JSON source generation │ ├── Services/ │ │ ├── DisplayRotationService.cs # Display rotation via ChangeDisplaySettingsEx -│ │ ├── MonitorStateManager.cs # State persistence (debounced save) and restore on startup -│ │ └── ProfileService.cs # Profile persistence +│ │ └── MonitorStateManager.cs # State persistence (debounced save) and restore on startup │ ├── Utils/ │ │ ├── ColorTemperatureHelper.cs # Color temp utilities │ │ ├── EventHelper.cs # Windows Event utilities @@ -221,11 +213,19 @@ src/modules/powerdisplay/ │ │ ├── MonitorMatchingHelper.cs # Profile-to-monitor matching │ │ ├── MonitorValueConverter.cs # Value conversion utilities │ │ ├── PnpIdHelper.cs # PnP manufacturer ID lookup -│ │ ├── ProfileHelper.cs # Profile helper utilities │ │ ├── SimpleDebouncer.cs # Generic debouncer │ │ └── VcpNames.cs # VCP code and value name lookup │ └── PathConstants.cs # File path constants │ +├── PowerDisplay.Models/ # Shared profile models and persistence +│ ├── ColorPresetItem.cs # Color preset UI item +│ ├── PowerDisplayProfile.cs # Profile definition +│ ├── PowerDisplayProfiles.cs # Profile collection +│ ├── ProfileMonitorSetting.cs # Per-monitor profile settings +│ ├── ProfileHelper.cs # Shared asynchronous profile entry points +│ ├── ProfileStore.cs # Atomic cross-process profile persistence +│ └── ProfileSerializationContext.cs # JSON source generation +│ ├── PowerDisplay/ # WinUI 3 application │ ├── Assets/ # App icons and images │ ├── Configuration/ @@ -304,7 +304,6 @@ flowchart TB subgraph PowerDisplayLib["PowerDisplay.Lib"] subgraph Services - ProfileService MonitorStateManager DisplayRotationService end @@ -316,6 +315,11 @@ flowchart TB PnpIdHelper["PnpIdHelper
(Manufacturer Names)"] end end + + subgraph PowerDisplayModels["PowerDisplay.Models"] + ProfileHelper + ProfileStore + end end subgraph Storage["Persistent Storage"] @@ -338,13 +342,14 @@ flowchart TB ThemeChangedEvent --> LightSwitchService %% App internal - LightSwitchService -.->|"Get profile name"| MainViewModel + LightSwitchService -.->|"Get profile id"| MainViewModel MainViewModel --> MonitorViewModel MonitorViewModel --> MonitorManager DisplayChangeWatcher -.->|"DisplayChanged event"| MainViewModel - %% App to Lib services - MainViewModel --> ProfileService + %% App to services and profile persistence + MainViewModel --> ProfileHelper + ProfileHelper --> ProfileStore MonitorViewModel --> MonitorStateManager MonitorManager --> Drivers MonitorManager --> DisplayRotationService @@ -352,8 +357,8 @@ flowchart TB %% Utils used during discovery WmiController --> PnpIdHelper - %% Services to Storage - ProfileService --> ProfilesJson + %% Persistence to Storage + ProfileStore --> ProfilesJson MonitorStateManager --> MonitorStateJson %% Drivers to Hardware @@ -1080,7 +1085,7 @@ flowchart TB StateManager["LightSwitchStateManager"] ThemeEval["Theme Evaluation
(Time/System)"] LightSwitchSettings["LightSwitchSettings"] - NotifyPD["NotifyPowerDisplay(isLight)"] + NotifyPD["NotifyPowerDisplayThemeChanged(isLight)"] end subgraph PowerDisplayModule["PowerDisplay Module (C#)"] @@ -1090,7 +1095,8 @@ flowchart TB MainViewModel["MainViewModel"] end - ProfileService["ProfileService"] + ProfileHelper["ProfileHelper
(PowerDisplay.Models)"] + ProfileStore["ProfileStore"] MonitorVMs["MonitorViewModels"] Controllers["IMonitorController"] end @@ -1113,17 +1119,18 @@ flowchart TB ThemeEval -->|"Time boundary
or manual"| StateManager StateManager --> LightSwitchSettings StateManager --> NotifyPD - NotifyPD -->|"isLight=true"| LightEvent - NotifyPD -->|"isLight=false"| DarkEvent + NotifyPD -->|"pure light theme event"| LightEvent + NotifyPD -->|"pure dark theme event"| DarkEvent %% PowerDisplay flow - theme determined from event LightEvent -->|"Event signaled"| EventWaiter DarkEvent -->|"Event signaled"| EventWaiter EventWaiter -->|"isLightMode"| LightSwitchSvc - LightSwitchSvc -->|"GetProfileForTheme()"| LSSettingsJson - LightSwitchSvc -->|"Profile name"| MainViewModel - MainViewModel -->|"LoadProfiles()"| ProfileService - ProfileService <--> PDProfilesJson + LightSwitchSvc -->|"GetProfileIdForTheme()"| LSSettingsJson + LightSwitchSvc -->|"Profile id"| MainViewModel + MainViewModel -->|"LoadProfilesAsync()"| ProfileHelper + ProfileHelper --> ProfileStore + ProfileStore <--> PDProfilesJson MainViewModel -->|"ApplyProfileAsync()"| MonitorVMs MonitorVMs --> Controllers Controllers --> Monitors @@ -1135,20 +1142,25 @@ flowchart TB style FileSystem fill:#fffde7 ``` +Native LightSwitch treats these named events as pure theme-change notifications and does not parse PowerDisplay profile enablement, names, or IDs. PowerDisplay reads the typed LightSwitch settings after receiving the event and is the sole authority that validates and applies the configured profile. + ### LightSwitch Settings JSON Structure ```json { "properties": { - "apply_monitor_settings": { "value": true }, - "enable_light_mode_profile": { "value": true }, - "light_mode_profile": { "value": "Productivity" }, - "enable_dark_mode_profile": { "value": true }, - "dark_mode_profile": { "value": "Night Mode" } + "enableLightModeProfile": { "value": true }, + "lightModeProfile": { "value": "" }, + "lightModeProfileId": { "value": 3 }, + "enableDarkModeProfile": { "value": true }, + "darkModeProfile": { "value": "" }, + "darkModeProfileId": { "value": 7 } } } ``` +The name fields are retained only for migration from pre-ID settings; current code persists and resolves the positive ID fields. + --- ## Data Flow and Communication @@ -1354,7 +1366,8 @@ sequenceDiagram participant SettingsPage as PowerDisplayPage participant ViewModel as PowerDisplayViewModel participant ProfileDialog as ProfileEditorDialog - participant ProfileService + participant ProfileHelper + participant ProfileStore participant FileSystem as profiles.json User->>SettingsPage: Clicks "Add Profile" button @@ -1369,20 +1382,21 @@ sequenceDiagram User->>ProfileDialog: Clicks "Save" ProfileDialog->>ProfileDialog: Validate inputs - Note over ProfileDialog: Check name unique,
at least one monitor selected + Note over ProfileDialog: Check non-empty name,
at least one monitor selected ProfileDialog-->>ViewModel: ResultProfile (PowerDisplayProfile) - ViewModel->>ProfileService: AddOrUpdateProfile(profile) + ViewModel->>ProfileHelper: ProfileHelper.AddOrUpdateProfileAsync(profile) + ProfileHelper->>ProfileStore: AddOrUpdateProfileAsync(profile) - ProfileService->>ProfileService: lock(_lock) - ProfileService->>FileSystem: Read profiles.json - FileSystem-->>ProfileService: Existing profiles - ProfileService->>ProfileService: Add/update profile in collection - ProfileService->>ProfileService: Set LastUpdated = DateTime.Now - ProfileService->>FileSystem: Write profiles.json - FileSystem-->>ProfileService: Success - ProfileService-->>ViewModel: true + ProfileStore->>ProfileStore: Acquire process lock and named mutex + ProfileStore->>FileSystem: Read profiles.json + FileSystem-->>ProfileStore: Existing profiles + ProfileStore->>ProfileStore: Assign id and update profile + ProfileStore->>FileSystem: Write temp file and atomically replace profiles.json + FileSystem-->>ProfileStore: Success + ProfileStore-->>ProfileHelper: Completed + ProfileHelper-->>ViewModel: Completed ViewModel->>ViewModel: RefreshProfilesList() ViewModel-->>SettingsPage: PropertyChanged(Profiles) @@ -1401,7 +1415,7 @@ sequenceDiagram participant EventWaiter as NativeEventWaiter participant LSSvc as LightSwitchService participant MainVM as MainViewModel - participant ProfileService + participant ProfileHelper participant MonitorVM as MonitorViewModel participant Controller as IMonitorController participant Monitor as Physical Monitor @@ -1412,8 +1426,8 @@ sequenceDiagram LightSwitch->>LightSwitch: EvaluateAndApplyIfNeeded() LightSwitch->>LightSwitch: ApplyTheme(isLight) - LightSwitch->>LightSwitch: NotifyPowerDisplay(isLight) - Note over LightSwitch: Check if profile enabled + LightSwitch->>LightSwitch: NotifyPowerDisplayThemeChanged(isLight) + Note over LightSwitch: Publish the resulting theme only;
PowerDisplay owns profile validation alt isLight == true LightSwitch->>WinEvent: SetEvent("Local\\PowerToys_LightSwitch_LightTheme") @@ -1425,16 +1439,16 @@ sequenceDiagram EventWaiter->>WinEvent: WaitAny([lightEvent, darkEvent]) returns index Note over EventWaiter: Theme determined from event:
index 0 = Light, index 1 = Dark - EventWaiter->>LSSvc: GetProfileForTheme(isLightMode) + EventWaiter->>LSSvc: GetProfileIdForTheme(isLightMode) LSSvc->>LSSvc: Read LightSwitch/settings.json - LSSvc-->>EventWaiter: profileName (or null) + LSSvc-->>EventWaiter: profileId (or null) - EventWaiter->>MainVM: Dispatch to UI thread with profileName + EventWaiter->>MainVM: Dispatch to UI thread with profileId - MainVM->>ProfileService: LoadProfiles() - ProfileService-->>MainVM: PowerDisplayProfiles + MainVM->>ProfileHelper: LoadProfilesAsync() + ProfileHelper-->>MainVM: PowerDisplayProfiles - MainVM->>MainVM: Find profile by name + MainVM->>MainVM: Find profile by id MainVM->>MainVM: ApplyProfileAsync(profile.MonitorSettings) loop For each ProfileMonitorSetting diff --git a/src/modules/LightSwitch/LightSwitchService/LightSwitchSettings.cpp b/src/modules/LightSwitch/LightSwitchService/LightSwitchSettings.cpp index 15e9f7c915..488142b95b 100644 --- a/src/modules/LightSwitch/LightSwitchService/LightSwitchSettings.cpp +++ b/src/modules/LightSwitch/LightSwitchService/LightSwitchSettings.cpp @@ -248,46 +248,6 @@ void LightSwitchSettings::LoadSettings() } } - // EnableDarkModeProfile - if (const auto jsonVal = values.get_bool_value(L"enableDarkModeProfile")) - { - auto val = *jsonVal; - if (m_settings.enableDarkModeProfile != val) - { - m_settings.enableDarkModeProfile = val; - } - } - - // EnableLightModeProfile - if (const auto jsonVal = values.get_bool_value(L"enableLightModeProfile")) - { - auto val = *jsonVal; - if (m_settings.enableLightModeProfile != val) - { - m_settings.enableLightModeProfile = val; - } - } - - // DarkModeProfile - if (const auto jsonVal = values.get_string_value(L"darkModeProfile")) - { - auto val = *jsonVal; - if (m_settings.darkModeProfile != val) - { - m_settings.darkModeProfile = val; - } - } - - // LightModeProfile - if (const auto jsonVal = values.get_string_value(L"lightModeProfile")) - { - auto val = *jsonVal; - if (m_settings.lightModeProfile != val) - { - m_settings.lightModeProfile = val; - } - } - // For ChangeSystem/ChangeApps changes, log telemetry if (themeTargetChanged) { diff --git a/src/modules/LightSwitch/LightSwitchService/LightSwitchSettings.h b/src/modules/LightSwitch/LightSwitchService/LightSwitchSettings.h index 4fd9777c5e..1d1c7953fe 100644 --- a/src/modules/LightSwitch/LightSwitchService/LightSwitchSettings.h +++ b/src/modules/LightSwitch/LightSwitchService/LightSwitchSettings.h @@ -67,11 +67,6 @@ struct LightSwitchConfig bool changeSystem = false; bool changeApps = false; - - bool enableDarkModeProfile = false; - bool enableLightModeProfile = false; - std::wstring darkModeProfile = L""; - std::wstring lightModeProfile = L""; }; class LightSwitchSettings diff --git a/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.cpp b/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.cpp index 3f77b2493c..c6b80ebe04 100644 --- a/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.cpp +++ b/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.cpp @@ -46,9 +46,8 @@ void LightSwitchStateManager::OnManualOverride() _state.isManualOverride = !_state.isManualOverride; // ModuleInterface has already flipped the Windows theme before signaling this event, - // regardless of which direction isManualOverride just toggled. Sync cached state and - // notify PowerDisplay on every call so the profile follows every hotkey press — the - // previous "if entering" gate silently dropped every even-numbered press. + // regardless of which direction isManualOverride just toggled. Sync cached state so the + // scheduler compares against the actual current theme on the next evaluation. _state.isSystemLightActive = GetCurrentSystemTheme(); _state.isAppsLightActive = GetCurrentAppsTheme(); @@ -56,7 +55,15 @@ void LightSwitchStateManager::OnManualOverride() (_state.isSystemLightActive ? L"light" : L"dark"), (_state.isAppsLightActive ? L"light" : L"dark")); - NotifyPowerDisplay(_state.isSystemLightActive); + const auto& settings = LightSwitchSettings::settings(); + if (settings.changeSystem) + { + NotifyPowerDisplayThemeChanged(_state.isSystemLightActive); + } + else if (settings.changeApps) + { + NotifyPowerDisplayThemeChanged(_state.isAppsLightActive); + } EvaluateAndApplyIfNeeded(); } @@ -271,40 +278,20 @@ void LightSwitchStateManager::EvaluateAndApplyIfNeeded() _state.isSystemLightActive = GetCurrentSystemTheme(); _state.isAppsLightActive = GetCurrentAppsTheme(); - // Notify PowerDisplay to apply display profile if configured - NotifyPowerDisplay(shouldBeLight); + // Notify PowerDisplay after the theme transition is complete. + NotifyPowerDisplayThemeChanged(shouldBeLight); } _state.lastTickMinutes = now; } -// Notify PowerDisplay module about theme change to apply display profiles -void LightSwitchStateManager::NotifyPowerDisplay(bool isLight) +// Notify PowerDisplay that LightSwitch applied a new theme. +void LightSwitchStateManager::NotifyPowerDisplayThemeChanged(bool isLight) { - const auto& settings = LightSwitchSettings::settings(); - - // Check if any profile is enabled and configured - bool shouldNotify = false; - - if (isLight && settings.enableLightModeProfile && !settings.lightModeProfile.empty()) - { - shouldNotify = true; - } - else if (!isLight && settings.enableDarkModeProfile && !settings.darkModeProfile.empty()) - { - shouldNotify = true; - } - - if (!shouldNotify) - { - return; - } - try { - // Signal PowerDisplay with the specific theme event - // Using separate events for light/dark eliminates race conditions where PowerDisplay - // might read the registry before LightSwitch has finished updating it + // The event carries only the resulting theme. PowerDisplay owns profile + // enablement, reference validation, and application. const wchar_t* eventName = isLight ? CommonSharedConstants::LIGHT_SWITCH_LIGHT_THEME_EVENT : CommonSharedConstants::LIGHT_SWITCH_DARK_THEME_EVENT; @@ -312,16 +299,22 @@ void LightSwitchStateManager::NotifyPowerDisplay(bool isLight) Logger::info(L"[LightSwitchStateManager] Notifying PowerDisplay about theme change (isLight: {})", isLight); HANDLE hThemeEvent = CreateEventW(nullptr, FALSE, FALSE, eventName); - if (hThemeEvent) + if (!hThemeEvent) { - SetEvent(hThemeEvent); - CloseHandle(hThemeEvent); - Logger::info(L"[LightSwitchStateManager] Theme event signaled to PowerDisplay: {}", eventName); + Logger::warn(L"[LightSwitchStateManager] Failed to create theme event (error: {})", GetLastError()); + return; + } + + if (!SetEvent(hThemeEvent)) + { + Logger::warn(L"[LightSwitchStateManager] Failed to signal theme event '{}' (error: {})", eventName, GetLastError()); } else { - Logger::warn(L"[LightSwitchStateManager] Failed to create theme event (error: {})", GetLastError()); + Logger::info(L"[LightSwitchStateManager] Theme event signaled to PowerDisplay: {}", eventName); } + + CloseHandle(hThemeEvent); } catch (...) { diff --git a/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.h b/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.h index b6c001fc64..4aea39f696 100644 --- a/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.h +++ b/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.h @@ -49,6 +49,6 @@ private: void EvaluateAndApplyIfNeeded(); bool CoordinatesAreValid(const std::wstring& lat, const std::wstring& lon); - // Notify PowerDisplay module about theme change to apply display profiles - void NotifyPowerDisplay(bool isLight); + // Notify PowerDisplay that LightSwitch applied a new theme. + void NotifyPowerDisplayThemeChanged(bool isLight); }; diff --git a/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileIdTests.cs b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileIdTests.cs new file mode 100644 index 0000000000..dcabee86aa --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileIdTests.cs @@ -0,0 +1,123 @@ +// 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.Text.Json; +using Microsoft.PowerToys.Settings.UI.Library; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace PowerDisplay.UnitTests; + +[TestClass] +public class LightSwitchProfileIdTests +{ + [TestMethod] + public void LightSwitchProperties_ProfileIds_RoundTripThroughJson_DefaultZero() + { + var props = new LightSwitchProperties(); + Assert.AreEqual(0, props.LightModeProfileId.Value); + Assert.AreEqual(0, props.DarkModeProfileId.Value); + + props.LightModeProfileId.Value = 3; + props.DarkModeProfileId.Value = 5; + + var json = JsonSerializer.Serialize(props); + var back = JsonSerializer.Deserialize(json); + + Assert.IsNotNull(back); + Assert.AreEqual(3, back!.LightModeProfileId.Value); + Assert.AreEqual(5, back.DarkModeProfileId.Value); + } + + [TestMethod] + public void LightSwitchSettings_Clone_PreservesProfileIds() + { + var settings = new LightSwitchSettings(); + settings.Properties.DarkModeProfileId.Value = 7; + settings.Properties.LightModeProfileId.Value = 3; + settings.Properties.DarkModeProfile.Value = "Night"; + settings.Properties.LightModeProfile.Value = "Day"; + + var clone = (LightSwitchSettings)settings.Clone(); + + Assert.AreEqual(7, clone.Properties.DarkModeProfileId.Value); + Assert.AreEqual(3, clone.Properties.LightModeProfileId.Value); + Assert.AreEqual("Night", clone.Properties.DarkModeProfile.Value); + Assert.AreEqual("Day", clone.Properties.LightModeProfile.Value); + } + + [TestMethod] + public void LightSwitchProperties_LegacyProfileNames_RemainDeserializable() + { + const string json = """ + { + "darkModeProfile": { "value": "Night" }, + "lightModeProfile": { "value": "Day" } + } + """; + + var properties = JsonSerializer.Deserialize(json); + + Assert.IsNotNull(properties); + Assert.AreEqual("Night", properties!.DarkModeProfile.Value); + Assert.AreEqual("Day", properties.LightModeProfile.Value); + Assert.AreEqual(0, properties.DarkModeProfileId.Value); + Assert.AreEqual(0, properties.LightModeProfileId.Value); + } + + [TestMethod] + public void LightSwitchSettings_ToJsonString_RoundTripsAllKnownProperties() + { + var settings = new LightSwitchSettings(); + settings.Properties.ChangeSystem.Value = false; + settings.Properties.ChangeApps.Value = false; + settings.Properties.ScheduleMode.Value = "SunsetToSunrise"; + settings.Properties.LightTime.Value = 451; + settings.Properties.DarkTime.Value = 1217; + settings.Properties.SunriseOffset.Value = -15; + settings.Properties.SunsetOffset.Value = 20; + settings.Properties.Latitude.Value = "47.642"; + settings.Properties.Longitude.Value = "-122.136"; + settings.Properties.ToggleThemeHotkey.Value = new HotkeySettings( + win: false, + ctrl: true, + alt: true, + shift: false, + code: 0x4C); + settings.Properties.EnableDarkModeProfile.Value = true; + settings.Properties.EnableLightModeProfile.Value = true; + settings.Properties.DarkModeProfile.Value = "Night"; + settings.Properties.LightModeProfile.Value = "Day"; + settings.Properties.DarkModeProfileId.Value = 7; + settings.Properties.LightModeProfileId.Value = 3; + + var json = settings.ToJsonString(); + var roundTripped = JsonSerializer.Deserialize( + json, + SettingsSerializationContext.Default.LightSwitchSettings); + + Assert.IsNotNull(roundTripped); + Assert.AreEqual(settings.Name, roundTripped.Name); + Assert.AreEqual(settings.Version, roundTripped.Version); + Assert.IsFalse(roundTripped.Properties.ChangeSystem.Value); + Assert.IsFalse(roundTripped.Properties.ChangeApps.Value); + Assert.AreEqual("SunsetToSunrise", roundTripped.Properties.ScheduleMode.Value); + Assert.AreEqual(451, roundTripped.Properties.LightTime.Value); + Assert.AreEqual(1217, roundTripped.Properties.DarkTime.Value); + Assert.AreEqual(-15, roundTripped.Properties.SunriseOffset.Value); + Assert.AreEqual(20, roundTripped.Properties.SunsetOffset.Value); + Assert.AreEqual("47.642", roundTripped.Properties.Latitude.Value); + Assert.AreEqual("-122.136", roundTripped.Properties.Longitude.Value); + Assert.IsFalse(roundTripped.Properties.ToggleThemeHotkey.Value.Win); + Assert.IsTrue(roundTripped.Properties.ToggleThemeHotkey.Value.Ctrl); + Assert.IsTrue(roundTripped.Properties.ToggleThemeHotkey.Value.Alt); + Assert.IsFalse(roundTripped.Properties.ToggleThemeHotkey.Value.Shift); + Assert.AreEqual(0x4C, roundTripped.Properties.ToggleThemeHotkey.Value.Code); + Assert.IsTrue(roundTripped.Properties.EnableDarkModeProfile.Value); + Assert.IsTrue(roundTripped.Properties.EnableLightModeProfile.Value); + Assert.AreEqual("Night", roundTripped.Properties.DarkModeProfile.Value); + Assert.AreEqual("Day", roundTripped.Properties.LightModeProfile.Value); + Assert.AreEqual(7, roundTripped.Properties.DarkModeProfileId.Value); + Assert.AreEqual(3, roundTripped.Properties.LightModeProfileId.Value); + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileReferenceHelperTests.cs b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileReferenceHelperTests.cs new file mode 100644 index 0000000000..3b48762f6e --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileReferenceHelperTests.cs @@ -0,0 +1,200 @@ +// 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 Microsoft.PowerToys.Settings.UI.Library; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using PowerDisplay.Models; + +namespace PowerDisplay.UnitTests; + +[TestClass] +public class LightSwitchProfileReferenceHelperTests +{ + [TestMethod] + public void GetProfileIdForTheme_DisabledOrZeroId_ReturnsNull() + { + var properties = new LightSwitchProperties(); + properties.EnableDarkModeProfile.Value = false; + properties.DarkModeProfileId.Value = 7; + properties.EnableLightModeProfile.Value = true; + properties.LightModeProfileId.Value = 0; + + Assert.IsNull(LightSwitchProfileReferenceHelper.GetProfileIdForTheme(properties, isLightMode: false)); + Assert.IsNull(LightSwitchProfileReferenceHelper.GetProfileIdForTheme(properties, isLightMode: true)); + } + + [TestMethod] + public void GetProfileIdForTheme_EnabledPositiveIds_ReturnsThemeId() + { + var properties = new LightSwitchProperties(); + properties.EnableDarkModeProfile.Value = true; + properties.DarkModeProfileId.Value = 7; + properties.EnableLightModeProfile.Value = true; + properties.LightModeProfileId.Value = 4; + + Assert.AreEqual(7, LightSwitchProfileReferenceHelper.GetProfileIdForTheme(properties, isLightMode: false)); + Assert.AreEqual(4, LightSwitchProfileReferenceHelper.GetProfileIdForTheme(properties, isLightMode: true)); + } + + [TestMethod] + public void ReconcileReferences_LegacyNames_MigratesIdsAndClearsNames() + { + var properties = new LightSwitchProperties(); + properties.DarkModeProfile.Value = "Night"; + properties.LightModeProfile.Value = "Day"; + + Assert.IsTrue(LightSwitchProfileReferenceHelper.ReconcileReferences( + properties, + Profiles(("Day", 4), ("Night", 7)))); + Assert.AreEqual(7, properties.DarkModeProfileId.Value); + Assert.AreEqual(4, properties.LightModeProfileId.Value); + Assert.AreEqual(string.Empty, properties.DarkModeProfile.Value); + Assert.AreEqual(string.Empty, properties.LightModeProfile.Value); + } + + [TestMethod] + public void ReconcileReferences_ValidId_ClearsLegacyNameAndBecomesIdempotent() + { + var properties = new LightSwitchProperties(); + properties.DarkModeProfileId.Value = 7; + properties.DarkModeProfile.Value = "Night"; + var profiles = Profiles(("Night", 7)); + + Assert.IsTrue(LightSwitchProfileReferenceHelper.ReconcileReferences(properties, profiles)); + Assert.AreEqual(7, properties.DarkModeProfileId.Value); + Assert.AreEqual(string.Empty, properties.DarkModeProfile.Value); + Assert.IsFalse(LightSwitchProfileReferenceHelper.ReconcileReferences(properties, profiles)); + } + + [TestMethod] + public void ReconcileReferences_StaleId_DoesNotFallBackToLegacyName() + { + var properties = new LightSwitchProperties(); + properties.DarkModeProfileId.Value = 99; + properties.DarkModeProfile.Value = "Night"; + + Assert.IsTrue(LightSwitchProfileReferenceHelper.ReconcileReferences( + properties, + Profiles(("Night", 7)))); + Assert.AreEqual(0, properties.DarkModeProfileId.Value); + Assert.AreEqual(string.Empty, properties.DarkModeProfile.Value); + } + + [TestMethod] + public void ReconcileReferences_UnknownLegacyName_ClearsNameAndLeavesZeroId() + { + var properties = new LightSwitchProperties(); + properties.DarkModeProfile.Value = "Deleted"; + + Assert.IsTrue(LightSwitchProfileReferenceHelper.ReconcileReferences(properties, Profiles())); + Assert.AreEqual(0, properties.DarkModeProfileId.Value); + Assert.AreEqual(string.Empty, properties.DarkModeProfile.Value); + } + + [TestMethod] + public void ReconcileReferences_EmptyReferences_RemainUnchanged() + { + var properties = new LightSwitchProperties(); + + Assert.IsFalse(LightSwitchProfileReferenceHelper.ReconcileReferences(properties, Profiles())); + } + + [TestMethod] + public void SetProfileId_StoresIdAndClearsLegacyName() + { + var idProperty = new IntProperty(3); + var legacyNameProperty = new StringProperty("Old Name"); + + Assert.IsTrue(LightSwitchProfileReferenceHelper.SetProfileId( + idProperty, + legacyNameProperty, + 7)); + Assert.AreEqual(7, idProperty.Value); + Assert.AreEqual(string.Empty, legacyNameProperty.Value); + } + + [TestMethod] + public void SetProfileId_UnchangedIdAndEmptyLegacyName_ReturnsFalse() + { + var idProperty = new IntProperty(7); + var legacyNameProperty = new StringProperty(string.Empty); + + Assert.IsFalse(LightSwitchProfileReferenceHelper.SetProfileId( + idProperty, + legacyNameProperty, + 7)); + } + + [TestMethod] + public void SetProfileId_NegativeId_Throws() + { + Assert.ThrowsExactly(() => + LightSwitchProfileReferenceHelper.SetProfileId( + new IntProperty(0), + new StringProperty(string.Empty), + -1)); + } + + [TestMethod] + public void ClearProfileIdReferences_ClearsOnlyMatchingIds() + { + var properties = new LightSwitchProperties(); + properties.DarkModeProfileId.Value = 7; + properties.LightModeProfileId.Value = 4; + properties.DarkModeProfile.Value = "Legacy dark"; + properties.LightModeProfile.Value = "Legacy light"; + + Assert.IsTrue(LightSwitchProfileReferenceHelper.ClearProfileIdReferences(properties, 7)); + Assert.AreEqual(0, properties.DarkModeProfileId.Value); + Assert.AreEqual(4, properties.LightModeProfileId.Value); + Assert.AreEqual("Legacy dark", properties.DarkModeProfile.Value); + Assert.AreEqual("Legacy light", properties.LightModeProfile.Value); + } + + [TestMethod] + public void ClearProfileIdReferences_BothMatchingIds_ClearsBothAndKeepsLegacyNames() + { + var properties = new LightSwitchProperties(); + properties.DarkModeProfileId.Value = 7; + properties.LightModeProfileId.Value = 7; + properties.DarkModeProfile.Value = "Legacy dark"; + properties.LightModeProfile.Value = "Legacy light"; + + Assert.IsTrue(LightSwitchProfileReferenceHelper.ClearProfileIdReferences(properties, 7)); + Assert.AreEqual(0, properties.DarkModeProfileId.Value); + Assert.AreEqual(0, properties.LightModeProfileId.Value); + Assert.AreEqual("Legacy dark", properties.DarkModeProfile.Value); + Assert.AreEqual("Legacy light", properties.LightModeProfile.Value); + } + + [TestMethod] + public void ClearProfileIdReferences_NonPositiveId_Throws() + { + Assert.ThrowsExactly(() => + LightSwitchProfileReferenceHelper.ClearProfileIdReferences( + new LightSwitchProperties(), + 0)); + } + + private static PowerDisplayProfiles Profiles(params (string Name, int Id)[] items) + { + var profiles = new PowerDisplayProfiles(); + foreach (var (name, id) in items) + { + profiles.Profiles.Add(new PowerDisplayProfile( + name, + new List + { + new ProfileMonitorSetting("MON1", 50, null, null, null), + }) + { + Id = id, + }); + } + + return profiles; + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileSettingsUpdaterTests.cs b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileSettingsUpdaterTests.cs new file mode 100644 index 0000000000..f64de295f1 --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/LightSwitchProfileSettingsUpdaterTests.cs @@ -0,0 +1,74 @@ +// 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 Microsoft.PowerToys.Settings.UI.Library; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace PowerDisplay.UnitTests; + +[TestClass] +public class LightSwitchProfileSettingsUpdaterTests +{ + [TestMethod] + public void ClearDeletedProfileAndSend_MatchingIds_ClearsAndSendsOnce() + { + var settings = new LightSwitchSettings(); + settings.Properties.DarkModeProfileId.Value = 7; + settings.Properties.LightModeProfileId.Value = 7; + settings.Properties.DarkModeProfile.Value = "Legacy dark"; + settings.Properties.LightModeProfile.Value = "Legacy light"; + var messages = new List(); + + var changed = LightSwitchProfileSettingsUpdater.ClearDeletedProfileAndSend( + settings, + 7, + message => + { + messages.Add(message); + return 0; + }); + + Assert.IsTrue(changed); + Assert.AreEqual(0, settings.Properties.DarkModeProfileId.Value); + Assert.AreEqual(0, settings.Properties.LightModeProfileId.Value); + Assert.AreEqual("Legacy dark", settings.Properties.DarkModeProfile.Value); + Assert.AreEqual("Legacy light", settings.Properties.LightModeProfile.Value); + Assert.AreEqual(1, messages.Count); + } + + [TestMethod] + public void ClearDeletedProfileAndSend_NonMatchingIds_DoesNotSend() + { + var settings = new LightSwitchSettings(); + settings.Properties.DarkModeProfileId.Value = 5; + settings.Properties.LightModeProfileId.Value = 6; + var sendCount = 0; + + var changed = LightSwitchProfileSettingsUpdater.ClearDeletedProfileAndSend( + settings, + 7, + _ => + { + sendCount++; + return 0; + }); + + Assert.IsFalse(changed); + Assert.AreEqual(5, settings.Properties.DarkModeProfileId.Value); + Assert.AreEqual(6, settings.Properties.LightModeProfileId.Value); + Assert.AreEqual(0, sendCount); + } + + [TestMethod] + public void ClearDeletedProfileAndSend_ZeroDeletedProfileId_Throws() + { + Assert.ThrowsExactly(() => + LightSwitchProfileSettingsUpdater.ClearDeletedProfileAndSend( + new LightSwitchSettings(), + 0, + _ => 0)); + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/PowerDisplayProfilesTests.cs b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/PowerDisplayProfilesTests.cs new file mode 100644 index 0000000000..6d33086a1e --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/PowerDisplayProfilesTests.cs @@ -0,0 +1,241 @@ +// 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.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Resources; +using System.Text.Json; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using PowerDisplay.Models; + +namespace PowerDisplay.UnitTests; + +[TestClass] +public class PowerDisplayProfilesTests +{ + private static readonly string[] ExpectedAssignedProfileNames = { "Assigned" }; + + private static PowerDisplayProfile MakeProfile(string name, int id = 0) + { + var p = new PowerDisplayProfile(name, new List + { + new ProfileMonitorSetting("MON1", 50, null, null, null), + }); + p.Id = id; + return p; + } + + [TestMethod] + public void IdAndNextId_RoundTripThroughJson_AndDefaultToZero() + { + var profiles = new PowerDisplayProfiles(); + var p = MakeProfile("Gaming", id: 7); + profiles.Profiles.Add(p); + profiles.NextId = 8; + + var json = JsonSerializer.Serialize(profiles, ProfileSerializationContext.Default.PowerDisplayProfiles); + var back = JsonSerializer.Deserialize(json, ProfileSerializationContext.Default.PowerDisplayProfiles); + + Assert.IsNotNull(back); + Assert.AreEqual(8, back!.NextId); + Assert.AreEqual(7, back.Profiles[0].Id); + Assert.AreEqual(0, new PowerDisplayProfile().Id); + Assert.AreEqual(0, new PowerDisplayProfiles().NextId); + } + + [TestMethod] + public void GetById_ReturnsMatch_OrNullForZeroAndMissing() + { + var profiles = new PowerDisplayProfiles(); + var a = MakeProfile("A", id: 1); + var b = MakeProfile("B", id: 2); + profiles.Profiles.Add(a); + profiles.Profiles.Add(b); + + Assert.AreSame(b, profiles.GetById(2)); + Assert.IsNull(profiles.GetById(0)); + Assert.IsNull(profiles.GetById(99)); + } + + [TestMethod] + public void GetAssignedProfiles_ExcludesNonPositiveIds() + { + var profiles = new PowerDisplayProfiles(); + profiles.Profiles.Add(MakeProfile("Negative", id: -1)); + profiles.Profiles.Add(MakeProfile("Legacy", id: 0)); + profiles.Profiles.Add(MakeProfile("Assigned", id: 4)); + + var assigned = profiles.GetAssignedProfiles().Select(profile => profile.Name).ToArray(); + + CollectionAssert.AreEqual(ExpectedAssignedProfileNames, assigned); + } + + [TestMethod] + public void GetLegacyProfileByName_ReturnsFirstCaseInsensitiveMatch() + { + var profiles = new PowerDisplayProfiles(); + var first = MakeProfile("Same", id: 1); + var second = MakeProfile("same", id: 2); + profiles.Profiles.Add(first); + profiles.Profiles.Add(second); + + Assert.AreSame(first, profiles.GetLegacyProfileByName("SAME")); + } + + [TestMethod] + public void GetLegacyProfileByName_ReturnsNull_WhenNoCaseInsensitiveMatchExists() + { + var profiles = new PowerDisplayProfiles(); + profiles.Profiles.Add(MakeProfile("Same", id: 1)); + + Assert.IsNull(profiles.GetLegacyProfileByName("Different")); + } + + [TestMethod] + public void RemoveProfileById_RemovesWhenPresent() + { + var profiles = new PowerDisplayProfiles(); + profiles.Profiles.Add(MakeProfile("A", id: 1)); + profiles.Profiles.Add(MakeProfile("B", id: 2)); + + Assert.IsTrue(profiles.RemoveProfile(2)); + Assert.AreEqual(1, profiles.Profiles.Count); + Assert.IsFalse(profiles.RemoveProfile(2)); + } + + [TestMethod] + public void SetProfile_AssignsIncreasingIds_AndAllowsDuplicateNames() + { + var profiles = new PowerDisplayProfiles { NextId = 1 }; + var a = MakeProfile("Same"); + var b = MakeProfile("Same"); + + profiles.SetProfile(a); + profiles.SetProfile(b); + + Assert.AreEqual(1, a.Id); + Assert.AreEqual(2, b.Id); + Assert.AreEqual(3, profiles.NextId); + Assert.AreEqual(2, profiles.Profiles.Count); // both kept: duplicate names allowed + } + + [TestMethod] + public void SetProfile_WithExplicitId_ReplacesSameIdAndHealsNextId() + { + var profiles = new PowerDisplayProfiles { NextId = 1 }; + var original = MakeProfile("A", id: 5); + profiles.SetProfile(original); // explicit id 5 + + var replacement = MakeProfile("A-edited", id: 5); + profiles.SetProfile(replacement); + + Assert.AreEqual(1, profiles.Profiles.Count); + Assert.AreSame(replacement, profiles.GetById(5)); + Assert.IsTrue(profiles.NextId > 5); // healed past the explicit id + } + + [TestMethod] + public void EnsureIds_BackfillsInOrder_SetsNextId_AndIsIdempotent() + { + var profiles = new PowerDisplayProfiles(); // NextId defaults to 0 (legacy file) + profiles.Profiles.Add(MakeProfile("A")); // Id 0 + profiles.Profiles.Add(MakeProfile("B")); // Id 0 + + Assert.IsTrue(profiles.EnsureIds()); + Assert.AreEqual(1, profiles.Profiles[0].Id); + Assert.AreEqual(2, profiles.Profiles[1].Id); + Assert.AreEqual(3, profiles.NextId); + + Assert.IsFalse(profiles.EnsureIds()); // second run: no change + } + + [TestMethod] + public void EnsureIds_SelfHealsNextId_AndPreservesExistingIds() + { + var profiles = new PowerDisplayProfiles { NextId = 2 }; // corrupt: <= existing max + profiles.Profiles.Add(MakeProfile("A", id: 5)); + profiles.Profiles.Add(MakeProfile("B")); // Id 0 + + Assert.IsTrue(profiles.EnsureIds()); + Assert.AreEqual(5, profiles.Profiles[0].Id); // preserved + Assert.AreEqual(6, profiles.Profiles[1].Id); // assigned above max + Assert.AreEqual(7, profiles.NextId); + } + + [TestMethod] + public void ProfileDisplayNameFormatter_UsesProvidedFormatOrder() + { + Assert.AreEqual( + "#4: Gaming", + ProfileDisplayNameFormatter.Format("Gaming", 4, "#{1}: {0}")); + } + + [TestMethod] + public void ProfileDisplayNameFormatter_InvalidFormat_FallsBackToNeutral() + { + Assert.AreEqual( + "Gaming (#4)", + ProfileDisplayNameFormatter.Format("Gaming", 4, "{0")); + } + + [TestMethod] + public void ProfileDisplayNameResource_ContainsNeutralFormat() + { + var resourceManager = new ResourceManager( + "PowerDisplay.Models.Properties.Resources", + typeof(PowerDisplayProfile).Assembly); + + Assert.AreEqual( + "{0} (#{1})", + resourceManager.GetString("ProfileDisplayNameFormat", CultureInfo.InvariantCulture)); + } + + [TestMethod] + public void DisplayName_CombinesNameAndId() + { + var p = MakeProfile("Gaming", id: 4); + Assert.AreEqual("Gaming (#4)", p.DisplayName); + } + + [TestMethod] + public void EnsureIds_ThenEditByAssignedId_ReplacesInsteadOfDuplicating() + { + // Legacy collection: profiles without ids (a pre-id profiles.json the app hasn't migrated). + var profiles = new PowerDisplayProfiles(); + profiles.Profiles.Add(MakeProfile("A")); // Id 0 + profiles.Profiles.Add(MakeProfile("B")); // Id 0 + + // The scanning migration back-fills ids before legacy profiles become editable, so the + // edited profile carries a stable id and SetProfile replaces it in place instead of adding a copy. + profiles.EnsureIds(); + var editedId = profiles.Profiles[0].Id; + + var edited = MakeProfile("A-renamed", id: editedId); + profiles.SetProfile(edited); + + Assert.AreEqual(2, profiles.Profiles.Count); // no duplicate created + Assert.AreSame(edited, profiles.GetById(editedId)); + Assert.AreEqual("A-renamed", profiles.GetById(editedId)!.Name); + } + + [TestMethod] + public void SetProfile_NewProfile_SelfHealsCorruptNextId_NoIdCollision() + { + // Corrupt/legacy counter: NextId sits at or below an id already in use. Adding new profiles + // (Id == 0) must still hand out ids above the highest in use, never colliding with it, even + // if SetProfile runs before EnsureIds has healed the counter. + var profiles = new PowerDisplayProfiles { NextId = 1 }; + profiles.Profiles.Add(MakeProfile("Existing", id: 5)); + + for (var i = 0; i < 6; i++) + { + profiles.SetProfile(MakeProfile("New")); + } + + var ids = profiles.Profiles.Select(p => p.Id).ToList(); + Assert.AreEqual(ids.Count, ids.Distinct().Count(), "profile ids must be unique"); + Assert.IsTrue(profiles.NextId > profiles.Profiles.Max(p => p.Id)); + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/ProfileMigrationTests.cs b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/ProfileMigrationTests.cs new file mode 100644 index 0000000000..de54f04243 --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/ProfileMigrationTests.cs @@ -0,0 +1,82 @@ +// 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.Collections.Generic; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using PowerDisplay.Common.Services; +using PowerDisplay.Models; + +namespace PowerDisplay.UnitTests; + +[TestClass] +public class ProfileMigrationTests +{ + private const string NewMonitorId = @"\\?\DISPLAY#DELD1A8#5&abc&0&UID12345"; + + [TestMethod] + public void Migrate_NoDiscoveredMonitors_BackfillsIdsWithoutChangingMonitorSettings() + { + var profile = MakeProfile("Legacy", "DDC_DELD1A8_1"); + var profiles = new PowerDisplayProfiles(); + profiles.Profiles.Add(profile); + + var changed = ProfileMigration.Migrate( + profiles, + System.Array.Empty<(string Id, int MonitorNumber)>()); + + Assert.IsTrue(changed); + Assert.AreEqual(1, profile.Id); + Assert.AreEqual("DDC_DELD1A8_1", profile.MonitorSettings[0].MonitorId); + Assert.AreEqual(2, profiles.NextId); + } + + [TestMethod] + public void Migrate_DiscoveredMonitor_BackfillsIdAndMigratesMonitorReference() + { + var profile = MakeProfile("Legacy", "DDC_DELD1A8_1"); + var profiles = new PowerDisplayProfiles(); + profiles.Profiles.Add(profile); + + var changed = ProfileMigration.Migrate( + profiles, + new[] { (NewMonitorId, 1) }); + + Assert.IsTrue(changed); + Assert.AreEqual(1, profile.Id); + Assert.AreEqual(1, profile.MonitorSettings.Count); + Assert.AreEqual(NewMonitorId, profile.MonitorSettings[0].MonitorId); + } + + [TestMethod] + public void Migrate_DiscoveredMonitor_DeduplicatesWhenNewSettingAlreadyExists() + { + var profile = new PowerDisplayProfile( + "Legacy", + new List + { + new ProfileMonitorSetting("DDC_DELD1A8_1", 50, null, null, null), + new ProfileMonitorSetting(NewMonitorId, 50, null, null, null), + }); + var profiles = new PowerDisplayProfiles(); + profiles.Profiles.Add(profile); + + var changed = ProfileMigration.Migrate( + profiles, + new[] { (NewMonitorId, 1) }); + + Assert.IsTrue(changed); + Assert.AreEqual(1, profile.MonitorSettings.Count); + Assert.AreEqual(NewMonitorId, profile.MonitorSettings[0].MonitorId); + } + + private static PowerDisplayProfile MakeProfile(string name, string monitorId) + { + return new PowerDisplayProfile( + name, + new List + { + new ProfileMonitorSetting(monitorId, 50, null, null, null), + }); + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/ProfileStoreTests.cs b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/ProfileStoreTests.cs new file mode 100644 index 0000000000..3924517c8a --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/ProfileStoreTests.cs @@ -0,0 +1,294 @@ +// 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 System.IO; +using System.Linq; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using PowerDisplay.Models; + +namespace PowerDisplay.UnitTests; + +[TestClass] +public class ProfileStoreTests +{ + private static readonly string[] ExpectedConcurrentProfileNames = { "First", "Second" }; + + private string _tempDir = string.Empty; + private string _profilesPath = string.Empty; + private string _mutexName = string.Empty; + + [TestInitialize] + public void SetUp() + { + _tempDir = Path.Combine(Path.GetTempPath(), $"pd-profile-store-test-{Guid.NewGuid():N}"); + Directory.CreateDirectory(_tempDir); + _profilesPath = Path.Combine(_tempDir, "profiles.json"); + _mutexName = $@"Local\PowerToys_PowerDisplay_ProfileStore_Test_{Guid.NewGuid():N}"; + } + + [TestCleanup] + public void TearDown() + { + try + { + if (File.Exists(_profilesPath)) + { + File.SetAttributes(_profilesPath, FileAttributes.Normal); + } + + if (Directory.Exists(_tempDir)) + { + Directory.Delete(_tempDir, recursive: true); + } + } + catch + { + // Best-effort cleanup. + } + } + + [TestMethod] + public void UpdateProfiles_CorruptJson_DoesNotOverwriteSource() + { + const string corruptJson = "{not-json"; + File.WriteAllText(_profilesPath, corruptJson); + var store = CreateStore(); + + Exception? exception = null; + try + { + store.UpdateProfiles(profiles => profiles.EnsureIds()); + } + catch (Exception ex) + { + exception = ex; + } + + Assert.IsInstanceOfType(exception); + Assert.AreEqual(corruptJson, File.ReadAllText(_profilesPath)); + } + + [TestMethod] + public void UpdateProfiles_SaveFails_DoesNotPublishTransientIdsOrReplaceSource() + { + var profiles = new PowerDisplayProfiles(); + profiles.Profiles.Add(MakeProfile("Legacy")); + var originalJson = JsonSerializer.Serialize(profiles, ProfileSerializationContext.Default.PowerDisplayProfiles); + File.WriteAllText(_profilesPath, originalJson); + File.SetAttributes(_profilesPath, FileAttributes.ReadOnly); + var store = CreateStore(); + + Exception? exception = null; + try + { + store.UpdateProfiles(loaded => loaded.EnsureIds()); + } + catch (Exception ex) + { + exception = ex; + } + + Assert.IsNotNull(exception); + Assert.AreEqual(originalJson, File.ReadAllText(_profilesPath)); + Assert.AreEqual(0, store.LoadProfiles().Profiles[0].Id); + Assert.IsFalse(Directory.EnumerateFiles(_tempDir, "*.tmp").Any()); + } + + [TestMethod] + public void AddOrUpdateProfile_WritesAtomicallyWithoutLeavingTemporaryFile() + { + var store = CreateStore(); + + store.AddOrUpdateProfile(MakeProfile("Gaming")); + + var loaded = store.LoadProfiles(); + Assert.AreEqual(1, loaded.Profiles.Count); + Assert.AreEqual(1, loaded.Profiles[0].Id); + Assert.IsFalse(Directory.EnumerateFiles(_tempDir, "*.tmp").Any()); + } + + [TestMethod] + public void AddOrUpdateProfile_SaveFails_RestoresIncomingProfileState() + { + var profiles = new PowerDisplayProfiles { NextId = 2 }; + profiles.Profiles.Add(MakeProfile("Existing", id: 1)); + var originalJson = JsonSerializer.Serialize(profiles, ProfileSerializationContext.Default.PowerDisplayProfiles); + File.WriteAllText(_profilesPath, originalJson); + File.SetAttributes(_profilesPath, FileAttributes.ReadOnly); + var incoming = MakeProfile("New"); + incoming.LastModified = DateTime.UnixEpoch; + var store = CreateStore(); + + Exception? exception = null; + try + { + store.AddOrUpdateProfile(incoming); + } + catch (Exception ex) + { + exception = ex; + } + + Assert.IsNotNull(exception); + Assert.AreEqual(0, incoming.Id); + Assert.AreEqual(DateTime.UnixEpoch, incoming.LastModified); + Assert.AreEqual(originalJson, File.ReadAllText(_profilesPath)); + Assert.IsFalse(Directory.EnumerateFiles(_tempDir, "*.tmp").Any()); + } + + [TestMethod] + public void SaveProfiles_SaveFails_RestoresLastUpdated() + { + File.WriteAllText(_profilesPath, "{}"); + File.SetAttributes(_profilesPath, FileAttributes.ReadOnly); + var profiles = new PowerDisplayProfiles { LastUpdated = DateTime.UnixEpoch }; + var store = CreateStore(); + + Exception? exception = null; + try + { + store.SaveProfiles(profiles); + } + catch (Exception ex) + { + exception = ex; + } + + Assert.IsNotNull(exception); + Assert.AreEqual(DateTime.UnixEpoch, profiles.LastUpdated); + } + + [TestMethod] + public void AddOrUpdateProfile_TwoStoresSharingMutex_PreserveBothUpdates() + { + var firstStore = CreateStore(); + var secondStore = CreateStore(); + using var start = new ManualResetEventSlim(); + + var first = Task.Run(() => + { + start.Wait(); + firstStore.AddOrUpdateProfile(MakeProfile("First")); + }); + var second = Task.Run(() => + { + start.Wait(); + secondStore.AddOrUpdateProfile(MakeProfile("Second")); + }); + + start.Set(); + Task.WaitAll(first, second); + + var loaded = firstStore.LoadProfiles(); + Assert.AreEqual(2, loaded.Profiles.Count); + Assert.AreEqual(2, loaded.Profiles.Select(profile => profile.Id).Distinct().Count()); + CollectionAssert.AreEquivalent( + ExpectedConcurrentProfileNames, + loaded.Profiles.Select(profile => profile.Name).ToArray()); + } + + [TestMethod] + public async Task UpdateProfilesAsync_WaitsWithoutBlockingCaller() + { + var firstStore = CreateStore(); + var secondStore = CreateStore(); + using var updateLoaded = new ManualResetEventSlim(); + using var continueUpdate = new ManualResetEventSlim(); + + var holder = Task.Run(() => + firstStore.UpdateProfiles(profiles => + { + updateLoaded.Set(); + continueUpdate.Wait(); + return false; + })); + + Assert.IsTrue(updateLoaded.Wait(TimeSpan.FromSeconds(5))); + + var waitingUpdate = secondStore.UpdateProfilesAsync(_ => false); + Assert.IsFalse(waitingUpdate.IsCompleted); + + continueUpdate.Set(); + await holder; + Assert.IsFalse(await waitingUpdate); + } + + [TestMethod] + public async Task AddOrUpdateProfileAsync_TwoStoresSharingMutex_PreserveBothUpdates() + { + var firstStore = CreateStore(); + var secondStore = CreateStore(); + using var start = new ManualResetEventSlim(); + + var first = Task.Run(async () => + { + start.Wait(); + await firstStore.AddOrUpdateProfileAsync(MakeProfile("First")); + }); + var second = Task.Run(async () => + { + start.Wait(); + await secondStore.AddOrUpdateProfileAsync(MakeProfile("Second")); + }); + + start.Set(); + await Task.WhenAll(first, second); + + var loaded = await firstStore.LoadProfilesAsync(); + Assert.AreEqual(2, loaded.Profiles.Count); + Assert.AreEqual(2, loaded.Profiles.Select(profile => profile.Id).Distinct().Count()); + } + + [TestMethod] + public void UpdateProfiles_HoldsMutexAcrossLoadModifySave() + { + var firstStore = CreateStore(); + var secondStore = CreateStore(); + using var updateLoaded = new ManualResetEventSlim(); + using var continueUpdate = new ManualResetEventSlim(); + + var first = Task.Run(() => + firstStore.UpdateProfiles(profiles => + { + updateLoaded.Set(); + continueUpdate.Wait(); + profiles.SetProfile(MakeProfile("First")); + return true; + })); + + Assert.IsTrue(updateLoaded.Wait(TimeSpan.FromSeconds(5))); + var second = Task.Run(() => secondStore.AddOrUpdateProfile(MakeProfile("Second"))); + continueUpdate.Set(); + Task.WaitAll(first, second); + + var loaded = firstStore.LoadProfiles(); + Assert.AreEqual(2, loaded.Profiles.Count); + CollectionAssert.AreEquivalent( + ExpectedConcurrentProfileNames, + loaded.Profiles.Select(profile => profile.Name).ToArray()); + } + + private ProfileStore CreateStore() + { + return new ProfileStore(_profilesPath, _mutexName, TimeSpan.FromSeconds(5)); + } + + private static PowerDisplayProfile MakeProfile(string name, int id = 0) + { + return new PowerDisplayProfile( + name, + new List + { + new ProfileMonitorSetting("MON1", 50, null, null, null), + }) + { + Id = id, + }; + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib/Services/ProfileMigration.cs b/src/modules/powerdisplay/PowerDisplay.Lib/Services/ProfileMigration.cs new file mode 100644 index 0000000000..af8da81749 --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Lib/Services/ProfileMigration.cs @@ -0,0 +1,77 @@ +// 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 System.Linq; +using ManagedCommon; +using PowerDisplay.Common.Models; +using PowerDisplay.Models; + +namespace PowerDisplay.Common.Services; + +public static class ProfileMigration +{ + public static bool Migrate( + PowerDisplayProfiles profiles, + IReadOnlyList<(string Id, int MonitorNumber)> discovered) + { + ArgumentNullException.ThrowIfNull(profiles); + ArgumentNullException.ThrowIfNull(discovered); + + var changed = profiles.EnsureIds(); + if (discovered.Count == 0) + { + return changed; + } + + foreach (var profile in profiles.Profiles) + { + if (profile?.MonitorSettings is null) + { + continue; + } + + var profileChanged = false; + foreach (var legacy in profile.MonitorSettings + .Where(setting => MonitorIdentity.IsLegacyId(setting?.MonitorId)) + .ToList()) + { + var newId = MonitorIdMigrator.MatchNewId(legacy.MonitorId, discovered); + if (newId != null + && profile.MonitorSettings.All( + setting => !MonitorIdComparer.Equal(setting.MonitorId, newId))) + { + profile.MonitorSettings.Add(new ProfileMonitorSetting( + newId, + legacy.Brightness, + legacy.ColorTemperatureVcp, + legacy.Contrast, + legacy.Volume)); + } + else if (newId != null) + { + Logger.LogInfo( + $"[LegacyMigration] Skipped duplicate profile setting for '{legacy.MonitorId}' in profile '{profile.Name}': '{newId}' already exists."); + } + else if (newId == null) + { + Logger.LogWarning( + $"[LegacyMigration] Dropping profile setting for '{legacy.MonitorId}' in profile '{profile.Name}': no current monitor with matching EdidId+MonitorNumber."); + } + + profile.MonitorSettings.Remove(legacy); + profileChanged = true; + } + + if (profileChanged) + { + profile.Touch(); + changed = true; + } + } + + return changed; + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Lib/Services/ProfileService.cs b/src/modules/powerdisplay/PowerDisplay.Lib/Services/ProfileService.cs deleted file mode 100644 index 86995bd390..0000000000 --- a/src/modules/powerdisplay/PowerDisplay.Lib/Services/ProfileService.cs +++ /dev/null @@ -1,23 +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 PowerDisplay.Models; -using ModelsProfileHelper = PowerDisplay.Models.ProfileHelper; - -namespace PowerDisplay.Common.Services -{ - /// - /// Thin facade over that provides named static entry points - /// for PowerDisplay.exe callers. - /// All locking and compound-operation atomicity is handled by . - /// - public static class ProfileService - { - /// - public static PowerDisplayProfiles LoadProfiles() => ModelsProfileHelper.LoadProfiles(); - - /// - public static bool SaveProfiles(PowerDisplayProfiles profiles) => ModelsProfileHelper.SaveProfiles(profiles); - } -} diff --git a/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplay.Models.csproj b/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplay.Models.csproj index 04e34dd8fd..079e18c770 100644 --- a/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplay.Models.csproj +++ b/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplay.Models.csproj @@ -17,6 +17,9 @@ true + + + diff --git a/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplayProfile.cs b/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplayProfile.cs index e509b7d58d..49646fa860 100644 --- a/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplayProfile.cs +++ b/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplayProfile.cs @@ -16,6 +16,9 @@ namespace PowerDisplay.Models [JsonPropertyName("name")] public string Name { get; set; } + [JsonPropertyName("id")] + public int Id { get; set; } + [JsonPropertyName("monitorSettings")] public List MonitorSettings { get; set; } @@ -56,5 +59,12 @@ namespace PowerDisplay.Models { LastModified = DateTime.UtcNow; } + + /// + /// Gets a human-readable label that disambiguates duplicate names, e.g. "Gaming (#4)". + /// Not serialized; UI display only. + /// + [JsonIgnore] + public string DisplayName => ProfileDisplayNameFormatter.Format(Name, Id); } } diff --git a/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplayProfiles.cs b/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplayProfiles.cs index c2baf0f4ed..46fe41a48a 100644 --- a/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplayProfiles.cs +++ b/src/modules/powerdisplay/PowerDisplay.Models/PowerDisplayProfiles.cs @@ -19,6 +19,9 @@ namespace PowerDisplay.Models [JsonPropertyName("profiles")] public List Profiles { get; set; } + [JsonPropertyName("nextId")] + public int NextId { get; set; } + [JsonPropertyName("lastUpdated")] public DateTime LastUpdated { get; set; } @@ -29,15 +32,36 @@ namespace PowerDisplay.Models } /// - /// Gets the profile by name + /// Gets the first profile whose name matches a pre-ID persisted reference. + /// This lookup is only for legacy migration because profile names are not unique. /// - public PowerDisplayProfile? GetProfile(string name) + public PowerDisplayProfile? GetLegacyProfileByName(string name) { - return Profiles.FirstOrDefault(p => p.Name.Equals(name, StringComparison.OrdinalIgnoreCase)); + return Profiles.FirstOrDefault( + profile => profile.Name.Equals(name, StringComparison.OrdinalIgnoreCase)); } /// - /// Adds or updates a profile + /// Gets the profile by its stable id, or null when id is not positive or no profile has it. + /// + public PowerDisplayProfile? GetById(int id) + { + return id <= 0 ? null : Profiles.FirstOrDefault(p => p.Id == id); + } + + /// + /// Returns profiles that have a usable stable id. + /// Legacy or corrupt profiles with non-positive ids remain hidden until migration. + /// + public IEnumerable GetAssignedProfiles() + { + return Profiles.Where(profile => profile is not null && profile.Id >= 1); + } + + /// + /// Adds or updates a profile, keyed by its stable id. When the incoming profile has no id + /// (Id == 0) a new one is assigned from the monotonic NextId counter. Names are not required + /// to be unique. /// public void SetProfile(PowerDisplayProfile profile) { @@ -46,10 +70,28 @@ namespace PowerDisplay.Models throw new ArgumentException("Profile is invalid"); } - var existing = GetProfile(profile.Name); - if (existing != null) + if (profile.Id == 0) { - Profiles.Remove(existing); + // Assign the next id, self-healing a corrupt/legacy NextId that isn't already past + // the highest id in use (mirrors EnsureIds). This guarantees a new profile never + // collides with an existing one even when SetProfile runs before EnsureIds. + var maxId = Profiles.Count == 0 ? 0 : Profiles.Max(p => p?.Id ?? 0); + var next = Math.Max(Math.Max(NextId, 1), maxId + 1); + profile.Id = next; + NextId = next + 1; + } + else + { + var existing = GetById(profile.Id); + if (existing != null) + { + Profiles.Remove(existing); + } + + if (NextId <= profile.Id) + { + NextId = profile.Id + 1; + } } profile.Touch(); @@ -58,11 +100,11 @@ namespace PowerDisplay.Models } /// - /// Removes a profile by name + /// Removes a profile by its stable id. /// - public bool RemoveProfile(string name) + public bool RemoveProfile(int id) { - var profile = GetProfile(name); + var profile = GetById(id); if (profile != null) { Profiles.Remove(profile); @@ -74,23 +116,33 @@ namespace PowerDisplay.Models } /// - /// Checks if a profile name is valid and available + /// One-shot upgrade: assigns a stable id to every profile still missing one (Id == 0), in + /// list order, and advances NextId past the highest id in use (self-healing a corrupt or + /// legacy counter). Returns true when anything changed. Idempotent on subsequent calls. /// - public bool IsNameAvailable(string name, string? excludeName = null) + public bool EnsureIds() { - if (string.IsNullOrWhiteSpace(name)) + var changed = false; + + var maxId = Profiles.Count == 0 ? 0 : Profiles.Max(p => p?.Id ?? 0); + var next = Math.Max(Math.Max(NextId, 1), maxId + 1); + + foreach (var p in Profiles) { - return false; + if (p is not null && p.Id == 0) + { + p.Id = next++; + changed = true; + } } - // Check if name is already used (excluding the profile being renamed) - var existing = GetProfile(name); - if (existing != null && (excludeName == null || !existing.Name.Equals(excludeName, StringComparison.OrdinalIgnoreCase))) + if (NextId != next) { - return false; + NextId = next; + changed = true; } - return true; + return changed; } } } diff --git a/src/modules/powerdisplay/PowerDisplay.Models/ProfileDisplayNameFormatter.cs b/src/modules/powerdisplay/PowerDisplay.Models/ProfileDisplayNameFormatter.cs new file mode 100644 index 0000000000..e86672e489 --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Models/ProfileDisplayNameFormatter.cs @@ -0,0 +1,61 @@ +// 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.Concurrent; +using System.Diagnostics; +using System.Globalization; +using System.Resources; +using System.Text; + +namespace PowerDisplay.Models +{ + internal static class ProfileDisplayNameFormatter + { + private const string NeutralFormat = "{0} (#{1})"; + private const string ResourceName = "ProfileDisplayNameFormat"; + private static readonly CompositeFormat NeutralCompositeFormat = CompositeFormat.Parse(NeutralFormat); + private static readonly ConcurrentDictionary ParsedFormats = new(StringComparer.Ordinal); + + private static readonly ResourceManager ResourceManager = new( + "PowerDisplay.Models.Properties.Resources", + typeof(ProfileDisplayNameFormatter).Assembly); + + public static string Format(string name, int id) + { + var format = ResourceManager.GetString( + ResourceName, + CultureInfo.CurrentUICulture); + return Format(name, id, format); + } + + internal static string Format(string name, int id, string? format) + { + try + { + var selectedFormat = string.IsNullOrEmpty(format) + ? NeutralCompositeFormat + : string.Equals(format, NeutralFormat, StringComparison.Ordinal) + ? NeutralCompositeFormat + : ParsedFormats.GetOrAdd(format, static value => CompositeFormat.Parse(value)); + + return string.Format( + CultureInfo.CurrentCulture, + selectedFormat, + name, + id); + } + catch (FormatException ex) + { + Trace.TraceError( + $"Invalid {ResourceName} resource: {ex.Message}"); + return string.Format( + CultureInfo.CurrentCulture, + NeutralCompositeFormat, + name, + id); + } + } + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Models/ProfileHelper.cs b/src/modules/powerdisplay/PowerDisplay.Models/ProfileHelper.cs index 2bd9f50180..3522c5fd2b 100644 --- a/src/modules/powerdisplay/PowerDisplay.Models/ProfileHelper.cs +++ b/src/modules/powerdisplay/PowerDisplay.Models/ProfileHelper.cs @@ -4,19 +4,20 @@ using System; using System.IO; -using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; namespace PowerDisplay.Models { /// /// Helper for loading and saving PowerDisplay profiles from/to disk. /// Provides shared file I/O logic used by both Settings UI and PowerDisplay module. - /// Thread-safe and AOT-compatible. - /// All compound operations (load → modify → save) are atomic within a single process. + /// Thread-safe across processes and AOT-compatible. + /// All compound operations (load → modify → save) are atomic. /// public static class ProfileHelper { - private static readonly object _lock = new object(); + private const string ProfilesMutexName = @"Local\PowerToys_PowerDisplay_Profiles"; private static readonly Lazy _profilesFilePath = new Lazy(() => Path.Combine( @@ -26,171 +27,28 @@ namespace PowerDisplay.Models "PowerDisplay", "profiles.json")); + private static readonly Lazy _profileStore = new Lazy(() => + new ProfileStore(ProfilesFilePath, ProfilesMutexName, TimeSpan.FromSeconds(5))); + /// /// Gets the full path to the profiles JSON file. /// public static string ProfilesFilePath => _profilesFilePath.Value; - /// - /// Loads PowerDisplay profiles from disk. - /// Thread-safe operation. - /// - /// PowerDisplayProfiles object, or a new empty instance if file doesn't exist or load fails. - public static PowerDisplayProfiles LoadProfiles() - { - lock (_lock) - { - return LoadProfilesCore(); - } - } + public static Task LoadProfilesAsync(CancellationToken cancellationToken = default) + => _profileStore.Value.LoadProfilesAsync(cancellationToken); - /// - /// Saves PowerDisplay profiles to disk. - /// Thread-safe operation with automatic timestamp update. - /// - /// The profiles collection to save. - /// True if save was successful, false otherwise. - public static bool SaveProfiles(PowerDisplayProfiles profiles) - { - lock (_lock) - { - return SaveProfilesCore(profiles); - } - } + public static Task AddOrUpdateProfileAsync( + PowerDisplayProfile profile, + CancellationToken cancellationToken = default) + => _profileStore.Value.AddOrUpdateProfileAsync(profile, cancellationToken); - /// - /// Adds or updates a profile and persists to disk atomically. - /// - /// The profile to add or update. - /// True if the operation was successful, false otherwise. - public static bool AddOrUpdateProfile(PowerDisplayProfile profile) - { - if (profile == null || !profile.IsValid()) - { - return false; - } + public static Task RemoveProfileByIdAsync(int id, CancellationToken cancellationToken = default) + => _profileStore.Value.RemoveProfileByIdAsync(id, cancellationToken); - lock (_lock) - { - var profiles = LoadProfilesCore(); - profiles.SetProfile(profile); - return SaveProfilesCore(profiles); - } - } - - /// - /// Renames and updates a profile atomically (for rename or edit operations). - /// Removes the old entry by and upserts the updated profile. - /// - /// The current name of the profile to replace. - /// The updated profile. - /// True if the operation was successful, false otherwise. - public static bool RenameAndUpdateProfile(string oldName, PowerDisplayProfile newProfile) - { - if (newProfile == null || !newProfile.IsValid()) - { - return false; - } - - lock (_lock) - { - var profiles = LoadProfilesCore(); - profiles.RemoveProfile(oldName); - profiles.SetProfile(newProfile); - return SaveProfilesCore(profiles); - } - } - - /// - /// Removes a profile by name and persists to disk atomically. - /// - /// The name of the profile to remove. - /// True if the profile was found and removed, false otherwise. - public static bool RemoveProfile(string profileName) - { - lock (_lock) - { - var profiles = LoadProfilesCore(); - bool removed = profiles.RemoveProfile(profileName); - if (removed) - { - SaveProfilesCore(profiles); - } - - return removed; - } - } - - /// - /// Gets a profile by name. - /// - /// The name of the profile to retrieve. - /// The profile if found, null otherwise. - public static PowerDisplayProfile? GetProfile(string profileName) - { - lock (_lock) - { - return LoadProfilesCore().GetProfile(profileName); - } - } - - // Lock-free core methods — only call from within a lock (_lock) block. - private static PowerDisplayProfiles LoadProfilesCore() - { - try - { - EnsureFolderExists(); - - if (File.Exists(ProfilesFilePath)) - { - var json = File.ReadAllText(ProfilesFilePath); - var profiles = JsonSerializer.Deserialize(json, ProfileSerializationContext.Default.PowerDisplayProfiles); - - if (profiles != null) - { - return profiles; - } - } - - return new PowerDisplayProfiles(); - } - catch (Exception) - { - return new PowerDisplayProfiles(); - } - } - - private static bool SaveProfilesCore(PowerDisplayProfiles profiles) - { - try - { - if (profiles == null) - { - return false; - } - - EnsureFolderExists(); - - profiles.LastUpdated = DateTime.UtcNow; - - var json = JsonSerializer.Serialize(profiles, ProfileSerializationContext.Default.PowerDisplayProfiles); - File.WriteAllText(ProfilesFilePath, json); - - return true; - } - catch (Exception) - { - return false; - } - } - - private static void EnsureFolderExists() - { - var folder = Path.GetDirectoryName(ProfilesFilePath); - if (folder != null && !Directory.Exists(folder)) - { - Directory.CreateDirectory(folder); - } - } + public static Task UpdateProfilesAsync( + Func update, + CancellationToken cancellationToken = default) + => _profileStore.Value.UpdateProfilesAsync(update, cancellationToken); } } diff --git a/src/modules/powerdisplay/PowerDisplay.Models/ProfileStore.cs b/src/modules/powerdisplay/PowerDisplay.Models/ProfileStore.cs new file mode 100644 index 0000000000..3ec3564e66 --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Models/ProfileStore.cs @@ -0,0 +1,231 @@ +// 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.IO; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; + +namespace PowerDisplay.Models +{ + internal sealed class ProfileStore + { + private readonly string _filePath; + private readonly string _mutexName; + private readonly TimeSpan _mutexTimeout; + private readonly object _processLock = new object(); + + internal ProfileStore(string filePath, string mutexName, TimeSpan mutexTimeout) + { + _filePath = filePath; + _mutexName = mutexName; + _mutexTimeout = mutexTimeout; + } + + internal PowerDisplayProfiles LoadProfiles() + { + return ExecuteLocked(LoadProfilesCore); + } + + internal Task LoadProfilesAsync(CancellationToken cancellationToken = default) + => RunAsync(LoadProfiles, cancellationToken); + + internal void SaveProfiles(PowerDisplayProfiles profiles) + { + ArgumentNullException.ThrowIfNull(profiles); + ExecuteLocked(() => SaveProfilesCore(profiles)); + } + + internal void AddOrUpdateProfile(PowerDisplayProfile profile) + { + if (profile == null || !profile.IsValid()) + { + throw new ArgumentException("Profile is invalid", nameof(profile)); + } + + ExecuteLocked(() => + { + var profiles = LoadProfilesCore(); + var originalId = profile.Id; + var originalLastModified = profile.LastModified; + try + { + profiles.SetProfile(profile); + SaveProfilesCore(profiles); + } + catch + { + profile.Id = originalId; + profile.LastModified = originalLastModified; + throw; + } + }); + } + + internal Task AddOrUpdateProfileAsync( + PowerDisplayProfile profile, + CancellationToken cancellationToken = default) + => RunAsync( + () => + { + AddOrUpdateProfile(profile); + return true; + }, + cancellationToken); + + internal bool RemoveProfileById(int id) + { + return ExecuteLocked(() => + { + var profiles = LoadProfilesCore(); + if (!profiles.RemoveProfile(id)) + { + return false; + } + + SaveProfilesCore(profiles); + return true; + }); + } + + internal Task RemoveProfileByIdAsync(int id, CancellationToken cancellationToken = default) + => RunAsync(() => RemoveProfileById(id), cancellationToken); + + internal bool UpdateProfiles(Func update) + { + ArgumentNullException.ThrowIfNull(update); + return ExecuteLocked(() => + { + var profiles = LoadProfilesCore(); + if (!update(profiles)) + { + return false; + } + + SaveProfilesCore(profiles); + return true; + }); + } + + internal Task UpdateProfilesAsync( + Func update, + CancellationToken cancellationToken = default) + => RunAsync(() => UpdateProfiles(update), cancellationToken); + + private T ExecuteLocked(Func operation) + { + lock (_processLock) + { + using var mutex = new Mutex(initiallyOwned: false, _mutexName); + var acquired = false; + try + { + try + { + acquired = mutex.WaitOne(_mutexTimeout); + } + catch (AbandonedMutexException) + { + acquired = true; + } + + if (!acquired) + { + throw new TimeoutException($"Timed out waiting for the profile store mutex after {_mutexTimeout}."); + } + + return operation(); + } + finally + { + if (acquired) + { + mutex.ReleaseMutex(); + } + } + } + } + + private void ExecuteLocked(Action operation) + { + ExecuteLocked(() => + { + operation(); + return true; + }); + } + + private static Task RunAsync(Func operation, CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(operation); + return Task.Run(operation, cancellationToken); + } + + private PowerDisplayProfiles LoadProfilesCore() + { + EnsureFolderExists(); + + if (!File.Exists(_filePath)) + { + return new PowerDisplayProfiles(); + } + + var json = File.ReadAllText(_filePath); + return JsonSerializer.Deserialize(json, ProfileSerializationContext.Default.PowerDisplayProfiles) + ?? throw new JsonException($"Profile file '{_filePath}' deserialized to null."); + } + + private void SaveProfilesCore(PowerDisplayProfiles profiles) + { + EnsureFolderExists(); + var originalLastUpdated = profiles.LastUpdated; + var temporaryPath = $"{_filePath}.{Guid.NewGuid():N}.tmp"; + + try + { + profiles.LastUpdated = DateTime.UtcNow; + var payload = JsonSerializer.SerializeToUtf8Bytes( + profiles, + ProfileSerializationContext.Default.PowerDisplayProfiles); + using (var stream = new FileStream( + temporaryPath, + FileMode.CreateNew, + FileAccess.Write, + FileShare.None, + bufferSize: 4096, + options: FileOptions.WriteThrough)) + { + stream.Write(payload); + stream.Flush(flushToDisk: true); + } + + File.Move(temporaryPath, _filePath, overwrite: true); + } + catch + { + profiles.LastUpdated = originalLastUpdated; + try + { + File.Delete(temporaryPath); + } + catch + { + // Best-effort cleanup; the original persistence exception is rethrown. + } + + throw; + } + } + + private void EnsureFolderExists() + { + var folder = Path.GetDirectoryName(_filePath); + if (!string.IsNullOrEmpty(folder)) + { + Directory.CreateDirectory(folder); + } + } + } +} diff --git a/src/modules/powerdisplay/PowerDisplay.Models/Properties/Resources.resx b/src/modules/powerdisplay/PowerDisplay.Models/Properties/Resources.resx new file mode 100644 index 0000000000..190f6f660d --- /dev/null +++ b/src/modules/powerdisplay/PowerDisplay.Models/Properties/Resources.resx @@ -0,0 +1,19 @@ + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + {0} (#{1}) + {0} is the profile name. {1} is the stable profile ID. + + diff --git a/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/App.xaml.cs b/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/App.xaml.cs index ab750c44f2..1040c576b9 100644 --- a/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/App.xaml.cs +++ b/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/App.xaml.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Globalization; using System.Threading; using System.Threading.Tasks; using ManagedCommon; @@ -377,12 +378,18 @@ namespace PowerDisplay } else if (messageType == Constants.PowerDisplayApplyProfileMessage()) { - // Apply profile by name - if (messageParts.Length > 1 && _mainWindow is MainWindow mainWindow && mainWindow.ViewModel != null) + if (messageParts.Length <= 1 + || !int.TryParse(messageParts[1].Trim(), NumberStyles.None, CultureInfo.InvariantCulture, out var profileId) + || profileId < 1) { - var profileName = messageParts[1].Trim(); - Logger.LogInfo($"[NamedPipe] Applying profile: {profileName}"); - await mainWindow.ViewModel.ApplyProfileByNameAsync(profileName); + Logger.LogWarning("[NamedPipe] ApplyProfile message is missing a valid positive profile id"); + return; + } + + if (_mainWindow is MainWindow mainWindow && mainWindow.ViewModel != null) + { + Logger.LogInfo($"[NamedPipe] Applying profile id: {profileId}"); + await mainWindow.ViewModel.ApplyProfileByIdAsync(profileId); } } else if (messageType == Constants.PowerDisplayTerminateAppMessage()) diff --git a/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml b/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml index f4b1a71b6c..d9b8e5918d 100644 --- a/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml +++ b/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml @@ -706,28 +706,42 @@ x:Name="ProfilesFlyout" Opened="Flyout_Opened" ShouldConstrainToRootBounds="False"> - - - - - - + + + + + + + - - - + x:Uid="ProfilesHeader" + Margin="{StaticResource FlyoutListHeaderMargin}" + FontSize="{StaticResource FlyoutSecondaryTextFontSize}" + Foreground="{ThemeResource TextFillColorSecondaryBrush}" /> + + + + + + + + diff --git a/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml.cs b/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml.cs index 7a8340aaed..da5d3a1628 100644 --- a/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml.cs +++ b/src/modules/powerdisplay/PowerDisplay/PowerDisplayXAML/MainWindow.xaml.cs @@ -479,7 +479,7 @@ namespace PowerDisplay return; } - Logger.LogInfo($"[UI] ProfileListView_SelectionChanged: Applying profile '{selectedProfile.Name}'"); + Logger.LogInfo($"[UI] ProfileListView_SelectionChanged: Applying profile '{selectedProfile.DisplayName}'"); // Apply profile via ViewModel command if (_viewModel?.ApplyProfileCommand?.CanExecute(selectedProfile) == true) diff --git a/src/modules/powerdisplay/PowerDisplay/Services/LightSwitchService.cs b/src/modules/powerdisplay/PowerDisplay/Services/LightSwitchService.cs index 7182fd32ed..bd90fdce23 100644 --- a/src/modules/powerdisplay/PowerDisplay/Services/LightSwitchService.cs +++ b/src/modules/powerdisplay/PowerDisplay/Services/LightSwitchService.cs @@ -5,67 +5,61 @@ using System; using ManagedCommon; using Microsoft.PowerToys.Settings.UI.Library; +using PowerDisplay.Models; using Settings.UI.Library; namespace PowerDisplay.Services { - /// - /// Service for handling LightSwitch theme change events. - /// Reads LightSwitch settings using the standard PowerToys settings pattern. - /// - public static class LightSwitchService + internal static class LightSwitchService { private const string LogPrefix = "[LightSwitch]"; - /// - /// Get the profile name to apply for the given theme. - /// - /// Whether the theme changed to light mode. - /// The profile name to apply, or null if no profile is configured. - public static string? GetProfileForTheme(bool isLightMode) + public static void MigrateLegacyProfileReferences(PowerDisplayProfiles profiles) + { + ArgumentNullException.ThrowIfNull(profiles); + + try + { + var settings = SettingsUtils.Default.GetSettingsOrDefault( + LightSwitchSettings.ModuleName); + + if (!LightSwitchProfileReferenceHelper.ReconcileReferences( + settings.Properties, + profiles)) + { + return; + } + + SettingsUtils.Default.SaveSettings( + settings.ToJsonString(), + LightSwitchSettings.ModuleName); + Logger.LogInfo($"{LogPrefix} Migrated legacy profile references to ids"); + } + catch (Exception ex) + { + Logger.LogError($"{LogPrefix} Failed to migrate legacy profile references: {ex.Message}"); + } + } + + public static int? GetProfileIdForTheme(bool isLightMode) { try { - Logger.LogInfo($"{LogPrefix} Processing theme change to {(isLightMode ? "light" : "dark")} mode"); + var settings = SettingsUtils.Default.GetSettingsOrDefault( + LightSwitchSettings.ModuleName); + var profileId = LightSwitchProfileReferenceHelper.GetProfileIdForTheme( + settings.Properties, + isLightMode); - var settings = SettingsUtils.Default.GetSettingsOrDefault(LightSwitchSettings.ModuleName); - - if (settings?.Properties == null) + if (profileId is null) { - Logger.LogWarning($"{LogPrefix} LightSwitch settings not found"); + Logger.LogTrace( + $"{LogPrefix} No enabled profile id configured for {(isLightMode ? "light" : "dark")} mode"); return null; } - string? profileName; - if (isLightMode) - { - if (!settings.Properties.EnableLightModeProfile.Value) - { - Logger.LogInfo($"{LogPrefix} Light mode profile is disabled"); - return null; - } - - profileName = settings.Properties.LightModeProfile.Value; - } - else - { - if (!settings.Properties.EnableDarkModeProfile.Value) - { - Logger.LogInfo($"{LogPrefix} Dark mode profile is disabled"); - return null; - } - - profileName = settings.Properties.DarkModeProfile.Value; - } - - if (string.IsNullOrEmpty(profileName) || profileName == "(None)") - { - Logger.LogInfo($"{LogPrefix} No profile configured for {(isLightMode ? "light" : "dark")} mode"); - return null; - } - - Logger.LogInfo($"{LogPrefix} Profile to apply: {profileName}"); - return profileName; + Logger.LogInfo($"{LogPrefix} Profile id to apply: {profileId.Value}"); + return profileId; } catch (Exception ex) { diff --git a/src/modules/powerdisplay/PowerDisplay/Strings/en-us/Resources.resw b/src/modules/powerdisplay/PowerDisplay/Strings/en-us/Resources.resw index a9daf7f0c2..5909db5e19 100644 --- a/src/modules/powerdisplay/PowerDisplay/Strings/en-us/Resources.resw +++ b/src/modules/powerdisplay/PowerDisplay/Strings/en-us/Resources.resw @@ -252,6 +252,12 @@ Profiles + + Loading profiles... + + + Loading profiles + Color temperature diff --git a/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.Monitors.cs b/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.Monitors.cs index dd4308a37e..1ad46e3bf7 100644 --- a/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.Monitors.cs +++ b/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.Monitors.cs @@ -40,7 +40,7 @@ public partial class MainViewModel { try { - UpdateMonitorList(monitors, isInitialLoad: true); + UpdateMonitorList(monitors); // Complete initialization asynchronously (restore settings if enabled) // IsScanning remains true until restore completes @@ -71,6 +71,13 @@ public partial class MainViewModel { try { + var discovered = Monitors + .Where(monitor => !string.IsNullOrEmpty(monitor.Id)) + .Select(monitor => (monitor.Id, monitor.MonitorNumber)) + .ToList(); + + await MigrateLegacySideFilesAsync(discovered, _cancellationTokenSource.Token); + // Check if we should restore settings on startup var settings = _settingsUtils.GetSettingsOrDefault(PowerDisplaySettings.ModuleName); if (settings.Properties.RestoreSettingsOnStartup) @@ -119,7 +126,7 @@ public partial class MainViewModel _dispatcherQueue.TryEnqueue(() => { - UpdateMonitorList(monitors, isInitialLoad: false); + UpdateMonitorList(monitors); IsScanning = false; }); } @@ -133,7 +140,7 @@ public partial class MainViewModel } } - private void UpdateMonitorList(IReadOnlyList monitors, bool isInitialLoad) + private void UpdateMonitorList(IReadOnlyList monitors) { CancelPendingLinkedBrightnessCommit(); @@ -170,14 +177,6 @@ public partial class MainViewModel // Save monitor information to settings SaveMonitorsToSettings(); - // First successful discovery after process start is the natural place to clean up - // any legacy "{Source}_{EdidId}_{N}" Ids still lingering in the side files that - // SaveMonitorsToSettings doesn't touch (profiles.json + monitor_state.json). - if (isInitialLoad) - { - MigrateLegacyMonitorIdsInSideFiles(); - } - // Note: RestoreMonitorSettingsAsync is now called from InitializeAsync/CompleteInitializationAsync // to ensure scanning state is maintained until restore completes } diff --git a/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.Settings.cs b/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.Settings.cs index 181e6ca81e..849c76dcf0 100644 --- a/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.Settings.cs +++ b/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.Settings.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using ManagedCommon; using Microsoft.PowerToys.Settings.UI.Library; @@ -99,7 +100,7 @@ public partial class MainViewModel { // Rebuild monitor list with updated hidden monitor settings // UpdateMonitorList already handles filtering hidden monitors - UpdateMonitorList(_monitorManager.Monitors, isInitialLoad: false); + UpdateMonitorList(_monitorManager.Monitors); // Reload UI display settings first (includes custom VCP mappings) // Must be loaded before ApplyUIConfiguration so names are available for UI refresh @@ -115,8 +116,8 @@ public partial class MainViewModel // RefreshMonitorsAsync, so this is a no-op-safe redundant push. _monitorManager.SetMaxCompatibilityMode(settings.Properties.MaxCompatibilityMode); - // Reload profiles in case they were added/updated/deleted in Settings UI - LoadProfiles(); + // Reload profiles in case they were added/updated/deleted in Settings UI. + _ = ReloadProfilesAsync(_cancellationTokenSource.Token); // Notify MonitorViewModels to refresh their custom VCP name displays foreach (var monitor in Monitors) @@ -154,33 +155,50 @@ public partial class MainViewModel } /// - /// Apply profile by name (called via Named Pipe from Settings UI) - /// This is the new direct method that receives the profile name via IPC. + /// Loads the saved profiles and returns the valid profile with the given id, or null (logging a + /// warning under ) when it is missing or invalid. /// - /// The name of the profile to apply. - public async Task ApplyProfileByNameAsync(string profileName) + private static async Task LoadValidProfileByIdAsync( + int profileId, + string logPrefix, + CancellationToken cancellationToken = default) + { + var profile = (await ProfileHelper.LoadProfilesAsync(cancellationToken)).GetById(profileId); + if (profile == null || !profile.IsValid()) + { + Logger.LogWarning($"{logPrefix} Profile id {profileId} not found or invalid"); + return null; + } + + return profile; + } + + /// + /// Apply profile by id (called via Named Pipe from Settings UI). Preserves GUI behavior; + /// only the lookup key changed from name to the stable id. + /// + /// The stable id of the profile to apply. + public async Task ApplyProfileByIdAsync(int profileId) { try { - Logger.LogInfo($"[Profile] Applying profile by name: {profileName}"); + Logger.LogInfo($"[Profile] Applying profile by id: {profileId}"); - // Load profiles and find the requested one - var profilesData = ProfileService.LoadProfiles(); - var profile = profilesData.GetProfile(profileName); - - if (profile == null || !profile.IsValid()) + var profile = await LoadValidProfileByIdAsync( + profileId, + "[Profile]", + _cancellationTokenSource.Token); + if (profile == null) { - Logger.LogWarning($"[Profile] Profile '{profileName}' not found or invalid"); return; } - // Apply the profile settings to monitors await ApplyProfileAsync(profile.MonitorSettings); - Logger.LogInfo($"[Profile] Successfully applied profile: {profileName}"); + Logger.LogInfo($"[Profile] Successfully applied profile id: {profileId}"); } catch (Exception ex) { - Logger.LogError($"[Profile] Failed to apply profile '{profileName}': {ex.Message}"); + Logger.LogError($"[Profile] Failed to apply profile id {profileId}: {ex.Message}"); } } @@ -191,38 +209,31 @@ public partial class MainViewModel /// Whether the theme changed to light mode. public void ApplyLightSwitchProfile(bool isLightMode) { - var profileName = LightSwitchService.GetProfileForTheme(isLightMode); - - if (string.IsNullOrEmpty(profileName)) - { - return; - } - _ = Task.Run(async () => { try { - Logger.LogInfo($"[LightSwitch Integration] Applying profile: {profileName}"); - - // Load and apply the profile - var profilesData = ProfileService.LoadProfiles(); - var profile = profilesData.GetProfile(profileName); - - if (profile == null || !profile.IsValid()) + var profileId = LightSwitchService.GetProfileIdForTheme(isLightMode); + if (profileId is null) + { + return; + } + + Logger.LogInfo($"[LightSwitch Integration] Applying profile id: {profileId.Value}"); + + var profile = await LoadValidProfileByIdAsync( + profileId.Value, + "[LightSwitch Integration]", + _cancellationTokenSource.Token); + if (profile == null) { - Logger.LogWarning($"[LightSwitch Integration] Profile '{profileName}' not found or invalid"); return; } // Apply the profile - need to dispatch to UI thread since MonitorViewModels are UI-bound - var tcs = new TaskCompletionSource(); - var enqueued = _dispatcherQueue.TryEnqueue(() => - { - // Start the async operation and handle completion - _ = ApplyProfileAndCompleteAsync(profile.MonitorSettings, tcs); - }); - - if (!enqueued) + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + if (!_dispatcherQueue.TryEnqueue( + () => _ = ApplyProfileAndCompleteAsync(profile.MonitorSettings, tcs))) { Logger.LogError($"[LightSwitch Integration] Failed to enqueue profile application to UI thread"); return; @@ -574,85 +585,77 @@ public partial class MainViewModel /// Invoked from the first successful discovery; on subsequent runs every entry is /// already in new-format and the filters short-circuit. /// - private void MigrateLegacyMonitorIdsInSideFiles() + private async Task MigrateLegacySideFilesAsync( + List<(string Id, int MonitorNumber)> discovered, + CancellationToken cancellationToken) { - var discovered = Monitors - .Where(m => !string.IsNullOrEmpty(m.Id)) - .Select(m => (m.Id, m.MonitorNumber)) - .ToList(); + PowerDisplayProfiles? migratedProfiles = null; + var profilesChanged = false; + + try + { + await RunProfileOperationAsync( + async token => + { + PowerDisplayProfiles? loadedProfiles = null; + profilesChanged = await ProfileHelper.UpdateProfilesAsync( + profiles => + { + var changed = ProfileMigration.Migrate(profiles, discovered); + loadedProfiles = profiles; + return changed; + }, + token); + token.ThrowIfCancellationRequested(); + + if (loadedProfiles is null) + { + throw new InvalidOperationException("Profile update completed without loaded profiles."); + } + + migratedProfiles = loadedProfiles; + ReplaceProfiles(loadedProfiles); + }, + cancellationToken); + + if (profilesChanged) + { + Logger.LogInfo("[LegacyMigration] profiles.json updated with stable profile and monitor ids."); + } + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + return; + } + catch (Exception ex) + { + Logger.LogError($"[LegacyMigration] Failed to migrate profiles.json: {ex.Message}"); + await ReloadProfilesAsync(cancellationToken); + if (cancellationToken.IsCancellationRequested) + { + return; + } + } + + if (migratedProfiles is not null) + { + LightSwitchService.MigrateLegacyProfileReferences(migratedProfiles); + } if (discovered.Count == 0) { return; } - // profiles.json and monitor_state.json are independent — a failure in one must - // not skip the other. MigrateLegacyKeys already has its own try/catch, so we - // only need to guard the profiles path here. try { - MigrateLegacyMonitorIdsInProfiles(discovered); + await Task.Run( + () => _stateManager.MigrateLegacyKeys(discovered), + CancellationToken.None); } catch (Exception ex) { - Logger.LogError($"[LegacyMigration] Failed to migrate profiles.json: {ex.Message}"); - } - - _stateManager.MigrateLegacyKeys(discovered); - } - - private static void MigrateLegacyMonitorIdsInProfiles(List<(string Id, int MonitorNumber)> discovered) - { - var profiles = ProfileService.LoadProfiles(); - if (profiles?.Profiles is null || profiles.Profiles.Count == 0) - { - return; - } - - bool anyChanged = false; - foreach (var profile in profiles.Profiles) - { - if (profile?.MonitorSettings is null) - { - continue; - } - - bool changed = false; - foreach (var legacy in profile.MonitorSettings - .Where(s => MonitorIdentity.IsLegacyId(s?.MonitorId)) - .ToList()) - { - var newId = MonitorIdMigrator.MatchNewId(legacy.MonitorId, discovered); - if (newId != null && profile.MonitorSettings.All(s => !MonitorIdComparer.Equal(s.MonitorId, newId))) - { - profile.MonitorSettings.Add(new ProfileMonitorSetting( - newId, - legacy.Brightness, - legacy.ColorTemperatureVcp, - legacy.Contrast, - legacy.Volume)); - } - else if (newId == null) - { - Logger.LogWarning( - $"[LegacyMigration] Dropping profile setting for '{legacy.MonitorId}' in profile '{profile.Name}': no current monitor with matching EdidId+MonitorNumber."); - } - - profile.MonitorSettings.Remove(legacy); - changed = true; - } - - if (changed) - { - profile.Touch(); - anyChanged = true; - } - } - - if (anyChanged) - { - ProfileService.SaveProfiles(profiles); - Logger.LogInfo("[LegacyMigration] profiles.json updated with DevicePath-based monitor Ids."); + Logger.LogError($"[LegacyMigration] Failed to migrate monitor_state.json: {ex.Message}"); } } @@ -716,15 +719,12 @@ public partial class MainViewModel // Load current settings to get hotkey and tray icon status var settings = _settingsUtils.GetSettingsOrDefault(PowerDisplaySettings.ModuleName); - // Load profiles to get count - var profilesData = ProfileService.LoadProfiles(); - var telemetryEvent = new PowerDisplaySettingsTelemetryEvent { HotkeyEnabled = settings.Properties.ActivationShortcut?.IsValid() ?? false, TrayIconEnabled = settings.Properties.ShowSystemTrayIcon, MonitorCount = Monitors.Count, - ProfileCount = profilesData?.Profiles?.Count ?? 0, + ProfileCount = Profiles.Count, }; PowerToysTelemetry.Log.WriteEvent(telemetryEvent); diff --git a/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs b/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs index c838f5bfcc..1eb19560ae 100644 --- a/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs +++ b/src/modules/powerdisplay/PowerDisplay/ViewModels/MainViewModel.cs @@ -48,6 +48,7 @@ public partial class MainViewModel : ObservableObject, IDisposable private readonly MonitorStateManager _stateManager; private readonly DisplayChangeWatcher _displayChangeWatcher; private readonly ISystemClock _clock; + private readonly SemaphoreSlim _profileOperationGate = new(1, 1); [ObservableProperty] [NotifyPropertyChangedFor(nameof(HasMonitors))] @@ -67,6 +68,10 @@ public partial class MainViewModel : ObservableObject, IDisposable [NotifyPropertyChangedFor(nameof(ShowProfileSwitcherButton))] public partial ObservableCollection Profiles { get; set; } + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(ShowProfileSwitcherButton))] + public partial bool IsProfilesLoading { get; private set; } + /// /// Event triggered when UI refresh is requested due to settings changes /// @@ -90,7 +95,6 @@ public partial class MainViewModel : ObservableObject, IDisposable _cancellationTokenSource = new CancellationTokenSource(); Monitors = new ObservableCollection(); Profiles = new ObservableCollection(); - IsScanning = true; ShowProfileSwitcher = true; ShowIdentifyMonitorsButton = true; MouseWheelIncrement = 5; @@ -102,9 +106,6 @@ public partial class MainViewModel : ObservableObject, IDisposable // Initialize the monitor manager _monitorManager = new MonitorManager(); - // Load profiles for quick apply feature - LoadProfiles(); - // Load UI display settings (profile switcher, identify button, color temp switcher) LoadUIDisplaySettings(); @@ -246,9 +247,9 @@ public partial class MainViewModel : ObservableObject, IDisposable /// /// Gets a value indicating whether to show the profile switcher button. - /// Combines settings value with HasProfiles check. + /// Combines the settings value with profile availability or loading state. /// - public bool ShowProfileSwitcherButton => ShowProfileSwitcher && HasProfiles; + public bool ShowProfileSwitcherButton => ShowProfileSwitcher && (HasProfiles || IsProfilesLoading); // Custom VCP mappings - loaded from settings private List _customVcpMappings = new(); @@ -455,28 +456,68 @@ public partial class MainViewModel : ObservableObject, IDisposable } /// - /// Load profiles from disk for quick apply feature + /// Reloads profiles from disk for the quick-apply feature without blocking the UI thread. /// - private void LoadProfiles() + private async Task ReloadProfilesAsync(CancellationToken cancellationToken = default) { try { - var profilesData = ProfileService.LoadProfiles(); - Profiles.Clear(); - foreach (var profile in profilesData.Profiles) - { - Profiles.Add(profile); - } - - OnPropertyChanged(nameof(HasProfiles)); - OnPropertyChanged(nameof(ShowProfileSwitcherButton)); + await RunProfileOperationAsync( + async token => + { + var profilesData = await ProfileHelper.LoadProfilesAsync(token); + ReplaceProfiles(profilesData); + }, + cancellationToken); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { } catch (Exception ex) { + Profiles.Clear(); + OnPropertyChanged(nameof(HasProfiles)); + OnPropertyChanged(nameof(ShowProfileSwitcherButton)); Logger.LogError($"[Profile] Failed to load profiles: {ex.Message}"); } } + private void ReplaceProfiles(PowerDisplayProfiles profilesData) + { + Profiles.Clear(); + foreach (var profile in profilesData.GetAssignedProfiles()) + { + Profiles.Add(profile); + } + + OnPropertyChanged(nameof(HasProfiles)); + OnPropertyChanged(nameof(ShowProfileSwitcherButton)); + } + + private async Task RunProfileOperationAsync( + Func operation, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(operation); + await _profileOperationGate.WaitAsync(cancellationToken); + try + { + IsProfilesLoading = true; + await operation(cancellationToken); + } + finally + { + try + { + IsProfilesLoading = false; + } + finally + { + _profileOperationGate.Release(); + } + } + } + /// /// Load UI display settings from settings file /// diff --git a/src/modules/powerdisplay/PowerDisplayModuleInterface/dllmain.cpp b/src/modules/powerdisplay/PowerDisplayModuleInterface/dllmain.cpp index 28099e1071..89e004b459 100644 --- a/src/modules/powerdisplay/PowerDisplayModuleInterface/dllmain.cpp +++ b/src/modules/powerdisplay/PowerDisplayModuleInterface/dllmain.cpp @@ -374,12 +374,12 @@ public: { Logger::trace(L"ApplyProfile action received"); - // Get the profile name from the action value - std::wstring profileName = action_object.get_value(); - Logger::trace(L"ApplyProfile: profile name = '{}'", profileName); + // Get the profile ID from the action value. + std::wstring profileId = action_object.get_value(); + Logger::trace(L"ApplyProfile: profile ID = '{}'", profileId); - // Send ApplyProfile message with profile name via Named Pipe - TrySendMessage(CommonSharedConstants::POWER_DISPLAY_APPLY_PROFILE_MESSAGE, profileName, L"ApplyProfile action"); + // Send the ApplyProfile message with the profile ID via Named Pipe. + TrySendMessage(CommonSharedConstants::POWER_DISPLAY_APPLY_PROFILE_MESSAGE, profileId, L"ApplyProfile action"); } } catch (std::exception&) diff --git a/src/settings-ui/Settings.UI.Library/LightSwitchProfileReferenceHelper.cs b/src/settings-ui/Settings.UI.Library/LightSwitchProfileReferenceHelper.cs new file mode 100644 index 0000000000..89530542e0 --- /dev/null +++ b/src/settings-ui/Settings.UI.Library/LightSwitchProfileReferenceHelper.cs @@ -0,0 +1,127 @@ +// 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. + +#nullable enable + +using System; +using PowerDisplay.Models; + +namespace Microsoft.PowerToys.Settings.UI.Library +{ + public static class LightSwitchProfileReferenceHelper + { + public const string NoneSentinel = "(None)"; + + public static int? GetProfileIdForTheme(LightSwitchProperties properties, bool isLightMode) + { + ArgumentNullException.ThrowIfNull(properties); + + var enabled = isLightMode + ? properties.EnableLightModeProfile.Value + : properties.EnableDarkModeProfile.Value; + var profileId = isLightMode + ? properties.LightModeProfileId.Value + : properties.DarkModeProfileId.Value; + + return enabled && profileId >= 1 ? profileId : null; + } + + public static bool SetProfileId( + IntProperty idProperty, + StringProperty legacyNameProperty, + int profileId) + { + ArgumentNullException.ThrowIfNull(idProperty); + ArgumentNullException.ThrowIfNull(legacyNameProperty); + + ArgumentOutOfRangeException.ThrowIfNegative(profileId); + + if (idProperty.Value == profileId + && string.IsNullOrEmpty(legacyNameProperty.Value)) + { + return false; + } + + idProperty.Value = profileId; + legacyNameProperty.Value = string.Empty; + return true; + } + + public static bool ClearProfileIdReferences( + LightSwitchProperties properties, + int profileId) + { + ArgumentNullException.ThrowIfNull(properties); + + ArgumentOutOfRangeException.ThrowIfLessThan(profileId, 1); + + var changed = false; + if (properties.LightModeProfileId.Value == profileId) + { + properties.LightModeProfileId.Value = 0; + changed = true; + } + + if (properties.DarkModeProfileId.Value == profileId) + { + properties.DarkModeProfileId.Value = 0; + changed = true; + } + + return changed; + } + + public static bool ReconcileReferences( + LightSwitchProperties properties, + PowerDisplayProfiles profiles) + { + ArgumentNullException.ThrowIfNull(properties); + ArgumentNullException.ThrowIfNull(profiles); + + var changed = false; + changed |= ReconcileOne( + profiles, + properties.LightModeProfileId, + properties.LightModeProfile); + changed |= ReconcileOne( + profiles, + properties.DarkModeProfileId, + properties.DarkModeProfile); + return changed; + } + + private static bool ReconcileOne( + PowerDisplayProfiles profiles, + IntProperty idProperty, + StringProperty legacyNameProperty) + { + var originalId = idProperty.Value; + var originalName = legacyNameProperty.Value; + + if (originalId >= 1) + { + if (profiles.GetById(originalId) is null) + { + idProperty.Value = 0; + } + } + else if (!string.IsNullOrEmpty(originalName) && originalName != NoneSentinel) + { + var profile = profiles.GetLegacyProfileByName(originalName); + if (profile is not null && profile.Id >= 1) + { + idProperty.Value = profile.Id; + } + } + + if (!string.IsNullOrEmpty(legacyNameProperty.Value)) + { + legacyNameProperty.Value = string.Empty; + } + + return idProperty.Value != originalId + || legacyNameProperty.Value != originalName; + } + } +} diff --git a/src/settings-ui/Settings.UI.Library/LightSwitchProfileSettingsUpdater.cs b/src/settings-ui/Settings.UI.Library/LightSwitchProfileSettingsUpdater.cs new file mode 100644 index 0000000000..1bbdad9b62 --- /dev/null +++ b/src/settings-ui/Settings.UI.Library/LightSwitchProfileSettingsUpdater.cs @@ -0,0 +1,32 @@ +// 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; + +namespace Microsoft.PowerToys.Settings.UI.Library +{ + public static class LightSwitchProfileSettingsUpdater + { + public static bool ClearDeletedProfileAndSend( + LightSwitchSettings settings, + int deletedProfileId, + Func sendConfigMessage) + { + ArgumentNullException.ThrowIfNull(settings); + ArgumentNullException.ThrowIfNull(sendConfigMessage); + + if (!LightSwitchProfileReferenceHelper.ClearProfileIdReferences( + settings.Properties, + deletedProfileId)) + { + return false; + } + + var outgoing = new SndModuleSettings( + new SndLightSwitchSettings(settings)); + sendConfigMessage(outgoing.ToJsonString()); + return true; + } + } +} diff --git a/src/settings-ui/Settings.UI.Library/LightSwitchProperties.cs b/src/settings-ui/Settings.UI.Library/LightSwitchProperties.cs index 4c56051ce9..054eb2b765 100644 --- a/src/settings-ui/Settings.UI.Library/LightSwitchProperties.cs +++ b/src/settings-ui/Settings.UI.Library/LightSwitchProperties.cs @@ -21,6 +21,8 @@ namespace Microsoft.PowerToys.Settings.UI.Library public const bool DefaultEnableLightModeProfile = false; public const string DefaultDarkModeProfile = ""; public const string DefaultLightModeProfile = ""; + public const int DefaultDarkModeProfileId = 0; + public const int DefaultLightModeProfileId = 0; public static readonly HotkeySettings DefaultToggleThemeHotkey = new HotkeySettings(true, true, false, true, 0x44); // Ctrl+Win+Shift+D public LightSwitchProperties() @@ -39,6 +41,8 @@ namespace Microsoft.PowerToys.Settings.UI.Library EnableLightModeProfile = new BoolProperty(DefaultEnableLightModeProfile); DarkModeProfile = new StringProperty(DefaultDarkModeProfile); LightModeProfile = new StringProperty(DefaultLightModeProfile); + DarkModeProfileId = new IntProperty(DefaultDarkModeProfileId); + LightModeProfileId = new IntProperty(DefaultLightModeProfileId); } [JsonPropertyName("changeSystem")] @@ -77,10 +81,24 @@ namespace Microsoft.PowerToys.Settings.UI.Library [JsonPropertyName("enableLightModeProfile")] public BoolProperty EnableLightModeProfile { get; set; } + /// + /// Legacy profile name retained only to migrate settings written before profile IDs. + /// New code must persist instead. + /// [JsonPropertyName("darkModeProfile")] public StringProperty DarkModeProfile { get; set; } + /// + /// Legacy profile name retained only to migrate settings written before profile IDs. + /// New code must persist instead. + /// [JsonPropertyName("lightModeProfile")] public StringProperty LightModeProfile { get; set; } + + [JsonPropertyName("darkModeProfileId")] + public IntProperty DarkModeProfileId { get; set; } + + [JsonPropertyName("lightModeProfileId")] + public IntProperty LightModeProfileId { get; set; } } } diff --git a/src/settings-ui/Settings.UI.Library/LightSwitchSettings.cs b/src/settings-ui/Settings.UI.Library/LightSwitchSettings.cs index 4aa5647102..20696db8ca 100644 --- a/src/settings-ui/Settings.UI.Library/LightSwitchSettings.cs +++ b/src/settings-ui/Settings.UI.Library/LightSwitchSettings.cs @@ -64,6 +64,8 @@ namespace Microsoft.PowerToys.Settings.UI.Library EnableLightModeProfile = new BoolProperty(Properties.EnableLightModeProfile.Value), DarkModeProfile = new StringProperty(Properties.DarkModeProfile.Value), LightModeProfile = new StringProperty(Properties.LightModeProfile.Value), + DarkModeProfileId = new IntProperty(Properties.DarkModeProfileId.Value), + LightModeProfileId = new IntProperty(Properties.LightModeProfileId.Value), }, }; } diff --git a/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/LightSwitch.cs b/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/LightSwitch.cs new file mode 100644 index 0000000000..c9b6463922 --- /dev/null +++ b/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/LightSwitch.cs @@ -0,0 +1,70 @@ +// 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.Collections.Generic; +using System.Reflection; +using Microsoft.PowerToys.Settings.UI.Library; +using Microsoft.PowerToys.Settings.UI.UnitTests.BackwardsCompatibility; +using Microsoft.PowerToys.Settings.UI.UnitTests.Mocks; +using Microsoft.PowerToys.Settings.UI.ViewModels; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using PowerDisplay.Models; + +namespace ViewModelTests; + +[TestClass] +public class LightSwitch +{ + [TestMethod] + public void SuppressedProfileSelectionChange_DoesNotPersistTemporaryZero() + { + var settings = new LightSwitchSettings(); + settings.Properties.DarkModeProfileId.Value = 7; + var messages = new List(); + var viewModel = CreateViewModel(settings, message => + { + messages.Add(message); + return 0; + }); + var selected = new PowerDisplayProfile( + "Night", + new List + { + new ProfileMonitorSetting("MON1", 50, null, null, null), + }) + { + Id = 7, + }; + + viewModel.SelectedDarkModeProfile = selected; + SetSuppression(viewModel, true); + viewModel.SelectedDarkModeProfile = null; + + Assert.AreEqual(7, settings.Properties.DarkModeProfileId.Value); + Assert.AreEqual(0, messages.Count); + } + + private static LightSwitchViewModel CreateViewModel( + LightSwitchSettings settings, + System.Func sendConfigMessage) + { + var generalSettingsRepository = + new BackCompatTestProperties.MockSettingsRepository( + ISettingsUtilsMocks.GetStubSettingsUtils().Object); + + return new LightSwitchViewModel( + generalSettingsRepository, + settings, + sendConfigMessage); + } + + private static void SetSuppression(LightSwitchViewModel viewModel, bool value) + { + var field = typeof(LightSwitchViewModel).GetField( + "_suppressProfileSelectionPersistence", + BindingFlags.Instance | BindingFlags.NonPublic); + Assert.IsNotNull(field); + field.SetValue(viewModel, value); + } +} diff --git a/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/ProfileEditorViewModelTests.cs b/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/ProfileEditorViewModelTests.cs new file mode 100644 index 0000000000..69b3bdd16b --- /dev/null +++ b/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/ProfileEditorViewModelTests.cs @@ -0,0 +1,41 @@ +// 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.Collections.ObjectModel; +using Microsoft.PowerToys.Settings.UI.Library; +using Microsoft.PowerToys.Settings.UI.ViewModels; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace ViewModelTests +{ + [TestClass] + public class ProfileEditorViewModelTests + { + [TestMethod] + public void CreateProfile_DefaultProfileId_ReturnsZero() + { + var viewModel = new ProfileEditorViewModel( + new ObservableCollection(), + "New profile"); + + var profile = viewModel.CreateProfile(); + + Assert.AreEqual(0, profile.Id); + } + + [TestMethod] + public void CreateProfile_ExistingProfileId_PreservesId() + { + const int profileId = 42; + var viewModel = new ProfileEditorViewModel( + new ObservableCollection(), + "Existing profile", + profileId); + + var profile = viewModel.CreateProfile(); + + Assert.AreEqual(profileId, profile.Id); + } + } +} diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml b/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml index a532cf18dd..102af80122 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml +++ b/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml @@ -241,12 +241,12 @@ + IsEnabled="{x:Bind ViewModel.CanSelectPowerDisplayProfile, Mode=OneWay}" /> @@ -255,12 +255,12 @@ + IsEnabled="{x:Bind ViewModel.CanSelectPowerDisplayProfile, Mode=OneWay}" /> diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml.cs b/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml.cs index 80dafde110..93de491f79 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml.cs +++ b/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml.cs @@ -83,8 +83,10 @@ namespace Microsoft.PowerToys.Settings.UI.Views this.ViewModel.RefreshEnabledState(); } - private void LightSwitchPage_Loaded(object sender, RoutedEventArgs e) + private async void LightSwitchPage_Loaded(object sender, RoutedEventArgs e) { + await ViewModel.InitializeProfilesAsync(); + if (this.ViewModel.SearchLocations.Count == 0) { foreach (var city in SearchLocationLoader.GetAll()) diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Views/PowerDisplayPage.xaml b/src/settings-ui/Settings.UI/SettingsXAML/Views/PowerDisplayPage.xaml index 374e4275e8..e6d7c0252e 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Views/PowerDisplayPage.xaml +++ b/src/settings-ui/Settings.UI/SettingsXAML/Views/PowerDisplayPage.xaml @@ -152,7 +152,7 @@ - + - +