From 825f68d1f0b45c1d0e9c2cff39987577bd324815 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Thu, 28 Sep 2023 21:56:23 -0400 Subject: [PATCH 1/5] fix!: Improve ResourceKey lazy resolution - Adjust lazy resource key resolution - Reduce dictionary lookups by removing empty ones - Don't resolve resource statically inside XAML and keep load time resolution BREAKING CHANGE: Resources dictionaries are now required to be explicitly referenced by URI to be considered during resource resolution. This change is present to align the inclusion behavior with WinUI, which does not automatically place dictionaries as ambiently available. This change can be disabled by using `FeatureConfiguration.ResourceDictionary.IncludeUnreferencedDictionaries`. --- .../Controls/ThemeResource_TCO_MyAlias.xaml | 13 ++ .../Controls/ThemeResource_TCO_MyBrush.xaml | 12 ++ .../Controls/ThemeResource_TCO_MyButton.xaml | 12 ++ .../ThemeResource_TCO_MyColorGreen.xaml | 13 ++ .../ThemeResource_TCO_MyColorRed.xaml | 14 +++ ...ThemeResource_Theme_Changing_Override.xaml | 119 ++++++++++++++++++ ...meResource_Theme_Changing_Override.xaml.cs | 65 ++++++++++ .../Windows_UI_Xaml/Given_ThemeResource.cs | 36 ++++++ src/Uno.UI/FeatureConfiguration.cs | 9 ++ .../UI/Xaml/Data/ResourceUpdateReason.cs | 5 + src/Uno.UI/UI/Xaml/DependencyObjectStore.cs | 25 +++- src/Uno.UI/UI/Xaml/ResourceDictionary.cs | 8 ++ src/Uno.UI/UI/Xaml/ResourceResolver.cs | 24 +++- .../UI/Xaml/ResourceResolverSingleton.cs | 12 +- src/Uno.UI/UI/Xaml/XamlScope.cs | 20 ++- 15 files changed, 361 insertions(+), 26 deletions(-) create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyAlias.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyBrush.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyButton.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyColorGreen.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyColorRed.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml.cs diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyAlias.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyAlias.xaml new file mode 100644 index 000000000000..1f9e95f135a8 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyAlias.xaml @@ -0,0 +1,13 @@ + + + + + + + + + + + + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyBrush.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyBrush.xaml new file mode 100644 index 000000000000..6e95977b53f7 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyBrush.xaml @@ -0,0 +1,12 @@ + + + + + + + + + + + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyButton.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyButton.xaml new file mode 100644 index 000000000000..13df52197fe0 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyButton.xaml @@ -0,0 +1,12 @@ + + + + + + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyColorGreen.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyColorGreen.xaml new file mode 100644 index 000000000000..a957ac87cfd4 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyColorGreen.xaml @@ -0,0 +1,13 @@ + + + + Green + + + DarkGreen + + + + Dummy + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyColorRed.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyColorRed.xaml new file mode 100644 index 000000000000..f09230fdbc1b --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyColorRed.xaml @@ -0,0 +1,14 @@ + + + + Red + + + + DarkRed + + + + Dummy + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml new file mode 100644 index 000000000000..eaa4a4594940 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml @@ -0,0 +1,119 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml.cs new file mode 100644 index 000000000000..812510131aa6 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml.cs @@ -0,0 +1,65 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices.WindowsRuntime; +using Windows.Foundation; +using Windows.Foundation.Collections; +using Windows.UI.Xaml; +using Windows.UI.Xaml.Controls; +using Windows.UI.Xaml.Controls.Primitives; +using Windows.UI.Xaml.Data; +using Windows.UI.Xaml.Input; +using Windows.UI.Xaml.Media; +using Windows.UI.Xaml.Navigation; + +// The Blank Page item template is documented at https://go.microsoft.com/fwlink/?LinkId=234238 + +namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls; + +/// +/// An empty page that can be used on its own or navigated to within a Frame. +/// +public sealed partial class ThemeResource_Theme_Changing_Override : Page +{ + public ThemeResource_Theme_Changing_Override() + { + this.InitializeComponent(); + } +} + +public class ThemeResource_Theme_Changing_Override_Custom : ResourceDictionary +{ + private const string GreenUri = "ms-appx:///Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyColorGreen.xaml"; + private const string RedUri = "ms-appx:///Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyColorRed.xaml"; + private const string BrushUri = "ms-appx:///Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyBrush.xaml"; + private const string AliasUri = "ms-appx:///Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyAlias.xaml"; + private const string ButtonUri = "ms-appx:///Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_TCO_MyButton.xaml"; + + private string _mode; + + public string Mode + { + get => _mode; + set + { + _mode = value; + var colorUri = _mode == "Green" ? GreenUri : RedUri; + + var myBrush = new ResourceDictionary { Source = new Uri(BrushUri) }; + var myAlias = new ResourceDictionary { Source = new Uri(AliasUri) }; + var myButton = new ResourceDictionary { Source = new Uri(ButtonUri) }; + + myBrush.MergedDictionaries.Add(new ResourceDictionary { Source = new Uri(colorUri) }); + myAlias.MergedDictionaries.Add(myBrush); + myButton.MergedDictionaries.Add(myAlias); + + MergedDictionaries.Add(myButton); + } + } + + public ThemeResource_Theme_Changing_Override_Custom() + { + + } +} diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ThemeResource.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ThemeResource.cs index be25605160b9..25d5d8641f07 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ThemeResource.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_ThemeResource.cs @@ -159,6 +159,42 @@ public async Task When_ThemeResource_Style_Switch() } } + [TestMethod] + public async Task When_Theme_Changed() + { + using var _ = StyleHelper.UseFluentStyles(); + + var control = new ThemeResource_Theme_Changing_Override(); + WindowHelper.WindowContent = control; + + await WindowHelper.WaitForIdle(); + + Assert.AreEqual(Colors.Red, (control.button01.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.Red, (control.button02.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.Red, (control.button03.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.Red, (control.button04.Background as SolidColorBrush)?.Color); + + Assert.AreEqual(Colors.Green, (control.button01_override.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.Green, (control.button02_override.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.Green, (control.button03_override.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.Green, (control.button04_override.Background as SolidColorBrush)?.Color); + + using (ThemeHelper.UseDarkTheme()) + { + await WindowHelper.WaitForIdle(); + + Assert.AreEqual(Colors.DarkRed, (control.button01.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.DarkRed, (control.button02.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.DarkRed, (control.button03.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.DarkRed, (control.button04.Background as SolidColorBrush)?.Color); + + Assert.AreEqual(Colors.DarkGreen, (control.button01_override.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.DarkGreen, (control.button02_override.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.DarkGreen, (control.button03_override.Background as SolidColorBrush)?.Color); + Assert.AreEqual(Colors.DarkGreen, (control.button04_override.Background as SolidColorBrush)?.Color); + } + } + private async Task When_DefaultForeground(Color lightThemeColor, Color darkThemeColor) { var run = new Run() diff --git a/src/Uno.UI/FeatureConfiguration.cs b/src/Uno.UI/FeatureConfiguration.cs index 973f4de4ada2..cea4ee76b0bb 100644 --- a/src/Uno.UI/FeatureConfiguration.cs +++ b/src/Uno.UI/FeatureConfiguration.cs @@ -169,6 +169,15 @@ public static class DependencyObject = true; } + public static class ResourceDictionary + { + /// + /// Determines whether unreferenced ResourceDictionary present in the assembly + /// are accessible from app resources. + /// + public static bool IncludeUnreferencedDictionaries { get; set; } + } + public static class Font { private static string _symbolsFont = diff --git a/src/Uno.UI/UI/Xaml/Data/ResourceUpdateReason.cs b/src/Uno.UI/UI/Xaml/Data/ResourceUpdateReason.cs index 7a3305f5081c..4b9f3185a186 100644 --- a/src/Uno.UI/UI/Xaml/Data/ResourceUpdateReason.cs +++ b/src/Uno.UI/UI/Xaml/Data/ResourceUpdateReason.cs @@ -24,6 +24,11 @@ internal enum ResourceUpdateReason /// HotReload = 4, + /// + /// Update marked as XamlLoader + /// + XamlParser = 8, + /// /// Updates that should be propagated recursively through the visual tree /// diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs index 32e83470fa22..661d314258cd 100644 --- a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs +++ b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs @@ -1330,6 +1330,14 @@ private void InnerUpdateResourceBindingsUnsafe(ResourceUpdateReason updateReason return; } + if ((updateReason & ResourceUpdateReason.ResolvedOnLoading) != 0) + { + for (var i = dictionariesInScope.Length - 1; i >= 0; i--) + { + ResourceResolver.PushSourceToScope(dictionariesInScope[i]); + } + } + var wasSet = false; foreach (var dict in dictionariesInScope) { @@ -1348,6 +1356,14 @@ private void InnerUpdateResourceBindingsUnsafe(ResourceUpdateReason updateReason SetResourceBindingValue(property, binding, value); } } + + if ((updateReason & ResourceUpdateReason.ResolvedOnLoading) != 0) + { + foreach (var dict in dictionariesInScope) + { + ResourceResolver.PopSourceFromScope(); + } + } } private void SetResourceBindingValue(DependencyProperty property, ResourceBinding binding, object? value) @@ -1469,7 +1485,7 @@ private IEnumerable GetChildrenDependencyObjects() /// internal IEnumerable GetResourceDictionaries(bool includeAppResources, ResourceDictionary? containingDictionary = null) { - if (containingDictionary != null) + if (containingDictionary is not null) { yield return containingDictionary; } @@ -1477,13 +1493,13 @@ internal IEnumerable GetResourceDictionaries(bool includeApp var candidate = ActualInstance; var candidateFE = candidate as FrameworkElement; - while (candidate != null) + while (candidate is not null) { var parent = candidate.GetParent() as DependencyObject; - if (candidateFE != null) + if (candidateFE is not null) { - if (candidateFE.Resources != null) // It's legal (if pointless) on UWP to set Resources to null from user code, so check + if (candidateFE.Resources is { IsEmpty: false}) // It's legal (if pointless) on UWP to set Resources to null from user code, so check { yield return candidateFE.Resources; } @@ -1505,6 +1521,7 @@ internal IEnumerable GetResourceDictionaries(bool includeApp candidate = parent; } } + if (includeAppResources && Application.Current != null) { // In the case of StaticResource resolution we skip Application.Resources because we assume these were already checked at initialize-time. diff --git a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs index 420a448c0bcc..f0e7b7e9a806 100644 --- a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs +++ b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs @@ -58,6 +58,14 @@ public Uri Source public IList MergedDictionaries => _mergedDictionaries; public IDictionary ThemeDictionaries => GetOrCreateThemeDictionaries(); + /// + /// Determines if this instance is empty + /// + internal bool IsEmpty + => Count == 0 + && ThemeDictionaries.Count == 0 + && MergedDictionaries.Count == 0; + private ResourceDictionary GetOrCreateThemeDictionaries() { if (_themeDictionaries is null) diff --git a/src/Uno.UI/UI/Xaml/ResourceResolver.cs b/src/Uno.UI/UI/Xaml/ResourceResolver.cs index d51c2adfe638..16fa298f1d60 100644 --- a/src/Uno.UI/UI/Xaml/ResourceResolver.cs +++ b/src/Uno.UI/UI/Xaml/ResourceResolver.cs @@ -244,7 +244,7 @@ internal static object ResolveTopLevelResource(SpecializedResourceDictionary.Res /// True for {ThemeResource Foo}, false for {StaticResource Foo} /// Optional parameter that provides parse-time context [EditorBrowsable(EditorBrowsableState.Never)] - public static void ApplyResource(DependencyObject owner, DependencyProperty property, object resourceKey, bool isThemeResourceExtension, bool isHotReloadSupported, object context = null) + public static void ApplyResource(DependencyObject owner, DependencyProperty property, object resourceKey, bool isThemeResourceExtension, bool isHotReloadSupported, bool fromXamlParser = false, object context = null) { var updateReason = ResourceUpdateReason.None; if (isThemeResourceExtension) @@ -260,13 +260,24 @@ public static void ApplyResource(DependencyObject owner, DependencyProperty prop updateReason |= ResourceUpdateReason.HotReload; } + if (fromXamlParser && ResourceResolver.CurrentScope.Sources.IsEmpty) + { + updateReason |= ResourceUpdateReason.XamlParser; + } + ApplyResource(owner, property, new SpecializedResourceDictionary.ResourceKey(resourceKey), updateReason, context, null); } internal static void ApplyResource(DependencyObject owner, DependencyProperty property, SpecializedResourceDictionary.ResourceKey specializedKey, ResourceUpdateReason updateReason, object context, DependencyPropertyValuePrecedences? precedence) { + // If the invocation comes from XAML and from theme resources, resolution + // must happen lazily, done through walking the visual tree. + var immediateResolution = + (updateReason & ResourceUpdateReason.XamlParser) != 0 + && (updateReason & ResourceUpdateReason.ThemeResource) != 0; + // Set initial value based on statically-available top-level resources. - if (TryStaticRetrieval(specializedKey, context, out var value)) + if (!immediateResolution && TryStaticRetrieval(specializedKey, context, out var value)) { owner.SetValue(property, BindingPropertyHelper.Convert(() => property.Type, value), precedence); @@ -383,9 +394,12 @@ private static bool TryStaticRetrieval(in SpecializedResourceDictionary.Resource internal static bool TryTopLevelRetrieval(in SpecializedResourceDictionary.ResourceKey resourceKey, object context, out object value) { value = null; - return (Application.Current?.Resources.TryGetValue(resourceKey, out value, shouldCheckSystem: false) ?? false) || - TryAssemblyResourceRetrieval(resourceKey, context, out value) || - TrySystemResourceRetrieval(resourceKey, out value); + return (Application.Current?.Resources.TryGetValue(resourceKey, out value, shouldCheckSystem: false) ?? false) + || ( + FeatureConfiguration.ResourceDictionary.IncludeUnreferencedDictionaries + && TryAssemblyResourceRetrieval(resourceKey, context, out value) + ) + || TrySystemResourceRetrieval(resourceKey, out value); } /// diff --git a/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs b/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs index 3f8377a16c42..ca2a9ac3fe02 100644 --- a/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs +++ b/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs @@ -25,15 +25,19 @@ namespace Uno.UI public sealed class ResourceResolverSingleton { private static ResourceResolverSingleton _instance; - public static ResourceResolverSingleton Instance => _instance ??= new ResourceResolverSingleton(); + public static ResourceResolverSingleton Instance + => _instance ??= new ResourceResolverSingleton(); [EditorBrowsable(EditorBrowsableState.Never)] - public object ResolveResourceStatic(object key, Type type, object context) => ResourceResolver.ResolveResourceStatic(key, type, context); + public object ResolveResourceStatic(object key, Type type, object context) + => ResourceResolver.ResolveResourceStatic(key, type, context); [EditorBrowsable(EditorBrowsableState.Never)] - public void ApplyResource(DependencyObject owner, DependencyProperty property, object resourceKey, bool isThemeResourceExtension, bool isHotReloadSupported, object context) => ResourceResolver.ApplyResource(owner, property, resourceKey, isThemeResourceExtension, isHotReloadSupported, context); + public void ApplyResource(DependencyObject owner, DependencyProperty property, object resourceKey, bool isThemeResourceExtension, bool isHotReloadSupported, object context) + => ResourceResolver.ApplyResource(owner, property, resourceKey, isThemeResourceExtension, isHotReloadSupported, true, context); [EditorBrowsable(EditorBrowsableState.Never)] - public object ResolveStaticResourceAlias(string resourceKey, object parseContext) => ResourceResolver.ResolveStaticResourceAlias(resourceKey, parseContext); + public object ResolveStaticResourceAlias(string resourceKey, object parseContext) + => ResourceResolver.ResolveStaticResourceAlias(resourceKey, parseContext); } } diff --git a/src/Uno.UI/UI/Xaml/XamlScope.cs b/src/Uno.UI/UI/Xaml/XamlScope.cs index 258994ffd10b..57a7e3a4e3ec 100644 --- a/src/Uno.UI/UI/Xaml/XamlScope.cs +++ b/src/Uno.UI/UI/Xaml/XamlScope.cs @@ -11,21 +11,15 @@ namespace Windows.UI.Xaml /// opposed to the visual tree. In particular it allows correct resolution during template resolution, where the visual tree may be /// arbitrarily distant from the xaml tree. /// - internal class XamlScope + internal record XamlScope(ImmutableStack Sources) { - private readonly ImmutableStack _resourceSources; + public XamlScope Push(ManagedWeakReference source) + => new(Sources.Push(source)); - public IEnumerable Sources => _resourceSources; + public XamlScope Pop() + => new(Sources.Pop()); - private XamlScope(ImmutableStack sources) - { - _resourceSources = sources; - } - - public XamlScope Push(ManagedWeakReference source) => new XamlScope(_resourceSources.Push(source)); - - public XamlScope Pop() => new XamlScope(_resourceSources.Pop()); - - public static XamlScope Create() => new XamlScope(ImmutableStack.Create()); + public static XamlScope Create() + => new(ImmutableStack.Create()); } } From ec1adb9d55bebb8bbfb6f54ed6a87fbad4852342 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Thu, 28 Sep 2023 23:00:25 -0400 Subject: [PATCH 2/5] chore: Allocate less during resource bindings enumeration --- ...meResource_Theme_Changing_Override.xaml.cs | 5 -- src/Uno.UI/FeatureConfiguration.cs | 2 +- src/Uno.UI/UI/Xaml/DependencyObjectStore.cs | 11 ++-- .../UI/Xaml/ResourceBindingCollection.cs | 66 ++++++++++++------- src/Uno.UI/UI/Xaml/ResourceDictionary.cs | 37 +++++++++-- src/Uno.UI/UI/Xaml/ResourceResolver.cs | 54 +++++++++++---- .../UI/Xaml/ResourceResolverSingleton.cs | 4 +- 7 files changed, 125 insertions(+), 54 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml.cs index 812510131aa6..245183c7bf7b 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/ThemeResource_Theme_Changing_Override.xaml.cs @@ -57,9 +57,4 @@ public string Mode MergedDictionaries.Add(myButton); } } - - public ThemeResource_Theme_Changing_Override_Custom() - { - - } } diff --git a/src/Uno.UI/FeatureConfiguration.cs b/src/Uno.UI/FeatureConfiguration.cs index cea4ee76b0bb..df6acbc1f789 100644 --- a/src/Uno.UI/FeatureConfiguration.cs +++ b/src/Uno.UI/FeatureConfiguration.cs @@ -174,7 +174,7 @@ public static class ResourceDictionary /// /// Determines whether unreferenced ResourceDictionary present in the assembly /// are accessible from app resources. - /// + /// public static bool IncludeUnreferencedDictionaries { get; set; } } diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs index 661d314258cd..490a628c78b7 100644 --- a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs +++ b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs @@ -1285,11 +1285,11 @@ internal void UpdateResourceBindings(ResourceUpdateReason updateReason, Resource var dictionariesInScope = GetResourceDictionaries(includeAppResources: false, containingDictionary).ToArray(); - var bindings = _resourceBindings.GetAllBindings().ToList(); //The original collection may be mutated during DP assignations + var bindings = _resourceBindings.GetAllBindings(); - foreach (var (property, binding) in bindings) + foreach (var binding in bindings) { - InnerUpdateResourceBindings(updateReason, dictionariesInScope, property, binding); + InnerUpdateResourceBindings(updateReason, dictionariesInScope, binding.Property, binding.Binding); } UpdateChildResourceBindings(updateReason); @@ -1332,6 +1332,9 @@ private void InnerUpdateResourceBindingsUnsafe(ResourceUpdateReason updateReason if ((updateReason & ResourceUpdateReason.ResolvedOnLoading) != 0) { + // Add the current dictionaries to the resolver scope, + // this allows for StaticResource.ResourceKey to resolve properly + for (var i = dictionariesInScope.Length - 1; i >= 0; i--) { ResourceResolver.PushSourceToScope(dictionariesInScope[i]); @@ -1499,7 +1502,7 @@ internal IEnumerable GetResourceDictionaries(bool includeApp if (candidateFE is not null) { - if (candidateFE.Resources is { IsEmpty: false}) // It's legal (if pointless) on UWP to set Resources to null from user code, so check + if (candidateFE.Resources is { IsEmpty: false }) // It's legal (if pointless) on UWP to set Resources to null from user code, so check { yield return candidateFE.Resources; } diff --git a/src/Uno.UI/UI/Xaml/ResourceBindingCollection.cs b/src/Uno.UI/UI/Xaml/ResourceBindingCollection.cs index 95f66aca2872..054c1b80e144 100644 --- a/src/Uno.UI/UI/Xaml/ResourceBindingCollection.cs +++ b/src/Uno.UI/UI/Xaml/ResourceBindingCollection.cs @@ -9,47 +9,69 @@ using Uno.UI.DataBinding; using Windows.UI.Xaml.Data; -namespace Windows.UI.Xaml +namespace Windows.UI.Xaml; + +internal class ResourceBindingCollection { - internal class ResourceBindingCollection - { - private readonly Dictionary> _bindings = new Dictionary>(); + private readonly Dictionary> _bindings = new(); + private BindingEntry[]? _cachedAllBindings; + + public bool HasBindings => _bindings.Count > 0 && _bindings.Any(b => b.Value.Any()); - public bool HasBindings => _bindings.Count > 0 && _bindings.Any(b => b.Value.Any()); + public record struct BindingEntry(DependencyProperty Property, ResourceBinding Binding); - public IEnumerable<(DependencyProperty Property, ResourceBinding Binding)> GetAllBindings() + public BindingEntry[] GetAllBindings() + { + if (_cachedAllBindings is null) { + List allBindings = new(); + foreach (var kvp in _bindings) { foreach (var kvpInner in kvp.Value) { - yield return (kvp.Key, kvpInner.Value); + allBindings.Add(new(kvp.Key, kvpInner.Value)); } } + + // We return a fully materialized list every time + // as the callers may enumerate the list and new items + // can be added when resource bindings are evaluated. + _cachedAllBindings = allBindings.ToArray(); } - public IEnumerable? GetBindingsForProperty(DependencyProperty property) - { - if (_bindings.TryGetValue(property, out var bindingsForProperty)) - { - return bindingsForProperty.Values; - } + return _cachedAllBindings; + } - return null; + public IEnumerable? GetBindingsForProperty(DependencyProperty property) + { + if (_bindings.TryGetValue(property, out var bindingsForProperty)) + { + return bindingsForProperty.Values; } - public void Add(DependencyProperty property, ResourceBinding resourceBinding) + return null; + } + + public void Add(DependencyProperty property, ResourceBinding resourceBinding) + { + if (!_bindings.TryGetValue(property, out var dict)) { - var dict = _bindings.FindOrCreate(property, () => new Dictionary()); - dict[resourceBinding.Precedence] = resourceBinding; + _bindings[property] = dict = new(); } - public void ClearBinding(DependencyProperty property, DependencyPropertyValuePrecedences precedence) + dict[resourceBinding.Precedence] = resourceBinding; + + _cachedAllBindings = null; + } + + public void ClearBinding(DependencyProperty property, DependencyPropertyValuePrecedences precedence) + { + if (_bindings.TryGetValue(property, out var bindingsByPrecedence)) { - if (_bindings.TryGetValue(property, out var bindingsByPrecedence)) - { - bindingsByPrecedence.Remove(precedence); - } + bindingsByPrecedence.Remove(precedence); + + _cachedAllBindings = null; } } } diff --git a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs index f0e7b7e9a806..a1e05d82bd0c 100644 --- a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs +++ b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs @@ -62,7 +62,7 @@ public Uri Source /// Determines if this instance is empty /// internal bool IsEmpty - => Count == 0 + => Count == 0 && ThemeDictionaries.Count == 0 && MergedDictionaries.Count == 0; @@ -203,14 +203,14 @@ private bool TryGetValue(in ResourceKey resourceKey, ResourceKey activeTheme, ou return true; } - if (activeTheme.IsEmpty) + if (GetFromMerged(resourceKey, out value)) { - activeTheme = Themes.Active; + return true; } - if (GetFromMerged(resourceKey, out value)) + if (activeTheme.IsEmpty) { - return true; + activeTheme = Themes.Active; } if (GetFromTheme(resourceKey, activeTheme, out value)) @@ -280,10 +280,24 @@ private void TryMaterializeLazy(in ResourceKey key, ref object value) if (value is LazyInitializer lazyInitializer) { object newValue = null; + bool hasEmptyCurrentScope = lazyInitializer.CurrentScope.Sources.IsEmpty; try { _values.Remove(key); // Temporarily remove the key to make this method safely reentrant, if it's a framework- or application-level theme dictionary - ResourceResolver.PushNewScope(lazyInitializer.CurrentScope); + + if (!hasEmptyCurrentScope) + { + ResourceResolver.PushNewScope(lazyInitializer.CurrentScope); + } + + // Lazy initialized resources must also resolve using the current dictionary + // In previous versions of Uno (4 and earlier), this used to not be needed because all ResourceDictionary + // files where implicitly available at the app level. + if (!FeatureConfiguration.ResourceDictionary.IncludeUnreferencedDictionaries) + { + ResourceResolver.PushSourceToScope(this); + } + newValue = lazyInitializer.Initializer(); } finally @@ -294,7 +308,16 @@ private void TryMaterializeLazy(in ResourceKey key, ref object value) { ResourceDictionaryValueChange?.Invoke(this, EventArgs.Empty); } - ResourceResolver.PopScope(); + + if (!FeatureConfiguration.ResourceDictionary.IncludeUnreferencedDictionaries) + { + ResourceResolver.PopSourceFromScope(); + } + + if (!hasEmptyCurrentScope) + { + ResourceResolver.PopScope(); + } } } } diff --git a/src/Uno.UI/UI/Xaml/ResourceResolver.cs b/src/Uno.UI/UI/Xaml/ResourceResolver.cs index 16fa298f1d60..77324a2d3af4 100644 --- a/src/Uno.UI/UI/Xaml/ResourceResolver.cs +++ b/src/Uno.UI/UI/Xaml/ResourceResolver.cs @@ -30,8 +30,8 @@ public static class ResourceResolver Uno.UI.GlobalStaticResources.MasterDictionary; #endif - private static readonly Dictionary> _registeredDictionariesByUri = new Dictionary>(StringComparer.OrdinalIgnoreCase); - private static readonly Dictionary _registeredDictionariesByAssembly = new Dictionary(StringComparer.Ordinal); + private static readonly Dictionary> _registeredDictionariesByUri = new(StringComparer.OrdinalIgnoreCase); + private static readonly Dictionary _registeredDictionariesByAssembly = new(StringComparer.Ordinal); /// /// This is used by hot reload (since converting the file path to a Source is impractical at runtime). /// @@ -244,7 +244,28 @@ internal static object ResolveTopLevelResource(SpecializedResourceDictionary.Res /// True for {ThemeResource Foo}, false for {StaticResource Foo} /// Optional parameter that provides parse-time context [EditorBrowsable(EditorBrowsableState.Never)] - public static void ApplyResource(DependencyObject owner, DependencyProperty property, object resourceKey, bool isThemeResourceExtension, bool isHotReloadSupported, bool fromXamlParser = false, object context = null) + public static void ApplyResource(DependencyObject owner, DependencyProperty property, object resourceKey, bool isThemeResourceExtension, bool isHotReloadSupported, object context = null) + => ApplyResource( + owner: owner, + property: property, + resourceKey: resourceKey, + isThemeResourceExtension: isThemeResourceExtension, + isHotReloadSupported, + fromXamlParser: false, + context: context); + + /// + /// Apply a StaticResource or ThemeResource assignment to a DependencyProperty of a DependencyObject. The assignment will be provisionally + /// made immediately using Application.Resources if possible, and retried at load-time using the visual-tree scope. + /// + /// Owner of the property + /// The property to assign + /// Key to the resource + /// True for {ThemeResource Foo}, false for {StaticResource Foo} + /// True when the invocation is performed from generated markup code + /// Optional parameter that provides parse-time context + [EditorBrowsable(EditorBrowsableState.Never)] + public static void ApplyResource(DependencyObject owner, DependencyProperty property, object resourceKey, bool isThemeResourceExtension, bool isHotReloadSupported, bool fromXamlParser, object context = null) { var updateReason = ResourceUpdateReason.None; if (isThemeResourceExtension) @@ -394,11 +415,8 @@ private static bool TryStaticRetrieval(in SpecializedResourceDictionary.Resource internal static bool TryTopLevelRetrieval(in SpecializedResourceDictionary.ResourceKey resourceKey, object context, out object value) { value = null; - return (Application.Current?.Resources.TryGetValue(resourceKey, out value, shouldCheckSystem: false) ?? false) - || ( - FeatureConfiguration.ResourceDictionary.IncludeUnreferencedDictionaries - && TryAssemblyResourceRetrieval(resourceKey, context, out value) - ) + return (Application.Current?.Resources.TryGetValue(resourceKey, out value, shouldCheckSystem: false) ?? false) + || TryAssemblyResourceRetrieval(resourceKey, context, out value) || TrySystemResourceRetrieval(resourceKey, out value); } @@ -517,11 +535,21 @@ public static void RegisterResourceDictionaryBySource(string uri, XamlParseConte if (context != null) { - // We store the dictionaries inside a ResourceDictionary to utilize the lazy-loading machinery - var assemblyDict = _registeredDictionariesByAssembly.FindOrCreate(context.AssemblyName, () => new ResourceDictionary()); - var initializer = new ResourceDictionary.ResourceInitializer(dictionary); - _assemblyRef++; // We don't actually use this key, we just need it to be unique - assemblyDict[_assemblyRef] = initializer; + var isGenericXaml = uri.EndsWith("themes/generic.xaml", StringComparison.OrdinalIgnoreCase); + + if (isGenericXaml || FeatureConfiguration.ResourceDictionary.IncludeUnreferencedDictionaries) + { + // We store the dictionaries inside a ResourceDictionary to utilize the lazy-loading machinery + // to convert ResourceDictionary.ResourceInitializer into actual instances + if (!_registeredDictionariesByAssembly.TryGetValue(context.AssemblyName, out var assemblyDict)) + { + _registeredDictionariesByAssembly[context.AssemblyName] = assemblyDict = new(); + } + + var initializer = new ResourceDictionary.ResourceInitializer(dictionary); + _assemblyRef++; // We don't actually use this key, we just need it to be unique + assemblyDict[_assemblyRef] = initializer; + } } if (filePath != null) diff --git a/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs b/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs index ca2a9ac3fe02..f527c4763ade 100644 --- a/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs +++ b/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs @@ -29,7 +29,7 @@ public static ResourceResolverSingleton Instance => _instance ??= new ResourceResolverSingleton(); [EditorBrowsable(EditorBrowsableState.Never)] - public object ResolveResourceStatic(object key, Type type, object context) + public object ResolveResourceStatic(object key, Type type, object context) => ResourceResolver.ResolveResourceStatic(key, type, context); [EditorBrowsable(EditorBrowsableState.Never)] @@ -37,7 +37,7 @@ public void ApplyResource(DependencyObject owner, DependencyProperty property, o => ResourceResolver.ApplyResource(owner, property, resourceKey, isThemeResourceExtension, isHotReloadSupported, true, context); [EditorBrowsable(EditorBrowsableState.Never)] - public object ResolveStaticResourceAlias(string resourceKey, object parseContext) + public object ResolveStaticResourceAlias(string resourceKey, object parseContext) => ResourceResolver.ResolveStaticResourceAlias(resourceKey, parseContext); } } From 00e8ceda7c56d68f9e840f4ac70a0111f194ed19 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Fri, 29 Sep 2023 14:16:00 -0400 Subject: [PATCH 3/5] test: Restore disabled test --- src/Uno.UI.Tests/Windows_UI_Xaml/Given_ThemeResource.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ThemeResource.cs b/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ThemeResource.cs index 6f5481212379..0d8ff599d64b 100644 --- a/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ThemeResource.cs +++ b/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ThemeResource.cs @@ -206,7 +206,6 @@ async Task GoTo(string stateName) } [TestMethod] - [Ignore("DoubleAnimation not supported by Uno.IS_UNIT_TESTS")] public async Task When_Visual_States_DoubleAnimation_Theme_Changed_Reapplied() { var page = new ThemeResource_In_Visual_States_Page(); From 7bd68b6e13b5aead0d70c1b5daf495d6476c4370 Mon Sep 17 00:00:00 2001 From: Jerome Laban Date: Fri, 29 Sep 2023 14:26:23 -0400 Subject: [PATCH 4/5] docs: Adjust for ambient dictionaries --- doc/articles/migrating-from-previous-releases.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/articles/migrating-from-previous-releases.md b/doc/articles/migrating-from-previous-releases.md index 049692fa8c64..82435fff1175 100644 --- a/doc/articles/migrating-from-previous-releases.md +++ b/doc/articles/migrating-from-previous-releases.md @@ -35,6 +35,14 @@ This change ensures that the XAML parser will only look for types in an explicit In order to resolve types properly in a conditional XAML namespace, make use to use the [new syntax introduced in Uno 4.8](https://platform.uno/docs/articles/platform-specific-xaml.html?q=condition#specifying-namespaces). +#### ResourceDictionary now require an explicit Uri reference + +Resources dictionaries are now required to be explicitly referenced by URI to be considered during resource resolution. Applications that are already running properly on WinAppSDK should not be impacted by this change. + +The reason for this change is the alignment of the inclusion behavior with WinUI, which does not automatically place dictionaries as ambiently available. + +This behavior can be disabled by using `FeatureConfiguration.ResourceDictionary.IncludeUnreferencedDictionaries`, by setting the value `true`. + #### `IsEnabled` property is moved from `FrameworkElement` to `Control` This property was incorrectly located on `FrameworkElement` but its behavior has not changed. From 9c6f2d89e09a5bf7ea9e64116137ddf9d3c6ba3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Laban?= Date: Fri, 29 Sep 2023 15:23:58 -0400 Subject: [PATCH 5/5] docs: Adjust spelling --- build/cSpell.json | 1 + 1 file changed, 1 insertion(+) diff --git a/build/cSpell.json b/build/cSpell.json index 6a3376f96db2..9c135ac4d296 100644 --- a/build/cSpell.json +++ b/build/cSpell.json @@ -3,6 +3,7 @@ "language": "en", "words": [ "Avalonia", + "ambiently", "binlog", "Blazor", "blockquotes",