From 56261f73c5a5fc9b032fb730124f8d4692eafbdf Mon Sep 17 00:00:00 2001 From: David Oliver Date: Fri, 14 Aug 2020 13:23:59 -0400 Subject: [PATCH 01/21] chore: Remove BuildPartials() It seems like there's no way _partials can be populated at that moment of execution. --- .../Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index adbc98547804..a5b0575d249d 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -878,7 +878,6 @@ private void BuildTopLevelResourceDictionary(IIndentedStringBuilder writer, Xaml AnalyzerSuppressionsGenerator.Generate(writer, _analyzerSuppressions); using (writer.BlockInvariant("public sealed partial class GlobalStaticResources")) { - BuildPartials(writer, isStatic: true); BuildResourceDictionaryGlobalProperties(writer, topLevelControl); var themeDictionaryMember = topLevelControl.Members.FirstOrDefault(m => m.Member.Name == "ThemeDictionaries"); From 0fd7cca13f4566c389ec221d0283674d0ebc3b49 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Fri, 14 Aug 2020 15:19:27 -0400 Subject: [PATCH 02/21] chore: Adjust annotated Xaml generation for debugging Don't try to decorate a null string, because this creates a non-null string, which can alter the generated code in undesired ways. --- .../Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index a5b0575d249d..358b65bafed4 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -5156,7 +5156,7 @@ private void TryAnnotateWithGeneratorSource(IIndentedStringBuilder writer, [Call private void TryAnnotateWithGeneratorSource(ref string str, [CallerMemberName] string callerName = null, [CallerLineNumber] int lineNumber = 0) { - if (_shouldAnnotateGeneratedXaml) + if (_shouldAnnotateGeneratedXaml && str != null) { str = GetGeneratorSourceAnnotation(callerName, lineNumber) + str; } From b6380f0721a13e1714a9102da1c2f2ec38407bab Mon Sep 17 00:00:00 2001 From: David Oliver Date: Fri, 14 Aug 2020 15:23:10 -0400 Subject: [PATCH 03/21] fix(xaml): Reduce size of AOT'ed compilations from Xaml Place ResourceDictionary properties (which may be generated in very large numbers) inside a non-static singleton class. This avoids a binary-size penalty from accessing static members in a static context, which adds checks for the static class initializer each time. --- .../XamlGenerator/XamlFileGenerator.cs | 68 ++++++++++++++++--- 1 file changed, 58 insertions(+), 10 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index 358b65bafed4..375fb3ca55e4 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -77,6 +77,10 @@ internal partial class XamlFileGenerator /// private bool _isTopLevelDictionary; private readonly bool _isUnoAssembly; + /// + /// True if the generator is currently creating child subclasses (for templates, etc) + /// + private bool _inChildSubclass = false; /// /// The current DefaultBindMode for x:Bind bindings, as set by app code for the current Xaml subtree. @@ -125,6 +129,10 @@ internal partial class XamlFileGenerator _defaultNamespace, XamlCodeGeneration.ParseContextPropertyName ); + /// + /// Name to use for inner singleton containing top-level ResourceDictionary properties + /// + private string SingletonClassName => $"ResourceDictionarySingleton__{_fileDefinition.ShortId}"; static XamlFileGenerator() { @@ -662,6 +670,7 @@ private static string SanitizeResourceName(string name) private void BuildChildSubclasses(IIndentedStringBuilder writer, bool isTopLevel = false) { + _inChildSubclass = true; TryAnnotateWithGeneratorSource(writer); var disposable = isTopLevel ? writer.BlockInvariant("namespace {0}.__Resources", _defaultNamespace) : null; @@ -878,6 +887,27 @@ private void BuildTopLevelResourceDictionary(IIndentedStringBuilder writer, Xaml AnalyzerSuppressionsGenerator.Generate(writer, _analyzerSuppressions); using (writer.BlockInvariant("public sealed partial class GlobalStaticResources")) { + var hasDefaultStyles = false; + writer.AppendLineInvariant("// This non-static inner class is a means of reducing size of AOT compilations by avoiding many accesses to static members from a static callsite, which adds costly class initializer checks each time."); + using (writer.BlockInvariant("internal sealed class {0}", SingletonClassName)) + { + // Build singleton + writer.AppendLineInvariant("private static {0} _instance;", SingletonClassName); + using (writer.BlockInvariant("internal static {0} Instance", SingletonClassName)) + { + using (writer.BlockInvariant("get")) + { + using (writer.BlockInvariant("if (_instance == null)")) + { + writer.AppendLineInvariant("_instance = new {0}();", SingletonClassName); + } + writer.AppendLine(); + writer.AppendLineInvariant("return _instance;"); + } + } + writer.AppendLine(); + writer.AppendLineInvariant("private {0}() {{ }}", SingletonClassName); + writer.AppendLine(); BuildResourceDictionaryGlobalProperties(writer, topLevelControl); var themeDictionaryMember = topLevelControl.Members.FirstOrDefault(m => m.Member.Name == "ThemeDictionaries"); @@ -887,9 +917,10 @@ private void BuildTopLevelResourceDictionary(IIndentedStringBuilder writer, Xaml BuildResourceDictionaryGlobalProperties(writer, dict); } - writer.AppendLineInvariant("private static global::Windows.UI.Xaml.ResourceDictionary _{0}_ResourceDictionary;", _fileUniqueId); + TryAnnotateWithGeneratorSource(writer); + writer.AppendLineInvariant("private global::Windows.UI.Xaml.ResourceDictionary _{0}_ResourceDictionary;", _fileUniqueId); writer.AppendLine(); - using (writer.BlockInvariant("internal static global::Windows.UI.Xaml.ResourceDictionary {0}_ResourceDictionary", _fileUniqueId)) + using (writer.BlockInvariant("internal global::Windows.UI.Xaml.ResourceDictionary {0}_ResourceDictionary", _fileUniqueId)) { using (writer.BlockInvariant("get")) { @@ -907,7 +938,14 @@ private void BuildTopLevelResourceDictionary(IIndentedStringBuilder writer, Xaml } } - BuildDefaultStylesRegistration(writer, FindImplicitContentMember(topLevelControl)); + hasDefaultStyles = BuildDefaultStylesRegistration(writer, FindImplicitContentMember(topLevelControl)); + } + writer.AppendLine(); + writer.AppendLineInvariant("internal static global::Windows.UI.Xaml.ResourceDictionary {0}_ResourceDictionary => {1}.Instance.{0}_ResourceDictionary;", _fileUniqueId, SingletonClassName); + if (hasDefaultStyles) + { + writer.AppendLineInvariant("static partial void RegisterDefaultStyles_{0}() => {1}.Instance.RegisterDefaultStyles_{0}();", _fileUniqueId, SingletonClassName); + } } } @@ -1082,18 +1120,18 @@ private string GetImplicitDictionaryResourceKey(XamlObjectDefinition resource) /// Note: if we're currently building an app, these registrations will never actually be called (the styles will be treated as implicit styles /// instead). /// - private void BuildDefaultStylesRegistration(IIndentedStringBuilder writer, XamlMemberDefinition resourcesRoot) + private bool BuildDefaultStylesRegistration(IIndentedStringBuilder writer, XamlMemberDefinition resourcesRoot) { TryAnnotateWithGeneratorSource(writer); var stylesCandidates = resourcesRoot?.Objects.Where(o => o.Type.Name == "Style" && GetExplicitDictionaryResourceKey(o, out var _) == null); if (stylesCandidates?.None() ?? true) { - return; + return false; } writer.AppendLine(); - using (writer.BlockInvariant("static partial void RegisterDefaultStyles_{0}()", _fileUniqueId)) + using (writer.BlockInvariant("public void RegisterDefaultStyles_{0}()", _fileUniqueId)) { foreach (var style in stylesCandidates) { @@ -1120,6 +1158,8 @@ private void BuildDefaultStylesRegistration(IIndentedStringBuilder writer, XamlM } } } + + return true; } /// @@ -1195,7 +1235,7 @@ private void BuildResourceDictionaryBackingClass(IIndentedStringBuilder writer, /// The name of the property. /// Function that builds the initialization logic for the property. /// - private void BuildSingleTimeInitializer(IIndentedStringBuilder writer, string propertyType, string propertyName, Action propertyBodyBuilder, bool isStatic = true) + private void BuildSingleTimeInitializer(IIndentedStringBuilder writer, string propertyType, string propertyName, Action propertyBodyBuilder, bool isStatic = false) { TryAnnotateWithGeneratorSource(writer); // The property type may be partially qualified, try resolving it through FindType @@ -3686,15 +3726,23 @@ private string GetSimpleStaticResourceRetrieval(INamedTypeSymbol targetPropertyT } /// - /// Get the name of global static property associated with the given resource key, if one exists, otherwise null. + /// Get the name of top-level ResourceDictionary property associated with the given resource key, if one exists, otherwise null. /// - private string GetResourceDictionaryPropertyName(string keyStr) => GetResourceDictionaryPropertyDetails(keyStr).PropertyName; + private string GetResourceDictionaryPropertyName(string keyStr) + { + var propertyName = GetResourceDictionaryPropertyDetails(keyStr).PropertyName; + TryAnnotateWithGeneratorSource(ref propertyName); + return propertyName; + } private (string PropertyName, INamedTypeSymbol PropertyType) GetResourceDictionaryPropertyDetails(string keyStr) { if (_topLevelDictionaryProperties.TryGetValue((_themeDictionaryCurrentlyBuilding, keyStr), out var propertyDetails)) { - return ("global::{0}.GlobalStaticResources.{1} /*{2}*/".InvariantCultureFormat(_defaultNamespace, propertyDetails.PropertyName, keyStr), propertyDetails.PropertyType); + var propertyAccess = _inChildSubclass ? + "global::{0}.GlobalStaticResources.{1}.Instance.{2} /*{3}*/".InvariantCultureFormat(_defaultNamespace, SingletonClassName, propertyDetails.PropertyName, keyStr) + : "this.{0} /*{1}*/".InvariantCultureFormat(propertyDetails.PropertyName, keyStr); + return (propertyAccess, propertyDetails.PropertyType); } return (null, null); From 9adc5323c26495130083e0bea45debe0b7b91424 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Sun, 16 Aug 2020 05:49:30 -0400 Subject: [PATCH 04/21] chore(xaml): Adjust parse context to avoid static access Access Xaml parse context in Xaml-generated code as an instance property wherever possible, to avoid the cost in AOT compilation of access to a static member from a static context. --- .../XamlGenerator/XamlCodeGeneration.cs | 3 +- .../XamlGenerator/XamlFileGenerator.cs | 117 ++++++++++++------ 2 files changed, 84 insertions(+), 36 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlCodeGeneration.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlCodeGeneration.cs index d8bdb2bf2735..9efc8bd0bc14 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlCodeGeneration.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlCodeGeneration.cs @@ -20,6 +20,7 @@ namespace Uno.UI.SourceGenerators.XamlGenerator internal partial class XamlCodeGeneration { internal const string ParseContextPropertyName = "__ParseContext_"; + internal const string ParseContextPropertyType = "global::Uno.UI.Xaml.XamlParseContext"; private readonly string[] _xamlSourceFiles; private readonly string[] _xamlSourceLinks; @@ -498,7 +499,7 @@ private string GenerateGlobalResources(IEnumerable files, Xa writer.AppendLineInvariant("private static bool _stylesRegistered;"); writer.AppendLineInvariant("private static bool _dictionariesRegistered;"); - using (writer.BlockInvariant("internal static global::Uno.UI.Xaml.XamlParseContext {0} {{get; }} = new global::Uno.UI.Xaml.XamlParseContext()", ParseContextPropertyName)) + using (writer.BlockInvariant("internal static {0} {1} {{get; }} = new {0}()", ParseContextPropertyType, ParseContextPropertyName)) { writer.AppendLineInvariant("AssemblyName = \"{0}\",", _projectInstance.GetPropertyValue("AssemblyName")); } diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index 375fb3ca55e4..d24b67e60dd2 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -80,7 +80,11 @@ internal partial class XamlFileGenerator /// /// True if the generator is currently creating child subclasses (for templates, etc) /// - private bool _inChildSubclass = false; + private bool _isInChildSubclass = false; + /// + /// True if the generator is currently creating the inner singleton class associated with a top-level resource dictionary + /// + private bool _isInSingletonInstance = false; /// /// The current DefaultBindMode for x:Bind bindings, as set by app code for the current Xaml subtree. @@ -124,11 +128,32 @@ internal partial class XamlFileGenerator /// private readonly bool _shouldAnnotateGeneratedXaml; - private string ParseContextPropertyAccess => "{0}{1}.GlobalStaticResources.{2}".InvariantCultureFormat( - GlobalPrefix, - _defaultNamespace, - XamlCodeGeneration.ParseContextPropertyName - ); + private string ParseContextPropertyAccess => + (_isTopLevelDictionary, _isInSingletonInstance) switch + { + (true, false) => "{0}{1}.GlobalStaticResources.{2}.Instance.{3}".InvariantCultureFormat( + GlobalPrefix, + _defaultNamespace, + SingletonClassName, + XamlCodeGeneration.ParseContextPropertyName + ), + (true, true) => "this.{0}".InvariantCultureFormat(XamlCodeGeneration.ParseContextPropertyName), + _ => "{0}{1}.GlobalStaticResources.{2}".InvariantCultureFormat( + GlobalPrefix, + _defaultNamespace, + XamlCodeGeneration.ParseContextPropertyName + ) + }; + /* + _isInSingletonInstance ? + "this.{0}".InvariantCultureFormat(XamlCodeGeneration.ParseContextPropertyName) + : "{0}{1}.GlobalStaticResources.{2}.Instance.{3}".InvariantCultureFormat( + GlobalPrefix, + _defaultNamespace, + SingletonClassName, + XamlCodeGeneration.ParseContextPropertyName + ); + */ /// /// Name to use for inner singleton containing top-level ResourceDictionary properties /// @@ -670,7 +695,7 @@ private static string SanitizeResourceName(string name) private void BuildChildSubclasses(IIndentedStringBuilder writer, bool isTopLevel = false) { - _inChildSubclass = true; + _isInChildSubclass = true; TryAnnotateWithGeneratorSource(writer); var disposable = isTopLevel ? writer.BlockInvariant("namespace {0}.__Resources", _defaultNamespace) : null; @@ -887,9 +912,21 @@ private void BuildTopLevelResourceDictionary(IIndentedStringBuilder writer, Xaml AnalyzerSuppressionsGenerator.Generate(writer, _analyzerSuppressions); using (writer.BlockInvariant("public sealed partial class GlobalStaticResources")) { + + IDisposable WrapSingleton() + { + writer.AppendLineInvariant("// This non-static inner class is a means of reducing size of AOT compilations by avoiding many accesses to static members from a static callsite, which adds costly class initializer checks each time."); + var block = writer.BlockInvariant("internal sealed class {0}", SingletonClassName); + _isInSingletonInstance = true; + return new DisposableAction(() => + { + block.Dispose(); + _isInSingletonInstance = false; + }); + } + var hasDefaultStyles = false; - writer.AppendLineInvariant("// This non-static inner class is a means of reducing size of AOT compilations by avoiding many accesses to static members from a static callsite, which adds costly class initializer checks each time."); - using (writer.BlockInvariant("internal sealed class {0}", SingletonClassName)) + using (WrapSingleton()) { // Build singleton writer.AppendLineInvariant("private static {0} _instance;", SingletonClassName); @@ -906,37 +943,47 @@ private void BuildTopLevelResourceDictionary(IIndentedStringBuilder writer, Xaml } } writer.AppendLine(); - writer.AppendLineInvariant("private {0}() {{ }}", SingletonClassName); + writer.AppendLineInvariant("internal {0} {1} {{get; }}", XamlCodeGeneration.ParseContextPropertyType, XamlCodeGeneration.ParseContextPropertyName); writer.AppendLine(); - BuildResourceDictionaryGlobalProperties(writer, topLevelControl); + using (writer.BlockInvariant("private {0}()", SingletonClassName)) + { + var outerProperty = "{0}{1}.GlobalStaticResources.{2}".InvariantCultureFormat( + GlobalPrefix, + _defaultNamespace, + XamlCodeGeneration.ParseContextPropertyName + ); + writer.AppendLineInvariant("{0} = {1};", XamlCodeGeneration.ParseContextPropertyName, outerProperty); + } + writer.AppendLine(); + BuildResourceDictionaryGlobalProperties(writer, topLevelControl); - var themeDictionaryMember = topLevelControl.Members.FirstOrDefault(m => m.Member.Name == "ThemeDictionaries"); + var themeDictionaryMember = topLevelControl.Members.FirstOrDefault(m => m.Member.Name == "ThemeDictionaries"); - foreach (var dict in (themeDictionaryMember?.Objects).Safe()) - { - BuildResourceDictionaryGlobalProperties(writer, dict); - } + foreach (var dict in (themeDictionaryMember?.Objects).Safe()) + { + BuildResourceDictionaryGlobalProperties(writer, dict); + } TryAnnotateWithGeneratorSource(writer); writer.AppendLineInvariant("private global::Windows.UI.Xaml.ResourceDictionary _{0}_ResourceDictionary;", _fileUniqueId); - writer.AppendLine(); + writer.AppendLine(); using (writer.BlockInvariant("internal global::Windows.UI.Xaml.ResourceDictionary {0}_ResourceDictionary", _fileUniqueId)) - { - using (writer.BlockInvariant("get")) { - using (writer.BlockInvariant("if (_{0}_ResourceDictionary == null)", _fileUniqueId)) + using (writer.BlockInvariant("get")) { - writer.AppendLineInvariant("_{0}_ResourceDictionary = ", _fileUniqueId); - InitializeAndBuildResourceDictionary(writer, topLevelControl, setIsParsing: true); - writer.AppendLineInvariant(";"); - var url = _globalStaticResourcesMap.GetSourceLink(_fileDefinition); - writer.AppendLineInvariant("_{0}_ResourceDictionary.Source = new global::System.Uri(\"{1}{2}\");", _fileUniqueId, XamlFilePathHelper.LocalResourcePrefix, url); - writer.AppendLineInvariant("_{0}_ResourceDictionary.CreationComplete();", _fileUniqueId); - } + using (writer.BlockInvariant("if (_{0}_ResourceDictionary == null)", _fileUniqueId)) + { + writer.AppendLineInvariant("_{0}_ResourceDictionary = ", _fileUniqueId); + InitializeAndBuildResourceDictionary(writer, topLevelControl, setIsParsing: true); + writer.AppendLineInvariant(";"); + var url = _globalStaticResourcesMap.GetSourceLink(_fileDefinition); + writer.AppendLineInvariant("_{0}_ResourceDictionary.Source = new global::System.Uri(\"{1}{2}\");", _fileUniqueId, XamlFilePathHelper.LocalResourcePrefix, url); + writer.AppendLineInvariant("_{0}_ResourceDictionary.CreationComplete();", _fileUniqueId); + } - writer.AppendLineInvariant("return _{0}_ResourceDictionary;", _fileUniqueId); + writer.AppendLineInvariant("return _{0}_ResourceDictionary;", _fileUniqueId); + } } - } hasDefaultStyles = BuildDefaultStylesRegistration(writer, FindImplicitContentMember(topLevelControl)); } @@ -988,7 +1035,7 @@ private void BuildResourceDictionaryGlobalProperties(IIndentedStringBuilder writ // overrides the aliased value. Perf impact needs to be evaluated. || _isUnoAssembly) { - _topLevelDictionaryProperties[(theme, key)] = (propertyName, FindType(resource.Type)); + _topLevelDictionaryProperties[(theme, key)] = (propertyName, FindType(resource.Type)); } } @@ -1011,7 +1058,7 @@ private void BuildResourceDictionaryGlobalProperties(IIndentedStringBuilder writ // overrides the aliased value. Perf impact needs to be evaluated. && !_isUnoAssembly) { - writer.AppendLineInvariant("// Skipping static property {0} for {1} {2} - StaticResource ResourceKey aliases are added directly to dictionary",_dictionaryPropertyIndex, key, theme); + writer.AppendLineInvariant("// Skipping static property {0} for {1} {2} - StaticResource ResourceKey aliases are added directly to dictionary", _dictionaryPropertyIndex, key, theme); } else { @@ -1477,7 +1524,7 @@ private void BuildPropertySetter(IIndentedStringBuilder writer, string fullTarge ); } - var valueObject = valueNode.Objects.First(); + var valueObject = valueNode.Objects.First(); if (HasMarkupExtension(valueNode)) { writer.AppendLineInvariant(BuildBindingOption(valueNode, propertyType, prependCastToType: true)); @@ -2378,7 +2425,7 @@ private void BuildDictionaryFromSource(IIndentedStringBuilder writer, XamlMember var source = (sourceDef?.Value as string)?.Replace('\\', '/'); var customResourceResourceId = GetCustomResourceResourceId(sourceDef); - + if (source == null && customResourceResourceId == null) { return; @@ -3739,7 +3786,7 @@ private string GetResourceDictionaryPropertyName(string keyStr) { if (_topLevelDictionaryProperties.TryGetValue((_themeDictionaryCurrentlyBuilding, keyStr), out var propertyDetails)) { - var propertyAccess = _inChildSubclass ? + var propertyAccess = _isInChildSubclass ? "global::{0}.GlobalStaticResources.{1}.Instance.{2} /*{3}*/".InvariantCultureFormat(_defaultNamespace, SingletonClassName, propertyDetails.PropertyName, keyStr) : "this.{0} /*{1}*/".InvariantCultureFormat(propertyDetails.PropertyName, keyStr); return (propertyAccess, propertyDetails.PropertyType); @@ -4118,7 +4165,7 @@ private static string BuildThickness(string memberValue) // Append 'f' to every decimal value in the thickness memberValue = AppendFloatSuffix(memberValue); } - + return "new global::Windows.UI.Xaml.Thickness(" + memberValue + ")"; } From 3c93d9565b58725d13b14a79a3b94f9113ad0027 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Wed, 16 Sep 2020 13:03:20 -0400 Subject: [PATCH 05/21] chore: Remove direct property special-casing for StaticResource aliases Preparatory to ripping out direct properties completely. --- .../XamlGenerator/XamlFileGenerator.cs | 30 ++++--------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index d24b67e60dd2..37f3f217141b 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -759,7 +759,7 @@ private void BuildCompiledBindingsInitializer(IndentedStringBuilder writer, stri writer.AppendLineInvariant($"Bindings = new {GetBindingsTypeNames(className).bindingsClassName}(this);"); } - if(hasXBindExpressions || hasResourceExtensions) + if (hasXBindExpressions || hasResourceExtensions) { using (writer.BlockInvariant($"Loading += delegate")) { @@ -772,7 +772,7 @@ private void BuildCompiledBindingsInitializer(IndentedStringBuilder writer, stri { var component = CurrentScope.Components[i]; - if(HasMarkupExtensionNeedingComponent(component) && IsDependencyObject(component)) + if (HasMarkupExtensionNeedingComponent(component) && IsDependencyObject(component)) { writer.AppendLineInvariant($"_component_{i}.UpdateResourceBindings();"); } @@ -1030,10 +1030,7 @@ private void BuildResourceDictionaryGlobalProperties(IIndentedStringBuilder writ throw new InvalidOperationException($"Dictionary Item {resource?.Type?.Name} has duplicate key `{key}` { (theme != null ? $" in theme {theme}" : "")}."); } var isStaticResourceAlias = resource.Type.Name == "StaticResource"; - if (!isStaticResourceAlias - // TODO: this case should be eventually removed, and the same behaviour applied within Uno.UI, to support the scenario where app code - // overrides the aliased value. Perf impact needs to be evaluated. - || _isUnoAssembly) + if (!isStaticResourceAlias) { _topLevelDictionaryProperties[(theme, key)] = (propertyName, FindType(resource.Type)); } @@ -1053,10 +1050,7 @@ private void BuildResourceDictionaryGlobalProperties(IIndentedStringBuilder writ _dictionaryPropertyIndex++; var isStaticResourceAlias = resource.Type.Name == "StaticResource"; - if (isStaticResourceAlias - // TODO: this case should be eventually removed, and the same behaviour applied within Uno.UI, to support the scenario where app code - // overrides the aliased value. Perf impact needs to be evaluated. - && !_isUnoAssembly) + if (isStaticResourceAlias) { writer.AppendLineInvariant("// Skipping static property {0} for {1} {2} - StaticResource ResourceKey aliases are added directly to dictionary", _dictionaryPropertyIndex, key, theme); } @@ -1093,21 +1087,7 @@ private void BuildStaticResourceResourceKeyReference(IIndentedStringBuilder writ TryAnnotateWithGeneratorSource(writer); var targetKey = resourceDefinition.Members.FirstOrDefault(m => m.Member.Name == "ResourceKey")?.Value as string; - var directProperty = GetResourceDictionaryPropertyName(targetKey); - if (directProperty != null) - { - // TODO: this case should be eventually removed, even when a match is present in the same dictionary, it should insert the passthrough to allow for - // the scenario where app code overrides the aliased value. Perf impact needs to be evaluated. - writer.AppendLineInvariant(directProperty); - } - else if (_isUnoAssembly) - { - writer.AppendLineInvariant(GetSimpleStaticResourceRetrieval(null, targetKey)); - } - else - { - writer.AppendLineInvariant("global::Uno.UI.ResourceResolver.ResolveStaticResourceAlias(\"{0}\", {1})", targetKey, ParseContextPropertyAccess); - } + writer.AppendLineInvariant("global::Uno.UI.ResourceResolver.ResolveStaticResourceAlias(\"{0}\", {1})", targetKey, ParseContextPropertyAccess); } /// From 3ae25da65cdf6cedd644fa1c5937ef0c3e11e539 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Wed, 16 Sep 2020 13:07:57 -0400 Subject: [PATCH 06/21] chore: Remove out-of-date comment It turned out to be necessary. --- .../XamlGenerator/XamlFileGenerator.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index 37f3f217141b..02afa096da97 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -3238,12 +3238,6 @@ string GetBindingOptions() } else if (IsDependencyProperty(member.Member)) { - // To be fully compatible with UWP here, we should check the nearest dictionary in the 'XAML scope' then outwards. For - // StaticResource extension that would be it, for ThemeResource we'd do additional resolution at load-time. - // - // Instead, Uno immediately sets any Application-level value, if it exists. Then we do load-time resolution by tree-walking - // for StaticResource *and* ThemeResource. (Note that initialize-time XAML scope resolution should be possible to implement, - // should it turn out to be necessary.) var propertyOwner = GetType(member.Member.DeclaringType); writer.AppendLineInvariant("global::Uno.UI.ResourceResolver.ApplyResource({0}, {1}.{2}Property, \"{3}\", isThemeResourceExtension: {4}, context: {5});", closureName, GetGlobalizedTypeName(propertyOwner.ToDisplayString()), member.Member.Name, resourceKey, isThemeResourceExtension ? "true" : "false", ParseContextPropertyAccess); } From a57e6d6a00207ea0d3a55589e05709c34f30db25 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Wed, 16 Sep 2020 14:18:11 -0400 Subject: [PATCH 07/21] chore: Avoid use of direct properties in parsed ResourceDictionaries Cut back on use of direct properties, preparatory to removing them completely. (The last holdout is default styles registration.) --- .../XamlGenerator/XamlFileGenerator.cs | 59 +++---------------- 1 file changed, 9 insertions(+), 50 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index 02afa096da97..843e2b4389a7 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -2230,22 +2230,19 @@ private void BuildResourceDictionary(IIndentedStringBuilder writer, XamlMemberDe { writer.AppendLineInvariant("{0}[{1}] = ", dictIdentifier, wrappedKey); } - var directproperty = GetResourceDictionaryPropertyName(key); - using (ShouldLazyInitializeResource(resource) ? BuildLazyResourceInitializer(writer) : null) + + if (resource.Type.Name == "StaticResource") { - if (resource.Type.Name == "StaticResource") // Direct properties aren't built for StaticResource aliases - { - BuildStaticResourceResourceKeyReference(writer, resource); - } - else if (directproperty != null) - { - writer.AppendLineInvariant(directproperty); - } - else + BuildStaticResourceResourceKeyReference(writer, resource); + } + else + { + using (BuildLazyResourceInitializer(writer)) { BuildChild(writer, null, resource); } } + writer.AppendLineInvariant(closingPunctuation); } @@ -3204,39 +3201,8 @@ string GetBindingOptions() if (resourceKey != null) { TryAnnotateWithGeneratorSource(writer); - (var directProperty, var directPropertyType) = GetResourceDictionaryPropertyDetails(resourceKey); - - if (directProperty != null) - { - // Prefer direct property reference (when we are in top-level ResourceDictionary and referencing resource in same dictionary) - var type = FindPropertyType(member.Member); - string rightSide; - if (type?.Name == "TimeSpan") - { - // explicit support for TimeSpan because we can't override the parsing. - rightSide = "global::System.TimeSpan.Parse({0})".InvariantCultureFormat(directProperty); - } - else - { - var rightSideInner = directProperty; - if (isThemeResourceExtension && type != null && !type.Equals(directPropertyType)) - { - // ThemeResource assignations are 'binding-like' in that they permit assignations that couldn't be directly made - perform the conversion if necessary - rightSideInner = $"global::Windows.UI.Xaml.Markup.XamlBindingHelper.ConvertValue(typeof({type}), {directProperty})"; - } - rightSide = "{0}{1}".InvariantCultureFormat(GetCastString(type, null), rightSideInner); - } - if (generateAssignation) - { - writer.AppendLineInvariant(formatLine("{0} = {1}"), member.Member.Name, rightSide); - } - else - { - writer.AppendLineInvariant(rightSide); - } - } - else if (IsDependencyProperty(member.Member)) + if (IsDependencyProperty(member.Member)) { var propertyOwner = GetType(member.Member.DeclaringType); writer.AppendLineInvariant("global::Uno.UI.ResourceResolver.ApplyResource({0}, {1}.{2}Property, \"{3}\", isThemeResourceExtension: {4}, context: {5});", closureName, GetGlobalizedTypeName(propertyOwner.ToDisplayString()), member.Member.Name, resourceKey, isThemeResourceExtension ? "true" : "false", ParseContextPropertyAccess); @@ -3717,13 +3683,6 @@ private string GetSimpleStaticResourceRetrieval(XamlMemberDefinition member, INa /// private string GetSimpleStaticResourceRetrieval(INamedTypeSymbol targetPropertyType, string keyStr) { - var directProperty = GetResourceDictionaryPropertyName(keyStr); - if (directProperty != null) - { - TryAnnotateWithGeneratorSource(ref directProperty); - return directProperty; - } - targetPropertyType = targetPropertyType ?? _objectSymbol; var targetPropertyFQT = targetPropertyType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); From 345b3db98caac569a274c18c2027ee78e4338b89 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Wed, 16 Sep 2020 14:39:20 -0400 Subject: [PATCH 08/21] chore: Add suffix option to Xaml generator source method decoration Makes it easier to interpret sources that aren't the entry point of the method. --- .../XamlGenerator/XamlFileGenerator.cs | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index 843e2b4389a7..3812401ab78a 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -964,7 +964,7 @@ IDisposable WrapSingleton() BuildResourceDictionaryGlobalProperties(writer, dict); } - TryAnnotateWithGeneratorSource(writer); + TryAnnotateWithGeneratorSource(writer, suffix: "DictionaryProperty"); writer.AppendLineInvariant("private global::Windows.UI.Xaml.ResourceDictionary _{0}_ResourceDictionary;", _fileUniqueId); writer.AppendLine(); using (writer.BlockInvariant("internal global::Windows.UI.Xaml.ResourceDictionary {0}_ResourceDictionary", _fileUniqueId)) @@ -2212,6 +2212,7 @@ private void BuildResourceDictionary(IIndentedStringBuilder writer, XamlMemberDe foreach (var resource in (resourcesRoot?.Objects).Safe()) { + TryAnnotateWithGeneratorSource(writer, suffix: "PerKey"); var key = GetDictionaryResourceKey(resource, out var name); if (key != null) @@ -3171,7 +3172,7 @@ string GetBindingOptions() if (bindingOptions != null) { - TryAnnotateWithGeneratorSource(writer); + TryAnnotateWithGeneratorSource(writer, suffix: "HasBindingOptions"); var isAttachedProperty = IsDependencyProperty(member.Member); var isBindingType = Equals(FindPropertyType(member.Member), _dataBindingSymbol); var isOwnerDependencyObject = member.Owner != null ? GetType(member.Owner.Type).GetAllInterfaces().Any(i => Equals(i, _dependencyObjectSymbol)) : false; @@ -3200,7 +3201,7 @@ string GetBindingOptions() if (resourceKey != null) { - TryAnnotateWithGeneratorSource(writer); + TryAnnotateWithGeneratorSource(writer, suffix: "HasResourceKey"); if (IsDependencyProperty(member.Member)) { @@ -5174,25 +5175,29 @@ private IDisposable Scope(string name) /// /// If flag is set, decorate the generated code with a marker of the current method. Useful for pinpointing the source of a bug or other undesired behavior. /// - private void TryAnnotateWithGeneratorSource(IIndentedStringBuilder writer, [CallerMemberName] string callerName = null, [CallerLineNumber] int lineNumber = 0) + private void TryAnnotateWithGeneratorSource(IIndentedStringBuilder writer, string suffix = null, [CallerMemberName] string callerName = null, [CallerLineNumber] int lineNumber = 0) { if (_shouldAnnotateGeneratedXaml) { - writer.Append(GetGeneratorSourceAnnotation(callerName, lineNumber)); + writer.Append(GetGeneratorSourceAnnotation(callerName, lineNumber, suffix)); } } - private void TryAnnotateWithGeneratorSource(ref string str, [CallerMemberName] string callerName = null, [CallerLineNumber] int lineNumber = 0) + private void TryAnnotateWithGeneratorSource(ref string str, string suffix = null, [CallerMemberName] string callerName = null, [CallerLineNumber] int lineNumber = 0) { if (_shouldAnnotateGeneratedXaml && str != null) { - str = GetGeneratorSourceAnnotation(callerName, lineNumber) + str; + str = GetGeneratorSourceAnnotation(callerName, lineNumber, suffix) + str; } } - private static string GetGeneratorSourceAnnotation(string callerName, int lineNumber) + private static string GetGeneratorSourceAnnotation(string callerName, int lineNumber, string suffix) { - return "/*{0} L:{1}*/".InvariantCultureFormat(callerName, lineNumber); + if (suffix != null) + { + suffix = "-" + suffix; + } + return "/*{0} L:{1}{2}*/".InvariantCultureFormat(callerName, lineNumber, suffix); } } } From b8c339557b95707fb1e89521f0d3369404c5eb47 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Thu, 17 Sep 2020 06:16:49 -0400 Subject: [PATCH 09/21] chore: Remove direct ResourceDictionary properties Adjust default style registration to retrieve the style from the containing ResourceDictionary, provided by the singleton. --- .../XamlGenerator/XamlFileGenerator.cs | 54 +++++++------------ .../Xaml/IXamlResourceDictionaryProvider.cs | 26 +++++++++ src/Uno.UI/UI/Xaml/Style/Style.cs | 32 ++++++++++- 3 files changed, 74 insertions(+), 38 deletions(-) create mode 100644 src/Uno.UI/UI/Xaml/IXamlResourceDictionaryProvider.cs diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index 3812401ab78a..bad8dbf308eb 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -144,21 +144,22 @@ internal partial class XamlFileGenerator XamlCodeGeneration.ParseContextPropertyName ) }; - /* - _isInSingletonInstance ? - "this.{0}".InvariantCultureFormat(XamlCodeGeneration.ParseContextPropertyName) - : "{0}{1}.GlobalStaticResources.{2}.Instance.{3}".InvariantCultureFormat( - GlobalPrefix, - _defaultNamespace, - SingletonClassName, - XamlCodeGeneration.ParseContextPropertyName - ); - */ + + private string SingletonInstanceAccess => _isInSingletonInstance ? + "this" : + "{0}{1}.GlobalStaticResources.{2}.Instance".InvariantCultureFormat( + GlobalPrefix, + _defaultNamespace, + SingletonClassName + ); + /// /// Name to use for inner singleton containing top-level ResourceDictionary properties /// private string SingletonClassName => $"ResourceDictionarySingleton__{_fileDefinition.ShortId}"; + private const string DictionaryProviderInterfaceName = "global::Uno.UI.IXamlResourceDictionaryProvider"; + static XamlFileGenerator() { _findContentProperty = Funcs.Create(SourceFindContentProperty).AsLockedMemoized(); @@ -916,7 +917,7 @@ private void BuildTopLevelResourceDictionary(IIndentedStringBuilder writer, Xaml IDisposable WrapSingleton() { writer.AppendLineInvariant("// This non-static inner class is a means of reducing size of AOT compilations by avoiding many accesses to static members from a static callsite, which adds costly class initializer checks each time."); - var block = writer.BlockInvariant("internal sealed class {0}", SingletonClassName); + var block = writer.BlockInvariant("internal sealed class {0} : {1}", SingletonClassName, DictionaryProviderInterfaceName); _isInSingletonInstance = true; return new DisposableAction(() => { @@ -986,6 +987,9 @@ IDisposable WrapSingleton() } hasDefaultStyles = BuildDefaultStylesRegistration(writer, FindImplicitContentMember(topLevelControl)); + + writer.AppendLine(); + writer.AppendLineInvariant("ResourceDictionary {0}.GetResourceDictionary() => {1}_ResourceDictionary;", DictionaryProviderInterfaceName, _fileUniqueId); } writer.AppendLine(); writer.AppendLineInvariant("internal static global::Windows.UI.Xaml.ResourceDictionary {0}_ResourceDictionary => {1}.Instance.{0}_ResourceDictionary;", _fileUniqueId, SingletonClassName); @@ -1007,6 +1011,7 @@ IDisposable WrapSingleton() /// The associated with the dictionary. private void BuildResourceDictionaryGlobalProperties(IIndentedStringBuilder writer, XamlObjectDefinition dictionaryObject) { + return; // This will be refactored forthwith TryAnnotateWithGeneratorSource(writer); var resourcesRoot = FindImplicitContentMember(dictionaryObject); var theme = GetDictionaryResourceKey(dictionaryObject); @@ -1173,9 +1178,9 @@ private bool BuildDefaultStylesRegistration(IIndentedStringBuilder writer, XamlM if (Equals(targetType.ContainingAssembly, _metadataHelper.Compilation.Assembly)) { var isNativeStyle = style.Members.FirstOrDefault(m => m.Member.Name == "IsNativeStyle")?.Value as string == "True"; - writer.AppendLineInvariant("global::Windows.UI.Xaml.Style.RegisterDefaultStyleForType({0}, () => {1}, /*isNativeStyle:*/{2});", + writer.AppendLineInvariant("global::Windows.UI.Xaml.Style.RegisterDefaultStyleForType({0}, {1}, /*isNativeStyle:*/{2});", implicitKey, - GetResourceDictionaryPropertyName(implicitKey), + SingletonInstanceAccess, isNativeStyle.ToString().ToLowerInvariant() ); } @@ -3706,29 +3711,6 @@ private string GetSimpleStaticResourceRetrieval(INamedTypeSymbol targetPropertyT return staticRetrieval; } - /// - /// Get the name of top-level ResourceDictionary property associated with the given resource key, if one exists, otherwise null. - /// - private string GetResourceDictionaryPropertyName(string keyStr) - { - var propertyName = GetResourceDictionaryPropertyDetails(keyStr).PropertyName; - TryAnnotateWithGeneratorSource(ref propertyName); - return propertyName; - } - - private (string PropertyName, INamedTypeSymbol PropertyType) GetResourceDictionaryPropertyDetails(string keyStr) - { - if (_topLevelDictionaryProperties.TryGetValue((_themeDictionaryCurrentlyBuilding, keyStr), out var propertyDetails)) - { - var propertyAccess = _isInChildSubclass ? - "global::{0}.GlobalStaticResources.{1}.Instance.{2} /*{3}*/".InvariantCultureFormat(_defaultNamespace, SingletonClassName, propertyDetails.PropertyName, keyStr) - : "this.{0} /*{1}*/".InvariantCultureFormat(propertyDetails.PropertyName, keyStr); - return (propertyAccess, propertyDetails.PropertyType); - } - - return (null, null); - } - private INamedTypeSymbol FindUnderlyingType(INamedTypeSymbol propertyType) { return (propertyType.IsNullable(out var underlyingType) && underlyingType is INamedTypeSymbol underlyingNamedType) ? underlyingNamedType : propertyType; diff --git a/src/Uno.UI/UI/Xaml/IXamlResourceDictionaryProvider.cs b/src/Uno.UI/UI/Xaml/IXamlResourceDictionaryProvider.cs new file mode 100644 index 000000000000..d644c043f2f9 --- /dev/null +++ b/src/Uno.UI/UI/Xaml/IXamlResourceDictionaryProvider.cs @@ -0,0 +1,26 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Diagnostics; +using System.Text; +using Microsoft.Extensions.Logging; +using Uno.Diagnostics.Eventing; +using Uno.Extensions; +using Uno.UI.DataBinding; +using Uno.UI.Xaml; +using Windows.UI.Xaml; +using Windows.UI.Xaml.Data; +using Windows.UI.Xaml.Resources; + +namespace Uno.UI +{ + /// + /// Provides lazy initialization for a resource dictionary. + /// + /// Normally only implemented and referenced by Xaml-generated code. + [EditorBrowsable(EditorBrowsableState.Never)] + public interface IXamlResourceDictionaryProvider + { + ResourceDictionary GetResourceDictionary(); + } +} diff --git a/src/Uno.UI/UI/Xaml/Style/Style.cs b/src/Uno.UI/UI/Xaml/Style/Style.cs index 20b2f79def0d..d4edc8ae917c 100644 --- a/src/Uno.UI/UI/Xaml/Style/Style.cs +++ b/src/Uno.UI/UI/Xaml/Style/Style.cs @@ -147,8 +147,10 @@ private void EnumerateSetters(Style style, Dictionary ma /// /// The type to which the style applies /// Function which generates the style. This will be called once when first used, then cached. - /// True if is is the native default style, false if it is the UWP default style. - /// This method should normally only be called from Xaml-generated code. + /// True if it is the native default style, false if it is the UWP default style. + /// + /// This is public for backward compatibility, but isn't called from Xaml-generated code any longer. + /// [EditorBrowsable(EditorBrowsableState.Never)] public static void RegisterDefaultStyleForType(Type type, StyleProviderHandler styleProvider, bool isNative) { @@ -162,6 +164,30 @@ public static void RegisterDefaultStyleForType(Type type, StyleProviderHandler s } } + /// + /// Register lazy default style provider for the nominated type. + /// + /// The type to which the style applies + /// Provides the dictionary in which the style is defined. + /// True if it is the native default style, false if it is the UWP default style. + /// This is an Uno-specific method, normally only called from Xaml-generated code. + [EditorBrowsable(EditorBrowsableState.Never)] + public static void RegisterDefaultStyleForType(Type type, IXamlResourceDictionaryProvider dictionaryProvider, bool isNative) + { + RegisterDefaultStyleForType(type, ProvideStyle, isNative); + + Style ProvideStyle() + { + var styleSource = dictionaryProvider.GetResourceDictionary(); + if (styleSource.TryGetValue(type, out var style, shouldCheckSystem: false)) + { + return (Style)style; + } + + throw new InvalidOperationException($"{styleSource} was registered as style provider for {type} but doesn't contain matching style."); + } + } + /// /// Returns the default Style for given type. /// @@ -186,6 +212,8 @@ private static Style GetDefaultStyleForType(Type type, bool useUWPDefaultStyles) style = styleProvider(); styleCache[type] = style; + + lookup.Remove(type); // The lookup won't be used again now that the style itself is cached } } From 6da1cbd1c79666d42f6c172d949373406fcb2f85 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Thu, 17 Sep 2020 10:50:59 -0400 Subject: [PATCH 10/21] chore: Enable nullable awareness for Style --- src/Uno.UI/UI/Xaml/Setter.cs | 24 ++++++++++++---------- src/Uno.UI/UI/Xaml/SetterBase.cs | 4 +++- src/Uno.UI/UI/Xaml/SetterBaseCollection.cs | 2 ++ src/Uno.UI/UI/Xaml/Style/Style.cs | 20 +++++++++++------- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Setter.cs b/src/Uno.UI/UI/Xaml/Setter.cs index c0dbec7b8ebd..4b542927fde1 100644 --- a/src/Uno.UI/UI/Xaml/Setter.cs +++ b/src/Uno.UI/UI/Xaml/Setter.cs @@ -1,3 +1,5 @@ +#nullable enable + using System; using System.ComponentModel; using System.Diagnostics; @@ -24,12 +26,12 @@ public Setter() } - private BindingPath _bindingPath; - private readonly SetterValueProviderHandler _valueProvider; - private object _value; + private BindingPath? _bindingPath; + private readonly SetterValueProviderHandler? _valueProvider; + private object? _value; private int _targetNameResolutionFailureCount; - public object Value + public object? Value { get { @@ -43,7 +45,7 @@ public object Value set => _value = value; } - public TargetPropertyPath Target + public TargetPropertyPath? Target { get; set; @@ -52,14 +54,14 @@ public TargetPropertyPath Target /// /// The property being set by this setter /// - public DependencyProperty Property { get; set; } + public DependencyProperty? Property { get; set; } /// /// The name of the ThemeResource applied to the value, if any. /// - internal string ThemeResourceName { get; set; } + internal string? ThemeResourceName { get; set; } - internal XamlParseContext ThemeResourceContext { get; set; } + internal XamlParseContext? ThemeResourceContext { get; set; } public Setter(DependencyProperty targetProperty, object value) { @@ -83,13 +85,13 @@ internal override void ApplyTo(DependencyObject o) { if (Property != null) { - object value = _valueProvider != null ? _valueProvider() : _value; if (!ThemeResourceName.IsNullOrEmpty()) { ResourceResolver.ApplyResource(o, Property, ThemeResourceName, isThemeResourceExtension: true, context: ThemeResourceContext); } else { + object? value = _valueProvider != null ? _valueProvider() : _value; o.SetValue(Property, BindingPropertyHelper.Convert(() => Property.Type, value)); } } @@ -119,12 +121,12 @@ internal void ApplyValue(DependencyPropertyValuePrecedences precedence, IFramewo } else { - path.Value = Value; + path.Value = Value; } } } - private BindingPath TryGetOrCreateBindingPath(DependencyPropertyValuePrecedences precedence, IFrameworkElement owner) + private BindingPath? TryGetOrCreateBindingPath(DependencyPropertyValuePrecedences precedence, IFrameworkElement owner) { if (_bindingPath != null) { diff --git a/src/Uno.UI/UI/Xaml/SetterBase.cs b/src/Uno.UI/UI/Xaml/SetterBase.cs index fd852a187188..635d4d2be3fd 100644 --- a/src/Uno.UI/UI/Xaml/SetterBase.cs +++ b/src/Uno.UI/UI/Xaml/SetterBase.cs @@ -1,4 +1,6 @@ -using System.ComponentModel; +#nullable enable + +using System.ComponentModel; using Uno.Extensions; using Uno.Logging; diff --git a/src/Uno.UI/UI/Xaml/SetterBaseCollection.cs b/src/Uno.UI/UI/Xaml/SetterBaseCollection.cs index 9e46820a65fe..d8e0f32da9b2 100644 --- a/src/Uno.UI/UI/Xaml/SetterBaseCollection.cs +++ b/src/Uno.UI/UI/Xaml/SetterBaseCollection.cs @@ -1,3 +1,5 @@ +#nullable enable + using System; using System.Collections.Generic; using System.Runtime.InteropServices; diff --git a/src/Uno.UI/UI/Xaml/Style/Style.cs b/src/Uno.UI/UI/Xaml/Style/Style.cs index d4edc8ae917c..46f5d88e1785 100644 --- a/src/Uno.UI/UI/Xaml/Style/Style.cs +++ b/src/Uno.UI/UI/Xaml/Style/Style.cs @@ -1,4 +1,6 @@ -using System; +#nullable enable + +using System; using System.Collections.Generic; using System.ComponentModel; using System.Linq; @@ -41,9 +43,9 @@ public Style(Type targetType) : this() TargetType = targetType; } - public Type TargetType { get; set; } + public Type? TargetType { get; set; } - public Style BasedOn { get; set; } + public Style? BasedOn { get; set; } public SetterBaseCollection Setters { get; } = new SetterBaseCollection(); @@ -69,7 +71,7 @@ internal void ApplyTo(DependencyObject o, DependencyPropertyValuePrecedences pre } // Check tree for resource binding values, since some Setters may have set ThemeResource-backed values - (o as IDependencyObjectStoreProvider).Store.UpdateResourceBindings(isThemeChangedUpdate: false); + (o as IDependencyObjectStoreProvider)!.Store.UpdateResourceBindings(isThemeChangedUpdate: false); } #if !HAS_EXPENSIVE_TRYFINALLY finally @@ -129,6 +131,10 @@ private void EnumerateSetters(Style style, Dictionary ma { if (setter is Setter s) { + if (s.Property == null) + { + throw new InvalidOperationException("Property must be set on Setter used in Style"); // TODO: We should also support Setter.Target inside Style https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.setter#remarks + } map[s.Property] = setter.ApplyTo; } else if (setter is ICSharpPropertySetter propertySetter) @@ -191,9 +197,9 @@ Style ProvideStyle() /// /// Returns the default Style for given type. /// - internal static Style GetDefaultStyleForType(Type type) => GetDefaultStyleForType(type, ShouldUseUWPDefaultStyle(type)); + internal static Style? GetDefaultStyleForType(Type type) => GetDefaultStyleForType(type, ShouldUseUWPDefaultStyle(type)); - private static Style GetDefaultStyleForType(Type type, bool useUWPDefaultStyles) + private static Style? GetDefaultStyleForType(Type type, bool useUWPDefaultStyles) { if (type == null) { @@ -205,7 +211,7 @@ private static Style GetDefaultStyleForType(Type type, bool useUWPDefaultStyles) var lookup = useUWPDefaultStyles ? _lookup : _nativeLookup; - if (!styleCache.TryGetValue(type, out var style)) + if (!styleCache.TryGetValue(type, out Style? style)) { if (lookup.TryGetValue(type, out var styleProvider)) { From 73d00d887aa6e04d5145ee2c5e5edb630df9abb2 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Fri, 18 Sep 2020 06:13:43 -0400 Subject: [PATCH 11/21] chore: Use giant switch case for resource initialization This is a device to reduce AOT package size, since method declarations (including lambdas) have a noticeable footprint in IR. --- .../XamlGenerator/XamlFileGenerator.cs | 150 +++++++++--------- .../UI/Xaml/IXamlLazyResourceInitializer.cs | 26 +++ src/Uno.UI/UI/Xaml/ResourceDictionary.cs | 78 ++++++--- .../UI/Xaml/ResourceResolverSingleton.cs | 32 ++++ 4 files changed, 184 insertions(+), 102 deletions(-) create mode 100644 src/Uno.UI/UI/Xaml/IXamlLazyResourceInitializer.cs create mode 100644 src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index bad8dbf308eb..fe4c304c9340 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -48,9 +48,9 @@ internal partial class XamlFileGenerator private readonly Dictionary _namedResources = new Dictionary(); private readonly List _partials = new List(); /// - /// Names of generated properties associated with resource definitions. These are created for top-level ResourceDictionary declarations only. + /// Names of disambiguated keys associated with resource definitions. These are created for top-level ResourceDictionary declarations only. /// - private readonly Dictionary<(string Theme, string ResourceKey), (string PropertyName, INamedTypeSymbol PropertyType)> _topLevelDictionaryProperties = new Dictionary<(string, string), (string, INamedTypeSymbol)>(); + private readonly Dictionary<(string Theme, string ResourceKey), string> _topLevelQualifiedKeys = new Dictionary<(string, string), string>(); private readonly Stack _scopeStack = new Stack(); private readonly XamlFileDefinition _fileDefinition; private readonly string _targetPath; @@ -159,6 +159,7 @@ internal partial class XamlFileGenerator private string SingletonClassName => $"ResourceDictionarySingleton__{_fileDefinition.ShortId}"; private const string DictionaryProviderInterfaceName = "global::Uno.UI.IXamlResourceDictionaryProvider"; + private const string LazyInitializerInterfaceName = "global::Uno.UI.IXamlLazyResourceInitializer"; static XamlFileGenerator() { @@ -917,7 +918,7 @@ private void BuildTopLevelResourceDictionary(IIndentedStringBuilder writer, Xaml IDisposable WrapSingleton() { writer.AppendLineInvariant("// This non-static inner class is a means of reducing size of AOT compilations by avoiding many accesses to static members from a static callsite, which adds costly class initializer checks each time."); - var block = writer.BlockInvariant("internal sealed class {0} : {1}", SingletonClassName, DictionaryProviderInterfaceName); + var block = writer.BlockInvariant("internal sealed class {0} : {1}, {2}", SingletonClassName, DictionaryProviderInterfaceName, LazyInitializerInterfaceName); _isInSingletonInstance = true; return new DisposableAction(() => { @@ -956,13 +957,28 @@ IDisposable WrapSingleton() writer.AppendLineInvariant("{0} = {1};", XamlCodeGeneration.ParseContextPropertyName, outerProperty); } writer.AppendLine(); - BuildResourceDictionaryGlobalProperties(writer, topLevelControl); - var themeDictionaryMember = topLevelControl.Members.FirstOrDefault(m => m.Member.Name == "ThemeDictionaries"); - - foreach (var dict in (themeDictionaryMember?.Objects).Safe()) + // Build giant switch case block for resource retrieval - the advantage here is reduced IR generation with associated reduced AOT footprint + using (writer.BlockInvariant("object {0}.GetInitializedValue(string resourceKey)", LazyInitializerInterfaceName)) { - BuildResourceDictionaryGlobalProperties(writer, dict); + using (writer.BlockInvariant("switch (resourceKey)")) + { + BuildTopLevelResourceDictionaryCases(writer, topLevelControl); + + var themeDictionaryMember = topLevelControl.Members.FirstOrDefault(m => m.Member.Name == "ThemeDictionaries"); + + foreach (var dict in (themeDictionaryMember?.Objects).Safe()) + { + BuildTopLevelResourceDictionaryCases(writer, dict); + } + + writer.AppendLineInvariant("default:"); + using (writer.Indent()) + { + writer.AppendLineInvariant("throw new global::System.ArgumentException(\"Entry '\" + resourceKey + \"' was not found\");"); + } + } + } TryAnnotateWithGeneratorSource(writer, suffix: "DictionaryProperty"); @@ -1005,19 +1021,18 @@ IDisposable WrapSingleton() } /// - ///Builds global static properties for each resource in a ResourceDictionary, for intra-dictionary static lookup. + ///Build entries in the giant switch case method for the current ResourceDictionary. /// /// The writer to use /// The associated with the dictionary. - private void BuildResourceDictionaryGlobalProperties(IIndentedStringBuilder writer, XamlObjectDefinition dictionaryObject) + private void BuildTopLevelResourceDictionaryCases(IIndentedStringBuilder writer, XamlObjectDefinition dictionaryObject) { - return; // This will be refactored forthwith TryAnnotateWithGeneratorSource(writer); var resourcesRoot = FindImplicitContentMember(dictionaryObject); var theme = GetDictionaryResourceKey(dictionaryObject); var resources = (resourcesRoot?.Objects).Safe(); - // Pre-populate property names (in case of forward lexical references) + // Pre-populate case names (though this is probably no longer necessary...?) var index = _dictionaryPropertyIndex; foreach (var resource in resources) { @@ -1029,15 +1044,15 @@ private void BuildResourceDictionaryGlobalProperties(IIndentedStringBuilder writ } index++; - var propertyName = GetPropertyNameForResourceKey(index); - if (_topLevelDictionaryProperties.ContainsKey((theme, key))) + var propertyName = GetCaseNameForResourceKey(key, index); + if (_topLevelQualifiedKeys.ContainsKey((theme, key))) { throw new InvalidOperationException($"Dictionary Item {resource?.Type?.Name} has duplicate key `{key}` { (theme != null ? $" in theme {theme}" : "")}."); } var isStaticResourceAlias = resource.Type.Name == "StaticResource"; if (!isStaticResourceAlias) { - _topLevelDictionaryProperties[(theme, key)] = (propertyName, FindType(resource.Type)); + _topLevelQualifiedKeys[(theme, key)] = propertyName; } } @@ -1057,28 +1072,17 @@ private void BuildResourceDictionaryGlobalProperties(IIndentedStringBuilder writ var isStaticResourceAlias = resource.Type.Name == "StaticResource"; if (isStaticResourceAlias) { - writer.AppendLineInvariant("// Skipping static property {0} for {1} {2} - StaticResource ResourceKey aliases are added directly to dictionary", _dictionaryPropertyIndex, key, theme); + writer.AppendLineInvariant("// Skipping case {0} for {1} {2} - StaticResource ResourceKey aliases are added directly to dictionary", _dictionaryPropertyIndex, key, theme); } else { - var propertyName = GetPropertyNameForResourceKey(_dictionaryPropertyIndex); - if (_topLevelDictionaryProperties[(theme, key)].PropertyName != propertyName) + var caseName = GetCaseNameForResourceKey(key, _dictionaryPropertyIndex); + if (_topLevelQualifiedKeys[(theme, key)] != caseName) { - throw new InvalidOperationException($"Property was not created correctly for {key} (theme={theme})."); + throw new InvalidOperationException($"Case name was not created correctly for {key} (theme={theme})."); } - writer.AppendLineInvariant("// Property for resource {0} {1}", key, theme != null ? "in theme {0}".InvariantCultureFormat(theme) : ""); - void BuildPropertyBody() - { - if (isStaticResourceAlias) - { - BuildStaticResourceResourceKeyReference(writer, resource); - } - else - { - BuildChild(writer, resourcesRoot, resource); - } - } - BuildSingleTimeInitializer(writer, isStaticResourceAlias ? "global::System.Object" : resource.Type.Name, propertyName, BuildPropertyBody); + writer.AppendLineInvariant("// Case for resource {0} {1}", key, theme != null ? "in theme {0}".InvariantCultureFormat(theme) : ""); + BuildSingleTimeInitializer(writer, caseName, () => BuildChild(writer, resourcesRoot, resource)); } } _themeDictionaryCurrentlyBuilding = former; @@ -1096,12 +1100,18 @@ private void BuildStaticResourceResourceKeyReference(IIndentedStringBuilder writ } /// - /// Get name to use for global static property associated with a resource. + /// Get name to use for case associated with a resource. /// + /// The resource key /// An index associated with the property. - private string GetPropertyNameForResourceKey(int index) + /// + /// We append the qualifier to the resource key because there may be multiple dictionary definitions (merged dictionaries, themed + /// dictionaries) in the same switch block, and it's possible (probable actually, in the case of theme dictionaries) to have + /// duplicate resource keys. + /// + private string GetCaseNameForResourceKey(string key, int index) { - return "Entry{0}_{1}".InvariantCultureFormat(_fileDefinition.ShortId, index); + return "{0}_{1}_{2}".InvariantCultureFormat(key, _fileDefinition.ShortId, index); } /// @@ -1260,53 +1270,18 @@ private void BuildResourceDictionaryBackingClass(IIndentedStringBuilder writer, } /// - /// Helper for building a lazily-initialized static property. + /// Build single case within the giant switch case block for resource retrieval /// - /// The writer to use - /// The type of the property. - /// The name of the property. - /// Function that builds the initialization logic for the property. - /// - private void BuildSingleTimeInitializer(IIndentedStringBuilder writer, string propertyType, string propertyName, Action propertyBodyBuilder, bool isStatic = false) + private void BuildSingleTimeInitializer(IIndentedStringBuilder writer, string caseName, Action propertyBodyBuilder) { TryAnnotateWithGeneratorSource(writer); - // The property type may be partially qualified, try resolving it through FindType - var propertySymbol = FindType(propertyType); - propertyType = propertySymbol?.GetFullName() ?? propertyType; - - var sanitizedPropertyName = SanitizeResourceName(propertyName); - - var propertyInitializedVariable = "_{0}Initialized".InvariantCultureFormat(sanitizedPropertyName); - var backingFieldVariable = "__{0}BackingField".InvariantCultureFormat(sanitizedPropertyName); - var staticModifier = isStatic ? "static" : ""; - var publicPropertyType = FindType(propertyType)?.DeclaredAccessibility == Accessibility.Public - ? propertyType - : "System.Object"; - - writer.AppendLineInvariant($"private {staticModifier} bool {propertyInitializedVariable} = false;"); - writer.AppendLineInvariant($"private {staticModifier} {GetGlobalizedTypeName(propertyType)} {backingFieldVariable};"); - - writer.AppendLine(); - - using (writer.BlockInvariant($"internal {staticModifier} {GetGlobalizedTypeName(publicPropertyType)} {sanitizedPropertyName}")) + writer.AppendLineInvariant("case \"{0}\":", caseName); + using (writer.Indent()) { - using (writer.BlockInvariant("get")) - { - using (writer.BlockInvariant($"if(!{propertyInitializedVariable})")) - { - writer.AppendLineInvariant($"{backingFieldVariable} = "); - using (writer.Indent()) - { - propertyBodyBuilder(); - } - writer.AppendLineInvariant(";"); - writer.AppendLineInvariant($"{propertyInitializedVariable} = true;"); - } - - writer.AppendLineInvariant($"return {backingFieldVariable};"); - } + writer.AppendLineInvariant("return"); + propertyBodyBuilder(); + writer.AppendLineInvariant(";"); } - writer.AppendLine(); } @@ -2241,6 +2216,14 @@ private void BuildResourceDictionary(IIndentedStringBuilder writer, XamlMemberDe { BuildStaticResourceResourceKeyReference(writer, resource); } + else if (_isTopLevelDictionary + // Note: It's possible to be in a top-level ResourceDictionary file but for caseName to be null, eg for + // a FrameworkElement.Resources declaration inside of a template + && GetResourceDictionaryCaseName(key) is string caseName) + { + // + writer.AppendLineInvariant("global::Uno.UI.ResourceResolverSingleton.Instance.ResolveLazyInitializer(\"{0}\", {1})", caseName, SingletonInstanceAccess); + } else { using (BuildLazyResourceInitializer(writer)) @@ -3711,6 +3694,19 @@ private string GetSimpleStaticResourceRetrieval(INamedTypeSymbol targetPropertyT return staticRetrieval; } + /// + /// Get the retrieval key associated with the given resource key, if one exists, otherwise null. + /// + private string GetResourceDictionaryCaseName(string keyStr) + { + if (_topLevelQualifiedKeys.TryGetValue((_themeDictionaryCurrentlyBuilding, keyStr), out var qualifiedKey)) + { + return qualifiedKey; + } + + return null; + } + private INamedTypeSymbol FindUnderlyingType(INamedTypeSymbol propertyType) { return (propertyType.IsNullable(out var underlyingType) && underlyingType is INamedTypeSymbol underlyingNamedType) ? underlyingNamedType : propertyType; diff --git a/src/Uno.UI/UI/Xaml/IXamlLazyResourceInitializer.cs b/src/Uno.UI/UI/Xaml/IXamlLazyResourceInitializer.cs new file mode 100644 index 000000000000..cc418e337a66 --- /dev/null +++ b/src/Uno.UI/UI/Xaml/IXamlLazyResourceInitializer.cs @@ -0,0 +1,26 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Diagnostics; +using System.Text; +using Microsoft.Extensions.Logging; +using Uno.Diagnostics.Eventing; +using Uno.Extensions; +using Uno.UI.DataBinding; +using Uno.UI.Xaml; +using Windows.UI.Xaml; +using Windows.UI.Xaml.Data; +using Windows.UI.Xaml.Resources; + +namespace Uno.UI +{ + /// + /// Provides initialization logic for resources defined in a ResourceDictionary. + /// + /// Normally only implemented and referenced by Xaml-generated code. + [EditorBrowsable(EditorBrowsableState.Never)] + public interface IXamlLazyResourceInitializer + { + object GetInitializedValue(string resourceRetrievalKey); + } +} diff --git a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs index 56cf87b43ecd..52899a340e39 100644 --- a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs +++ b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs @@ -69,7 +69,7 @@ public bool HasKey(object key) /// and can be removed as breaking change later. public bool Insert(object key, object value) { - Set(key, value); + Set(key, value, throwIfPresent: false); return true; } @@ -79,18 +79,7 @@ public bool Insert(object key, object value) public void Clear() => _values.Clear(); - public void Add(object key, object value) - { - if (value is ResourceInitializer resourceInitializer) - { - _hasUnmaterializedItems = true; - _values.Add(key, new LazyInitializer(ResourceResolver.CurrentScope, resourceInitializer)); - } - else - { - _values.Add(key, value); - } - } + public void Add(object key, object value) => Set(key, value, throwIfPresent: true); public bool ContainsKey(object key) => ContainsKey(key, shouldCheckSystem: true); public bool ContainsKey(object key, bool shouldCheckSystem) => _values.ContainsKey(key) || ContainsKeyMerged(key) || ContainsKeyTheme(key) @@ -133,15 +122,24 @@ public object this[object key] return value; } - set => Set(key, value); + set => Set(key, value, throwIfPresent: false); } - private void Set(object key, object value) + private void Set(object key, object value, bool throwIfPresent) { - if (value is ResourceInitializer resourceInitializer) + if (throwIfPresent && _values.ContainsKey(key)) + { + throw new ArgumentException("An entry with the same key already exists."); + } + + if (value is ResourceInitializer || value is ProviderLazyInitializer) { _hasUnmaterializedItems = true; - _values[key] = new LazyInitializer(ResourceResolver.CurrentScope, resourceInitializer); + } + + if (value is ResourceInitializer resourceInitializer) + { + _values[key] = new DelegateLazyInitializer(ResourceResolver.CurrentScope, resourceInitializer); } else { @@ -163,7 +161,7 @@ private void TryMaterializeLazy(object key, ref object value) { _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); - newValue = lazyInitializer.Initializer(); + newValue = lazyInitializer.GetInitializedValue(); } #if !HAS_EXPENSIVE_TRYFINALLY finally @@ -299,7 +297,7 @@ private void CopyFrom(ResourceDictionary source) public global::System.Collections.Generic.ICollection Keys => _values.Keys; - // TODO: this doesn't handle lazy initializers or aliases + // TODO: this doesn't handle lazy initializers or aliases public global::System.Collections.Generic.ICollection Values => _values.Values; public void Add(global::System.Collections.Generic.KeyValuePair item) => Add(item.Key, item.Value); @@ -374,7 +372,7 @@ private void TryMaterializeAll() foreach (var kvp in _values) { - if (kvp.Value is LazyInitializer lazyInitializer) + if (kvp.Value is LazyInitializer) { unmaterialized.Add(kvp); } @@ -420,21 +418,49 @@ internal void UpdateThemeBindings() } [EditorBrowsable(EditorBrowsableState.Never)] + // This is present for backward compatibility, but it's no longer used in Xaml-generated code with newer versions of Uno. public delegate object ResourceInitializer(); + private abstract class LazyInitializer + { + public XamlScope CurrentScope { get; } + + protected LazyInitializer(XamlScope currentScope) + { + CurrentScope = currentScope; + } + + public abstract object GetInitializedValue(); + } + /// /// Allows resources to be initialized on-demand using correct scope. /// - private struct LazyInitializer + /// This is present for backward compatibility, but it's no longer used in Xaml-generated code with newer versions of Uno. + private class DelegateLazyInitializer : LazyInitializer { - public XamlScope CurrentScope { get; } - public ResourceInitializer Initializer { get; } + private ResourceInitializer Initializer { get; } - public LazyInitializer(XamlScope currentScope, ResourceInitializer initializer) + public DelegateLazyInitializer(XamlScope currentScope, ResourceInitializer initializer) : base(currentScope) { - CurrentScope = currentScope; Initializer = initializer; } + + public override object GetInitializedValue() => Initializer(); + } + + private class ProviderLazyInitializer : LazyInitializer + { + private readonly string _qualifiedResourceKey; + private readonly IXamlLazyResourceInitializer _initializer; + + public ProviderLazyInitializer(XamlScope currentScope, string qualifiedResourceKey, IXamlLazyResourceInitializer initializer) : base(currentScope) + { + _qualifiedResourceKey = qualifiedResourceKey; + _initializer = initializer; + } + + public override object GetInitializedValue() => _initializer.GetInitializedValue(_qualifiedResourceKey); } /// @@ -455,6 +481,8 @@ public StaticResourceAliasRedirect(string resourceKey, XamlParseContext parseCon internal static object GetStaticResourceAliasPassthrough(string resourceKey, XamlParseContext parseContext) => new StaticResourceAliasRedirect(resourceKey, parseContext); + internal static object GetLazyInitializer(string qualifiedResourceKey, IXamlLazyResourceInitializer initializer) => new ProviderLazyInitializer(ResourceResolver.CurrentScope, qualifiedResourceKey, initializer); + private static class Themes { public const string Default = "Default"; diff --git a/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs b/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs new file mode 100644 index 000000000000..483550bb0992 --- /dev/null +++ b/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs @@ -0,0 +1,32 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Diagnostics; +using System.Text; +using Microsoft.Extensions.Logging; +using Uno.Diagnostics.Eventing; +using Uno.Extensions; +using Uno.UI.DataBinding; +using Uno.UI.Xaml; +using Windows.UI.Xaml; +using Windows.UI.Xaml.Data; +using Windows.UI.Xaml.Resources; + +namespace Uno.UI +{ + /// + /// A wrapper for methods following the singleton pattern. + /// + /// + /// The motivation is to avoid additional overhead associated with static method calls into types with static state. This is normally + /// only called from Xaml-generated code. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + public sealed class ResourceResolverSingleton + { + private static ResourceResolverSingleton _instance; + public static ResourceResolverSingleton Instance => _instance ??= new ResourceResolverSingleton(); + + public object ResolveLazyInitializer(string qualifiedResourceKey, IXamlLazyResourceInitializer initializer) => ResourceDictionary.GetLazyInitializer(qualifiedResourceKey, initializer); + } +} From e283aa6d7151e51003c1bb8f0bd15241178977fd Mon Sep 17 00:00:00 2001 From: David Oliver Date: Fri, 18 Sep 2020 06:31:32 -0400 Subject: [PATCH 12/21] chore: Send frequently-called ResourceResolver methods through singleton --- .../XamlGenerator/XamlFileGenerator.cs | 6 +++--- src/Uno.UI/UI/Xaml/ResourceResolver.cs | 7 ++++--- src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs | 9 +++++++++ 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index fe4c304c9340..a83e6bb9c22b 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -1096,7 +1096,7 @@ private void BuildStaticResourceResourceKeyReference(IIndentedStringBuilder writ TryAnnotateWithGeneratorSource(writer); var targetKey = resourceDefinition.Members.FirstOrDefault(m => m.Member.Name == "ResourceKey")?.Value as string; - writer.AppendLineInvariant("global::Uno.UI.ResourceResolver.ResolveStaticResourceAlias(\"{0}\", {1})", targetKey, ParseContextPropertyAccess); + writer.AppendLineInvariant("global::Uno.UI.ResourceResolverSingleton.Instance.ResolveStaticResourceAlias(\"{0}\", {1})", targetKey, ParseContextPropertyAccess); } /// @@ -3194,7 +3194,7 @@ string GetBindingOptions() if (IsDependencyProperty(member.Member)) { var propertyOwner = GetType(member.Member.DeclaringType); - writer.AppendLineInvariant("global::Uno.UI.ResourceResolver.ApplyResource({0}, {1}.{2}Property, \"{3}\", isThemeResourceExtension: {4}, context: {5});", closureName, GetGlobalizedTypeName(propertyOwner.ToDisplayString()), member.Member.Name, resourceKey, isThemeResourceExtension ? "true" : "false", ParseContextPropertyAccess); + writer.AppendLineInvariant("global::Uno.UI.ResourceResolverSingleton.Instance.ApplyResource({0}, {1}.{2}Property, \"{3}\", isThemeResourceExtension: {4}, context: {5});", closureName, GetGlobalizedTypeName(propertyOwner.ToDisplayString()), member.Member.Name, resourceKey, isThemeResourceExtension ? "true" : "false", ParseContextPropertyAccess); } else if (IsAttachedProperty(member)) { @@ -3683,7 +3683,7 @@ private string GetSimpleStaticResourceRetrieval(INamedTypeSymbol targetPropertyT var staticRetrieval = $"(" + - $"global::Uno.UI.ResourceResolver.ResolveResourceStatic(" + + $"global::Uno.UI.ResourceResolverSingleton.Instance.ResolveResourceStatic(" + $"\"{keyStr}\", " + $"out var {propertyOutputVar}, " + $"context: {ParseContextPropertyAccess}) " + diff --git a/src/Uno.UI/UI/Xaml/ResourceResolver.cs b/src/Uno.UI/UI/Xaml/ResourceResolver.cs index 7114a17e26b7..11a714026384 100644 --- a/src/Uno.UI/UI/Xaml/ResourceResolver.cs +++ b/src/Uno.UI/UI/Xaml/ResourceResolver.cs @@ -60,7 +60,6 @@ static ResourceResolver() /// /// Performs a one-time, typed resolution of a named resource, using Application.Resources. /// - /// [EditorBrowsable(EditorBrowsableState.Never)] public static T ResolveResourceStatic(object key, object context = null) { @@ -73,9 +72,8 @@ public static T ResolveResourceStatic(object key, object context = null) } /// - /// Performs a one-time, typed resolution of a named resource, using Application.Resources. + /// Performs a one-time resolution of a named resource, using Application.Resources. /// - /// [EditorBrowsable(EditorBrowsableState.Never)] [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)] public static bool ResolveResourceStatic(object key, out object value, object context = null) @@ -398,4 +396,7 @@ public static object ResolveStaticResourceAlias(string resourceKey, object parse internal static void UpdateSystemThemeBindings() => MasterDictionary.UpdateThemeBindings(); } + + + } diff --git a/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs b/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs index 483550bb0992..deeb85736245 100644 --- a/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs +++ b/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs @@ -28,5 +28,14 @@ public sealed class ResourceResolverSingleton public static ResourceResolverSingleton Instance => _instance ??= new ResourceResolverSingleton(); public object ResolveLazyInitializer(string qualifiedResourceKey, IXamlLazyResourceInitializer initializer) => ResourceDictionary.GetLazyInitializer(qualifiedResourceKey, initializer); + + [EditorBrowsable(EditorBrowsableState.Never)] + public bool ResolveResourceStatic(object key, out object value, object context) => ResourceResolver.ResolveResourceStatic(key, out value, context); + + [EditorBrowsable(EditorBrowsableState.Never)] + public void ApplyResource(DependencyObject owner, DependencyProperty property, object resourceKey, bool isThemeResourceExtension, object context) => ResourceResolver.ApplyResource(owner, property, resourceKey, isThemeResourceExtension, context); + + [EditorBrowsable(EditorBrowsableState.Never)] + public object ResolveStaticResourceAlias(string resourceKey, object parseContext) => ResourceResolver.ResolveStaticResourceAlias(resourceKey, parseContext); } } From 00e746451df22a59588cc8ded52b1f3a2b2f1e01 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Tue, 22 Sep 2020 06:15:00 -0400 Subject: [PATCH 13/21] chore: Use individual instance methods for ResourceDictionary initializers This in intermediate in binary footprint between inline lambdas (heavier) and the giant switch case method (lighter), while avoiding the problems of the gscm in terms of JITter and PG-AOT unfriendliness. --- .../XamlGenerator/XamlFileGenerator.cs | 76 ++++++++----------- src/Uno.UI/UI/Xaml/ResourceDictionary.cs | 52 +++---------- .../UI/Xaml/ResourceResolverSingleton.cs | 2 - 3 files changed, 42 insertions(+), 88 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index a83e6bb9c22b..543a0e4c39ee 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -918,7 +918,7 @@ private void BuildTopLevelResourceDictionary(IIndentedStringBuilder writer, Xaml IDisposable WrapSingleton() { writer.AppendLineInvariant("// This non-static inner class is a means of reducing size of AOT compilations by avoiding many accesses to static members from a static callsite, which adds costly class initializer checks each time."); - var block = writer.BlockInvariant("internal sealed class {0} : {1}, {2}", SingletonClassName, DictionaryProviderInterfaceName, LazyInitializerInterfaceName); + var block = writer.BlockInvariant("internal sealed class {0} : {1}", SingletonClassName, DictionaryProviderInterfaceName); _isInSingletonInstance = true; return new DisposableAction(() => { @@ -958,27 +958,14 @@ IDisposable WrapSingleton() } writer.AppendLine(); - // Build giant switch case block for resource retrieval - the advantage here is reduced IR generation with associated reduced AOT footprint - using (writer.BlockInvariant("object {0}.GetInitializedValue(string resourceKey)", LazyInitializerInterfaceName)) - { - using (writer.BlockInvariant("switch (resourceKey)")) - { - BuildTopLevelResourceDictionaryCases(writer, topLevelControl); - - var themeDictionaryMember = topLevelControl.Members.FirstOrDefault(m => m.Member.Name == "ThemeDictionaries"); - - foreach (var dict in (themeDictionaryMember?.Objects).Safe()) - { - BuildTopLevelResourceDictionaryCases(writer, dict); - } + // Build initializer methods for resource retrieval + BuildTopLevelResourceDictionaryInitializers(writer, topLevelControl); - writer.AppendLineInvariant("default:"); - using (writer.Indent()) - { - writer.AppendLineInvariant("throw new global::System.ArgumentException(\"Entry '\" + resourceKey + \"' was not found\");"); - } - } + var themeDictionaryMember = topLevelControl.Members.FirstOrDefault(m => m.Member.Name == "ThemeDictionaries"); + foreach (var dict in (themeDictionaryMember?.Objects).Safe()) + { + BuildTopLevelResourceDictionaryInitializers(writer, dict); } TryAnnotateWithGeneratorSource(writer, suffix: "DictionaryProperty"); @@ -1021,18 +1008,18 @@ IDisposable WrapSingleton() } /// - ///Build entries in the giant switch case method for the current ResourceDictionary. + ///Build initializers for the current ResourceDictionary. /// /// The writer to use /// The associated with the dictionary. - private void BuildTopLevelResourceDictionaryCases(IIndentedStringBuilder writer, XamlObjectDefinition dictionaryObject) + private void BuildTopLevelResourceDictionaryInitializers(IIndentedStringBuilder writer, XamlObjectDefinition dictionaryObject) { TryAnnotateWithGeneratorSource(writer); var resourcesRoot = FindImplicitContentMember(dictionaryObject); var theme = GetDictionaryResourceKey(dictionaryObject); var resources = (resourcesRoot?.Objects).Safe(); - // Pre-populate case names (though this is probably no longer necessary...?) + // Pre-populate initializer names (though this is probably no longer necessary...?) var index = _dictionaryPropertyIndex; foreach (var resource in resources) { @@ -1044,7 +1031,7 @@ private void BuildTopLevelResourceDictionaryCases(IIndentedStringBuilder writer, } index++; - var propertyName = GetCaseNameForResourceKey(key, index); + var propertyName = GetInitializerNameForResourceKey(key, index); if (_topLevelQualifiedKeys.ContainsKey((theme, key))) { throw new InvalidOperationException($"Dictionary Item {resource?.Type?.Name} has duplicate key `{key}` { (theme != null ? $" in theme {theme}" : "")}."); @@ -1056,7 +1043,6 @@ private void BuildTopLevelResourceDictionaryCases(IIndentedStringBuilder writer, } } - //Create static properties var former = _themeDictionaryCurrentlyBuilding; //Will 99% of the time be null. (Mainly this is half-heartedly trying to support funky usage of recursive merged dictionaries.) _themeDictionaryCurrentlyBuilding = theme; foreach (var resource in resources) @@ -1072,17 +1058,17 @@ private void BuildTopLevelResourceDictionaryCases(IIndentedStringBuilder writer, var isStaticResourceAlias = resource.Type.Name == "StaticResource"; if (isStaticResourceAlias) { - writer.AppendLineInvariant("// Skipping case {0} for {1} {2} - StaticResource ResourceKey aliases are added directly to dictionary", _dictionaryPropertyIndex, key, theme); + writer.AppendLineInvariant("// Skipping initializer {0} for {1} {2} - StaticResource ResourceKey aliases are added directly to dictionary", _dictionaryPropertyIndex, key, theme); } else { - var caseName = GetCaseNameForResourceKey(key, _dictionaryPropertyIndex); - if (_topLevelQualifiedKeys[(theme, key)] != caseName) + var initializerName = GetInitializerNameForResourceKey(key, _dictionaryPropertyIndex); + if (_topLevelQualifiedKeys[(theme, key)] != initializerName) { - throw new InvalidOperationException($"Case name was not created correctly for {key} (theme={theme})."); + throw new InvalidOperationException($"Method name was not created correctly for {key} (theme={theme})."); } - writer.AppendLineInvariant("// Case for resource {0} {1}", key, theme != null ? "in theme {0}".InvariantCultureFormat(theme) : ""); - BuildSingleTimeInitializer(writer, caseName, () => BuildChild(writer, resourcesRoot, resource)); + writer.AppendLineInvariant("// Method for resource {0} {1}", key, theme != null ? "in theme {0}".InvariantCultureFormat(theme) : ""); + BuildSingleTimeInitializer(writer, initializerName, () => BuildChild(writer, resourcesRoot, resource)); } } _themeDictionaryCurrentlyBuilding = former; @@ -1100,18 +1086,18 @@ private void BuildStaticResourceResourceKeyReference(IIndentedStringBuilder writ } /// - /// Get name to use for case associated with a resource. + /// Get name to use for initializer associated with a resource. /// /// The resource key /// An index associated with the property. /// - /// We append the qualifier to the resource key because there may be multiple dictionary definitions (merged dictionaries, themed + /// We don't use the unqualified resource key because there may be multiple dictionary definitions (merged dictionaries, themed /// dictionaries) in the same switch block, and it's possible (probable actually, in the case of theme dictionaries) to have /// duplicate resource keys. /// - private string GetCaseNameForResourceKey(string key, int index) + private string GetInitializerNameForResourceKey(string key, int index) { - return "{0}_{1}_{2}".InvariantCultureFormat(key, _fileDefinition.ShortId, index); + return "Get_{0}_{1}".InvariantCultureFormat(_fileDefinition.ShortId, index); } /// @@ -1270,15 +1256,14 @@ private void BuildResourceDictionaryBackingClass(IIndentedStringBuilder writer, } /// - /// Build single case within the giant switch case block for resource retrieval + /// Build single initializer for resource retrieval /// - private void BuildSingleTimeInitializer(IIndentedStringBuilder writer, string caseName, Action propertyBodyBuilder) + private void BuildSingleTimeInitializer(IIndentedStringBuilder writer, string initializerName, Action propertyBodyBuilder) { TryAnnotateWithGeneratorSource(writer); - writer.AppendLineInvariant("case \"{0}\":", caseName); + writer.AppendLineInvariant("private object {0}() =>", initializerName); using (writer.Indent()) { - writer.AppendLineInvariant("return"); propertyBodyBuilder(); writer.AppendLineInvariant(";"); } @@ -2217,12 +2202,17 @@ private void BuildResourceDictionary(IIndentedStringBuilder writer, XamlMemberDe BuildStaticResourceResourceKeyReference(writer, resource); } else if (_isTopLevelDictionary - // Note: It's possible to be in a top-level ResourceDictionary file but for caseName to be null, eg for + // Note: It's possible to be in a top-level ResourceDictionary file but for initializerName to be null, eg for // a FrameworkElement.Resources declaration inside of a template - && GetResourceDictionaryCaseName(key) is string caseName) + // + // Actually in this case if it's non-null it's worse still, eg if the main dictionary sets an implicit style for + // Button, and a FrameworkElement.Resources declaration in a template *also* sets an implicit Button style, we + // don't want to use the former as a backing for the latter + && !_isInChildSubclass + && GetResourceDictionaryInitializerName(key) is string initializerName) { // - writer.AppendLineInvariant("global::Uno.UI.ResourceResolverSingleton.Instance.ResolveLazyInitializer(\"{0}\", {1})", caseName, SingletonInstanceAccess); + writer.AppendLineInvariant("(global::Windows.UI.Xaml.ResourceDictionary.ResourceInitializer){0}", initializerName); } else { @@ -3697,7 +3687,7 @@ private string GetSimpleStaticResourceRetrieval(INamedTypeSymbol targetPropertyT /// /// Get the retrieval key associated with the given resource key, if one exists, otherwise null. /// - private string GetResourceDictionaryCaseName(string keyStr) + private string GetResourceDictionaryInitializerName(string keyStr) { if (_topLevelQualifiedKeys.TryGetValue((_themeDictionaryCurrentlyBuilding, keyStr), out var qualifiedKey)) { diff --git a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs index 52899a340e39..9cec9eadf755 100644 --- a/src/Uno.UI/UI/Xaml/ResourceDictionary.cs +++ b/src/Uno.UI/UI/Xaml/ResourceDictionary.cs @@ -132,14 +132,10 @@ private void Set(object key, object value, bool throwIfPresent) throw new ArgumentException("An entry with the same key already exists."); } - if (value is ResourceInitializer || value is ProviderLazyInitializer) - { - _hasUnmaterializedItems = true; - } - if (value is ResourceInitializer resourceInitializer) { - _values[key] = new DelegateLazyInitializer(ResourceResolver.CurrentScope, resourceInitializer); + _hasUnmaterializedItems = true; + _values[key] = new LazyInitializer(ResourceResolver.CurrentScope, resourceInitializer); } else { @@ -161,7 +157,7 @@ private void TryMaterializeLazy(object key, ref object value) { _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); - newValue = lazyInitializer.GetInitializedValue(); + newValue = lazyInitializer.Initializer(); } #if !HAS_EXPENSIVE_TRYFINALLY finally @@ -372,7 +368,7 @@ private void TryMaterializeAll() foreach (var kvp in _values) { - if (kvp.Value is LazyInitializer) + if (kvp.Value is LazyInitializer lazyInitializer) { unmaterialized.Add(kvp); } @@ -418,49 +414,21 @@ internal void UpdateThemeBindings() } [EditorBrowsable(EditorBrowsableState.Never)] - // This is present for backward compatibility, but it's no longer used in Xaml-generated code with newer versions of Uno. public delegate object ResourceInitializer(); - private abstract class LazyInitializer - { - public XamlScope CurrentScope { get; } - - protected LazyInitializer(XamlScope currentScope) - { - CurrentScope = currentScope; - } - - public abstract object GetInitializedValue(); - } - /// /// Allows resources to be initialized on-demand using correct scope. /// - /// This is present for backward compatibility, but it's no longer used in Xaml-generated code with newer versions of Uno. - private class DelegateLazyInitializer : LazyInitializer + private struct LazyInitializer { - private ResourceInitializer Initializer { get; } + public XamlScope CurrentScope { get; } + public ResourceInitializer Initializer { get; } - public DelegateLazyInitializer(XamlScope currentScope, ResourceInitializer initializer) : base(currentScope) + public LazyInitializer(XamlScope currentScope, ResourceInitializer initializer) { + CurrentScope = currentScope; Initializer = initializer; } - - public override object GetInitializedValue() => Initializer(); - } - - private class ProviderLazyInitializer : LazyInitializer - { - private readonly string _qualifiedResourceKey; - private readonly IXamlLazyResourceInitializer _initializer; - - public ProviderLazyInitializer(XamlScope currentScope, string qualifiedResourceKey, IXamlLazyResourceInitializer initializer) : base(currentScope) - { - _qualifiedResourceKey = qualifiedResourceKey; - _initializer = initializer; - } - - public override object GetInitializedValue() => _initializer.GetInitializedValue(_qualifiedResourceKey); } /// @@ -481,8 +449,6 @@ public StaticResourceAliasRedirect(string resourceKey, XamlParseContext parseCon internal static object GetStaticResourceAliasPassthrough(string resourceKey, XamlParseContext parseContext) => new StaticResourceAliasRedirect(resourceKey, parseContext); - internal static object GetLazyInitializer(string qualifiedResourceKey, IXamlLazyResourceInitializer initializer) => new ProviderLazyInitializer(ResourceResolver.CurrentScope, qualifiedResourceKey, initializer); - private static class Themes { public const string Default = "Default"; diff --git a/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs b/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs index deeb85736245..19cc42c793e5 100644 --- a/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs +++ b/src/Uno.UI/UI/Xaml/ResourceResolverSingleton.cs @@ -27,8 +27,6 @@ public sealed class ResourceResolverSingleton private static ResourceResolverSingleton _instance; public static ResourceResolverSingleton Instance => _instance ??= new ResourceResolverSingleton(); - public object ResolveLazyInitializer(string qualifiedResourceKey, IXamlLazyResourceInitializer initializer) => ResourceDictionary.GetLazyInitializer(qualifiedResourceKey, initializer); - [EditorBrowsable(EditorBrowsableState.Never)] public bool ResolveResourceStatic(object key, out object value, object context) => ResourceResolver.ResolveResourceStatic(key, out value, context); From fb394325be935a61e6208fd25eadabf2c38a033e Mon Sep 17 00:00:00 2001 From: David Oliver Date: Tue, 22 Sep 2020 11:42:34 -0400 Subject: [PATCH 14/21] chore: Add more Xaml-generated debug annotations --- .../XamlGenerator/XamlFileGenerator.cs | 67 ++++++++++++------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index 543a0e4c39ee..c0ae73347c97 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -1395,6 +1395,9 @@ private void BuildInlineStyle(IIndentedStringBuilder writer, XamlObjectDefinitio } } + /// + /// Build Setter inside of a Style declaration. + /// private void BuildPropertySetter(IIndentedStringBuilder writer, string fullTargetType, string lineEnding, string property, XamlMemberDefinition valueNode, string targetInstance = null) { TryAnnotateWithGeneratorSource(writer); @@ -1429,6 +1432,7 @@ private void BuildPropertySetter(IIndentedStringBuilder writer, string fullTarge { if (isDependencyProperty) { + TryAnnotateWithGeneratorSource(writer, suffix: "InlineValueDP"); writer.AppendLineInvariant( "new global::Windows.UI.Xaml.Setter({0}.{1}Property, ({2}){3})" + lineEnding, GetGlobalizedTypeName(fullTargetType), @@ -1439,6 +1443,7 @@ private void BuildPropertySetter(IIndentedStringBuilder writer, string fullTarge } else { + TryAnnotateWithGeneratorSource(writer, suffix: "InlineValuePOCO"); writer.AppendLineInvariant( "new global::Windows.UI.Xaml.Setter<{0}>(\"{1}\", o => {3}.{1} = {2})" + lineEnding, GetGlobalizedTypeName(fullTargetType), @@ -1472,10 +1477,12 @@ private void BuildPropertySetter(IIndentedStringBuilder writer, string fullTarge var valueObject = valueNode.Objects.First(); if (HasMarkupExtension(valueNode)) { + TryAnnotateWithGeneratorSource(writer, suffix: isDependencyProperty ? "MarkupValueDP" : "MarkupValuePOCO"); writer.AppendLineInvariant(BuildBindingOption(valueNode, propertyType, prependCastToType: true)); } else { + TryAnnotateWithGeneratorSource(writer, suffix: isDependencyProperty ? "ChildValueDP" : "ChildValuePOCO"); BuildChild(writer, valueNode, valueObject); } @@ -3982,42 +3989,50 @@ private static string BuildGridLength(string memberValue) private string BuildLiteralValue(XamlMemberDefinition member, INamedTypeSymbol propertyType = null, XamlMemberDefinition owner = null, string objectUid = "") { - if (member.Objects.None()) + var literal = Inner(); + TryAnnotateWithGeneratorSource(ref literal); + + return literal; + + string Inner() { - var memberValue = member.Value?.ToString(); + if (member.Objects.None()) + { + var memberValue = member.Value?.ToString(); - var originalType = propertyType; + var originalType = propertyType; - propertyType = propertyType ?? FindPropertyType(member.Member); + propertyType = propertyType ?? FindPropertyType(member.Member); - if (propertyType != null) - { - var s = BuildLiteralValue(propertyType, memberValue, owner, member.Member.Name, objectUid); + if (propertyType != null) + { + var s = BuildLiteralValue(propertyType, memberValue, owner, member.Member.Name, objectUid); - s += $"/* {propertyType}/{originalType}, {memberValue}, {member?.Member?.DeclaringType?.Name}/{member?.Member?.Name} */"; + s += $"/* {propertyType}/{originalType}, {memberValue}, {member?.Member?.DeclaringType?.Name}/{member?.Member?.Name} */"; - return s; + return s; + } + else + { + throw new Exception($"The property {member.Owner?.Type?.Name}.{member.Member?.Name} is unknown".InvariantCultureFormat(member.Member?.Name)); + } } else { - throw new Exception($"The property {member.Owner?.Type?.Name}.{member.Member?.Name} is unknown".InvariantCultureFormat(member.Member?.Name)); - } - } - else - { - var expression = member.Objects.First(); + var expression = member.Objects.First(); - if (expression.Type.Name == "StaticResource" || expression.Type.Name == "ThemeResource") - { - return GetSimpleStaticResourceRetrieval(propertyType, expression.Members.First().Value?.ToString()); - } - else if (expression.Type.Name == "NullExtension") - { - return "null"; - } - else - { - throw new NotSupportedException("MarkupExtension {0} is not supported.".InvariantCultureFormat(expression.Type.Name)); + if (expression.Type.Name == "StaticResource" || expression.Type.Name == "ThemeResource") + { + return GetSimpleStaticResourceRetrieval(propertyType, expression.Members.First().Value?.ToString()); + } + else if (expression.Type.Name == "NullExtension") + { + return "null"; + } + else + { + throw new NotSupportedException("MarkupExtension {0} is not supported.".InvariantCultureFormat(expression.Type.Name)); + } } } } From eb04d4ac18452b53a8765d3cd5b924908be8df98 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Tue, 22 Sep 2020 14:59:14 -0400 Subject: [PATCH 15/21] chore: Reduce Xaml generated code size by not creating lambdas for Setters Where we're creating a Style Setter of form Value={StaticResource/ThemeResource ...}, call a special helper instead of declaring an unnecessary inline lambda. --- .../XamlGenerator/XamlFileGenerator.cs | 49 ++++++++++++++++++- src/Uno.UI/Helpers/Xaml/SetterHelper.cs | 32 ++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 src/Uno.UI/Helpers/Xaml/SetterHelper.cs diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index c0ae73347c97..c47f7a13c17c 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -1453,6 +1453,31 @@ private void BuildPropertySetter(IIndentedStringBuilder writer, string fullTarge ); } } + else if (isDependencyProperty && HasResourceMarkupExtension(valueNode)) + { + TryAnnotateWithGeneratorSource(writer, suffix: "IsResourceMarkupValueDP"); + var valueObject = valueNode.Objects.First(); + var key = valueObject.Members.FirstOrDefault()?.Value.ToString(); + if (key == null) + { + GenerateError(writer, "Resource markup did not define a key."); + } + // Call helper to avoid unnecessary AOT binary footprint of creating a lambda, etc + writer.AppendLineInvariant("global::Uno.UI.Helpers.Xaml.SetterHelper.GetPropertySetterWithResourceValue({0}.{1}Property, \"{2}\", {3}, default({4}))", + GetGlobalizedTypeName(fullTargetType), + property, + key, + ParseContextPropertyAccess, + propertyType + ); + + if (valueObject.Type.Name == "ThemeResource") + { + writer.AppendLineInvariant(".ApplyThemeResourceUpdateValues(\"{0}\", {1})", valueObject.Members.FirstOrDefault()?.Value, ParseContextPropertyAccess); + } + + writer.AppendLineInvariant(lineEnding); + } else { if (isDependencyProperty) @@ -1477,7 +1502,7 @@ private void BuildPropertySetter(IIndentedStringBuilder writer, string fullTarge var valueObject = valueNode.Objects.First(); if (HasMarkupExtension(valueNode)) { - TryAnnotateWithGeneratorSource(writer, suffix: isDependencyProperty ? "MarkupValueDP" : "MarkupValuePOCO"); + TryAnnotateWithGeneratorSource(writer, suffix: isDependencyProperty ? "NonResourceMarkupValueDP" : "MarkupValuePOCO"); writer.AppendLineInvariant(BuildBindingOption(valueNode, propertyType, prependCastToType: true)); } else @@ -1531,6 +1556,9 @@ private void GenerateSilentWarning(IIndentedStringBuilder writer, string message writer.AppendLineInvariant("// WARNING " + message, options); } + /// + /// Is the kind of thing that involves squiggly brackets? + /// private bool HasMarkupExtension(XamlMemberDefinition valueNode) { // Return false if the Owner is a custom markup extension @@ -1551,6 +1579,25 @@ private bool HasMarkupExtension(XamlMemberDefinition valueNode) ); } + /// + /// Is a {StaticResource ...} or {ThemeResource ...} reference? + /// + private bool HasResourceMarkupExtension(XamlMemberDefinition valueNode) + { + // Return false if the Owner is a custom markup extension + if (IsCustomMarkupExtensionType(valueNode.Owner?.Type)) + { + return false; + } + + return valueNode + .Objects + .Any(o => + o.Type.Name == "StaticResource" + || o.Type.Name == "ThemeResource" + ); + } + private bool HasCustomMarkupExtension(XamlMemberDefinition valueNode) { // Verify if a custom markup extension exists diff --git a/src/Uno.UI/Helpers/Xaml/SetterHelper.cs b/src/Uno.UI/Helpers/Xaml/SetterHelper.cs new file mode 100644 index 000000000000..763c53e3bcea --- /dev/null +++ b/src/Uno.UI/Helpers/Xaml/SetterHelper.cs @@ -0,0 +1,32 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Windows.UI.Xaml; + +namespace Uno.UI.Helpers.Xaml +{ + [EditorBrowsable(EditorBrowsableState.Never)] + // This is normally called from code generated by the Xaml parser + // + // NOTE: This class' methods may be called often from generated code. It MUST NOT have static state or a static ctor. + public static class SetterHelper + { + public static Setter GetPropertySetterWithResourceValue(DependencyProperty dependencyProperty, object key, object context, object defaultValue) + { + return new Setter(dependencyProperty, ProvideSetterValue); + + object ProvideSetterValue() + { + if (ResourceResolver.ResolveResourceStatic(key, out var value, context)) + { + return value; + } + + return defaultValue; + } + } + } +} From 44e17587e7355b18bb40f44af3cac1709752e75d Mon Sep 17 00:00:00 2001 From: David Oliver Date: Wed, 23 Sep 2020 11:54:25 -0400 Subject: [PATCH 16/21] fix: Materialize certain ResourceDictionary entries eagerly Skip the lazy initialization for resources that are 1. Not styles nor templates, and 2. Not referring to other resources. This improves the AOT binary footprint, and also fixes some scenarios where existing UWP code may be relying upon resources being eagerly created. --- .../XamlGenerator/XamlFileGenerator.cs | 53 ++++++++++++++++--- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index c47f7a13c17c..7b822530e482 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -1060,6 +1060,10 @@ private void BuildTopLevelResourceDictionaryInitializers(IIndentedStringBuilder { writer.AppendLineInvariant("// Skipping initializer {0} for {1} {2} - StaticResource ResourceKey aliases are added directly to dictionary", _dictionaryPropertyIndex, key, theme); } + else if (!ShouldLazyInitializeResource(resource)) + { + writer.AppendLineInvariant("// Skipping initializer {0} for {1} {2} - Literal declaration, will be eagerly materialized and added to the dictionary", _dictionaryPropertyIndex, key, theme); + } else { var initializerName = GetInitializerNameForResourceKey(key, _dictionaryPropertyIndex); @@ -2255,6 +2259,10 @@ private void BuildResourceDictionary(IIndentedStringBuilder writer, XamlMemberDe { BuildStaticResourceResourceKeyReference(writer, resource); } + else if (!ShouldLazyInitializeResource(resource)) + { + BuildChild(writer, null, resource); + } else if (_isTopLevelDictionary // Note: It's possible to be in a top-level ResourceDictionary file but for initializerName to be null, eg for // a FrameworkElement.Resources declaration inside of a template @@ -2297,13 +2305,44 @@ private bool ShouldLazyInitializeResource(XamlObjectDefinition resource) { var typeName = resource.Type.Name; - return - // Styles are lazily initialized for perf considerations - typeName == "Style" - // All resources not in a top-level dictionary (ie FrameworkElement.Resources and Application.Resources declarations) are lazily - // initialized, this is to be able to handle lexically-forward references correctly. (In top-level dictionaries, this is already - // handled by creating lazy static properties for each resource.) - || !_isTopLevelDictionary; + if (typeName == "Style" || typeName == "ControlTemplate" || typeName == "DataTemplate") + { + // Always lazily initialize styles and templates, since they may be large + return true; + } + + // If value declaration contains no markup, we can safely create it eagerly. Otherwise, we will wrap it in a lazy initializer + // to be able to handle lexically-forward resource references correctly. + return HasDescendantsWithMarkupExtension(resource); + + bool HasDescendantsWithMarkupExtension(XamlObjectDefinition xamlObjectDefinition) + { + foreach (var member in xamlObjectDefinition.Members) + { + if (HasMarkupExtension(member)) + { + return true; + } + + foreach (var obj in member.Objects) + { + if (HasDescendantsWithMarkupExtension(obj)) + { + return true; + } + } + } + + foreach (var obj in xamlObjectDefinition.Objects) + { + if (HasDescendantsWithMarkupExtension(obj)) + { + return true; + } + } + + return false; + } } /// From 8ad2265403e7a319abd1307ecd6c82b9f5133f2d Mon Sep 17 00:00:00 2001 From: David Oliver Date: Wed, 23 Sep 2020 12:02:33 -0400 Subject: [PATCH 17/21] test: Add unit test for eager ResourceDictionary initialization --- .../TestInitializer.cs | 19 +++++++++++ .../More/Test_Dictionary_Initializer.xaml | 7 ++++ .../Uno.UI.Tests.ViewLibrary.csproj | 5 +-- .../Xaml/Test_Control_With_Initializer.xaml | 17 ++++++++++ .../Test_Control_With_Initializer.xaml.cs | 32 +++++++++++++++++++ src/Uno.UI.Tests/Uno.UI.Tests.csproj | 14 +------- .../Given_ResourceDictionary.cs | 9 ++++++ 7 files changed, 86 insertions(+), 17 deletions(-) create mode 100644 src/Uno.UI.Tests.ViewLibrary/TestInitializer.cs create mode 100644 src/Uno.UI.Tests.ViewLibrary/Themes/More/Test_Dictionary_Initializer.xaml create mode 100644 src/Uno.UI.Tests/App/Xaml/Test_Control_With_Initializer.xaml create mode 100644 src/Uno.UI.Tests/App/Xaml/Test_Control_With_Initializer.xaml.cs diff --git a/src/Uno.UI.Tests.ViewLibrary/TestInitializer.cs b/src/Uno.UI.Tests.ViewLibrary/TestInitializer.cs new file mode 100644 index 000000000000..e010deff40ad --- /dev/null +++ b/src/Uno.UI.Tests.ViewLibrary/TestInitializer.cs @@ -0,0 +1,19 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Uno.UI.Tests.ViewLibrary +{ + // This type should only be created by the Test_Dictionary_Initializer dictionary + public class TestInitializer + { + public static bool IsInitialized { get; private set; } + + public TestInitializer() + { + IsInitialized = true; + } + } +} diff --git a/src/Uno.UI.Tests.ViewLibrary/Themes/More/Test_Dictionary_Initializer.xaml b/src/Uno.UI.Tests.ViewLibrary/Themes/More/Test_Dictionary_Initializer.xaml new file mode 100644 index 000000000000..848e070afa6a --- /dev/null +++ b/src/Uno.UI.Tests.ViewLibrary/Themes/More/Test_Dictionary_Initializer.xaml @@ -0,0 +1,7 @@ + + + + diff --git a/src/Uno.UI.Tests.ViewLibrary/Uno.UI.Tests.ViewLibrary.csproj b/src/Uno.UI.Tests.ViewLibrary/Uno.UI.Tests.ViewLibrary.csproj index 68aa7c2a2610..bec830897e7a 100644 --- a/src/Uno.UI.Tests.ViewLibrary/Uno.UI.Tests.ViewLibrary.csproj +++ b/src/Uno.UI.Tests.ViewLibrary/Uno.UI.Tests.ViewLibrary.csproj @@ -64,10 +64,7 @@ - - - - + diff --git a/src/Uno.UI.Tests/App/Xaml/Test_Control_With_Initializer.xaml b/src/Uno.UI.Tests/App/Xaml/Test_Control_With_Initializer.xaml new file mode 100644 index 000000000000..a1b6ab727fc6 --- /dev/null +++ b/src/Uno.UI.Tests/App/Xaml/Test_Control_With_Initializer.xaml @@ -0,0 +1,17 @@ + + + + + + + + + diff --git a/src/Uno.UI.Tests/App/Xaml/Test_Control_With_Initializer.xaml.cs b/src/Uno.UI.Tests/App/Xaml/Test_Control_With_Initializer.xaml.cs new file mode 100644 index 000000000000..f85b180abdae --- /dev/null +++ b/src/Uno.UI.Tests/App/Xaml/Test_Control_With_Initializer.xaml.cs @@ -0,0 +1,32 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices.WindowsRuntime; +using Uno.UI.Tests.App.Views; +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.Tests.App.Xaml +{ + /// + /// An empty page that can be used on its own or navigated to within a Frame. + /// + public sealed partial class Test_Control_With_Initializer : UserControl + { + + public Test_Control_With_Initializer() + { + this.InitializeComponent(); + } + } +} diff --git a/src/Uno.UI.Tests/Uno.UI.Tests.csproj b/src/Uno.UI.Tests/Uno.UI.Tests.csproj index b69c1e7f987c..8e0b6aa33e51 100644 --- a/src/Uno.UI.Tests/Uno.UI.Tests.csproj +++ b/src/Uno.UI.Tests/Uno.UI.Tests.csproj @@ -96,20 +96,8 @@ - - - - - - - - + - - - - - diff --git a/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs b/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs index 5961d79c4329..81792c065f67 100644 --- a/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs +++ b/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs @@ -6,6 +6,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Uno.UI.Tests.App.Xaml; using Uno.UI.Tests.Helpers; +using Uno.UI.Tests.ViewLibrary; #if !NETFX_CORE using Uno.UI.Xaml; #endif @@ -684,5 +685,13 @@ public void When_XamlControlsResources() Assert.IsTrue(xcr.ContainsKey(typeof(Button))); Assert.IsInstanceOfType(xcr[typeof(Button)], typeof(Style)); } + + [TestMethod] + public void When_Needs_Eager_Materialization() + { + Assert.IsFalse(TestInitializer.IsInitialized); + var control = new Test_Control_With_Initializer(); + Assert.IsTrue(TestInitializer.IsInitialized); + } } } From f19e1668b662fc691aa65f177ef6186652e24db6 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Wed, 23 Sep 2020 12:09:26 -0400 Subject: [PATCH 18/21] chore: Remove unused members --- .../Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index 7b822530e482..fde6146332fd 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -25,7 +25,6 @@ namespace Uno.UI.SourceGenerators.XamlGenerator { internal partial class XamlFileGenerator { - private const string ImplicitStyleMarker = "__ImplicitStyle_"; private const string GlobalPrefix = "global::"; private const string QualifiedNamespaceMarker = "."; @@ -159,7 +158,6 @@ internal partial class XamlFileGenerator private string SingletonClassName => $"ResourceDictionarySingleton__{_fileDefinition.ShortId}"; private const string DictionaryProviderInterfaceName = "global::Uno.UI.IXamlResourceDictionaryProvider"; - private const string LazyInitializerInterfaceName = "global::Uno.UI.IXamlLazyResourceInitializer"; static XamlFileGenerator() { From 3e4d1aa4a63bb6e13078a4fd25c2d3f1b3086243 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Wed, 23 Sep 2020 13:30:50 -0400 Subject: [PATCH 19/21] test: Verify that spaces in ResourceDictionary keys are now supported --- src/Uno.UI.Tests/App/Xaml/Test_Dictionary.xaml | 2 ++ src/Uno.UI.Tests/App/Xaml/Test_Page_Other.xaml | 3 +++ .../Windows_UI_Xaml/Given_ResourceDictionary.cs | 8 ++++++++ 3 files changed, 13 insertions(+) diff --git a/src/Uno.UI.Tests/App/Xaml/Test_Dictionary.xaml b/src/Uno.UI.Tests/App/Xaml/Test_Dictionary.xaml index 2ec243d973a6..ffc2420f000e 100644 --- a/src/Uno.UI.Tests/App/Xaml/Test_Dictionary.xaml +++ b/src/Uno.UI.Tests/App/Xaml/Test_Dictionary.xaml @@ -55,4 +55,6 @@ + diff --git a/src/Uno.UI.Tests/App/Xaml/Test_Page_Other.xaml b/src/Uno.UI.Tests/App/Xaml/Test_Page_Other.xaml index 7ce55554e798..cab2132d45b0 100644 --- a/src/Uno.UI.Tests/App/Xaml/Test_Page_Other.xaml +++ b/src/Uno.UI.Tests/App/Xaml/Test_Page_Other.xaml @@ -45,5 +45,8 @@ x:FieldModifier="public" Foreground="{StaticResource BituminousColorBrush}" /> + diff --git a/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs b/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs index 81792c065f67..d96acf8ada44 100644 --- a/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs +++ b/src/Uno.UI.Tests/Windows_UI_Xaml/Given_ResourceDictionary.cs @@ -693,5 +693,13 @@ public void When_Needs_Eager_Materialization() var control = new Test_Control_With_Initializer(); Assert.IsTrue(TestInitializer.IsInitialized); } + + [TestMethod] + public void When_Space_In_Key() + { + var page = new Test_Page_Other(); + var border = page.SpaceInKeyBorder; + Assert.AreEqual(Colors.SlateBlue, (border.Background as SolidColorBrush).Color); + } } } From 79771afd4e144f0e2066ca56768c43094411bd5b Mon Sep 17 00:00:00 2001 From: David Oliver Date: Wed, 23 Sep 2020 14:37:23 -0400 Subject: [PATCH 20/21] chore: Make HasDescendantsWithMarkupExtension top level --- .../XamlGenerator/XamlFileGenerator.cs | 63 ++++++++++--------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index fde6146332fd..31597d7ceeb3 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -1629,6 +1629,38 @@ private bool HasMarkupExtensionNeedingComponent(XamlObjectDefinition objectDefin .Members .Any(o => o.Objects.Any(o => o.Type.Name == "Bind" || o.Type.Name == "StaticResource" || o.Type.Name == "ThemeResource")); + /// + /// Does this node or any nested nodes have markup extensions? + /// + private bool HasDescendantsWithMarkupExtension(XamlObjectDefinition xamlObjectDefinition) + { + foreach (var member in xamlObjectDefinition.Members) + { + if (HasMarkupExtension(member)) + { + return true; + } + + foreach (var obj in member.Objects) + { + if (HasDescendantsWithMarkupExtension(obj)) + { + return true; + } + } + } + + foreach (var obj in xamlObjectDefinition.Objects) + { + if (HasDescendantsWithMarkupExtension(obj)) + { + return true; + } + } + + return false; + } + private bool IsCustomMarkupExtensionType(XamlType xamlType) { if (xamlType == null) @@ -2311,36 +2343,7 @@ private bool ShouldLazyInitializeResource(XamlObjectDefinition resource) // If value declaration contains no markup, we can safely create it eagerly. Otherwise, we will wrap it in a lazy initializer // to be able to handle lexically-forward resource references correctly. - return HasDescendantsWithMarkupExtension(resource); - - bool HasDescendantsWithMarkupExtension(XamlObjectDefinition xamlObjectDefinition) - { - foreach (var member in xamlObjectDefinition.Members) - { - if (HasMarkupExtension(member)) - { - return true; - } - - foreach (var obj in member.Objects) - { - if (HasDescendantsWithMarkupExtension(obj)) - { - return true; - } - } - } - - foreach (var obj in xamlObjectDefinition.Objects) - { - if (HasDescendantsWithMarkupExtension(obj)) - { - return true; - } - } - - return false; - } + return HasDescendantsWithMarkupExtension(resource); } /// From 50356398fd8d351567553b7db39272edd6136f65 Mon Sep 17 00:00:00 2001 From: David Oliver Date: Wed, 23 Sep 2020 14:38:16 -0400 Subject: [PATCH 21/21] chore: Adjust comment wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jérôme Laban --- src/Uno.UI/Helpers/Xaml/SetterHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI/Helpers/Xaml/SetterHelper.cs b/src/Uno.UI/Helpers/Xaml/SetterHelper.cs index 763c53e3bcea..57e22da8f203 100644 --- a/src/Uno.UI/Helpers/Xaml/SetterHelper.cs +++ b/src/Uno.UI/Helpers/Xaml/SetterHelper.cs @@ -11,7 +11,7 @@ namespace Uno.UI.Helpers.Xaml [EditorBrowsable(EditorBrowsableState.Never)] // This is normally called from code generated by the Xaml parser // - // NOTE: This class' methods may be called often from generated code. It MUST NOT have static state or a static ctor. + // NOTE: This class' methods may be called often from generated code. It MUST NOT have static state or a static ctor as it impacts the size of the generated code of the call site of this method when using mono's AOT engine. public static class SetterHelper { public static Setter GetPropertySetterWithResourceValue(DependencyProperty dependencyProperty, object key, object context, object defaultValue)