From 84b39a9edcf324da3a4d9280d196e939622f34ef Mon Sep 17 00:00:00 2001 From: Jaylyn Barbee <51131738+Jaylyn-Barbee@users.noreply.github.com> Date: Mon, 2 Feb 2026 09:33:25 -0500 Subject: [PATCH 1/7] [Light Switch] Changed the rules surrounding the max/min value of the Offset field (#45125) ## Summary of the Pull Request This PR introduces new logic that dictates the max and min value for the `Offset` field that the user can change when using Sunrise to Sunset mode. ## PR Checklist - [x] Closes: #44959 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected ## Detailed Description of the Pull Request / Additional comments The new logic is as follows: - The sunrise offset cannot go into the previous day and cannot overlap the current sunset transition time - The sunset offset cannot overlap the last sunrise time and cannot overlap into the next day. These values are dynamic and update when the VM updates with new times. ## Validation Steps Performed - Manual testing --- .../SettingsXAML/Views/LightSwitchPage.xaml | 8 ++-- .../ViewModels/LightSwitchViewModel.cs | 44 +++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml b/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml index a44d482a04..e1d6d8013c 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml +++ b/src/settings-ui/Settings.UI/SettingsXAML/Views/LightSwitchPage.xaml @@ -112,8 +112,8 @@ @@ -121,8 +121,8 @@ diff --git a/src/settings-ui/Settings.UI/ViewModels/LightSwitchViewModel.cs b/src/settings-ui/Settings.UI/ViewModels/LightSwitchViewModel.cs index 05aec49c9a..6d3c83c8ba 100644 --- a/src/settings-ui/Settings.UI/ViewModels/LightSwitchViewModel.cs +++ b/src/settings-ui/Settings.UI/ViewModels/LightSwitchViewModel.cs @@ -232,6 +232,8 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels NotifyPropertyChanged(); OnPropertyChanged(nameof(LightTimeTimeSpan)); + OnPropertyChanged(nameof(SunriseOffsetMin)); + OnPropertyChanged(nameof(SunsetOffsetMin)); if (ScheduleMode == "SunsetToSunrise") { @@ -252,6 +254,8 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels NotifyPropertyChanged(); OnPropertyChanged(nameof(DarkTimeTimeSpan)); + OnPropertyChanged(nameof(SunriseOffsetMax)); + OnPropertyChanged(nameof(SunsetOffsetMax)); if (ScheduleMode == "SunsetToSunrise") { @@ -270,6 +274,7 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels { ModuleSettings.Properties.SunriseOffset.Value = value; OnPropertyChanged(nameof(LightTimeTimeSpan)); + OnPropertyChanged(nameof(SunsetOffsetMin)); } } } @@ -283,10 +288,49 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels { ModuleSettings.Properties.SunsetOffset.Value = value; OnPropertyChanged(nameof(DarkTimeTimeSpan)); + OnPropertyChanged(nameof(SunriseOffsetMax)); } } } + public int SunriseOffsetMin + { + get + { + // Minimum: don't let adjusted sunrise go before 00:00 + return -LightTime; + } + } + + public int SunriseOffsetMax + { + get + { + // Maximum: adjusted sunrise must stay before adjusted sunset + int adjustedSunset = DarkTime + SunsetOffset; + return Math.Max(0, adjustedSunset - LightTime - 1); + } + } + + public int SunsetOffsetMin + { + get + { + // Minimum: adjusted sunset must stay after adjusted sunrise + int adjustedSunrise = LightTime + SunriseOffset; + return Math.Min(0, adjustedSunrise - DarkTime + 1); + } + } + + public int SunsetOffsetMax + { + get + { + // Maximum: don't let adjusted sunset go past 23:59 (1439 minutes) + return 1439 - DarkTime; + } + } + // === Computed projections (OneWay bindings only) === public TimeSpan LightTimeTimeSpan { From b5991642f8cc7a5bb7b5de0cc046ce92148b2a56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Mon, 2 Feb 2026 18:10:36 +0100 Subject: [PATCH 2/7] CmdPal: Add trailing backslash to OutDir in Microsoft.Terminal.UI project file (#45250) ## Summary of the Pull Request Ensures the OutDir path in Microsoft.Terminal.UI.vcxproj ends with a backslash, making it explicit as a directory, and fixes warning MSB8004. ## PR Checklist - [ ] Closes: #xxx - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed --- .../cmdpal/Microsoft.Terminal.UI/Microsoft.Terminal.UI.vcxproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/cmdpal/Microsoft.Terminal.UI/Microsoft.Terminal.UI.vcxproj b/src/modules/cmdpal/Microsoft.Terminal.UI/Microsoft.Terminal.UI.vcxproj index 42243c2305..208ad4166e 100644 --- a/src/modules/cmdpal/Microsoft.Terminal.UI/Microsoft.Terminal.UI.vcxproj +++ b/src/modules/cmdpal/Microsoft.Terminal.UI/Microsoft.Terminal.UI.vcxproj @@ -201,7 +201,7 @@ - ..\..\..\..\$(Platform)\$(Configuration)\WinUI3Apps\CmdPal + ..\..\..\..\$(Platform)\$(Configuration)\WinUI3Apps\CmdPal\ obj\$(Platform)\$(Configuration)\ From dca532cf4bef4c67c77683dad693de5f8b2448de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Mon, 2 Feb 2026 18:16:43 +0100 Subject: [PATCH 3/7] CmdPal: Icon cache (#44538) ## Summary of the Pull Request This PR implements actual cache in IconCacheService and adds some fixes on top for free. The good - `IconCacheService` now caches decoded icons - Ensures that UI thread is not starved by loading icons by limiting number of threads that can load icons at any given time - `IconCacheService` decodes bitmaps directly to the required size to reduce memory usage - `IconBox` now reacts to theme, DPI scale, and size changes immediately - Introduced `AdaptiveCache` with time-based decay to improve icon reuse - Updated `IconCacheProvider` and `IconCacheService` to handle multiple icon sizes and scale-aware caching - Added priority-based decoding in `IconCacheService` for more responsive loading - Extended `IconPathConverter` to support target icon sizes - Switched hero images in `ShellPage` to use the jumbo icon cache - Made `MainWindow` title bar logic resilient to a null `XamlRoot` - Fixed Tag icon positioning - Removes custom `TypedEventHandlerExtensions` in favor of `CommunityToolkit.WinUI.Deferred`. The bad - Since IconData lacks a unique identity, when it includes a stream, it relies on simple reference equality, acknowledging that it might not be stable. We might cache some obsolete garbage because of this, but it is fast and better than nothing at all. Yet another task for the future me. ## PR Checklist - [ ] Closes: - [ ] Closes: #38284 - [ ] Related to: #44407 - [ ] Related to: https://github.com/zadjii-msft/PowerToys/issues/333 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed --- .github/actions/spell-check/expect.txt | 1 + .../cmdpal/Microsoft.CmdPal.UI/App.xaml.cs | 10 +- .../Controls/ContextMenu.xaml | 4 +- .../Controls/FallbackRanker.xaml | 2 +- .../Controls/FiltersDropDown.xaml | 2 +- .../Microsoft.CmdPal.UI/Controls/IconBox.cs | 294 +++++++++++------ .../Controls/SourceRequestedEventArgs.cs | 4 +- .../Microsoft.CmdPal.UI/Controls/Tag.xaml | 1 + .../Microsoft.CmdPal.UI/Controls/Tag.xaml.cs | 2 +- .../ExtViews/ListPage.xaml | 33 +- .../Helpers/AdaptiveCache`2.cs | 299 ++++++++++++++++++ .../Helpers/IconCacheProvider.cs | 44 --- .../Helpers/IconCacheService.cs | 99 ------ .../Helpers/Icons/CachedIconSourceProvider.cs | 103 ++++++ .../Helpers/Icons/IIconLoaderService.cs | 21 ++ .../Helpers/Icons/IIconSourceProvider.cs | 13 + .../Helpers/Icons/IconCacheProvider.cs | 79 +++++ .../Helpers/Icons/IconLoadPriority.cs | 11 + .../Helpers/Icons/IconLoaderService.cs | 224 +++++++++++++ .../Helpers/Icons/IconServiceRegistration.cs | 37 +++ .../Helpers/Icons/IconSourceProvider.cs | 41 +++ .../Helpers/Icons/WellKnownIconSize.cs | 13 + .../Helpers/TypedEventHandlerExtensions.cs | 75 ----- .../Microsoft.CmdPal.UI/MainWindow.xaml.cs | 8 +- .../Microsoft.CmdPal.UI/Pages/ShellPage.xaml | 6 +- .../Settings/ExtensionPage.xaml | 6 +- .../Settings/ExtensionsPage.xaml | 2 +- .../IconPathConverter.idl | 2 +- 28 files changed, 1091 insertions(+), 345 deletions(-) create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/AdaptiveCache`2.cs delete mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/IconCacheProvider.cs delete mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/IconCacheService.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/CachedIconSourceProvider.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IIconLoaderService.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IIconSourceProvider.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconCacheProvider.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconLoadPriority.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconLoaderService.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconServiceRegistration.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconSourceProvider.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/WellKnownIconSize.cs delete mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/TypedEventHandlerExtensions.cs diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index d94db89953..15e48cff17 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -899,6 +899,7 @@ LEFTTEXT LError LEVELID LExit +LFU lhwnd LIBFUZZER LIBID diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/App.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/App.xaml.cs index 3d6a7ef634..eb103d3157 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/App.xaml.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/App.xaml.cs @@ -32,6 +32,7 @@ using Microsoft.CmdPal.UI.ViewModels.Services; using Microsoft.CommandPalette.Extensions; using Microsoft.Extensions.DependencyInjection; using Microsoft.PowerToys.Telemetry; +using Microsoft.UI.Dispatching; using Microsoft.UI.Xaml; // To learn more about WinUI, the WinUI project structure, @@ -72,6 +73,8 @@ public partial class App : Application, IDisposable Services = ConfigureServices(); + IconCacheProvider.Initialize(Services); + this.InitializeComponent(); // Ensure types used in XAML are preserved for AOT compilation @@ -113,12 +116,13 @@ public partial class App : Application, IDisposable // Root services services.AddSingleton(TaskScheduler.FromCurrentSynchronizationContext()); + var dispatcherQueue = DispatcherQueue.GetForCurrentThread(); AddBuiltInCommands(services); AddCoreServices(services); - AddUIServices(services); + AddUIServices(services, dispatcherQueue); return services.BuildServiceProvider(); } @@ -169,7 +173,7 @@ public partial class App : Application, IDisposable services.AddSingleton(); } - private static void AddUIServices(ServiceCollection services) + private static void AddUIServices(ServiceCollection services, DispatcherQueue dispatcherQueue) { // Models var sm = SettingsModel.LoadSettings(); @@ -188,6 +192,8 @@ public partial class App : Application, IDisposable services.AddSingleton(); services.AddSingleton(); + + services.AddIconServices(dispatcherQueue); } private static void AddCoreServices(ServiceCollection services) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ContextMenu.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ContextMenu.xaml index 36167717ea..4d72e91b6a 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ContextMenu.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ContextMenu.xaml @@ -42,7 +42,7 @@ Margin="4,0,0,0" HorizontalAlignment="Left" SourceKey="{x:Bind Icon, Mode=OneWay}" - SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested}" /> + SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested20}" /> + SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested20}" /> + SourceRequested="{x:Bind helpers:IconCacheProvider.SourceRequested20}" /> diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/FiltersDropDown.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/FiltersDropDown.xaml index f8c888e8f8..e92dbd912e 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/FiltersDropDown.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/FiltersDropDown.xaml @@ -46,7 +46,7 @@ HorizontalAlignment="Left" VerticalAlignment="Center" SourceKey="{x:Bind Icon}" - SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested}" /> + SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested20}" /> public partial class IconBox : ContentControl { - private readonly DispatcherQueue _queue = DispatcherQueue.GetForCurrentThread(); + private double _lastScale; + private ElementTheme _lastTheme; + private double _lastFontSize; + + private const double DefaultIconFontSize = 16.0; /// /// Gets or sets the to display within the . Overwritten, if is used instead. @@ -48,10 +48,23 @@ public partial class IconBox : ContentControl public static readonly DependencyProperty SourceKeyProperty = DependencyProperty.Register(nameof(SourceKey), typeof(object), typeof(IconBox), new PropertyMetadata(null, OnSourceKeyPropertyChanged)); + private TypedEventHandler? _sourceRequested; + /// /// Gets or sets the event handler to provide the value of the for the property from the provided . /// - public event TypedEventHandler? SourceRequested; + public event TypedEventHandler? SourceRequested + { + add + { + _sourceRequested += value; + if (_sourceRequested?.GetInvocationList().Length == 1) + { + Refresh(); + } + } + remove => _sourceRequested -= value; + } public IconBox() { @@ -59,119 +72,208 @@ public partial class IconBox : ContentControl IsTabStop = false; HorizontalContentAlignment = HorizontalAlignment.Center; VerticalContentAlignment = VerticalAlignment.Center; + + Loaded += OnLoaded; + Unloaded += OnUnloaded; + ActualThemeChanged += OnActualThemeChanged; + SizeChanged += OnSizeChanged; + + UpdateLastFontSize(); + } + + private void UpdateLastFontSize() + { + _lastFontSize = + Pick(Width) + ?? Pick(Height) + ?? Pick(ActualWidth) + ?? Pick(ActualHeight) + ?? DefaultIconFontSize; + + return; + + static double? Pick(double value) => double.IsFinite(value) && value > 0 ? value : null; + } + + private void OnSizeChanged(object s, SizeChangedEventArgs e) + { + UpdateLastFontSize(); + + if (Source is FontIconSource fontIcon) + { + fontIcon.FontSize = _lastFontSize; + } + } + + private void OnActualThemeChanged(FrameworkElement sender, object args) + { + if (_lastTheme == ActualTheme) + { + return; + } + + _lastTheme = ActualTheme; + Refresh(); + } + + private void OnLoaded(object sender, RoutedEventArgs e) + { + _lastTheme = ActualTheme; + UpdateLastFontSize(); + + if (XamlRoot is not null) + { + _lastScale = XamlRoot.RasterizationScale; + XamlRoot.Changed += OnXamlRootChanged; + } + } + + private void OnUnloaded(object sender, RoutedEventArgs e) + { + if (XamlRoot is not null) + { + XamlRoot.Changed -= OnXamlRootChanged; + } + } + + private void OnXamlRootChanged(XamlRoot sender, XamlRootChangedEventArgs args) + { + var newScale = sender.RasterizationScale; + var changedLastTheme = _lastTheme != ActualTheme; + _lastTheme = ActualTheme; + if ((changedLastTheme || Math.Abs(newScale - _lastScale) > 0.01) && SourceKey is not null) + { + _lastScale = newScale; + UpdateSourceKey(this, SourceKey); + } + } + + private void Refresh() + { + if (SourceKey is not null) + { + UpdateSourceKey(this, SourceKey); + } } private static void OnSourcePropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) { - if (d is IconBox @this) + if (d is not IconBox self) { - switch (e.NewValue) - { - case null: - @this.Content = null; - break; - case FontIconSource fontIco: - fontIco.FontSize = double.IsNaN(@this.Width) ? @this.Height : @this.Width; + return; + } + + switch (e.NewValue) + { + case null: + self.Content = null; + self.Padding = default; + break; + case FontIconSource fontIcon: + if (self.Content is IconSourceElement iconSourceElement) + { + iconSourceElement.IconSource = fontIcon; + } + else + { + fontIcon.FontSize = self._lastFontSize; // For inexplicable reasons, FontIconSource.CreateIconElement // doesn't work, so do it ourselves // TODO: File platform bug? IconSourceElement elem = new() { - IconSource = fontIco, HorizontalAlignment = HorizontalAlignment.Center, VerticalAlignment = VerticalAlignment.Center, + IconSource = fontIcon, }; - @this.Content = elem; - break; - case IconSource source: - @this.Content = source.CreateIconElement(); - break; - default: - throw new InvalidOperationException($"New value of {e.NewValue} is not of type IconSource."); - } + self.Content = elem; + } + + self.Padding = new Thickness(Math.Round(self._lastFontSize * -0.2)); + + break; + case BitmapIconSource bitmapIcon: + if (self.Content is IconSourceElement iconSourceElement2) + { + iconSourceElement2.IconSource = bitmapIcon; + } + else + { + self.Content = bitmapIcon.CreateIconElement(); + } + + self.Padding = default; + + break; + case IconSource source: + self.Content = source.CreateIconElement(); + self.Padding = default; + break; + default: + throw new InvalidOperationException($"New value of {e.NewValue} is not of type IconSource."); } } private static void OnSourceKeyPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) { - if (d is IconBox @this) + if (d is not IconBox self) { - if (e.NewValue is null) + return; + } + + UpdateSourceKey(self, e.NewValue); + } + + private static void UpdateSourceKey(IconBox iconBox, object? sourceKey) + { + if (sourceKey is null) + { + iconBox.Source = null; + return; + } + + Callback(iconBox, sourceKey); + } + + private static async void Callback(IconBox iconBox, object? sourceKey) + { + try + { + var iconBoxSourceRequestedHandler = iconBox._sourceRequested; + + if (iconBoxSourceRequestedHandler is null) { - @this.Source = null; + return; } - else + + var eventArgs = new SourceRequestedEventArgs(sourceKey, iconBox._lastTheme, iconBox._lastScale); + await iconBoxSourceRequestedHandler.InvokeAsync(iconBox, eventArgs); + + // After the await: + // Is the icon we're looking up now, the one we still + // want to find? Since this IconBox might be used in a + // list virtualization situation, it's very possible we + // may have already been set to a new icon before we + // even got back from the await. + if (eventArgs.Key != sourceKey) { - // TODO GH #239 switch back when using the new MD text block - // Switching back to EnqueueAsync has broken icons in tags (they don't show) - // _ = @this._queue.EnqueueAsync(() => - @this._queue.TryEnqueue(async void () => - { - try - { - if (@this.SourceRequested is null) - { - return; - } - - var requestedTheme = @this.ActualTheme; - var eventArgs = new SourceRequestedEventArgs(e.NewValue, requestedTheme); - - await @this.SourceRequested.InvokeAsync(@this, eventArgs); - - // After the await: - // Is the icon we're looking up now, the one we still - // want to find? Since this IconBox might be used in a - // list virtualization situation, it's very possible we - // may have already been set to a new icon before we - // even got back from the await. - if (eventArgs.Key != @this.SourceKey) - { - // If the requested icon has changed, then just bail - return; - } - - @this.Source = eventArgs.Value; - - // Here's a little lesson in trickery: - // Emoji are rendered just a bit bigger than Segoe Icons. - // Just enough bigger that they get clipped if you put - // them in a box at the same size. - // - // So, if the icon we get back was a font icon, - // and the glyph for that icon is NOT in the range of - // Segoe icons, then let's give the icon some extra space - var iconData = eventArgs.Key switch - { - IconDataViewModel key => key, - IconInfoViewModel info => requestedTheme == ElementTheme.Light ? info.Light : info.Dark, - _ => null, - }; - - if (iconData?.Icon is not null && @this.Source is FontIconSource) - { - var iconSize = - !double.IsNaN(@this.Width) ? @this.Width : - !double.IsNaN(@this.Height) ? @this.Height : - @this.ActualWidth > 0 ? @this.ActualWidth : - @this.ActualHeight; - - @this.Padding = new Thickness(Math.Round(iconSize * -0.2)); - } - else - { - @this.Padding = default; - } - } - catch (Exception ex) - { - // Exception from TryEnqueue bypasses the global error handler, - // and crashes the app. - Logger.LogError("Failed to set icon", ex); - } - }); + // If the requested icon has changed, then just bail + return; } + + if (eventArgs.Value == iconBox.Source) + { + return; + } + + iconBox.Source = eventArgs.Value; + } + catch (Exception ex) + { + // Exception from TryEnqueue bypasses the global error handler, + // and crashes the app. + Logger.LogError("Failed to set icon", ex); } } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/SourceRequestedEventArgs.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/SourceRequestedEventArgs.cs index 670bf13a7a..5528217f89 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/SourceRequestedEventArgs.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/SourceRequestedEventArgs.cs @@ -11,11 +11,13 @@ namespace Microsoft.CmdPal.UI.Controls; /// /// See event. /// -public class SourceRequestedEventArgs(object? key, ElementTheme requestedTheme) : DeferredEventArgs +public class SourceRequestedEventArgs(object? key, ElementTheme requestedTheme, double scale = 1.0) : DeferredEventArgs { public object? Key { get; private set; } = key; public IconSource? Value { get; set; } public ElementTheme Theme => requestedTheme; + + public double Scale => scale; } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/Tag.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/Tag.xaml index 7cf917b21c..14386896f3 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/Tag.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/Tag.xaml @@ -72,6 +72,7 @@ diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/Tag.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/Tag.xaml.cs index b74cc54687..405d4341a9 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/Tag.xaml.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/Tag.xaml.cs @@ -72,7 +72,7 @@ public partial class Tag : Control if (GetTemplateChild(TagIconBox) is IconBox iconBox) { - iconBox.SourceRequested += IconCacheProvider.SourceRequested; + iconBox.SourceRequested += IconCacheProvider.SourceRequested20; iconBox.Visibility = HasIcon ? Visibility.Visible : Visibility.Collapsed; } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml index c0edf83390..2f670d62b4 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml @@ -27,6 +27,11 @@ 8 8 + 32 + 48 + 100 + 160 + 40 0 0 @@ -288,7 +293,7 @@ AutomationProperties.AccessibilityView="Raw" Foreground="{ThemeResource TextFillColorSecondaryBrush}" SourceKey="{x:Bind Icon, Mode=OneWay}" - SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested}" /> + SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested20}" /> + SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested32}" /> + SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested64}" /> + SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested256}" /> @@ -582,7 +587,7 @@ Margin="8" Foreground="{ThemeResource TextFillColorSecondaryBrush}" SourceKey="{x:Bind ViewModel.EmptyContent.Icon, Mode=OneWay}" - SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested}" /> + SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested64}" /> +/// A high-performance, near-lock-free adaptive cache optimized for UI Icons. +/// Eviction merely drops references to allow the GC to manage UI-bound lifetimes. +/// +internal sealed class AdaptiveCache + where TKey : IEquatable +{ + private readonly int _capacity; + private readonly double _decayFactor; + private readonly TimeSpan _decayInterval; + + private readonly ConcurrentDictionary _map; + private readonly ConcurrentStack _pool = []; + private readonly WaitCallback _maintenanceCallback; + + private long _currentTick; + private long _lastDecayTicks = DateTime.UtcNow.Ticks; + private InterlockedBoolean _maintenanceSwitch = new(false); + + public AdaptiveCache(int capacity = 384, TimeSpan? decayInterval = null, double decayFactor = 0.5) + { + _capacity = capacity; + _decayInterval = decayInterval ?? TimeSpan.FromMinutes(5); + _decayFactor = decayFactor; + _map = new ConcurrentDictionary(Environment.ProcessorCount, capacity); + + _maintenanceCallback = static state => + { + var cache = (AdaptiveCache)state!; + try + { + cache.PerformCleanup(); + } + finally + { + cache._maintenanceSwitch.Clear(); + } + }; + } + + public TValue GetOrAdd(TKey key, Func factory, TArg arg) + { + if (_map.TryGetValue(key, out var entry)) + { + entry.Update(Interlocked.Increment(ref _currentTick)); + return entry.Value!; + } + + if (!_pool.TryPop(out var newEntry)) + { + newEntry = new CacheEntry(); + } + + var value = factory(key, arg); + var tick = Interlocked.Increment(ref _currentTick); + newEntry.Initialize(key, value, 1.0, tick); + + if (!_map.TryAdd(key, newEntry)) + { + newEntry.Clear(); + _pool.Push(newEntry); + + if (_map.TryGetValue(key, out var existing)) + { + existing.Update(tick); + return existing.Value!; + } + } + + if (ShouldMaintenanceRun()) + { + TryRunMaintenance(); + } + + return value; + } + + public bool TryGet(TKey key, [MaybeNullWhen(false)] out TValue value) + { + if (_map.TryGetValue(key, out var entry)) + { + entry.Update(Interlocked.Increment(ref _currentTick)); + value = entry.Value; + return true; + } + + value = default; + return false; + } + + public void Add(TKey key, TValue value) + { + var tick = Interlocked.Increment(ref _currentTick); + + if (_map.TryGetValue(key, out var existing)) + { + existing.Update(tick); + existing.SetValue(value); + return; + } + + if (!_pool.TryPop(out var newEntry)) + { + newEntry = new CacheEntry(); + } + + newEntry.Initialize(key, value, 1.0, tick); + + if (!_map.TryAdd(key, newEntry)) + { + newEntry.Clear(); + _pool.Push(newEntry); + } + + if (ShouldMaintenanceRun()) + { + TryRunMaintenance(); + } + } + + public bool TryRemove(TKey key) + { + if (_map.TryRemove(key, out var evicted)) + { + evicted.Clear(); + _pool.Push(evicted); + return true; + } + + return false; + } + + public void Clear() + { + foreach (var key in _map.Keys) + { + TryRemove(key); + } + + Interlocked.Exchange(ref _currentTick, 0); + } + + private bool ShouldMaintenanceRun() + { + return _map.Count > _capacity || (DateTime.UtcNow.Ticks - Interlocked.Read(ref _lastDecayTicks)) > _decayInterval.Ticks; + } + + private void TryRunMaintenance() + { + if (_maintenanceSwitch.Set()) + { + ThreadPool.UnsafeQueueUserWorkItem(_maintenanceCallback, this); + } + } + + private void PerformCleanup() + { + var nowTicks = DateTime.UtcNow.Ticks; + var isDecay = (nowTicks - Interlocked.Read(ref _lastDecayTicks)) > _decayInterval.Ticks; + if (isDecay) + { + Interlocked.Exchange(ref _lastDecayTicks, nowTicks); + } + + var currentTick = Interlocked.Read(ref _currentTick); + + foreach (var (key, entry) in _map) + { + if (isDecay) + { + entry.Decay(_decayFactor); + } + + var score = CalculateScore(entry, currentTick); + + if (score < 0.1 || _map.Count > _capacity) + { + if (_map.TryRemove(key, out var evicted)) + { + evicted.Clear(); + _pool.Push(evicted); + } + } + } + } + + /// + /// Calculates the survival score of an entry. + /// Higher score = stay in cache; Lower score = priority for eviction. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static double CalculateScore(CacheEntry entry, long currentTick) + { + // Tuning parameter: How much weight to give recency vs frequency. + // - a larger ageWeight makes the cache behave more like LRU (Least Recently Used). + // - a smaller ageWeight makes it behave more like LFU (Least Frequently Used). + const double ageWeight = 0.001; + + var frequency = entry.GetFrequency(); + var age = currentTick - entry.GetLastAccess(); + + return frequency - (age * ageWeight); + } + + /// + /// Represents a single pooled entry in the cache, containing the value and + /// atomic metadata for adaptive eviction logic. + /// + private sealed class CacheEntry + { + /// + /// Gets the key associated with this entry. Used primarily for identification during cleanup. + /// + public TKey Key { get; private set; } = default!; + + /// + /// Gets the cached value. This reference is cleared on eviction to allow GC collection. + /// + public TValue Value { get; private set; } = default!; + + /// + /// Stores the frequency count as double bits to allow for Interlocked atomic math. + /// Frequencies are decayed over time to ensure the cache adapts to new usage patterns. + /// + /// + /// This allows the use of Interlocked.CompareExchange to perform thread-safe floating point + /// arithmetic without a global lock. + /// + private long _frequencyBits; + + /// + /// The tick (monotonically increasing counter) of the last time this entry was accessed. + /// + private long _lastAccessTick; + + public void Initialize(TKey key, TValue value, double frequency, long lastAccessTick) + { + Key = key; + Value = value; + _frequencyBits = BitConverter.DoubleToInt64Bits(frequency); + _lastAccessTick = lastAccessTick; + } + + public void SetValue(TValue value) + { + Value = value; + } + + public void Clear() + { + Key = default!; + Value = default!; + } + + public void Update(long tick) + { + Interlocked.Exchange(ref _lastAccessTick, tick); + long initial, updated; + do + { + initial = Interlocked.Read(ref _frequencyBits); + updated = BitConverter.DoubleToInt64Bits(BitConverter.Int64BitsToDouble(initial) + 1.0); + } + while (Interlocked.CompareExchange(ref _frequencyBits, updated, initial) != initial); + } + + public void Decay(double factor) + { + long initial, updated; + do + { + initial = Interlocked.Read(ref _frequencyBits); + updated = BitConverter.DoubleToInt64Bits(BitConverter.Int64BitsToDouble(initial) * factor); + } + while (Interlocked.CompareExchange(ref _frequencyBits, updated, initial) != initial); + } + + public double GetFrequency() + { + return BitConverter.Int64BitsToDouble(Interlocked.Read(ref _frequencyBits)); + } + + public long GetLastAccess() + { + return Interlocked.Read(ref _lastAccessTick); + } + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/IconCacheProvider.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/IconCacheProvider.cs deleted file mode 100644 index 2687909cfa..0000000000 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/IconCacheProvider.cs +++ /dev/null @@ -1,44 +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 Microsoft.CmdPal.Core.ViewModels; -using Microsoft.CmdPal.UI.Controls; - -namespace Microsoft.CmdPal.UI.Helpers; - -/// -/// Common async event handler provides the cache lookup function for the deferred event. -/// -public static partial class IconCacheProvider -{ - private static readonly IconCacheService IconService = new(Microsoft.UI.Dispatching.DispatcherQueue.GetForCurrentThread()); - -#pragma warning disable IDE0060 // Remove unused parameter - public static async void SourceRequested(IconBox sender, SourceRequestedEventArgs args) -#pragma warning restore IDE0060 // Remove unused parameter - { - if (args.Key is null) - { - return; - } - - if (args.Key is IconDataViewModel iconData) - { - var deferral = args.GetDeferral(); - - args.Value = await IconService.GetIconSource(iconData); - - deferral.Complete(); - } - else if (args.Key is IconInfoViewModel iconInfo) - { - var deferral = args.GetDeferral(); - - var data = args.Theme == Microsoft.UI.Xaml.ElementTheme.Dark ? iconInfo.Dark : iconInfo.Light; - args.Value = await IconService.GetIconSource(data); - - deferral.Complete(); - } - } -} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/IconCacheService.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/IconCacheService.cs deleted file mode 100644 index a506344f61..0000000000 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/IconCacheService.cs +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright (c) Microsoft Corporation -// The Microsoft Corporation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System.Diagnostics; -using Microsoft.CmdPal.Core.ViewModels; -using Microsoft.Terminal.UI; -using Microsoft.UI.Dispatching; -using Microsoft.UI.Xaml.Controls; -using Microsoft.UI.Xaml.Media.Imaging; -using Windows.Storage.Streams; - -namespace Microsoft.CmdPal.UI.Helpers; - -public sealed class IconCacheService(DispatcherQueue dispatcherQueue) -{ - public Task GetIconSource(IconDataViewModel icon) => - - // todo: actually implement a cache of some sort - IconToSource(icon); - - private async Task IconToSource(IconDataViewModel icon) - { - try - { - if (!string.IsNullOrEmpty(icon.Icon)) - { - var source = IconPathConverter.IconSourceMUX(icon.Icon, false, icon.FontFamily); - return source; - } - else if (icon.Data is not null) - { - try - { - return await StreamToIconSource(icon.Data.Unsafe!); - } - catch (Exception ex) - { - Debug.WriteLine("Failed to load icon from stream: " + ex); - } - } - } - catch - { - } - - return null; - } - - private async Task StreamToIconSource(IRandomAccessStreamReference iconStreamRef) - { - if (iconStreamRef is null) - { - return null; - } - - var bitmap = await IconStreamToBitmapImageAsync(iconStreamRef); - var icon = new ImageIconSource() { ImageSource = bitmap }; - return icon; - } - - private async Task IconStreamToBitmapImageAsync(IRandomAccessStreamReference iconStreamRef) - { - // Return the bitmap image via TaskCompletionSource. Using WCT's EnqueueAsync does not suffice here, since if - // we're already on the thread of the DispatcherQueue then it just directly calls the function, with no async involved. - return await TryEnqueueAsync(dispatcherQueue, async () => - { - using var bitmapStream = await iconStreamRef.OpenReadAsync(); - var itemImage = new BitmapImage(); - await itemImage.SetSourceAsync(bitmapStream); - return itemImage; - }); - } - - private static Task TryEnqueueAsync(DispatcherQueue dispatcher, Func> function) - { - var completionSource = new TaskCompletionSource(); - - var enqueued = dispatcher.TryEnqueue(DispatcherQueuePriority.Normal, async void () => - { - try - { - var result = await function(); - completionSource.SetResult(result); - } - catch (Exception ex) - { - completionSource.SetException(ex); - } - }); - - if (!enqueued) - { - completionSource.SetException(new InvalidOperationException("Failed to enqueue the operation on the UI dispatcher")); - } - - return completionSource.Task; - } -} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/CachedIconSourceProvider.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/CachedIconSourceProvider.cs new file mode 100644 index 0000000000..589e40d10f --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/CachedIconSourceProvider.cs @@ -0,0 +1,103 @@ +// 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.Runtime.CompilerServices; +using Microsoft.CmdPal.Core.ViewModels; +using Microsoft.UI.Xaml.Controls; +using Windows.Foundation; + +namespace Microsoft.CmdPal.UI.Helpers; + +internal sealed class CachedIconSourceProvider : IIconSourceProvider +{ + private readonly AdaptiveCache> _cache; + private readonly Size _iconSize; + private readonly IconLoaderService _loader; + private readonly Lock _lock = new(); + + public CachedIconSourceProvider(IconLoaderService loader, Size iconSize, int cacheSize) + { + _loader = loader; + _iconSize = iconSize; + _cache = new AdaptiveCache>(cacheSize, TimeSpan.FromMinutes(60)); + } + + public CachedIconSourceProvider(IconLoaderService loader, int iconSize, int cacheSize) + : this(loader, new Size(iconSize, iconSize), cacheSize) + { + } + + public Task GetIconSource(IconDataViewModel icon, double scale) + { + var key = new IconCacheKey(icon, scale); + + return _cache.TryGet(key, out var existingTask) + ? existingTask + : GetOrCreateSlowPath(key, icon, scale); + } + + private Task GetOrCreateSlowPath(IconCacheKey key, IconDataViewModel icon, double scale) + { + lock (_lock) + { + if (_cache.TryGet(key, out var existingTask)) + { + return existingTask; + } + + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + _loader.EnqueueLoad( + icon.Icon, + icon.FontFamily, + icon.Data?.Unsafe, + _iconSize, + scale, + tcs); + + var task = tcs.Task; + + _ = task.ContinueWith( + _ => + { + lock (_lock) + { + _cache.TryRemove(key); + } + }, + TaskContinuationOptions.OnlyOnFaulted); + + _cache.Add(key, task); + return task; + } + } + + private readonly struct IconCacheKey : IEquatable + { + private readonly string? _icon; + private readonly string? _fontFamily; + private readonly int _streamRefHashCode; + private readonly int _scale; + + public IconCacheKey(IconDataViewModel icon, double scale) + { + _icon = icon.Icon; + _fontFamily = icon.FontFamily; + _streamRefHashCode = icon.Data?.Unsafe is { } stream + ? RuntimeHelpers.GetHashCode(stream) + : 0; + _scale = (int)(100 * Math.Round(scale, 2)); + } + + public bool Equals(IconCacheKey other) => + _icon == other._icon && + _fontFamily == other._fontFamily && + _streamRefHashCode == other._streamRefHashCode && + _scale == other._scale; + + public override bool Equals(object? obj) => obj is IconCacheKey other && Equals(other); + + public override int GetHashCode() => HashCode.Combine(_icon, _fontFamily, _streamRefHashCode, _scale); + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IIconLoaderService.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IIconLoaderService.cs new file mode 100644 index 0000000000..c7f7f27e1b --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IIconLoaderService.cs @@ -0,0 +1,21 @@ +// 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 Microsoft.UI.Xaml.Controls; +using Windows.Foundation; +using Windows.Storage.Streams; + +namespace Microsoft.CmdPal.UI.Helpers; + +internal interface IIconLoaderService : IAsyncDisposable +{ + void EnqueueLoad( + string? iconString, + string? fontFamily, + IRandomAccessStreamReference? streamRef, + Size iconSize, + double scale, + TaskCompletionSource tcs, + IconLoadPriority priority); +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IIconSourceProvider.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IIconSourceProvider.cs new file mode 100644 index 0000000000..1d3f3fc646 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IIconSourceProvider.cs @@ -0,0 +1,13 @@ +// 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 Microsoft.CmdPal.Core.ViewModels; +using Microsoft.UI.Xaml.Controls; + +namespace Microsoft.CmdPal.UI.Helpers; + +internal interface IIconSourceProvider +{ + Task GetIconSource(IconDataViewModel icon, double scale); +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconCacheProvider.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconCacheProvider.cs new file mode 100644 index 0000000000..40de39b5ab --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconCacheProvider.cs @@ -0,0 +1,79 @@ +// 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 Microsoft.CmdPal.Core.ViewModels; +using Microsoft.CmdPal.UI.Controls; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.CmdPal.UI.Helpers; + +/// +/// Common async event handler provides the cache lookup function for the deferred event. +/// +public static partial class IconCacheProvider +{ + /* + Memory Usage Considerations (raw estimates): + | Icon Size | Per Icon | Count | Total | Per Icon @ 200% | Total @ 200% | Per Icon @ 300% | Total @ 300% | + | --------- | -------: | ----: | -------: | --------------: | -----------: | --------------: | -----------: | + | 20×20 | 1.6 KB | 1024 | 1.6 MB | 6.4 KB | 6.4 MB | 14.4 KB | 14.4 MB | + | 32×32 | 4.0 KB | 512 | 2.0 MB | 16 KB | 8.0 MB | 36.0 KB | 18.0 MB | + | 48×48 | 9.0 KB | 256 | 2.3 MB | 36 KB | 9.0 MB | 81.0 KB | 20.3 MB | + | 64×64 | 16.0 KB | 64 | 1.0 MB | 64 KB | 4.0 MB | 144.0 KB | 9.0 MB | + | 256×256 | 256.0 KB | 64 | 16.0 MB | 1 MB | 64.0 MB | 2.3 MB | 144 MB | + */ + + private static IIconSourceProvider _provider20 = null!; + private static IIconSourceProvider _provider32 = null!; + private static IIconSourceProvider _provider64 = null!; + private static IIconSourceProvider _provider256 = null!; + + public static void Initialize(IServiceProvider serviceProvider) + { + _provider20 = serviceProvider.GetRequiredKeyedService(WellKnownIconSize.Size20); + _provider32 = serviceProvider.GetRequiredKeyedService(WellKnownIconSize.Size32); + _provider64 = serviceProvider.GetRequiredKeyedService(WellKnownIconSize.Size64); + _provider256 = serviceProvider.GetRequiredKeyedService(WellKnownIconSize.Size256); + } + + private static async void SourceRequestedCore(IIconSourceProvider service, SourceRequestedEventArgs args) + { + if (args.Key is null) + { + return; + } + + var deferral = args.GetDeferral(); + + try + { + args.Value = args.Key switch + { + IconDataViewModel iconData => await service.GetIconSource(iconData, args.Scale), + IconInfoViewModel iconInfo => await service.GetIconSource( + args.Theme == Microsoft.UI.Xaml.ElementTheme.Light ? iconInfo.Light : iconInfo.Dark, + args.Scale), + _ => null, + }; + } + finally + { + deferral.Complete(); + } + } + +#pragma warning disable IDE0060 // Remove unused parameter + public static void SourceRequested20(IconBox sender, SourceRequestedEventArgs args) + => SourceRequestedCore(_provider20, args); + + public static void SourceRequested32(IconBox sender, SourceRequestedEventArgs args) + => SourceRequestedCore(_provider32, args); + + public static void SourceRequested64(IconBox sender, SourceRequestedEventArgs args) + => SourceRequestedCore(_provider64, args); + + public static void SourceRequested256(IconBox sender, SourceRequestedEventArgs args) + => SourceRequestedCore(_provider256, args); +#pragma warning restore IDE0060 // Remove unused parameter +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconLoadPriority.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconLoadPriority.cs new file mode 100644 index 0000000000..ff824da548 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconLoadPriority.cs @@ -0,0 +1,11 @@ +// 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. + +namespace Microsoft.CmdPal.UI.Helpers; + +internal enum IconLoadPriority +{ + Low, + High, +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconLoaderService.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconLoaderService.cs new file mode 100644 index 0000000000..ef93ccb040 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconLoaderService.cs @@ -0,0 +1,224 @@ +// 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.Threading.Channels; +using CommunityToolkit.WinUI; +using ManagedCommon; +using Microsoft.Terminal.UI; +using Microsoft.UI.Dispatching; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Media.Imaging; +using Windows.Foundation; +using Windows.Storage.Streams; + +namespace Microsoft.CmdPal.UI.Helpers; + +internal sealed partial class IconLoaderService : IIconLoaderService +{ + private const DispatcherQueuePriority LoadingPriorityOnDispatcher = DispatcherQueuePriority.Low; + private const int DefaultIconSize = 256; + private const int MaxWorkerCount = 4; + + private static readonly int WorkerCount = Math.Clamp(Environment.ProcessorCount / 2, 1, MaxWorkerCount); + + private readonly Channel> _highPriorityQueue = Channel.CreateBounded>(32); + private readonly Channel> _lowPriorityQueue = Channel.CreateUnbounded>(); + private readonly Task[] _workers; + private readonly DispatcherQueue _dispatcherQueue; + + public IconLoaderService(DispatcherQueue dispatcherQueue) + { + _dispatcherQueue = dispatcherQueue; + _workers = new Task[WorkerCount]; + + for (var i = 0; i < WorkerCount; i++) + { + _workers[i] = Task.Run(ProcessQueueAsync); + } + } + + public void EnqueueLoad( + string? iconString, + string? fontFamily, + IRandomAccessStreamReference? streamRef, + Size iconSize, + double scale, + TaskCompletionSource tcs, + IconLoadPriority priority = IconLoadPriority.Low) + { + var workItem = () => LoadAndCompleteAsync(iconString, fontFamily, streamRef, iconSize, scale, tcs); + + if (priority == IconLoadPriority.High) + { + if (_highPriorityQueue.Writer.TryWrite(workItem)) + { + return; + } + +#if DEBUG + Logger.LogDebug("High priority icon queue full, falling back to low priority"); +#endif + } + + _lowPriorityQueue.Writer.TryWrite(workItem); + } + + public async ValueTask DisposeAsync() + { + _highPriorityQueue.Writer.Complete(); + _lowPriorityQueue.Writer.Complete(); + + await Task.WhenAll(_workers).ConfigureAwait(false); + } + + private async Task ProcessQueueAsync() + { + while (true) + { + Func? workItem; + + if (_highPriorityQueue.Reader.TryRead(out workItem)) + { + await ExecuteWork(workItem).ConfigureAwait(false); + continue; + } + + var highWait = _highPriorityQueue.Reader.WaitToReadAsync().AsTask(); + var lowWait = _lowPriorityQueue.Reader.WaitToReadAsync().AsTask(); + + await Task.WhenAny(highWait, lowWait).ConfigureAwait(false); + + // Check if both channels are completed (disposal) + if (_highPriorityQueue.Reader.Completion.IsCompleted && + _lowPriorityQueue.Reader.Completion.IsCompleted) + { + // Drain any remaining items + while (_highPriorityQueue.Reader.TryRead(out workItem)) + { + await ExecuteWork(workItem).ConfigureAwait(false); + } + + while (_lowPriorityQueue.Reader.TryRead(out workItem)) + { + await ExecuteWork(workItem).ConfigureAwait(false); + } + + break; + } + + if (_highPriorityQueue.Reader.TryRead(out workItem) || + _lowPriorityQueue.Reader.TryRead(out workItem)) + { + await ExecuteWork(workItem).ConfigureAwait(false); + } + } + + static async Task ExecuteWork(Func workItem) + { + try + { + await workItem().ConfigureAwait(false); + } + catch (Exception ex) + { + Logger.LogError("Failed to load icon", ex); + } + } + } + + private async Task LoadAndCompleteAsync( + string? iconString, + string? fontFamily, + IRandomAccessStreamReference? streamRef, + Size iconSize, + double scale, + TaskCompletionSource tcs) + { + try + { + var result = await LoadIconCoreAsync(iconString, fontFamily, streamRef, iconSize, scale).ConfigureAwait(false); + tcs.TrySetResult(result); + } + catch (Exception ex) + { + tcs.TrySetException(ex); + } + } + + private async Task LoadIconCoreAsync( + string? iconString, + string? fontFamily, + IRandomAccessStreamReference? streamRef, + Size iconSize, + double scale) + { + var scaledSize = new Size(iconSize.Width * scale, iconSize.Height * scale); + + if (!string.IsNullOrEmpty(iconString)) + { + return await _dispatcherQueue + .EnqueueAsync(() => GetStringIconSource(iconString, fontFamily, scaledSize), LoadingPriorityOnDispatcher) + .ConfigureAwait(false); + } + + if (streamRef != null) + { + try + { + using var bitmapStream = await streamRef.OpenReadAsync().AsTask().ConfigureAwait(false); + + return await _dispatcherQueue + .EnqueueAsync(BuildImageSource, LoadingPriorityOnDispatcher) + .ConfigureAwait(false); + + async Task BuildImageSource() + { + var bitmap = new BitmapImage(); + ApplyDecodeSize(bitmap, scaledSize); + await bitmap.SetSourceAsync(bitmapStream); + return new ImageIconSource { ImageSource = bitmap }; + } + } +#pragma warning disable CS0168 // Variable is declared but never used + catch (Exception ex) +#pragma warning restore CS0168 // Variable is declared but never used + { +#if DEBUG + Logger.LogDebug($"Failed to open icon stream: {ex}"); +#endif + return null; + } + } + + return null; + } + + private static void ApplyDecodeSize(BitmapImage bitmap, Size size) + { + if (size.IsEmpty) + { + return; + } + + if (size.Width >= size.Height) + { + bitmap.DecodePixelWidth = (int)size.Width; + } + else + { + bitmap.DecodePixelHeight = (int)size.Height; + } + } + + private static IconSource? GetStringIconSource(string iconString, string? fontFamily, Size size) + { + var iconSize = (int)Math.Max(size.Width, size.Height); + if (iconSize == 0) + { + iconSize = DefaultIconSize; + } + + return IconPathConverter.IconSourceMUX(iconString, false, fontFamily, iconSize); + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconServiceRegistration.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconServiceRegistration.cs new file mode 100644 index 0000000000..c5f3709276 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconServiceRegistration.cs @@ -0,0 +1,37 @@ +// 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 Microsoft.Extensions.DependencyInjection; +using Microsoft.UI.Dispatching; + +namespace Microsoft.CmdPal.UI.Helpers; + +internal static class IconServiceRegistration +{ + public static IServiceCollection AddIconServices(this IServiceCollection services, DispatcherQueue dispatcherQueue) + { + // Single shared loader + var loader = new IconLoaderService(dispatcherQueue); + services.AddSingleton(loader); + + // Keyed providers by size + services.AddKeyedSingleton( + WellKnownIconSize.Size20, + (_, _) => new CachedIconSourceProvider(loader, 20, 1024)); + + services.AddKeyedSingleton( + WellKnownIconSize.Size32, + (_, _) => new IconSourceProvider(loader, 32)); + + services.AddKeyedSingleton( + WellKnownIconSize.Size64, + (_, _) => new CachedIconSourceProvider(loader, 64, 256)); + + services.AddKeyedSingleton( + WellKnownIconSize.Size256, + (_, _) => new CachedIconSourceProvider(loader, 256, 64)); + + return services; + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconSourceProvider.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconSourceProvider.cs new file mode 100644 index 0000000000..13c6ed764a --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/IconSourceProvider.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 Microsoft.CmdPal.Core.ViewModels; +using Microsoft.UI.Xaml.Controls; +using Windows.Foundation; + +namespace Microsoft.CmdPal.UI.Helpers; + +internal sealed class IconSourceProvider : IIconSourceProvider +{ + private readonly IconLoaderService _loader; + private readonly Size _iconSize; + + public IconSourceProvider(IconLoaderService loader, Size iconSize) + { + _loader = loader; + _iconSize = iconSize; + } + + public IconSourceProvider(IconLoaderService loader, int iconSize) + : this(loader, new Size(iconSize, iconSize)) + { + } + + public Task GetIconSource(IconDataViewModel icon, double scale) + { + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + _loader.EnqueueLoad( + icon.Icon, + icon.FontFamily, + icon.Data?.Unsafe, + _iconSize, + scale, + tcs); + + return tcs.Task; + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/WellKnownIconSize.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/WellKnownIconSize.cs new file mode 100644 index 0000000000..e35cbd46f5 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/Icons/WellKnownIconSize.cs @@ -0,0 +1,13 @@ +// 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. + +namespace Microsoft.CmdPal.UI.Helpers; + +internal enum WellKnownIconSize +{ + Size20 = 20, + Size32 = 32, + Size64 = 64, + Size256 = 256, +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/TypedEventHandlerExtensions.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/TypedEventHandlerExtensions.cs deleted file mode 100644 index 70bfffe6b3..0000000000 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/TypedEventHandlerExtensions.cs +++ /dev/null @@ -1,75 +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 CommunityToolkit.Common.Deferred; -using Windows.Foundation; - -// Pilfered from CommunityToolkit.WinUI.Deferred -namespace Microsoft.CmdPal.UI.Deferred; - -/// -/// Extensions to for Deferred Events. -/// -public static class TypedEventHandlerExtensions -{ - /// - /// Use to invoke an async using . - /// - /// Type of sender. - /// type. - /// to be invoked. - /// Sender of the event. - /// instance. - /// to wait on deferred event handler. -#pragma warning disable CA1715 // Identifiers should have correct prefix -#pragma warning disable SA1314 // Type parameter names should begin with T - public static Task InvokeAsync(this TypedEventHandler eventHandler, S sender, R eventArgs) -#pragma warning restore SA1314 // Type parameter names should begin with T -#pragma warning restore CA1715 // Identifiers should have correct prefix - where R : DeferredEventArgs => InvokeAsync(eventHandler, sender, eventArgs, CancellationToken.None); - - /// - /// Use to invoke an async using with a . - /// - /// Type of sender. - /// type. - /// to be invoked. - /// Sender of the event. - /// instance. - /// option. - /// to wait on deferred event handler. -#pragma warning disable CA1715 // Identifiers should have correct prefix -#pragma warning disable SA1314 // Type parameter names should begin with T - public static Task InvokeAsync(this TypedEventHandler eventHandler, S sender, R eventArgs, CancellationToken cancellationToken) -#pragma warning restore SA1314 // Type parameter names should begin with T -#pragma warning restore CA1715 // Identifiers should have correct prefix - where R : DeferredEventArgs - { - if (eventHandler is null) - { - return Task.CompletedTask; - } - - var tasks = eventHandler.GetInvocationList() - .OfType>() - .Select(invocationDelegate => - { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - - invocationDelegate(sender, eventArgs); - -#pragma warning disable CS0618 // Type or member is obsolete - var deferral = eventArgs.GetCurrentDeferralAndReset(); - - return deferral?.WaitForCompletion(cancellationToken) ?? Task.CompletedTask; -#pragma warning restore CS0618 // Type or member is obsolete - }) - .ToArray(); - - return Task.WhenAll(tasks); - } -} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs index bc083cd1bd..d54fbe93d2 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs @@ -648,8 +648,14 @@ public sealed partial class MainWindow : WindowEx, // Updates our window s.t. the top of the window is draggable. private void UpdateRegionsForCustomTitleBar() { + var xamlRoot = RootElement.XamlRoot; + if (xamlRoot is null) + { + return; + } + // Specify the interactive regions of the title bar. - var scaleAdjustment = RootElement.XamlRoot.RasterizationScale; + var scaleAdjustment = xamlRoot.RasterizationScale; // Get the rectangle around our XAML content. We're going to mark this // rectangle as "Passthrough", so that the normal window operations diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml index df40e597c3..18bc15298d 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml @@ -92,7 +92,7 @@ Height="16" Margin="0,3,8,0" SourceKey="{x:Bind Icon, Mode=OneWay}" - SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested}" /> + SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested20}" /> @@ -296,7 +296,7 @@ ui:VisualExtensions.NormalizedCenterPoint="0.5,0.5" Foreground="{ThemeResource TextFillColorSecondaryBrush}" SourceKey="{x:Bind ViewModel.CurrentPage.Icon, Mode=OneWay}" - SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested}" + SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested20}" Visibility="{x:Bind ViewModel.CurrentPage.IsNested, Mode=OneWay}"> + SourceRequested="{x:Bind helpers:IconCacheProvider.SourceRequested20}" /> @@ -94,7 +94,7 @@ Height="20" AutomationProperties.AccessibilityView="Raw" SourceKey="{x:Bind Icon, Mode=OneWay}" - SourceRequested="{x:Bind helpers:IconCacheProvider.SourceRequested}" /> + SourceRequested="{x:Bind helpers:IconCacheProvider.SourceRequested20}" /> @@ -165,7 +165,7 @@ Height="20" AutomationProperties.AccessibilityView="Raw" SourceKey="{x:Bind Icon, Mode=OneWay}" - SourceRequested="{x:Bind helpers:IconCacheProvider.SourceRequested}" /> + SourceRequested="{x:Bind helpers:IconCacheProvider.SourceRequested20}" /> diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml index 528e8d2351..e01f26b571 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml @@ -239,7 +239,7 @@ Height="20" AutomationProperties.AccessibilityView="Raw" SourceKey="{x:Bind Icon, Mode=OneWay}" - SourceRequested="{x:Bind helpers:IconCacheProvider.SourceRequested}" /> + SourceRequested="{x:Bind helpers:IconCacheProvider.SourceRequested20}" /> Date: Mon, 2 Feb 2026 18:23:34 +0100 Subject: [PATCH 4/7] CmdPal: Make Indexer great again - part 1 - hotfix (#44729) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request This PR introduces a rough hotfix for several indexer-related issues. - Premise: patch what we can in-place and fix the core later (reworking `SeachEngine` and `SearchQuery` is slightly trickier). This patch also removes some dead code for future refactor. - Adds search cancellation to the File Search page and the indexer fallback. - Prevents older searches from overwriting newer model state and reduces wasted work. - Stops reusing the search engine; creates a new instance per search to avoid synchronization issues. - That `SeachEngine` and `SearchQuery` are not multi-threading friendly. - Removes search priming to simplify the code and improve performance. - Since `SearchQuery` cancels and re-primes on every search, priming provides little benefit and can hide extra work (for example, cancellation triggering re-priming). - Fixes the indexer fallback subject line not updating when there is more than one match. - It previously kept the old value, which was confusing. - ~Shows the number of matched files in the fallback result.~ - Fetching total number of rows was reverted, performance was not stable :( - Optimizes the indexer fallback by reducing the number of items processed but not used. - Only fetches the item(s) needed for the fallback itself—no extra work on the hot path. - Stops reusing the fallback result when navigating to the File Search page to show more results. This requires querying again, but it simplifies the flow and keeps components isolated. - Fixes the English mnemonic keyword `kind` being hardcoded in the search page filter. Windows Search uses localized mnemonic keyword names, so this PR replaces it with canonical keyword `System.Kind` that is universaly recognized. - Adds extra diagnostics to `SearchQuery` and makes logging more precise. - DataPackage for the IndexerListItem now defers including of storage items - boost performance and we avoid touching the item until its needed. - IndexerPage with a prepopulated query will delay loading until the items are actually enumerated (to avoid populating from fallback hot path) and it no longer takes external SearchEngine. It just recreates everything. ## PR Checklist - [x] Related to: #44728 - [x] Closes: #44731 - [x] Closes: #44732 - [x] Closes: #44743 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed --- .github/actions/spell-check/expect.txt | 1 + .../FallbackOpenFileItem.cs | 295 ++++++++++------ .../Helpers/DataPackageHelper.cs | 72 ++-- .../Indexer/SearchQuery.cs | 314 +++++++----------- .../Indexer/Utils/QueryStringBuilder.cs | 13 +- .../Pages/IndexerPage.cs | 200 ++++++++--- .../Properties/Resources.Designer.cs | 11 +- .../Properties/Resources.resx | 3 + .../SearchEngine.cs | 111 ++++--- 9 files changed, 614 insertions(+), 406 deletions(-) diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index 15e48cff17..9d6188fa24 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -44,6 +44,7 @@ ALLCHILDREN ALLINPUT Allman Allmodule +ALLNOISE ALLOWUNDO ALLVIEW ALPHATYPE diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/FallbackOpenFileItem.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/FallbackOpenFileItem.cs index 967e962085..96f66729ac 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/FallbackOpenFileItem.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/FallbackOpenFileItem.cs @@ -2,33 +2,43 @@ // 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 System.Globalization; using System.IO; using System.Text; +using System.Threading; +using System.Threading.Tasks; using Microsoft.CmdPal.Ext.Indexer.Data; using Microsoft.CmdPal.Ext.Indexer.Helpers; using Microsoft.CmdPal.Ext.Indexer.Properties; +using Microsoft.CommandPalette.Extensions; using Microsoft.CommandPalette.Extensions.Toolkit; +using Windows.ApplicationModel.DataTransfer; using Windows.Storage.Streams; namespace Microsoft.CmdPal.Ext.Indexer; -internal sealed partial class FallbackOpenFileItem : FallbackCommandItem, System.IDisposable +internal sealed partial class FallbackOpenFileItem : FallbackCommandItem, IDisposable { - private const string _id = "com.microsoft.cmdpal.builtin.indexer.fallback"; - private static readonly NoOpCommand _baseCommandWithId = new() { Id = _id }; + private const string CommandId = "com.microsoft.cmdpal.builtin.indexer.fallback"; - private readonly CompositeFormat fallbackItemSearchPageTitleCompositeFormat = CompositeFormat.Parse(Resources.Indexer_fallback_searchPage_title); + // Cookie to identify our queries; since we replace the SearchEngine on each search, + // this can be a constant. + private const uint HardQueryCookie = 10; + private static readonly NoOpCommand BaseCommandWithId = new() { Id = CommandId }; - private readonly SearchEngine _searchEngine = new(); + private readonly CompositeFormat _fallbackItemSearchPageTitleFormat = CompositeFormat.Parse(Resources.Indexer_fallback_searchPage_title); + private readonly CompositeFormat _fallbackItemSearchSubtitleMultipleResults = CompositeFormat.Parse(Resources.Indexer_Fallback_MultipleResults_Subtitle); + private readonly Lock _querySwitchLock = new(); + private readonly Lock _resultLock = new(); - private uint _queryCookie = 10; - - private Func _suppressCallback; + private CancellationTokenSource? _currentQueryCts; + private Func? _suppressCallback; public FallbackOpenFileItem() - : base(_baseCommandWithId, Resources.Indexer_Find_Path_fallback_display_title, _id) + : base(BaseCommandWithId, Resources.Indexer_Find_Path_fallback_display_title, CommandId) { Title = string.Empty; Subtitle = string.Empty; @@ -37,118 +47,209 @@ internal sealed partial class FallbackOpenFileItem : FallbackCommandItem, System public override void UpdateQuery(string query) { - if (string.IsNullOrWhiteSpace(query)) - { - Command = new NoOpCommand(); - Title = string.Empty; - Subtitle = string.Empty; - Icon = null; - MoreCommands = null; - DataPackage = null; + UpdateQueryCore(query); + } + private void UpdateQueryCore(string query) + { + // Calling this will cancel any ongoing query processing. We always use a new SearchEngine + // instance per query, as SearchEngine.Query cancels/reinitializes internally. + CancellationToken cancellationToken; + + lock (_querySwitchLock) + { + _currentQueryCts?.Cancel(); + _currentQueryCts?.Dispose(); + _currentQueryCts = new CancellationTokenSource(); + cancellationToken = _currentQueryCts.Token; + } + + var suppressCallback = _suppressCallback; + if (string.IsNullOrWhiteSpace(query) || (suppressCallback is not null && suppressCallback(query))) + { + ClearResultForCurrentQuery(cancellationToken); return; } - if (_suppressCallback is not null && _suppressCallback(query)) + try { - Command = new NoOpCommand(); - Title = string.Empty; - Subtitle = string.Empty; - Icon = null; - MoreCommands = null; - DataPackage = null; - - return; - } - - if (Path.Exists(query)) - { - // Exit 1: The query is a direct path to a file. Great! Return it. - var item = new IndexerItem(fullPath: query); - var listItemForUs = new IndexerListItem(item, IncludeBrowseCommand.AsDefault); - Command = listItemForUs.Command; - MoreCommands = listItemForUs.MoreCommands; - Subtitle = item.FileName; - Title = item.FullPath; - Icon = listItemForUs.Icon; - DataPackage = DataPackageHelper.CreateDataPackageForPath(listItemForUs, item.FullPath); - - try + var exists = Path.Exists(query); + if (exists) { - var stream = ThumbnailHelper.GetThumbnail(item.FullPath).Result; - if (stream is not null) + ProcessDirectPath(query, cancellationToken); + } + else + { + ProcessSearchQuery(query, cancellationToken); + } + } + catch (OperationCanceledException) + { + // Query was superseded by a newer one - discard silently. + } + catch + { + if (!cancellationToken.IsCancellationRequested) + { + ClearResultForCurrentQuery(cancellationToken); + } + } + } + + private void ProcessDirectPath(string query, CancellationToken ct) + { + var item = new IndexerItem(fullPath: query); + var indexerListItem = new IndexerListItem(item, IncludeBrowseCommand.AsDefault); + + ct.ThrowIfCancellationRequested(); + UpdateResultForCurrentQuery(indexerListItem, skipIcon: true, ct); + _ = LoadIconAsync(item.FullPath, ct); + } + + private void ProcessSearchQuery(string query, CancellationToken ct) + { + ct.ThrowIfCancellationRequested(); + + // for now the SearchEngine and SearchQuery are not thread-safe, so we create a new instance per query + // since SearchEngine will re-initialize on a new query anyway, it doesn't seem to be a big overhead for now + var searchEngine = new SearchEngine(); + + try + { + searchEngine.Query(query, queryCookie: HardQueryCookie); + ct.ThrowIfCancellationRequested(); + + // We only need to know whether there are 0, 1, or more than one result + var results = searchEngine.FetchItems(0, 2, queryCookie: HardQueryCookie, out _, noIcons: true); + var count = results.Count; + + if (count == 0) + { + ClearResultForCurrentQuery(ct); + } + else if (count == 1) + { + if (results[0] is IndexerListItem indexerListItem) { - var data = new IconData(RandomAccessStreamReference.CreateFromStream(stream)); - Icon = new IconInfo(data, data); + UpdateResultForCurrentQuery(indexerListItem, skipIcon: true, ct); + _ = LoadIconAsync(indexerListItem.FilePath, ct); + } + else + { + ClearResultForCurrentQuery(ct); } } - catch + else { + var indexerPage = new IndexerPage(query); + + var set = UpdateResultForCurrentQuery( + string.Format(CultureInfo.CurrentCulture, _fallbackItemSearchPageTitleFormat, query), + string.Format(CultureInfo.CurrentCulture, _fallbackItemSearchSubtitleMultipleResults), + Icons.FileExplorerIcon, + indexerPage, + MoreCommands, + DataPackage, + skipIcon: false, + ct); + + if (!set) + { + // if we failed to set the result (query was cancelled), dispose the page and search engine + indexerPage.Dispose(); + } } - - return; } - else + finally { - _queryCookie++; + searchEngine?.Dispose(); + } + } - try + private async Task LoadIconAsync(string path, CancellationToken ct) + { + try + { + var stream = await ThumbnailHelper.GetThumbnail(path).ConfigureAwait(false); + if (stream is null || ct.IsCancellationRequested) { - _searchEngine.Query(query, _queryCookie); - var results = _searchEngine.FetchItems(0, 20, _queryCookie, out var _); - - if (results.Count == 0 || (results[0] is not IndexerListItem indexerListItem)) - { - // Exit 2: We searched for the file, and found nothing. Oh well. - // Hide ourselves. - Title = string.Empty; - Subtitle = string.Empty; - Command = new NoOpCommand(); - MoreCommands = null; - DataPackage = null; - return; - } - - if (results.Count == 1) - { - // Exit 3: We searched for the file, and found exactly one thing. Awesome! - // Return it. - Title = indexerListItem.Title; - Subtitle = indexerListItem.Subtitle; - Icon = indexerListItem.Icon; - Command = indexerListItem.Command; - MoreCommands = indexerListItem.MoreCommands; - DataPackage = DataPackageHelper.CreateDataPackageForPath(indexerListItem, indexerListItem.FilePath); - - return; - } - - // Exit 4: We found more than one result. Make our command take - // us to the file search page, prepopulated with this search. - var indexerPage = new IndexerPage(query, _searchEngine, _queryCookie, results); - Title = string.Format(CultureInfo.CurrentCulture, fallbackItemSearchPageTitleCompositeFormat, query); - Icon = Icons.FileExplorerIcon; - Command = indexerPage; - MoreCommands = null; - DataPackage = null; - return; } - catch + + var thumbnailStream = RandomAccessStreamReference.CreateFromStream(stream); + if (ct.IsCancellationRequested) { - Title = string.Empty; - Subtitle = string.Empty; - Icon = null; - Command = new NoOpCommand(); - MoreCommands = null; - DataPackage = null; + return; } + + var data = new IconData(thumbnailStream); + UpdateIconForCurrentQuery(new IconInfo(data), ct); + } + catch + { + // ignore - keep default icon + UpdateIconForCurrentQuery(Icons.FileExplorerIcon, ct); + } + } + + private bool ClearResultForCurrentQuery(CancellationToken ct) + { + return UpdateResultForCurrentQuery(string.Empty, string.Empty, Icons.FileExplorerIcon, BaseCommandWithId, null, null, false, ct); + } + + private bool UpdateResultForCurrentQuery(IndexerListItem listItem, bool skipIcon, CancellationToken ct) + { + return UpdateResultForCurrentQuery( + listItem.Title, + listItem.Subtitle, + listItem.Icon, + listItem.Command, + listItem.MoreCommands, + DataPackageHelper.CreateDataPackageForPath(listItem, listItem.FilePath), + skipIcon, + ct); + } + + private bool UpdateResultForCurrentQuery(string title, string subtitle, IIconInfo? iconInfo, ICommand? command, IContextItem[]? moreCommands, DataPackage? dataPackage, bool skipIcon, CancellationToken ct) + { + lock (_resultLock) + { + if (ct.IsCancellationRequested) + { + return false; + } + + Title = title; + Subtitle = subtitle; + if (!skipIcon) + { + Icon = iconInfo!; + } + + MoreCommands = moreCommands!; + DataPackage = dataPackage; + Command = command; + return true; + } + } + + private void UpdateIconForCurrentQuery(IIconInfo icon, CancellationToken ct) + { + lock (_resultLock) + { + if (ct.IsCancellationRequested) + { + return; + } + + Icon = icon; } } public void Dispose() { - _searchEngine.Dispose(); + _currentQueryCts?.Cancel(); + _currentQueryCts?.Dispose(); GC.SuppressFinalize(this); } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Helpers/DataPackageHelper.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Helpers/DataPackageHelper.cs index 65d18a0e2a..a4e7873189 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Helpers/DataPackageHelper.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Helpers/DataPackageHelper.cs @@ -2,6 +2,8 @@ // 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 System.IO; using System.Threading.Tasks; @@ -14,51 +16,83 @@ namespace Microsoft.CmdPal.Ext.Indexer.Helpers; internal static class DataPackageHelper { - public static DataPackage CreateDataPackageForPath(ICommandItem listItem, string path) + public static DataPackage? CreateDataPackageForPath(ICommandItem listItem, string? path) { - if (string.IsNullOrEmpty(path)) + if (string.IsNullOrWhiteSpace(path)) { return null; } - var dataPackage = new DataPackage(); - dataPackage.SetText(path); - _ = dataPackage.TrySetStorageItemsAsync(path); - dataPackage.Properties.Title = listItem.Title; - dataPackage.Properties.Description = listItem.Subtitle; - dataPackage.RequestedOperation = DataPackageOperation.Copy; + // Capture now; don't rely on listItem still being valid later. + var title = listItem.Title; + var description = listItem.Subtitle; + var capturedPath = path; + + var dataPackage = new DataPackage + { + RequestedOperation = DataPackageOperation.Copy, + Properties = + { + Title = title, + Description = description, + }, + }; + + // Cheap + immediate. + dataPackage.SetText(capturedPath); + + // Expensive + only computed if the consumer asks for StorageItems. + dataPackage.SetDataProvider(StandardDataFormats.StorageItems, async void (request) => + { + var deferral = request.GetDeferral(); + try + { + var items = await TryGetStorageItemAsync(capturedPath).ConfigureAwait(false); + if (items is not null) + { + request.SetData(items); + } + + // If null: just don't provide StorageItems. Text still works. + } + catch + { + // Swallow: better to provide partial data (text) than fail the whole package. + } + finally + { + deferral.Complete(); + } + }); + return dataPackage; } - public static async Task TrySetStorageItemsAsync(this DataPackage dataPackage, string filePath) + private static async Task TryGetStorageItemAsync(string filePath) { try { if (File.Exists(filePath)) { var file = await StorageFile.GetFileFromPathAsync(filePath); - dataPackage.SetStorageItems([file]); - return true; + return [file]; } if (Directory.Exists(filePath)) { var folder = await StorageFolder.GetFolderFromPathAsync(filePath); - dataPackage.SetStorageItems([folder]); - return true; + return [folder]; } - // nothing there - return false; + return null; } catch (UnauthorizedAccessException) { - // Access denied – skip or report, but don't crash - return false; + return null; } - catch (Exception) + catch { - return false; + return null; } } } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Indexer/SearchQuery.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Indexer/SearchQuery.cs index 6b85834bb8..6dd3137dbb 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Indexer/SearchQuery.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Indexer/SearchQuery.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Concurrent; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Threading; @@ -17,66 +18,25 @@ namespace Microsoft.CmdPal.Ext.Indexer.Indexer; internal sealed partial class SearchQuery : IDisposable { - private readonly Lock _lockObject = new(); // Lock object for synchronization - private readonly DBPROPIDSET dbPropIdSet; + [SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Indexing Service constant")] + private const int QUERY_E_ALLNOISE = unchecked((int)0x80041605); - private uint reuseWhereID; - private EventWaitHandle queryCompletedEvent; - private Timer queryTpTimer; - private IRowset currentRowset; - private IRowset reuseRowset; + private readonly Lock _lockObject = new(); - public uint Cookie { get; set; } + private IRowset _currentRowset; + + public QueryState State { get; private set; } = QueryState.NotStarted; + + private int? LastHResult { get; set; } + + private string LastErrorMessage { get; set; } + + public uint Cookie { get; private set; } public string SearchText { get; private set; } public ConcurrentQueue SearchResults { get; private set; } = []; - public SearchQuery() - { - dbPropIdSet = new DBPROPIDSET - { - rgPropertyIDs = Marshal.AllocCoTaskMem(sizeof(uint)), // Allocate memory for the property ID array - cPropertyIDs = 1, - guidPropertySet = new Guid("AA6EE6B0-E828-11D0-B23E-00AA0047FC01"), // DBPROPSET_MSIDXS_ROWSETEXT, - }; - - // Copy the property ID into the allocated memory - Marshal.WriteInt32(dbPropIdSet.rgPropertyIDs, 8); // MSIDXSPROP_WHEREID - - Init(); - } - - private void Init() - { - // Create all the objects we will want cached - try - { - queryTpTimer = new Timer(QueryTimerCallback, this, Timeout.Infinite, Timeout.Infinite); - if (queryTpTimer is null) - { - Logger.LogError("Failed to create query timer"); - return; - } - - queryCompletedEvent = new EventWaitHandle(false, EventResetMode.ManualReset); - if (queryCompletedEvent is null) - { - Logger.LogError("Failed to create query completed event"); - return; - } - - // Execute a synchronous query on file items to prime the index and keep that handle around - PrimeIndexAndCacheWhereId(); - } - catch (Exception ex) - { - Logger.LogError("Exception at SearchUXQueryHelper Init", ex); - } - } - - public void WaitForQueryCompletedEvent() => queryCompletedEvent.WaitOne(); - public void CancelOutstandingQueries() { Logger.LogDebug("Cancel query " + SearchText); @@ -84,14 +44,7 @@ internal sealed partial class SearchQuery : IDisposable // Are we currently doing work? If so, let's cancel lock (_lockObject) { - if (queryTpTimer is not null) - { - queryTpTimer.Change(Timeout.Infinite, Timeout.Infinite); - queryTpTimer.Dispose(); - queryTpTimer = null; - } - - Init(); + State = QueryState.Cancelled; } } @@ -102,40 +55,32 @@ internal sealed partial class SearchQuery : IDisposable ExecuteSyncInternal(); } - public static void QueryTimerCallback(object state) - { - var pQueryHelper = (SearchQuery)state; - pQueryHelper.ExecuteSyncInternal(); - } - private void ExecuteSyncInternal() { lock (_lockObject) { - var queryStr = QueryStringBuilder.GenerateQuery(SearchText, reuseWhereID); + State = QueryState.Running; + LastHResult = null; + LastErrorMessage = null; + + var queryStr = QueryStringBuilder.GenerateQuery(SearchText); try { - // We need to generate a search query string with the search text the user entered above - if (currentRowset is not null) - { - // We have a previous rowset, this means the user is typing and we should store this - // recapture the where ID from this so the next ExecuteSync call will be faster - reuseRowset = currentRowset; - reuseWhereID = GetReuseWhereId(reuseRowset); - } - - currentRowset = ExecuteCommand(queryStr); + var result = ExecuteCommand(queryStr); + _currentRowset = result.Rowset; + State = result.State; + LastHResult = result.HResult; + LastErrorMessage = result.ErrorMessage; SearchResults.Clear(); } catch (Exception ex) { + State = QueryState.ExecuteFailed; + LastHResult = ex.HResult; + LastErrorMessage = ex.Message; Logger.LogError("Error executing query", ex); } - finally - { - queryCompletedEvent.Set(); - } } } @@ -170,31 +115,68 @@ internal sealed partial class SearchQuery : IDisposable public bool FetchRows(int offset, int limit) { - if (currentRowset is null) + if (_currentRowset is null) { - Logger.LogError("No rowset to fetch rows from"); + var message = $"No rowset to fetch rows from. State={State}, HResult={LastHResult}, Error='{LastErrorMessage}'"; + + switch (State) + { + case QueryState.NoResults: + case QueryState.AllNoise: + Logger.LogDebug(message); + break; + case QueryState.NotStarted: + case QueryState.Cancelled: + case QueryState.Running: + Logger.LogInfo(message); + break; + default: + Logger.LogError(message); + break; + } + return false; } - IGetRow getRow = null; + IGetRow getRow; try { - getRow = (IGetRow)currentRowset; + getRow = (IGetRow)_currentRowset; } - catch (Exception) + catch (Exception ex) { - Logger.LogInfo("Reset the current rowset"); + Logger.LogInfo($"Reset the current rowset. State={State}, HResult={LastHResult}, Error='{LastErrorMessage}'"); + Logger.LogError("Failed to cast current rowset to IGetRow", ex); + ExecuteSyncInternal(); - getRow = (IGetRow)currentRowset; + + if (_currentRowset is null) + { + var message = $"Failed to reset rowset. State={State}, HResult={LastHResult}, Error='{LastErrorMessage}'"; + + switch (State) + { + case QueryState.NoResults: + case QueryState.AllNoise: + Logger.LogDebug(message); + break; + default: + Logger.LogError(message); + break; + } + + return false; + } + + getRow = (IGetRow)_currentRowset; } - uint rowCountReturned; var prghRows = IntPtr.Zero; try { - currentRowset.GetNextRows(IntPtr.Zero, offset, limit, out rowCountReturned, out prghRows); + _currentRowset.GetNextRows(IntPtr.Zero, offset, limit, out var rowCountReturned, out prghRows); if (rowCountReturned == 0) { @@ -215,7 +197,7 @@ internal sealed partial class SearchQuery : IDisposable } } - currentRowset.ReleaseRows(rowCountReturned, rowHandles, IntPtr.Zero, null, null); + _currentRowset.ReleaseRows(rowCountReturned, rowHandles, IntPtr.Zero, null, null); Marshal.FreeCoTaskMem(prghRows); prghRows = IntPtr.Zero; @@ -236,141 +218,91 @@ internal sealed partial class SearchQuery : IDisposable } } - private void PrimeIndexAndCacheWhereId() - { - var queryStr = QueryStringBuilder.GeneratePrimingQuery(); - var rowset = ExecuteCommand(queryStr); - if (rowset is not null) - { - reuseRowset = rowset; - reuseWhereID = GetReuseWhereId(reuseRowset); - } - } - - private unsafe IRowset ExecuteCommand(string queryStr) + private static ExecuteCommandResult ExecuteCommand(string queryStr) { if (string.IsNullOrEmpty(queryStr)) { - return null; + return new ExecuteCommandResult(Rowset: null, State: QueryState.ExecuteFailed, HResult: null, ErrorMessage: "Query string was empty."); } try { - var session = (IDBCreateSession)DataSourceManager.GetDataSource(); + var dataSource = DataSourceManager.GetDataSource(); + if (dataSource is null) + { + Logger.LogError("GetDataSource returned null"); + return new ExecuteCommandResult(Rowset: null, State: QueryState.NullDataSource, HResult: null, ErrorMessage: "GetDataSource returned null."); + } + + var session = (IDBCreateSession)dataSource; var guid = typeof(IDBCreateCommand).GUID; session.CreateSession(IntPtr.Zero, ref guid, out var ppDBSession); if (ppDBSession is null) { Logger.LogError("CreateSession failed"); - return null; + return new ExecuteCommandResult(Rowset: null, State: QueryState.CreateSessionFailed, HResult: null, ErrorMessage: "CreateSession returned null session."); } var createCommand = (IDBCreateCommand)ppDBSession; guid = typeof(ICommandText).GUID; - createCommand.CreateCommand(IntPtr.Zero, ref guid, out ICommandText commandText); + createCommand.CreateCommand(IntPtr.Zero, ref guid, out var commandText); if (commandText is null) { Logger.LogError("Failed to get ICommandText interface"); - return null; + return new ExecuteCommandResult(Rowset: null, State: QueryState.CreateCommandFailed, HResult: null, ErrorMessage: "CreateCommand returned null command."); } var riid = NativeHelpers.OleDb.DbGuidDefault; - var irowSetRiid = typeof(IRowset).GUID; commandText.SetCommandText(ref riid, queryStr); - commandText.Execute(null, ref irowSetRiid, null, out var pcRowsAffected, out var rowsetPointer); + commandText.Execute(null, ref irowSetRiid, null, out _, out var rowsetPointer); - return rowsetPointer; + return rowsetPointer is null + ? new ExecuteCommandResult(Rowset: null, State: QueryState.NoResults, HResult: null, ErrorMessage: null) + : new ExecuteCommandResult(Rowset: rowsetPointer, State: QueryState.Completed, HResult: null, ErrorMessage: null); + } + catch (COMException ex) when (ex.HResult == QUERY_E_ALLNOISE) + { + Logger.LogDebug($"Query returned all noise, no results. ({queryStr})"); + return new ExecuteCommandResult(Rowset: null, State: QueryState.AllNoise, HResult: ex.HResult, ErrorMessage: ex.Message); + } + catch (COMException ex) + { + Logger.LogError($"Unexpected COM error for query '{queryStr}'.", ex); + return new ExecuteCommandResult(Rowset: null, State: QueryState.ExecuteFailed, HResult: ex.HResult, ErrorMessage: ex.Message); } catch (Exception ex) { - Logger.LogError("Unexpected error.", ex); - return null; + Logger.LogError($"Unexpected error for query '{queryStr}'.", ex); + return new ExecuteCommandResult(Rowset: null, State: QueryState.ExecuteFailed, HResult: ex.HResult, ErrorMessage: ex.Message); } } - private unsafe DBPROP? GetPropset(IRowsetInfo rowsetInfo) - { - var prgPropSetsPtr = IntPtr.Zero; - - try - { - ulong cPropertySets; - var res = rowsetInfo.GetProperties(1, [dbPropIdSet], out cPropertySets, out prgPropSetsPtr); - if (res != 0) - { - Logger.LogError($"Error getting properties: {res}"); - return null; - } - - if (cPropertySets == 0 || prgPropSetsPtr == IntPtr.Zero) - { - Logger.LogError("No property sets returned"); - return null; - } - - var firstPropSetPtr = (DBPROPSET*)prgPropSetsPtr.ToInt64(); - var propSet = *firstPropSetPtr; - if (propSet.cProperties == 0 || propSet.rgProperties == IntPtr.Zero) - { - return null; - } - - var propPtr = (DBPROP*)propSet.rgProperties.ToInt64(); - return *propPtr; - } - catch (Exception ex) - { - Logger.LogError($"Exception occurred while getting properties,", ex); - return null; - } - finally - { - // Free the property sets pointer returned by GetProperties, if necessary - if (prgPropSetsPtr != IntPtr.Zero) - { - Marshal.FreeCoTaskMem(prgPropSetsPtr); - } - } - } - - private uint GetReuseWhereId(IRowset rowset) - { - var rowsetInfo = (IRowsetInfo)rowset; - - if (rowsetInfo is null) - { - return 0; - } - - var prop = GetPropset(rowsetInfo); - if (prop is null) - { - return 0; - } - - if (prop?.vValue.VarType == VarEnum.VT_UI4) - { - var value = prop?.vValue._ulong; - return (uint)value; - } - - return 0; - } - public void Dispose() { CancelOutstandingQueries(); - - // Free the allocated memory for rgPropertyIDs - if (dbPropIdSet.rgPropertyIDs != IntPtr.Zero) - { - Marshal.FreeCoTaskMem(dbPropIdSet.rgPropertyIDs); - } - - queryCompletedEvent?.Dispose(); } + + internal enum QueryState + { + NotStarted = 0, + Running, + Completed, + NoResults, + AllNoise, + NullDataSource, + CreateSessionFailed, + CreateCommandFailed, + ExecuteFailed, + Cancelled, + } + + private readonly record struct ExecuteCommandResult( + IRowset Rowset, + QueryState State, + int? HResult, + string ErrorMessage); } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Indexer/Utils/QueryStringBuilder.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Indexer/Utils/QueryStringBuilder.cs index 068ea08750..52b130ee68 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Indexer/Utils/QueryStringBuilder.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Indexer/Utils/QueryStringBuilder.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System; -using System.Globalization; using System.Runtime.CompilerServices; using ManagedCommon; using ManagedCsWin32; @@ -11,20 +10,16 @@ using Microsoft.CmdPal.Ext.Indexer.Indexer.SystemSearch; namespace Microsoft.CmdPal.Ext.Indexer.Indexer.Utils; -internal sealed partial class QueryStringBuilder +internal static class QueryStringBuilder { private const string Properties = "System.ItemUrl, System.ItemNameDisplay, path, System.Search.EntryID, System.Kind, System.KindText"; private const string SystemIndex = "SystemIndex"; private const string ScopeFileConditions = "SCOPE='file:'"; private const string OrderConditions = "System.DateModified DESC"; - private const string SelectQueryWithScope = "SELECT " + Properties + " FROM " + SystemIndex + " WHERE (" + ScopeFileConditions + ")"; - private const string SelectQueryWithScopeAndOrderConditions = SelectQueryWithScope + " ORDER BY " + OrderConditions; private static ISearchQueryHelper queryHelper; - public static string GeneratePrimingQuery() => SelectQueryWithScopeAndOrderConditions; - - public static string GenerateQuery(string searchText, uint whereId) + public static string GenerateQuery(string searchText) { if (queryHelper is null) { @@ -40,7 +35,7 @@ internal sealed partial class QueryStringBuilder throw; } - ISearchCatalogManager catalogManager = searchManager.GetCatalog(SystemIndex); + var catalogManager = searchManager.GetCatalog(SystemIndex); if (catalogManager is null) { throw new ArgumentException($"Failed to get catalog manager for {SystemIndex}"); @@ -57,7 +52,7 @@ internal sealed partial class QueryStringBuilder queryHelper.SetQuerySorting(OrderConditions); } - queryHelper.SetQueryWhereRestrictions("AND " + ScopeFileConditions + "AND ReuseWhere(" + whereId.ToString(CultureInfo.InvariantCulture) + ")"); + queryHelper.SetQueryWhereRestrictions($"AND {ScopeFileConditions}"); return queryHelper.GenerateSQLFromUserQuery(searchText); } } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Pages/IndexerPage.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Pages/IndexerPage.cs index 41abc0b018..f355db27bc 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Pages/IndexerPage.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Pages/IndexerPage.cs @@ -2,10 +2,13 @@ // 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 System.Collections.Generic; using System.Globalization; using System.Text.Encodings.Web; +using System.Threading; using System.Threading.Tasks; using Microsoft.CmdPal.Ext.Indexer.Indexer; using Microsoft.CmdPal.Ext.Indexer.Properties; @@ -16,18 +19,25 @@ namespace Microsoft.CmdPal.Ext.Indexer; internal sealed partial class IndexerPage : DynamicListPage, IDisposable { + // Cookie to identify our queries; since we replace the SearchEngine on each search, + // this can be a constant. + private const uint HardQueryCookie = 10; + private readonly List _indexerListItems = []; - private readonly SearchEngine _searchEngine; - private readonly bool disposeSearchEngine = true; + private readonly Lock _searchLock = new(); - private uint _queryCookie; - - private string initialQuery = string.Empty; + private SearchEngine? _searchEngine; + private CancellationTokenSource? _searchCts; + private string _initialQuery = string.Empty; private bool _isEmptyQuery = true; - private CommandItem _noSearchEmptyContent; - private CommandItem _nothingFoundEmptyContent; + private CommandItem? _noSearchEmptyContent; + private CommandItem? _nothingFoundEmptyContent; + + private bool _deferredLoad; + + public override ICommandItem EmptyContent => _isEmptyQuery ? _noSearchEmptyContent! : _nothingFoundEmptyContent!; public IndexerPage() { @@ -35,8 +45,8 @@ internal sealed partial class IndexerPage : DynamicListPage, IDisposable Icon = Icons.FileExplorerIcon; Name = Resources.Indexer_Title; PlaceholderText = Resources.Indexer_PlaceholderText; + _searchEngine = new(); - _queryCookie = 10; var filters = new SearchFilters(); filters.PropChanged += Filters_PropChanged; @@ -45,22 +55,23 @@ internal sealed partial class IndexerPage : DynamicListPage, IDisposable CreateEmptyContent(); } - public IndexerPage(string query, SearchEngine searchEngine, uint queryCookie, IList firstPageData) + public IndexerPage(string query) { Icon = Icons.FileExplorerIcon; Name = Resources.Indexer_Title; - _searchEngine = searchEngine; - _queryCookie = queryCookie; - _indexerListItems.AddRange(firstPageData); - initialQuery = query; + + _searchEngine = new(); + + _initialQuery = query; SearchText = query; - disposeSearchEngine = false; var filters = new SearchFilters(); filters.PropChanged += Filters_PropChanged; Filters = filters; CreateEmptyContent(); + IsLoading = true; + _deferredLoad = true; } private void CreateEmptyContent() @@ -95,8 +106,6 @@ internal sealed partial class IndexerPage : DynamicListPage, IDisposable ShellHelpers.OpenInShell(command); } - public override ICommandItem EmptyContent => _isEmptyQuery ? _noSearchEmptyContent : _nothingFoundEmptyContent; - private void Filters_PropChanged(object sender, IPropChangedEventArgs args) { PerformSearch(SearchText); @@ -104,35 +113,31 @@ internal sealed partial class IndexerPage : DynamicListPage, IDisposable public override void UpdateSearchText(string oldSearch, string newSearch) { - if (oldSearch != newSearch && newSearch != initialQuery) + if (oldSearch != newSearch && newSearch != _initialQuery) { PerformSearch(newSearch); } } - private void PerformSearch(string newSearch) + public override IListItem[] GetItems() { - var actualSearch = FullSearchString(newSearch); - _ = Task.Run(() => + if (_deferredLoad) { - _isEmptyQuery = string.IsNullOrWhiteSpace(actualSearch); - Query(actualSearch); - LoadMore(); - OnPropertyChanged(nameof(EmptyContent)); - initialQuery = null; - }); - } + PerformSearch(_initialQuery); + _deferredLoad = false; + } - public override IListItem[] GetItems() => [.. _indexerListItems]; + return [.. _indexerListItems]; + } private string FullSearchString(string query) { - switch (Filters.CurrentFilterId) + switch (Filters?.CurrentFilterId) { case "folders": - return $"{query} kind:folders"; + return $"System.Kind:folders {query}"; case "files": - return $"{query} kind:NOT folders"; + return $"System.Kind:NOT folders {query}"; case "all": default: return query; @@ -141,28 +146,139 @@ internal sealed partial class IndexerPage : DynamicListPage, IDisposable public override void LoadMore() { + var ct = Volatile.Read(ref _searchCts)?.Token; + IsLoading = true; - var results = _searchEngine.FetchItems(_indexerListItems.Count, 20, _queryCookie, out var hasMore); - _indexerListItems.AddRange(results); - HasMoreItems = hasMore; - IsLoading = false; - RaiseItemsChanged(_indexerListItems.Count); + + var hasMore = false; + SearchEngine? searchEngine; + int offset; + + lock (_searchLock) + { + searchEngine = _searchEngine; + offset = _indexerListItems.Count; + } + + var results = searchEngine?.FetchItems(offset, 20, queryCookie: HardQueryCookie, out hasMore) ?? []; + + if (ct?.IsCancellationRequested == true) + { + IsLoading = false; + return; + } + + lock (_searchLock) + { + if (ct?.IsCancellationRequested == true) + { + IsLoading = false; + return; + } + + _indexerListItems.AddRange(results); + HasMoreItems = hasMore; + IsLoading = false; + RaiseItemsChanged(_indexerListItems.Count); + } } private void Query(string query) { - ++_queryCookie; - _indexerListItems.Clear(); + lock (_searchLock) + { + _indexerListItems.Clear(); + _searchEngine?.Query(query, queryCookie: HardQueryCookie); + } + } - _searchEngine.Query(query, _queryCookie); + private void ReplaceSearchEngine(SearchEngine newSearchEngine) + { + SearchEngine? oldEngine; + + lock (_searchLock) + { + oldEngine = _searchEngine; + _searchEngine = newSearchEngine; + } + + oldEngine?.Dispose(); + } + + private void PerformSearch(string newSearch) + { + var actualSearch = FullSearchString(newSearch); + + var newCts = new CancellationTokenSource(); + var oldCts = Interlocked.Exchange(ref _searchCts, newCts); + oldCts?.Cancel(); + oldCts?.Dispose(); + + var ct = newCts.Token; + + _ = Task.Run( + () => + { + ct.ThrowIfCancellationRequested(); + + lock (_searchLock) + { + // If the user hasn't provided any base query text, results should be empty + // regardless of the currently selected filter. + _isEmptyQuery = string.IsNullOrWhiteSpace(newSearch); + + if (_isEmptyQuery) + { + _indexerListItems.Clear(); + HasMoreItems = false; + IsLoading = false; + RaiseItemsChanged(0); + OnPropertyChanged(nameof(EmptyContent)); + _initialQuery = string.Empty; + return; + } + + // Track the most recent query we initiated, so UpdateSearchText doesn't + // spuriously suppress a search when SearchText gets set programmatically. + _initialQuery = newSearch; + } + + ct.ThrowIfCancellationRequested(); + ReplaceSearchEngine(new SearchEngine()); + + ct.ThrowIfCancellationRequested(); + Query(actualSearch); + + ct.ThrowIfCancellationRequested(); + LoadMore(); + + ct.ThrowIfCancellationRequested(); + + lock (_searchLock) + { + OnPropertyChanged(nameof(EmptyContent)); + } + }, + ct); } public void Dispose() { - if (disposeSearchEngine) + var cts = Interlocked.Exchange(ref _searchCts, null); + cts?.Cancel(); + cts?.Dispose(); + + SearchEngine? searchEngine; + + lock (_searchLock) { - _searchEngine.Dispose(); - GC.SuppressFinalize(this); + searchEngine = _searchEngine; + _searchEngine = null; + _indexerListItems.Clear(); } + + searchEngine?.Dispose(); + + GC.SuppressFinalize(this); } } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Properties/Resources.Designer.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Properties/Resources.Designer.cs index 469c5faf61..1dfde65a28 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Properties/Resources.Designer.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Properties/Resources.Designer.cs @@ -19,7 +19,7 @@ namespace Microsoft.CmdPal.Ext.Indexer.Properties { // class via a tool like ResGen or Visual Studio. // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "18.0.0.0")] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class Resources { @@ -177,6 +177,15 @@ namespace Microsoft.CmdPal.Ext.Indexer.Properties { } } + /// + /// Looks up a localized string similar to The query matches multiple items. + /// + internal static string Indexer_Fallback_MultipleResults_Subtitle { + get { + return ResourceManager.GetString("Indexer_Fallback_MultipleResults_Subtitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to Search for "{0}" in files. /// diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Properties/Resources.resx b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Properties/Resources.resx index 8f5f760137..6c7e6483c9 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Properties/Resources.resx +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/Properties/Resources.resx @@ -211,4 +211,7 @@ You can try searching all files on this PC or adjust your indexing settings. Failed to launch Peek + + The query matches multiple items + \ No newline at end of file diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/SearchEngine.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/SearchEngine.cs index eb1ca563b4..15fff442c1 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/SearchEngine.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Indexer/SearchEngine.cs @@ -2,6 +2,8 @@ // 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 System.Collections.Generic; using System.Diagnostics; @@ -16,15 +18,20 @@ namespace Microsoft.CmdPal.Ext.Indexer; public sealed partial class SearchEngine : IDisposable { - private SearchQuery _searchQuery = new(); + private SearchQuery? _searchQuery = new(); public void Query(string query, uint queryCookie) { - // _indexerListItems.Clear(); - _searchQuery.SearchResults.Clear(); - _searchQuery.CancelOutstandingQueries(); + var searchQuery = _searchQuery; + if (searchQuery is null) + { + return; + } - if (query == string.Empty) + searchQuery.SearchResults.Clear(); + searchQuery.CancelOutstandingQueries(); + + if (string.IsNullOrWhiteSpace(query)) { return; } @@ -32,64 +39,74 @@ public sealed partial class SearchEngine : IDisposable Stopwatch stopwatch = new(); stopwatch.Start(); - _searchQuery.Execute(query, queryCookie); + searchQuery.Execute(query, queryCookie); stopwatch.Stop(); Logger.LogDebug($"Query time: {stopwatch.ElapsedMilliseconds} ms, query: \"{query}\""); } - public IList FetchItems(int offset, int limit, uint queryCookie, out bool hasMore) + public IList FetchItems(int offset, int limit, uint queryCookie, out bool hasMore, bool noIcons = false) { hasMore = false; - var results = new List(); - if (_searchQuery is not null) + + var searchQuery = _searchQuery; + if (searchQuery is null) { - var cookie = _searchQuery.Cookie; - if (cookie == queryCookie) - { - var index = 0; - SearchResult result; - - // var hasMoreItems = _searchQuery.FetchRows(_indexerListItems.Count, limit); - var hasMoreItems = _searchQuery.FetchRows(offset, limit); - - while (!_searchQuery.SearchResults.IsEmpty && _searchQuery.SearchResults.TryDequeue(out result) && ++index <= limit) - { - IconInfo icon = null; - try - { - var stream = ThumbnailHelper.GetThumbnail(result.LaunchUri).Result; - if (stream is not null) - { - var data = new IconData(RandomAccessStreamReference.CreateFromStream(stream)); - icon = new IconInfo(data, data); - } - } - catch (Exception ex) - { - Logger.LogError("Failed to get the icon.", ex); - } - - results.Add(new IndexerListItem(new IndexerItem - { - FileName = result.ItemDisplayName, - FullPath = result.LaunchUri, - }) - { - Icon = icon, - }); - } - - hasMore = hasMoreItems; - } + return []; } + var cookie = searchQuery.Cookie; + if (cookie != queryCookie) + { + return []; + } + + var results = new List(); + var index = 0; + var hasMoreItems = searchQuery.FetchRows(offset, limit); + + while (!searchQuery.SearchResults.IsEmpty && searchQuery.SearchResults.TryDequeue(out var result) && ++index <= limit) + { + var indexerListItem = new IndexerListItem(new IndexerItem + { + FileName = result.ItemDisplayName, + FullPath = result.LaunchUri, + }); + + if (!noIcons) + { + IconInfo? icon = null; + try + { + var stream = ThumbnailHelper.GetThumbnail(result.LaunchUri).Result; + if (stream is not null) + { + var data = new IconData(RandomAccessStreamReference.CreateFromStream(stream)); + icon = new IconInfo(data, data); + } + } + catch (Exception ex) + { + Logger.LogError("Failed to get the icon.", ex); + } + + indexerListItem.Icon = icon; + } + + results.Add(indexerListItem); + } + + hasMore = hasMoreItems; return results; } public void Dispose() { + var searchQuery = _searchQuery; _searchQuery = null; + + searchQuery?.Dispose(); + GC.SuppressFinalize(this); } } From 18c6d6b0f30a3ab2964737b1e3b0339ed30acd95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Mon, 2 Feb 2026 18:53:40 +0100 Subject: [PATCH 5/7] CmdPal: Improve loading of application icons (uwp and jumbo icons) - part 2 (#44973) ## Summary of the Pull Request This PR improves icons for app items: - Refactors icon detection and selection from the AppX manifest out of `UWPApplication` - Prefer *unplated* UWP app logos so icons no longer appear smaller than expected - Adds an icon loader based on `IShellItemImageFactory` to correctly load large icons - Jumbo icons loaded from shortcuts are now crisp - Jumbo icons loaded from shortcuts are no longer scaled down - Refactors detail loading in `AppListItem` to prevent potential deadlocks - Makes PWA icons more crisp - Fixes fallback item (now it gets used not only when the icon is null, but also when it's empty).
Old New
1 image image
2 image image
3 image image
4 image image
5 image image
6 image image
## PR Checklist - [x] Closes: #44970 - [x] Closes: #43320 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed --- .github/actions/spell-check/expect.txt | 4 + .../ext/Microsoft.CmdPal.Ext.Apps/AppItem.cs | 2 + .../Microsoft.CmdPal.Ext.Apps/AppListItem.cs | 83 +++- .../Helpers/AppxIconLoader.cs | 295 +++++++++++++ .../Helpers/IconExtractor.cs | 132 ++++++ .../Helpers/IconSearchResult.cs | 44 ++ .../NativeMethods.txt | 6 + .../Programs/UWPApplication.cs | 406 +++--------------- 8 files changed, 601 insertions(+), 371 deletions(-) create mode 100644 src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/AppxIconLoader.cs create mode 100644 src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/IconExtractor.cs create mode 100644 src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/IconSearchResult.cs diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index 9d6188fa24..1f450a10e6 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -130,6 +130,7 @@ bezelled bhid BIF bigbar +BIGGERSIZEOK bigobj binlog binres @@ -311,6 +312,7 @@ CRECT CRH critsec cropandlock +CROPTOSQUARE Crossdevice csdevkit CSearch @@ -761,6 +763,7 @@ IAI icf ICONERROR ICONLOCATION +ICONONLY IDCANCEL IDD idk @@ -1672,6 +1675,7 @@ sigdn Signedness SIGNINGSCENARIO signtool +SIIGBF SINGLEKEY sipolicy SIZEBOX diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AppItem.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AppItem.cs index 14f9597418..7b111c922b 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AppItem.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AppItem.cs @@ -33,6 +33,8 @@ public sealed class AppItem public string? FullExecutablePath { get; set; } + public string? JumboIconPath { get; set; } + public AppItem() { } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AppListItem.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AppListItem.cs index d907277ddc..8d1a05d641 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AppListItem.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AppListItem.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using ManagedCommon; using Microsoft.CmdPal.Core.Common.Helpers; using Microsoft.CmdPal.Ext.Apps.Commands; +using Microsoft.CmdPal.Ext.Apps.Helpers; using Microsoft.CommandPalette.Extensions; using Microsoft.CommandPalette.Extensions.Toolkit; @@ -17,6 +18,7 @@ public sealed partial class AppListItem : ListItem { private readonly AppCommand _appCommand; private readonly AppItem _app; + private readonly Lazy> _iconLoadTask; private readonly Lazy> _detailsLoadTask; @@ -66,7 +68,7 @@ public sealed partial class AppListItem : ListItem MoreCommands = AddPinCommands(_app.Commands!, isPinned); _detailsLoadTask = new Lazy>(BuildDetails); - _iconLoadTask = new Lazy>(async () => await FetchIcon(useThumbnails)); + _iconLoadTask = new Lazy>(async () => await FetchIcon(useThumbnails).ConfigureAwait(false)); } private async Task LoadDetailsAsync() @@ -85,7 +87,7 @@ public sealed partial class AppListItem : ListItem { try { - Icon = _appCommand.Icon = await _iconLoadTask.Value ?? Icons.GenericAppIcon; + Icon = _appCommand.Icon = CoalesceIcon(await _iconLoadTask.Value); } catch (Exception ex) { @@ -93,6 +95,21 @@ public sealed partial class AppListItem : ListItem } } + private static IconInfo CoalesceIcon(IconInfo? value) + { + return CoalesceIcon(value, Icons.GenericAppIcon)!; + } + + private static IconInfo? CoalesceIcon(IconInfo? value, IconInfo? replacement) + { + return IconIsNullOrEmpty(value) ? replacement : value; + } + + private static bool IconIsNullOrEmpty(IconInfo? value) + { + return value == null || (string.IsNullOrEmpty(value.Light?.Icon) && value.Light?.Data is null) || (string.IsNullOrEmpty(value.Dark?.Icon) && value.Dark?.Data is null); + } + private async Task
BuildDetails() { // Build metadata, with app type, path, etc. @@ -107,24 +124,49 @@ public sealed partial class AppListItem : ListItem metadata.Add(new DetailsElement() { Key = "[DEBUG] AppIdentifier", Data = new DetailsLink() { Text = _app.AppIdentifier } }); metadata.Add(new DetailsElement() { Key = "[DEBUG] ExePath", Data = new DetailsLink() { Text = _app.ExePath } }); metadata.Add(new DetailsElement() { Key = "[DEBUG] IcoPath", Data = new DetailsLink() { Text = _app.IcoPath } }); + metadata.Add(new DetailsElement() { Key = "[DEBUG] JumboIconPath", Data = new DetailsLink() { Text = _app.JumboIconPath ?? "(null)" } }); #endif // Icon IconInfo? heroImage = null; if (_app.IsPackaged) { - heroImage = new IconInfo(_app.IcoPath); + heroImage = new IconInfo(_app.JumboIconPath ?? _app.IcoPath); } else { + // Get the icon from the system + if (!string.IsNullOrEmpty(_app.JumboIconPath)) + { + var randomAccessStream = await IconExtractor.GetIconStreamAsync(_app.JumboIconPath, 64); + if (randomAccessStream != null) + { + heroImage = IconInfo.FromStream(randomAccessStream); + } + } + + if (IconIsNullOrEmpty(heroImage) && !string.IsNullOrEmpty(_app.IcoPath)) + { + var randomAccessStream = await IconExtractor.GetIconStreamAsync(_app.IcoPath, 64); + if (randomAccessStream != null) + { + heroImage = IconInfo.FromStream(randomAccessStream); + } + } + // do nothing if we fail to load an icon. // Logging it would be too NOISY, there's really no need. - if (!string.IsNullOrEmpty(_app.IcoPath)) + if (IconIsNullOrEmpty(heroImage) && !string.IsNullOrEmpty(_app.JumboIconPath)) + { + heroImage = await TryLoadThumbnail(_app.JumboIconPath, jumbo: true, logOnFailure: false); + } + + if (IconIsNullOrEmpty(heroImage) && !string.IsNullOrEmpty(_app.IcoPath)) { heroImage = await TryLoadThumbnail(_app.IcoPath, jumbo: true, logOnFailure: false); } - if (heroImage == null && !string.IsNullOrEmpty(_app.ExePath)) + if (IconIsNullOrEmpty(heroImage) && !string.IsNullOrEmpty(_app.ExePath)) { heroImage = await TryLoadThumbnail(_app.ExePath, jumbo: true, logOnFailure: false); } @@ -133,8 +175,8 @@ public sealed partial class AppListItem : ListItem return new Details() { Title = this.Title, - HeroImage = heroImage ?? this.Icon ?? Icons.GenericAppIcon, - Metadata = metadata.ToArray(), + HeroImage = CoalesceIcon(CoalesceIcon(heroImage, this.Icon as IconInfo)), + Metadata = [..metadata], }; } @@ -154,7 +196,7 @@ public sealed partial class AppListItem : ListItem icon = await TryLoadThumbnail(_app.IcoPath, jumbo: false, logOnFailure: true); } - if (icon == null && !string.IsNullOrEmpty(_app.ExePath)) + if (IconIsNullOrEmpty(icon) && !string.IsNullOrEmpty(_app.ExePath)) { icon = await TryLoadThumbnail(_app.ExePath, jumbo: false, logOnFailure: true); } @@ -196,22 +238,25 @@ public sealed partial class AppListItem : ListItem private async Task TryLoadThumbnail(string path, bool jumbo, bool logOnFailure) { - try + return await Task.Run(async () => { - var stream = await ThumbnailHelper.GetThumbnail(path, jumbo); - if (stream is not null) + try { - return IconInfo.FromStream(stream); + var stream = await ThumbnailHelper.GetThumbnail(path, jumbo).ConfigureAwait(false); + if (stream is not null) + { + return IconInfo.FromStream(stream); + } } - } - catch (Exception ex) - { - if (logOnFailure) + catch (Exception ex) { - Logger.LogDebug($"Failed to load icon {path} for {AppIdentifier}:\n{ex}"); + if (logOnFailure) + { + Logger.LogDebug($"Failed to load icon {path} for {AppIdentifier}:\n{ex}"); + } } - } - return null; + return null; + }).ConfigureAwait(false); } } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/AppxIconLoader.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/AppxIconLoader.cs new file mode 100644 index 0000000000..589cff5214 --- /dev/null +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/AppxIconLoader.cs @@ -0,0 +1,295 @@ +// 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 Microsoft.CmdPal.Ext.Apps.Programs; +using Microsoft.CmdPal.Ext.Apps.Utils; + +namespace Microsoft.CmdPal.Ext.Apps.Helpers; + +internal static class AppxIconLoader +{ + private const string ContrastWhite = "contrast-white"; + private const string ContrastBlack = "contrast-black"; + + private static readonly Dictionary> _scaleFactors = new() + { + { UWP.PackageVersion.Windows10, [100, 125, 150, 200, 400] }, + { UWP.PackageVersion.Windows81, [100, 120, 140, 160, 180] }, + { UWP.PackageVersion.Windows8, [100] }, + }; + + private static readonly List TargetSizes = [16, 24, 30, 36, 44, 60, 72, 96, 128, 180, 256]; + + private static IconSearchResult GetScaleIcons( + string path, + string colorscheme, + UWP.PackageVersion packageVersion, + bool highContrast = false) + { + var extension = Path.GetExtension(path); + if (extension is null) + { + return IconSearchResult.NotFound(); + } + + var end = path.Length - extension.Length; + var prefix = path[..end]; + + if (!_scaleFactors.TryGetValue(packageVersion, out var factors)) + { + return IconSearchResult.NotFound(); + } + + var logoType = highContrast ? LogoType.HighContrast : LogoType.Colored; + + // Check from highest scale factor to lowest for best quality + for (var i = factors.Count - 1; i >= 0; i--) + { + var factor = factors[i]; + string[] pathsToTry = highContrast + ? + [ + $"{prefix}.scale-{factor}_{colorscheme}{extension}", + $"{prefix}.{colorscheme}_scale-{factor}{extension}", + ] + : + [ + $"{prefix}.scale-{factor}{extension}", + ]; + + foreach (var p in pathsToTry) + { + if (File.Exists(p)) + { + return IconSearchResult.FoundScaled(p, logoType); + } + } + } + + // Check base path (100% scale) as last resort + if (!highContrast && File.Exists(path)) + { + return IconSearchResult.FoundScaled(path, logoType); + } + + return IconSearchResult.NotFound(); + } + + private static IconSearchResult GetTargetSizeIcon( + string path, + string colorscheme, + bool highContrast = false, + int appIconSize = 36, + double maxSizeCoefficient = 8.0) + { + var extension = Path.GetExtension(path); + if (extension is null) + { + return IconSearchResult.NotFound(); + } + + var end = path.Length - extension.Length; + var prefix = path[..end]; + var pathSizePairs = new List<(string Path, int Size)>(); + + foreach (var size in TargetSizes) + { + if (highContrast) + { + pathSizePairs.Add(($"{prefix}.targetsize-{size}_{colorscheme}{extension}", size)); + pathSizePairs.Add(($"{prefix}.{colorscheme}_targetsize-{size}{extension}", size)); + } + else + { + pathSizePairs.Add(($"{prefix}.targetsize-{size}_altform-unplated{extension}", size)); + pathSizePairs.Add(($"{prefix}.targetsize-{size}{extension}", size)); + } + } + + var maxAllowedSize = (int)(appIconSize * maxSizeCoefficient); + var logoType = highContrast ? LogoType.HighContrast : LogoType.Colored; + + string? bestLargerPath = null; + var bestLargerSize = int.MaxValue; + + string? bestSmallerPath = null; + var bestSmallerSize = 0; + + foreach (var (p, size) in pathSizePairs) + { + if (!File.Exists(p)) + { + continue; + } + + if (size >= appIconSize && size <= maxAllowedSize) + { + if (size < bestLargerSize) + { + bestLargerSize = size; + bestLargerPath = p; + } + } + else if (size < appIconSize) + { + if (size > bestSmallerSize) + { + bestSmallerSize = size; + bestSmallerPath = p; + } + } + } + + if (bestLargerPath is not null) + { + return IconSearchResult.FoundTargetSize(bestLargerPath, logoType, bestLargerSize); + } + + if (bestSmallerPath is not null) + { + return IconSearchResult.FoundTargetSize(bestSmallerPath, logoType, bestSmallerSize); + } + + return IconSearchResult.NotFound(); + } + + private static IconSearchResult GetColoredIcon( + string path, + string colorscheme, + int iconSize, + UWP package) + { + // First priority: targetsize icons (we know the exact size) + var targetResult = GetTargetSizeIcon(path, colorscheme, highContrast: false, appIconSize: iconSize); + if (targetResult.MeetsMinimumSize(iconSize)) + { + return targetResult; + } + + var hcTargetResult = GetTargetSizeIcon(path, colorscheme, highContrast: true, appIconSize: iconSize); + if (hcTargetResult.MeetsMinimumSize(iconSize)) + { + return hcTargetResult; + } + + // Second priority: scale icons (size unknown, but higher scale = likely better) + var scaleResult = GetScaleIcons(path, colorscheme, package.Version, highContrast: false); + if (scaleResult.IsFound) + { + return scaleResult; + } + + var hcScaleResult = GetScaleIcons(path, colorscheme, package.Version, highContrast: true); + if (hcScaleResult.IsFound) + { + return hcScaleResult; + } + + // Last resort: return undersized targetsize if we found one + if (targetResult.IsFound) + { + return targetResult; + } + + if (hcTargetResult.IsFound) + { + return hcTargetResult; + } + + return IconSearchResult.NotFound(); + } + + private static IconSearchResult SetHighContrastIcon( + string path, + string colorscheme, + int iconSize, + UWP package) + { + // First priority: HC targetsize icons (we know the exact size) + var hcTargetResult = GetTargetSizeIcon(path, colorscheme, highContrast: true, appIconSize: iconSize); + if (hcTargetResult.MeetsMinimumSize(iconSize)) + { + return hcTargetResult; + } + + var targetResult = GetTargetSizeIcon(path, colorscheme, highContrast: false, appIconSize: iconSize); + if (targetResult.MeetsMinimumSize(iconSize)) + { + return targetResult; + } + + // Second priority: scale icons + var hcScaleResult = GetScaleIcons(path, colorscheme, package.Version, highContrast: true); + if (hcScaleResult.IsFound) + { + return hcScaleResult; + } + + var scaleResult = GetScaleIcons(path, colorscheme, package.Version, highContrast: false); + if (scaleResult.IsFound) + { + return scaleResult; + } + + // Last resort: undersized targetsize + if (hcTargetResult.IsFound) + { + return hcTargetResult; + } + + if (targetResult.IsFound) + { + return targetResult; + } + + return IconSearchResult.NotFound(); + } + + /// + /// Loads an icon from a UWP package, attempting to find the best match for the requested size. + /// + /// The relative URI to the logo asset. + /// The current theme. + /// The requested icon size in pixels. + /// The UWP package. + /// + /// An IconSearchResult. Use to check if + /// the icon is confirmed to be large enough, or + /// to determine if the size is known. + /// + internal static IconSearchResult LogoPathFromUri( + string uri, + Theme theme, + int iconSize, + UWP package) + { + var path = Path.Combine(package.Location, uri); + var logo = Probe(theme, path, iconSize, package); + if (!logo.IsFound && !uri.Contains('\\', StringComparison.Ordinal)) + { + path = Path.Combine(package.Location, "Assets", uri); + logo = Probe(theme, path, iconSize, package); + } + + return logo; + } + + private static IconSearchResult Probe(Theme theme, string path, int iconSize, UWP package) + { + return theme switch + { + Theme.HighContrastBlack or Theme.HighContrastOne or Theme.HighContrastTwo + => SetHighContrastIcon(path, ContrastBlack, iconSize, package), + Theme.HighContrastWhite + => SetHighContrastIcon(path, ContrastWhite, iconSize, package), + Theme.Light + => GetColoredIcon(path, ContrastWhite, iconSize, package), + _ + => GetColoredIcon(path, ContrastBlack, iconSize, package), + }; + } +} diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/IconExtractor.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/IconExtractor.cs new file mode 100644 index 0000000000..c1d04e286c --- /dev/null +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/IconExtractor.cs @@ -0,0 +1,132 @@ +// 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.Runtime.InteropServices.WindowsRuntime; +using System.Threading.Tasks; +using ManagedCommon; +using Windows.Graphics.Imaging; +using Windows.Storage.Streams; +using Windows.Win32; +using Windows.Win32.Foundation; +using Windows.Win32.Graphics.Gdi; +using Windows.Win32.UI.Shell; + +namespace Microsoft.CmdPal.Ext.Apps.Helpers; + +internal static class IconExtractor +{ + public static async Task GetIconStreamAsync(string path, int size) + { + var bitmap = GetIcon(path, size); + if (bitmap == null) + { + return null; + } + + var stream = new InMemoryRandomAccessStream(); + var encoder = await BitmapEncoder.CreateAsync(BitmapEncoder.PngEncoderId, stream); + encoder.SetSoftwareBitmap(bitmap); + await encoder.FlushAsync(); + + stream.Seek(0); + return stream; + } + + public static unsafe SoftwareBitmap? GetIcon(string path, int size) + { + IShellItemImageFactory* factory = null; + HBITMAP hBitmap = default; + + try + { + fixed (char* pPath = path) + { + var iid = IShellItemImageFactory.IID_Guid; + var hr = PInvoke.SHCreateItemFromParsingName( + pPath, + null, + &iid, + (void**)&factory); + + if (hr.Failed || factory == null) + { + return null; + } + } + + var requestedSize = new SIZE { cx = size, cy = size }; + var hr2 = factory->GetImage( + requestedSize, + SIIGBF.SIIGBF_ICONONLY | SIIGBF.SIIGBF_BIGGERSIZEOK | SIIGBF.SIIGBF_CROPTOSQUARE, + &hBitmap); + + if (hr2.Failed || hBitmap.IsNull) + { + return null; + } + + return CreateSoftwareBitmap(hBitmap, size); + } + catch (Exception ex) + { + Logger.LogError($"Failed to load icon from path='{path}',size={size}", ex); + return null; + } + finally + { + if (!hBitmap.IsNull) + { + PInvoke.DeleteObject(hBitmap); + } + + if (factory != null) + { + factory->Release(); + } + } + } + + private static unsafe SoftwareBitmap CreateSoftwareBitmap(HBITMAP hBitmap, int size) + { + var pixels = new byte[size * size * 4]; + + var bmi = new BITMAPINFO + { + bmiHeader = new BITMAPINFOHEADER + { + biSize = (uint)sizeof(BITMAPINFOHEADER), + biWidth = size, + biHeight = -size, + biPlanes = 1, + biBitCount = 32, + biCompression = 0, + }, + }; + + var hdc = PInvoke.GetDC(default); + try + { + fixed (byte* pPixels = pixels) + { + _ = PInvoke.GetDIBits( + hdc, + hBitmap, + 0, + (uint)size, + pPixels, + &bmi, + DIB_USAGE.DIB_RGB_COLORS); + } + } + finally + { + _ = PInvoke.ReleaseDC(default, hdc); + } + + var bitmap = new SoftwareBitmap(BitmapPixelFormat.Bgra8, size, size, BitmapAlphaMode.Premultiplied); + bitmap.CopyFromBuffer(pixels.AsBuffer()); + return bitmap; + } +} diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/IconSearchResult.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/IconSearchResult.cs new file mode 100644 index 0000000000..51c8a142cf --- /dev/null +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Helpers/IconSearchResult.cs @@ -0,0 +1,44 @@ +// 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 Microsoft.CmdPal.Ext.Apps.Programs; + +namespace Microsoft.CmdPal.Ext.Apps.Helpers; + +/// +/// Result of an icon search operation. +/// +internal readonly record struct IconSearchResult( + string? LogoPath, + LogoType LogoType, + bool IsTargetSizeIcon, + int? KnownSize = null) +{ + /// + /// Gets a value indicating whether an icon was found. + /// + public bool IsFound => LogoPath is not null; + + /// + /// Returns true if we can confirm the icon meets the minimum size. + /// Only possible for targetsize icons where the size is encoded in the filename. + /// + public bool MeetsMinimumSize(int minimumSize) => + IsTargetSizeIcon && KnownSize >= minimumSize; + + /// + /// Returns true if we know the icon is undersized. + /// Returns false if not found, or if size is unknown (scale-based icons). + /// + public bool IsKnownUndersized(int minimumSize) => + IsTargetSizeIcon && KnownSize < minimumSize; + + public static IconSearchResult NotFound() => new(null, default, false); + + public static IconSearchResult FoundTargetSize(string path, LogoType logoType, int size) + => new(path, logoType, IsTargetSizeIcon: true, size); + + public static IconSearchResult FoundScaled(string path, LogoType logoType) + => new(path, logoType, IsTargetSizeIcon: false); +} diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/NativeMethods.txt b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/NativeMethods.txt index 017871d42f..86138d3fb2 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/NativeMethods.txt +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/NativeMethods.txt @@ -18,3 +18,9 @@ ShellLink IPersistFile CoTaskMemFree IUnknown +IShellItemImageFactory +DeleteObject +GetDIBits +GetDC +ReleaseDC +SIIGBF diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Programs/UWPApplication.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Programs/UWPApplication.cs index 4ec9598483..91b08d3b86 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Programs/UWPApplication.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/Programs/UWPApplication.cs @@ -8,6 +8,7 @@ using System.IO.Abstractions; using System.Xml; using ManagedCommon; using Microsoft.CmdPal.Ext.Apps.Commands; +using Microsoft.CmdPal.Ext.Apps.Helpers; using Microsoft.CmdPal.Ext.Apps.Properties; using Microsoft.CmdPal.Ext.Apps.Utils; using Microsoft.CommandPalette.Extensions; @@ -23,8 +24,10 @@ namespace Microsoft.CmdPal.Ext.Apps.Programs; [Serializable] public class UWPApplication : IUWPApplication { + private const int ListIconSize = 20; + private const int JumboIconSize = 64; + private static readonly IFileSystem FileSystem = new FileSystem(); - private static readonly IPath Path = FileSystem.Path; private static readonly IFile File = FileSystem.File; public string AppListEntry { get; set; } = string.Empty; @@ -56,13 +59,15 @@ public class UWPApplication : IUWPApplication public LogoType LogoType { get; set; } + public string JumboLogoPath { get; set; } = string.Empty; + + public LogoType JumboLogoType { get; set; } + public UWP Package { get; set; } - private string logoUri; + private string _logoUri; - private const string ContrastWhite = "contrast-white"; - - private const string ContrastBlack = "contrast-black"; + private string _jumboLogoUri; // Function to set the subtitle based on the Type of application public static string Type() @@ -154,7 +159,8 @@ public class UWPApplication : IUWPApplication DisplayName = ResourceFromPri(package.FullName, DisplayName); Description = ResourceFromPri(package.FullName, Description); - logoUri = LogoUriFromManifest(manifestApp); + _logoUri = LogoUriFromManifest(manifestApp); + _jumboLogoUri = LogoUriFromManifest(manifestApp, jumbo: true); Enabled = true; CanRunElevated = IfApplicationCanRunElevated(); @@ -280,16 +286,24 @@ public class UWPApplication : IUWPApplication } } - private static readonly Dictionary _logoKeyFromVersion = new Dictionary + private static readonly Dictionary _smallLogoKeyFromVersion = new Dictionary { { PackageVersion.Windows10, "Square44x44Logo" }, { PackageVersion.Windows81, "Square30x30Logo" }, { PackageVersion.Windows8, "SmallLogo" }, }; - internal unsafe string LogoUriFromManifest(IAppxManifestApplication* app) + private static readonly Dictionary _largeLogoKeyFromVersion = new Dictionary { - if (_logoKeyFromVersion.TryGetValue(Package.Version, out var key)) + { PackageVersion.Windows10, "Square150x150Logo" }, + { PackageVersion.Windows81, "Square150x150Logo" }, + { PackageVersion.Windows8, "Logo" }, + }; + + internal unsafe string LogoUriFromManifest(IAppxManifestApplication* app, bool jumbo = false) + { + var logoMap = jumbo ? _largeLogoKeyFromVersion : _smallLogoKeyFromVersion; + if (logoMap.TryGetValue(Package.Version, out var key)) { var hr = app->GetStringValue(key, out var logoUriFromAppPtr); return ComFreeHelper.GetStringAndFree(hr, logoUriFromAppPtr); @@ -302,257 +316,55 @@ public class UWPApplication : IUWPApplication public void UpdateLogoPath(Theme theme) { - LogoPathFromUri(logoUri, theme); - } - - // scale factors on win10: https://learn.microsoft.com/windows/uwp/controls-and-patterns/tiles-and-notifications-app-assets#asset-size-tables, - private static readonly Dictionary> _scaleFactors = new Dictionary> + // Update small logo + var logo = AppxIconLoader.LogoPathFromUri(_logoUri, theme, ListIconSize, Package); + if (logo.IsFound) { - { PackageVersion.Windows10, new List { 100, 125, 150, 200, 400 } }, - { PackageVersion.Windows81, new List { 100, 120, 140, 160, 180 } }, - { PackageVersion.Windows8, new List { 100 } }, - }; - - private bool SetScaleIcons(string path, string colorscheme, bool highContrast = false) - { - var extension = Path.GetExtension(path); - if (extension is not null) - { - var end = path.Length - extension.Length; - var prefix = path.Substring(0, end); - var paths = new List { }; - - if (!highContrast) - { - paths.Add(path); - } - - if (_scaleFactors.TryGetValue(Package.Version, out var factors)) - { - foreach (var factor in factors) - { - if (highContrast) - { - paths.Add($"{prefix}.scale-{factor}_{colorscheme}{extension}"); - paths.Add($"{prefix}.{colorscheme}_scale-{factor}{extension}"); - } - else - { - paths.Add($"{prefix}.scale-{factor}{extension}"); - } - } - } - - // By working from the highest resolution to the lowest, we make - // sure that we use the highest quality possible icon for the app. - // - // FirstOrDefault would result in us using the 1x scaled icon - // always, which is usually too small for our needs. - for (var i = paths.Count - 1; i >= 0; i--) - { - if (File.Exists(paths[i])) - { - LogoPath = paths[i]; - if (highContrast) - { - LogoType = LogoType.HighContrast; - } - else - { - LogoType = LogoType.Colored; - } - - return true; - } - } - } - - return false; - } - - private bool SetTargetSizeIcon(string path, string colorscheme, bool highContrast = false) - { - var extension = Path.GetExtension(path); - if (extension is not null) - { - var end = path.Length - extension.Length; - var prefix = path.Substring(0, end); - var paths = new List { }; - const int appIconSize = 36; - var targetSizes = new List { 16, 24, 30, 36, 44, 60, 72, 96, 128, 180, 256 }; - var pathFactorPairs = new Dictionary(); - - foreach (var factor in targetSizes) - { - if (highContrast) - { - var suffixThemePath = $"{prefix}.targetsize-{factor}_{colorscheme}{extension}"; - var prefixThemePath = $"{prefix}.{colorscheme}_targetsize-{factor}{extension}"; - paths.Add(suffixThemePath); - paths.Add(prefixThemePath); - pathFactorPairs.Add(suffixThemePath, factor); - pathFactorPairs.Add(prefixThemePath, factor); - } - else - { - var simplePath = $"{prefix}.targetsize-{factor}{extension}"; - var altformUnPlatedPath = $"{prefix}.targetsize-{factor}_altform-unplated{extension}"; - paths.Add(simplePath); - paths.Add(altformUnPlatedPath); - pathFactorPairs.Add(simplePath, factor); - pathFactorPairs.Add(altformUnPlatedPath, factor); - } - } - - // Sort paths by distance to desired app icon size - var selectedIconPath = string.Empty; - var closestDistance = int.MaxValue; - - foreach (var p in paths) - { - if (File.Exists(p) && pathFactorPairs.TryGetValue(p, out var factor)) - { - var distance = Math.Abs(factor - appIconSize); - if (distance < closestDistance) - { - closestDistance = distance; - selectedIconPath = p; - } - } - } - - if (!string.IsNullOrEmpty(selectedIconPath)) - { - LogoPath = selectedIconPath; - if (highContrast) - { - LogoType = LogoType.HighContrast; - } - else - { - LogoType = LogoType.Colored; - } - - return true; - } - } - - return false; - } - - private bool SetColoredIcon(string path, string colorscheme) - { - var isSetColoredScaleIcon = SetScaleIcons(path, colorscheme); - if (isSetColoredScaleIcon) - { - return true; - } - - var isSetColoredTargetIcon = SetTargetSizeIcon(path, colorscheme); - if (isSetColoredTargetIcon) - { - return true; - } - - var isSetHighContrastScaleIcon = SetScaleIcons(path, colorscheme, true); - if (isSetHighContrastScaleIcon) - { - return true; - } - - var isSetHighContrastTargetIcon = SetTargetSizeIcon(path, colorscheme, true); - if (isSetHighContrastTargetIcon) - { - return true; - } - - return false; - } - - private bool SetHighContrastIcon(string path, string colorscheme) - { - var isSetHighContrastScaleIcon = SetScaleIcons(path, colorscheme, true); - if (isSetHighContrastScaleIcon) - { - return true; - } - - var isSetHighContrastTargetIcon = SetTargetSizeIcon(path, colorscheme, true); - if (isSetHighContrastTargetIcon) - { - return true; - } - - var isSetColoredScaleIcon = SetScaleIcons(path, colorscheme); - if (isSetColoredScaleIcon) - { - return true; - } - - var isSetColoredTargetIcon = SetTargetSizeIcon(path, colorscheme); - if (isSetColoredTargetIcon) - { - return true; - } - - return false; - } - - internal void LogoPathFromUri(string uri, Theme theme) - { - // all https://learn.microsoft.com/windows/uwp/controls-and-patterns/tiles-and-notifications-app-assets - // windows 10 https://msdn.microsoft.com/library/windows/apps/dn934817.aspx - // windows 8.1 https://msdn.microsoft.com/library/windows/apps/hh965372.aspx#target_size - // windows 8 https://msdn.microsoft.com/library/windows/apps/br211475.aspx - string path; - bool isLogoUriSet; - - // Using Ordinal since this is used internally with uri - if (uri.Contains('\\', StringComparison.Ordinal)) - { - path = Path.Combine(Package.Location, uri); + LogoPath = logo.LogoPath!; + LogoType = logo.LogoType; } else - { - // for C:\Windows\MiracastView, etc. - path = Path.Combine(Package.Location, "Assets", uri); - } - - switch (theme) - { - case Theme.HighContrastBlack: - case Theme.HighContrastOne: - case Theme.HighContrastTwo: - isLogoUriSet = SetHighContrastIcon(path, ContrastBlack); - break; - case Theme.HighContrastWhite: - isLogoUriSet = SetHighContrastIcon(path, ContrastWhite); - break; - case Theme.Light: - isLogoUriSet = SetColoredIcon(path, ContrastWhite); - break; - default: - isLogoUriSet = SetColoredIcon(path, ContrastBlack); - break; - } - - if (!isLogoUriSet) { LogoPath = string.Empty; LogoType = LogoType.Error; } + + // Jumbo logo ... small logo can actually provide better result + var jumboLogo = AppxIconLoader.LogoPathFromUri(_logoUri, theme, JumboIconSize, Package); + if (jumboLogo.IsFound) + { + JumboLogoPath = jumboLogo.LogoPath!; + JumboLogoType = jumboLogo.LogoType; + } + else + { + JumboLogoPath = string.Empty; + JumboLogoType = LogoType.Error; + } + + if (!jumboLogo.MeetsMinimumSize(JumboIconSize) || !jumboLogo.IsFound) + { + var jumboLogoAlt = AppxIconLoader.LogoPathFromUri(_jumboLogoUri, theme, JumboIconSize, Package); + if (jumboLogoAlt.IsFound) + { + JumboLogoPath = jumboLogoAlt.LogoPath!; + JumboLogoType = jumboLogoAlt.LogoType; + } + } } public AppItem ToAppItem() { var app = this; var iconPath = app.LogoType != LogoType.Error ? app.LogoPath : string.Empty; - var item = new AppItem() + var jumboIconPath = app.JumboLogoType != LogoType.Error ? app.JumboLogoPath : string.Empty; + var item = new AppItem { Name = app.Name, Subtitle = app.Description, Type = UWPApplication.Type(), IcoPath = iconPath, + JumboIconPath = jumboIconPath, DirPath = app.Location, UserModelId = app.UserModelId, IsPackaged = true, @@ -563,116 +375,6 @@ public class UWPApplication : IUWPApplication return item; } - /* - public ImageSource Logo() - { - if (LogoType == LogoType.Colored) - { - var logo = ImageFromPath(LogoPath); - var platedImage = PlatedImage(logo); - return platedImage; - } - else - { - return ImageFromPath(LogoPath); - } - } - - private const int _dpiScale100 = 96; - - private ImageSource PlatedImage(BitmapImage image) - { - if (!string.IsNullOrEmpty(BackgroundColor)) - { - string currentBackgroundColor; - if (BackgroundColor == "transparent") - { - // Using InvariantCulture since this is internal - currentBackgroundColor = SystemParameters.WindowGlassBrush.ToString(CultureInfo.InvariantCulture); - } - else - { - currentBackgroundColor = BackgroundColor; - } - - var padding = 8; - var width = image.Width + (2 * padding); - var height = image.Height + (2 * padding); - var x = 0; - var y = 0; - - var group = new DrawingGroup(); - var converted = ColorConverter.ConvertFromString(currentBackgroundColor); - if (converted is not null) - { - var color = (Color)converted; - var brush = new SolidColorBrush(color); - var pen = new Pen(brush, 1); - var backgroundArea = new Rect(0, 0, width, height); - var rectangleGeometry = new RectangleGeometry(backgroundArea, 8, 8); - var rectDrawing = new GeometryDrawing(brush, pen, rectangleGeometry); - group.Children.Add(rectDrawing); - - var imageArea = new Rect(x + padding, y + padding, image.Width, image.Height); - var imageDrawing = new ImageDrawing(image, imageArea); - group.Children.Add(imageDrawing); - - // http://stackoverflow.com/questions/6676072/get-system-drawing-bitmap-of-a-wpf-area-using-visualbrush - var visual = new DrawingVisual(); - var context = visual.RenderOpen(); - context.DrawDrawing(group); - context.Close(); - - var bitmap = new RenderTargetBitmap( - Convert.ToInt32(width), - Convert.ToInt32(height), - _dpiScale100, - _dpiScale100, - PixelFormats.Pbgra32); - - bitmap.Render(visual); - - return bitmap; - } - else - { - ProgramLogger.Exception($"Unable to convert background string {BackgroundColor} to color for {Package.Location}", new InvalidOperationException(), GetType(), Package.Location); - - return new BitmapImage(new Uri(Constant.ErrorIcon)); - } - } - else - { - // todo use windows theme as background - return image; - } - } - - private BitmapImage ImageFromPath(string path) - { - if (File.Exists(path)) - { - var memoryStream = new MemoryStream(); - using (var fileStream = File.OpenRead(path)) - { - fileStream.CopyTo(memoryStream); - memoryStream.Position = 0; - - var image = new BitmapImage(); - image.BeginInit(); - image.StreamSource = memoryStream; - image.EndInit(); - return image; - } - } - else - { - // ProgramLogger.Exception($"Unable to get logo for {UserModelId} from {path} and located in {Package.Location}", new FileNotFoundException(), GetType(), path); - return new BitmapImage(new Uri(ImageLoader.ErrorIconPath)); - } - } - */ - public override string ToString() { return $"{DisplayName}: {Description}"; From 49cc504d94f65651efeb81d2f324e4d46e3bb838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Mon, 2 Feb 2026 19:30:00 +0100 Subject: [PATCH 6/7] CmdPal: Improve fuzzy matcher Unicode and emoji robustness (#45275) ## Summary of the Pull Request Add comprehensive unit tests for emoji, ZWJ sequences, skin tone modifiers, and UTF-16 edge cases (unpaired surrogates, combining marks, random garbage). Update matcher logic to skip normalization of lone surrogates, preventing errors with malformed Unicode. Expand comparison test data to cover emoji scenarios. Adds regression guards for diacritic handling and surrogate processing. Fixes #45246 introduced in #44809. ## PR Checklist - [x] Closes: #45246 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed --- .github/actions/spell-check/expect.txt | 2 +- .../FuzzyMatcherComparisonTests.cs | 6 +- .../FuzzyMatcherComplexEmojiTests.cs | 29 +++ .../FuzzyMatcherEmojiTests.cs | 83 +++++++ .../FuzzyMatcherNormalizationTests.cs | 78 ++++++ .../FuzzyMatcherUnicodeGarbageTests.cs | 223 ++++++++++++++++++ .../FuzzyStringMatcher.cs | 9 +- 7 files changed, 427 insertions(+), 3 deletions(-) create mode 100644 src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherComplexEmojiTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherEmojiTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherNormalizationTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherUnicodeGarbageTests.cs diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index 1f450a10e6..7c1b9f65dd 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -2180,4 +2180,4 @@ Zoneszonabletester Zoomin zoomit ZOOMITX -Zorder \ No newline at end of file +Zorder diff --git a/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherComparisonTests.cs b/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherComparisonTests.cs index 11c3113dac..99a6af73af 100644 --- a/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherComparisonTests.cs +++ b/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherComparisonTests.cs @@ -117,7 +117,11 @@ public class FuzzyMatcherComparisonTests ["_a", "_a"], ["a_", "a_"], ["-a", "-a"], - ["a-", "a-"] + ["a-", "a-"], + ["🐿️", "🐿️"], // Squirrel emoji + ["\U0001F44D", "\U0001F44D\U0001F3FB"], // Base thumbs-up vs thumbs-up with LIGHT skin tone modifier + ["\U0001F44D\U0001F3FB", "\U0001F44D\U0001F3FB"], // Thumbs-up with LIGHT skin tone vs itself (exact same sequence) + ["\U0001F44D\U0001F3FB", "\U0001F44D\U0001F3FF"], // Thumbs-up with LIGHT skin tone vs thumbs-up with DARK skin tone ]; [TestMethod] diff --git a/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherComplexEmojiTests.cs b/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherComplexEmojiTests.cs new file mode 100644 index 0000000000..f418402aed --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherComplexEmojiTests.cs @@ -0,0 +1,29 @@ +// 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 Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CommandPalette.Extensions.Toolkit.UnitTests; + +[TestClass] +public sealed class FuzzyMatcherComplexEmojiTests +{ + [TestMethod] + [Ignore("For now this is not supported")] + public void Mismatch_DifferentSkinTone_PartialMatch() + { + // "👍🏻" (Light) vs "👍🏿" (Dark) + // They share the base "👍". + const string needle = "👍🏻"; + const string haystack = "👍🏿"; + + var result = FuzzyStringMatcher.ScoreFuzzyWithPositions(needle, haystack, allowNonContiguousMatches: true); + + // Should have a positive score because of the base emoji match + Assert.IsTrue(result.Score > 0, "Expected partial match based on base emoji"); + + // Should match the base emoji (2 chars) + Assert.AreEqual(2, result.Positions.Count, "Expected match on base emoji only"); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherEmojiTests.cs b/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherEmojiTests.cs new file mode 100644 index 0000000000..623325f3fc --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherEmojiTests.cs @@ -0,0 +1,83 @@ +// 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 Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CommandPalette.Extensions.Toolkit.UnitTests; + +[TestClass] +public sealed class FuzzyMatcherEmojiTests +{ + [TestMethod] + public void ExactMatch_SimpleEmoji_ReturnsScore() + { + const string needle = "🚀"; + const string haystack = "Launch 🚀 sequence"; + + var result = FuzzyStringMatcher.ScoreFuzzyWithPositions(needle, haystack, allowNonContiguousMatches: true); + + Assert.IsTrue(result.Score > 0, "Expected match for simple emoji"); + + // 🚀 is 2 chars (surrogates) + Assert.AreEqual(2, result.Positions.Count, "Expected 2 matched characters positions for the emoji"); + } + + [TestMethod] + public void ExactMatch_SkinTone_ReturnsScore() + { + const string needle = "👍🏽"; // Medium skin tone + const string haystack = "Thumbs up 👍🏽 here"; + + var result = FuzzyStringMatcher.ScoreFuzzyWithPositions(needle, haystack, allowNonContiguousMatches: true); + + Assert.IsTrue(result.Score > 0, "Expected match for emoji with skin tone"); + + // 👍🏽 is 4 chars: U+1F44D (2 chars) + U+1F3FD (2 chars) + Assert.AreEqual(4, result.Positions.Count, "Expected 4 matched characters positions for the emoji with modifier"); + } + + [TestMethod] + public void ZWJSequence_Family_Match() + { + const string needle = "👨‍👩‍👧‍👦"; // Family: Man, Woman, Girl, Boy + const string haystack = "Emoji 👨‍👩‍👧‍👦 Test"; + + var result = FuzzyStringMatcher.ScoreFuzzyWithPositions(needle, haystack, allowNonContiguousMatches: true); + + Assert.IsTrue(result.Score > 0, "Expected match for ZWJ sequence"); + + // This emoji is 11 code points? No. + // Man (2) + ZWJ (1) + Woman (2) + ZWJ (1) + Girl (2) + ZWJ (1) + Boy (2) = 11 chars? + // Let's just check score > 0. + Assert.IsTrue(result.Positions.Count > 0); + } + + [TestMethod] + public void Flags_Match() + { + const string needle = "🇺🇸"; // US Flag (Regional Indicator U + Regional Indicator S) + const string haystack = "USA 🇺🇸"; + + var result = FuzzyStringMatcher.ScoreFuzzyWithPositions(needle, haystack, allowNonContiguousMatches: true); + + Assert.IsTrue(result.Score > 0, "Expected match for flag emoji"); + + // 2 code points, each is surrogate pair? + // U+1F1FA (REGIONAL INDICATOR SYMBOL LETTER U) -> 2 chars + // U+1F1F8 (REGIONAL INDICATOR SYMBOL LETTER S) -> 2 chars + // Total 4 chars. + Assert.AreEqual(4, result.Positions.Count); + } + + [TestMethod] + public void Emoji_MixedWithText_Search() + { + const string needle = "t🌮o"; // "t" + taco + "o" + const string haystack = "taco 🌮 on tuesday"; + + var result = FuzzyStringMatcher.ScoreFuzzyWithPositions(needle, haystack, allowNonContiguousMatches: true); + + Assert.IsTrue(result.Score > 0); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherNormalizationTests.cs b/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherNormalizationTests.cs new file mode 100644 index 0000000000..ccc5174f00 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherNormalizationTests.cs @@ -0,0 +1,78 @@ +// 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 Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CommandPalette.Extensions.Toolkit.UnitTests; + +[TestClass] +public sealed class FuzzyMatcherNormalizationTests +{ + [TestMethod] + public void Normalization_ShouldBeLengthPreserving_GermanEszett() + { + // "Straße" (6 chars) + // Standard "SS" expansion would change length to 7. + // Our normalizer must preserve length. + var input = "Straße"; + var expectedLength = input.Length; + + // Case 1: Remove Diacritics = true + var normalized = Fold(input, removeDiacritics: true); + Assert.AreEqual(expectedLength, normalized.Length, "Normalization (removeDiacritics=true) must be length preserving for 'Straße'"); + + // Verify expected mapping: ß -> ß (length 1) + Assert.AreEqual("STRAßE", normalized); + + // Case 2: Remove Diacritics = false + var normalizedKeep = Fold(input, removeDiacritics: false); + Assert.AreEqual(expectedLength, normalizedKeep.Length, "Normalization (removeDiacritics=false) must be length preserving for 'Straße'"); + + // ß maps to ß in invariant culture (length 1) + Assert.AreEqual("STRAßE", normalizedKeep); + } + + [TestMethod] + public void Normalization_ShouldBeLengthPreserving_CommonDiacritics() + { + var input = "Crème Brûlée"; + var expected = "CREME BRULEE"; + + var normalized = Fold(input, removeDiacritics: true); + + Assert.AreEqual(input.Length, normalized.Length); + Assert.AreEqual(expected, normalized); + } + + [TestMethod] + public void Normalization_ShouldBeLengthPreserving_MixedComposed() + { + // "Ångström" -> A + ring, o + umlaut /* #no-spell-check-line */ + var input = "Ångström"; /* #no-spell-check-line */ + var expected = "ANGSTROM"; + + var normalized = Fold(input, removeDiacritics: true); + + Assert.AreEqual(input.Length, normalized.Length); + Assert.AreEqual(expected, normalized); + } + + [TestMethod] + public void Normalization_ShouldNormalizeSlashes() + { + var input = @"Folder\File.txt"; + var expected = "FOLDER/FILE.TXT"; + + var normalized = Fold(input, removeDiacritics: true); + + Assert.AreEqual(input.Length, normalized.Length); + Assert.AreEqual(expected, normalized); + } + + private string Fold(string input, bool removeDiacritics) + { + return FuzzyStringMatcher.Folding.FoldForComparison(input, removeDiacritics); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherUnicodeGarbageTests.cs b/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherUnicodeGarbageTests.cs new file mode 100644 index 0000000000..4532f19b71 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CommandPalette.Extensions.Toolkit.UnitTests/FuzzyMatcherUnicodeGarbageTests.cs @@ -0,0 +1,223 @@ +// 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 Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CommandPalette.Extensions.Toolkit.UnitTests; + +[TestClass] +public sealed class FuzzyMatcherUnicodeGarbageTests +{ + [TestMethod] + public void UnpairedHighSurrogateInNeedle_RemoveDiacritics_ShouldNotThrow() + { + const string needle = "\uD83D"; // high surrogate (unpaired) + const string haystack = "abc"; + + _ = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: true); + } + + [TestMethod] + public void UnpairedLowSurrogateInNeedle_RemoveDiacritics_ShouldNotThrow() + { + const string needle = "\uDC00"; // low surrogate (unpaired) + const string haystack = "abc"; + + _ = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: true); + } + + [TestMethod] + public void UnpairedHighSurrogateInHaystack_RemoveDiacritics_ShouldNotThrow() + { + const string needle = "a"; + const string haystack = "a\uD83D" + "bc"; // inject unpaired high surrogate + + _ = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: true); + } + + [TestMethod] + public void UnpairedLowSurrogateInHaystack_RemoveDiacritics_ShouldNotThrow() + { + const string needle = "a"; + const string haystack = "a\uDC00" + "bc"; // inject unpaired low surrogate + + _ = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: true); + } + + [TestMethod] + public void MixedSurrogatesAndMarks_RemoveDiacritics_ShouldNotThrow() + { + // "Garbage smoothie": unpaired surrogate + combining mark + emoji surrogate pair + const string needle = "a\uD83D\u0301"; // 'a' + unpaired high surrogate + combining acute + const string haystack = "a\u0301 \U0001F600"; // 'a' + combining acute + space + 😀 (valid pair) + + _ = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: true); + } + + [TestMethod] + public void ValidEmojiSurrogatePair_RemoveDiacritics_ShouldNotThrow_AndCanMatch() + { + // 😀 U+1F600 encoded as surrogate pair in UTF-16 + const string needle = "\U0001F600"; + const string haystack = "x \U0001F600 y"; + + var result = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: true); + + // Keep assertions minimal: just ensure it doesn't act like "no match". + // If your API returns score=0 for no match, this is stable. + Assert.IsTrue(result.Score > 0, "Expected emoji to produce a match score > 0."); + Assert.IsTrue(result.Positions.Count > 0, "Expected at least one matched position."); + } + + [TestMethod] + public void DiacriticStripping_StillWorks_OnBMPNonSurrogate() + { + // This is a regression guard: we fixed surrogates; don't break diacritic stripping. + // "é" should fold like "e" when removeDiacritics=true. + const string needle = "cafe"; + const string haystack = "CAFÉ"; + + var withDiacriticsRemoved = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: true); + + var withoutDiacriticsRemoved = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: false); + + Assert.IsTrue(withDiacriticsRemoved.Score >= withoutDiacriticsRemoved.Score, "Removing diacritics should not make matching worse for 'CAFÉ' vs 'cafe'."); + Assert.IsTrue(withDiacriticsRemoved.Score > 0, "Expected a match when diacritics are removed."); + } + + [TestMethod] + public void RandomUtf16Garbage_RemoveDiacritics_ShouldNotThrow() + { + // Deterministic pseudo-random "UTF-16 garbage", including surrogates. + // This is a quick fuzz-lite test that’s stable across runs. + var s1 = MakeDeterministicGarbage(seed: 1234, length: 512); + var s2 = MakeDeterministicGarbage(seed: 5678, length: 1024); + + _ = FuzzyStringMatcher.ScoreFuzzyWithPositions( + s1, + s2, + allowNonContiguousMatches: true, + removeDiacritics: true); + } + + [TestMethod] + public void RandomUtf16Garbage_NoDiacritics_ShouldNotThrow() + { + var s1 = MakeDeterministicGarbage(seed: 42, length: 512); + var s2 = MakeDeterministicGarbage(seed: 43, length: 1024); + + _ = FuzzyStringMatcher.ScoreFuzzyWithPositions( + s1, + s2, + allowNonContiguousMatches: true, + removeDiacritics: false); + } + + [TestMethod] + public void HighSurrogateAtEndOfHaystack_RemoveDiacritics_ShouldNotThrow() + { + const string needle = "a"; + const string haystack = "abc\uD83D"; // Ends with high surrogate + + _ = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: true); + } + + [TestMethod] + public void ComplexEmojiSequence_RemoveDiacritics_ShouldNotThrow() + { + // Family: Man, Woman, Girl, Boy + // U+1F468 U+200D U+1F469 U+200D U+1F467 U+200D U+1F466 + const string needle = "\U0001F468"; + const string haystack = "Info: \U0001F468\u200D\U0001F469\u200D\U0001F467\u200D\U0001F466 family"; + + _ = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: true); + } + + [TestMethod] + public void NullOrEmptyInputs_ShouldNotThrow() + { + // Empty needle + var result1 = FuzzyStringMatcher.ScoreFuzzyWithPositions(string.Empty, "abc", true, true); + Assert.AreEqual(0, result1.Score); + + // Empty haystack + var result2 = FuzzyStringMatcher.ScoreFuzzyWithPositions("abc", string.Empty, true, true); + Assert.AreEqual(0, result2.Score); + + // Null haystack + var result3 = FuzzyStringMatcher.ScoreFuzzyWithPositions("abc", null!, true, true); + Assert.AreEqual(0, result3.Score); + } + + [TestMethod] + public void VeryLongStrings_ShouldNotThrow() + { + var needle = new string('a', 100); + var haystack = new string('b', 10000) + needle + new string('c', 10000); + + _ = FuzzyStringMatcher.ScoreFuzzyWithPositions( + needle, + haystack, + allowNonContiguousMatches: true, + removeDiacritics: true); + } + + private static string MakeDeterministicGarbage(int seed, int length) + { + // LCG for deterministic generation without Random’s platform/version surprises. + var x = (uint)seed; + var chars = length <= 2048 ? stackalloc char[length] : new char[length]; + + for (var i = 0; i < chars.Length; i++) + { + // LCG: x = (a*x + c) mod 2^32 + x = unchecked((1664525u * x) + 1013904223u); + + // Take top 16 bits as UTF-16 code unit (includes surrogates). + chars[i] = (char)(x >> 16); + } + + return new string(chars); + } +} diff --git a/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/FuzzyStringMatcher.cs b/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/FuzzyStringMatcher.cs index 40491970b3..a4b7084555 100644 --- a/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/FuzzyStringMatcher.cs +++ b/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/FuzzyStringMatcher.cs @@ -643,7 +643,7 @@ public static class FuzzyStringMatcher // ============================================================ // Folding: slash normalization + upper case + optional diacritics stripping - private static class Folding + internal static class Folding { // Cache maps an upper case char to its diacritics-stripped upper case char. // '\0' means "not cached yet". @@ -820,6 +820,13 @@ public static class FuzzyStringMatcher return upper; } + // Emoji and other astral symbols come through as surrogate pairs in UTF-16. + // We process char-by-char, so never try to normalize a lone surrogate. + if (char.IsSurrogate(upper)) + { + return upper; + } + var cached = StripCacheUpper[upper]; if (cached != '\0') { From b3e7c9d22752429caa9f6103e43b845676c3e3a9 Mon Sep 17 00:00:00 2001 From: Jaylyn Barbee <51131738+Jaylyn-Barbee@users.noreply.github.com> Date: Mon, 2 Feb 2026 20:23:54 -0500 Subject: [PATCH 7/7] [Light Switch] Fix Light Switch start up logic (#45304) ## Summary of the Pull Request Title ## PR Checklist - [x] Closes: https://github.com/microsoft/PowerToys/issues/45291 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected ## Detailed Description of the Pull Request / Additional comments Before, there was a function that initialized some variables about the current system state that were later used to check against if that state needed to change in a different function. That caused from some issues because I was reusing the function for a double purpose. Now the `SyncInitialThemeState()` function in the State Manager will sync those initial variables and apply the correct theme if needed. I also removed an unnecessary parameter from `onTick` ## Validation Steps Performed Manual testing --- .../LightSwitch/LightSwitchService/LightSwitchService.cpp | 3 +-- .../LightSwitchService/LightSwitchStateManager.cpp | 6 +++++- .../LightSwitchService/LightSwitchStateManager.h | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/modules/LightSwitch/LightSwitchService/LightSwitchService.cpp b/src/modules/LightSwitch/LightSwitchService/LightSwitchService.cpp index 8919f4274b..5b4fa8297b 100644 --- a/src/modules/LightSwitch/LightSwitchService/LightSwitchService.cpp +++ b/src/modules/LightSwitch/LightSwitchService/LightSwitchService.cpp @@ -250,7 +250,6 @@ DWORD WINAPI ServiceWorkerThread(LPVOID lpParam) Logger::info(L"[LightSwitchService] Initialized at {:02d}:{:02d}.", st.wHour, st.wMinute); stateManager.SyncInitialThemeState(); - stateManager.OnTick(nowMinutes); // ──────────────────────────────────────────────────────────────── // Worker Loop @@ -281,7 +280,7 @@ DWORD WINAPI ServiceWorkerThread(LPVOID lpParam) GetLocalTime(&st); nowMinutes = st.wHour * 60 + st.wMinute; DetectAndHandleExternalThemeChange(stateManager); - stateManager.OnTick(nowMinutes); + stateManager.OnTick(); continue; } diff --git a/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.cpp b/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.cpp index f562d38c41..cc4f959881 100644 --- a/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.cpp +++ b/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.cpp @@ -28,7 +28,7 @@ void LightSwitchStateManager::OnSettingsChanged() } // Called once per minute -void LightSwitchStateManager::OnTick(int currentMinutes) +void LightSwitchStateManager::OnTick() { std::lock_guard lock(_stateMutex); if (_state.lastAppliedMode != ScheduleMode::FollowNightLight) @@ -109,10 +109,14 @@ void LightSwitchStateManager::SyncInitialThemeState() std::lock_guard lock(_stateMutex); _state.isSystemLightActive = GetCurrentSystemTheme(); _state.isAppsLightActive = GetCurrentAppsTheme(); + _state.isNightLightActive = IsNightLightEnabled(); Logger::debug(L"[LightSwitchStateManager] Synced initial state to current system theme ({})", _state.isSystemLightActive ? L"light" : L"dark"); Logger::debug(L"[LightSwitchStateManager] Synced initial state to current apps theme ({})", _state.isAppsLightActive ? L"light" : L"dark"); + + // This will ensure that the theme is applied according to current settings at startup + EvaluateAndApplyIfNeeded(); } static std::pair update_sun_times(auto& settings) diff --git a/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.h b/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.h index c4f39a2e9a..65d6f7ada7 100644 --- a/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.h +++ b/src/modules/LightSwitch/LightSwitchService/LightSwitchStateManager.h @@ -28,7 +28,7 @@ public: void OnSettingsChanged(); // Called every minute (from service worker tick). - void OnTick(int currentMinutes); + void OnTick(); // Called when manual override is toggled (via shortcut or system change). void OnManualOverride();