Merge pull request #214 from zadjii-msft/llama/image-loading-perf

Just some minor clean-up/perf things I was trying, doesn't actually fix anything though, unfortunately (outside of maybe making startup a bit faster)...

Does NOT fix https://github.com/zadjii-msft/PowerToys/issues/213

We were effectively getting called to load images twice in a few spots, and we were trying to load them everywhere at startup. This minimized that in a few places, but we can't use x:Phase in the DataTemplate for the List Items (as it needs a UIElement), so that caused some issues.

I thought I had a fix for all the duplicate items loading on the home page, but alas no. Have to continue digging into that, but got side-tracked by the above.

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListItemViewModel.cs

@@ -107,4 +107,8 @@ public partial class ListItemViewModel(IListItem model, TaskScheduler scheduler)
     public bool MatchesFilter(string filter) => StringMatcher.FuzzySearch(filter, Title).Success || StringMatcher.FuzzySearch(filter, Subtitle).Success;
 
     public override string ToString() => $"{Name} ListItemViewModel";
+
+    public override bool Equals(object? obj) => obj is ListItemViewModel vm && vm._listItemModel.Equals(this._listItemModel);
+
+    public override int GetHashCode() => _listItemModel.GetHashCode();
 }

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListViewModel.cs

@@ -90,7 +90,9 @@ public partial class ListViewModel : PageViewModel
                 viewModel.InitializeProperties();
                 _itemCache.Add(viewModel); // TODO: Figure out when we clear/remove things from cache...
 
-                if (Filter == string.Empty || viewModel.MatchesFilter(Filter))
+                // We may already have items from the new items here.
+                if ((Filter == string.Empty || viewModel.MatchesFilter(Filter))
+                    && !Items.Contains(viewModel)) //// TODO: We should be smarter about the contains here somehow (also in OnFilterUpdated)
                 {
                     // Am I really allowed to modify that observable collection on a BG
                     // thread and have it just work in the UI??

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/ExtensionObject`1.cs

@@ -7,4 +7,8 @@ namespace Microsoft.CmdPal.UI.ViewModels.Models;
 public class ExtensionObject<T>(T? value) // where T : IInspectable
 {
     public T? Unsafe { get; } = value;
+
+    public override bool Equals(object? obj) => obj is ExtensionObject<T> ext && ext.Unsafe?.Equals(this.Unsafe) == true;
+
+    public override int GetHashCode() => Unsafe?.GetHashCode() ?? base.GetHashCode();
 }

src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ActionBar.xaml

@@ -57,6 +57,7 @@
 
         
         <Border x:Name="IconBorder"
+                x:Load="{x:Bind IsLoaded, Mode=OneWay}"
                 Width="20"
                 Height="20"
                 Margin="12,0,0,0"
@@ -79,6 +80,8 @@
             Orientation="Horizontal"
             Spacing="6">
             <Button
+                x:Name="PrimaryButton"
+                x:Load="{x:Bind IsLoaded, Mode=OneWay}"
                 Height="40"
                 Padding="8,4,8,4"
                 Visibility="{x:Bind ViewModel.PrimaryAction.Name, Converter={StaticResource StringNotEmptyToVisibilityConverter}, Mode=OneWay}">
@@ -109,6 +112,8 @@
                 </StackPanel>
             </Button>
             <Button
+                x:Name="SecondaryButton"
+                x:Load="{x:Bind IsLoaded, Mode=OneWay}"
                 Height="40"
                 Padding="8,4,8,4"
                 Visibility="{x:Bind ViewModel.HasSecondaryCommand, Mode=OneWay}">
@@ -120,7 +125,7 @@
                             Margin="4,4,4,4">
                         <!-- LoadIconBehavior will magically fill this border up with an icon -->
                         <Interactivity:Interaction.Behaviors>
-                            <cmdpalUI:LoadIconBehavior Source="{x:Bind ViewModel.SecondaryAction.Icon, Mode=OneWay}"/>
+                            <cmdpalUI:LoadIconBehavior Source="{x:Bind ViewModel.SecondaryAction.Icon, Mode=OneWay}" />
                         </Interactivity:Interaction.Behaviors>
                     </Border>
 

src/modules/cmdpal/Microsoft.CmdPal.UI/LoadIconBehavior.xaml.cs

@@ -14,19 +14,15 @@ public partial class LoadIconBehavior : DependencyObject, IBehavior
 {
     private static readonly IconCacheService IconService = new(Microsoft.UI.Dispatching.DispatcherQueue.GetForCurrentThread());
 
-    public IconDataType Source
+    public IconDataType? Source
     {
-        get => (IconDataType)GetValue(SourceProperty);
-        set
-        {
-            SetValue(SourceProperty, value);
-            OnSourcePropertyChanged();
-        }
+        get => (IconDataType?)GetValue(SourceProperty);
+        set => SetValue(SourceProperty, value);
     }
 
     // Using a DependencyProperty as the backing store for Source.  This enables animation, styling, binding, etc...
     public static readonly DependencyProperty SourceProperty =
-        DependencyProperty.Register(nameof(Source), typeof(IconDataType), typeof(LoadIconBehavior), new PropertyMetadata(new IconDataType(string.Empty)));
+        DependencyProperty.Register(nameof(Source), typeof(IconDataType), typeof(LoadIconBehavior), new PropertyMetadata(null, OnSourcePropertyChanged));
 
     public DependencyObject? AssociatedObject { get; private set; }
 
@@ -34,12 +30,20 @@ public partial class LoadIconBehavior : DependencyObject, IBehavior
 
     public void Detach() => AssociatedObject = null;
 
-    public async void OnSourcePropertyChanged()
+    private static async void OnSourcePropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
     {
-        var icoSource = await IconService.GetIconSource(Source ?? new(string.Empty));
-
-        if (AssociatedObject is Border border)
+        if (d is LoadIconBehavior @this
+            && @this.AssociatedObject is Border border)
         {
+            var icoSource = await IconService.GetIconSource(@this.Source ?? new(string.Empty));
+
+            /* This causes a catastrophic failure...
+            if (border.Child != null)
+            {
+                VisualTreeHelper.DisconnectChildrenRecursive(border.Child);
+                border.Child = null;
+            }*/
+
             if (icoSource is FontIconSource fontIco)
             {
                 fontIco.FontSize = border.Width;

src/modules/cmdpal/Microsoft.CmdPal.UI/ShellPage.xaml

@@ -86,12 +86,13 @@
                 </Grid.RowDefinitions>
 
                 <Border x:Name="HeroImageBorder"
+                        x:Load="{x:Bind IsLoaded, Mode=OneWay}"
                         Width="64" 
                         Height="64"
                         Visibility="{x:Bind ViewModel.Details.HasHeroImage, Mode=OneWay}" >
                     <!-- LoadIconBehavior will magically fill this border up with an icon -->
                     <Interactivity:Interaction.Behaviors>
-                        <local:LoadIconBehavior Source="{x:Bind ViewModel.Details.HeroImage, Mode=OneWay}"/>
+                        <local:LoadIconBehavior Source="{x:Bind ViewModel.Details.HeroImage, Mode=OneWay}" />
                     </Interactivity:Interaction.Behaviors>
                 </Border>