From 0c1fb1b179a1c72177827d7d2ecfd2c0de9a26e3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 5 May 2021 00:29:46 +0200 Subject: [PATCH 1/2] Improved Guid.ToUppercaseAsciiLetters() extension --- .../Extensions/System/GuidExtensions.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Microsoft.Toolkit.Uwp.UI.Media/Extensions/System/GuidExtensions.cs b/Microsoft.Toolkit.Uwp.UI.Media/Extensions/System/GuidExtensions.cs index 6d3fa5e600d..e096e52e0cd 100644 --- a/Microsoft.Toolkit.Uwp.UI.Media/Extensions/System/GuidExtensions.cs +++ b/Microsoft.Toolkit.Uwp.UI.Media/Extensions/System/GuidExtensions.cs @@ -4,7 +4,6 @@ using System; using System.Diagnostics.Contracts; -using System.Linq; namespace Microsoft.Toolkit.Uwp.UI.Media { @@ -19,12 +18,19 @@ internal static class GuidExtensions /// The input to process /// A representation of only made up of letters in the [A-Z] range [Pure] - public static string ToUppercaseAsciiLetters(this Guid guid) + public static string ToUppercaseAsciiLetters(in this Guid guid) { - return new string(( - from c in guid.ToString("N") - let l = char.IsDigit(c) ? (char)('G' + c - '0') : c - select l).ToArray()); + // Composition IDs must only be composed of characters in the [A-Z0-9_] set, + // and also have the restriction that the initial character cannot be a digit. + // Because of this, we need to prepend an underscore to a serialized guid to + // avoid cases where the first character is a digit. Additionally, we're forced + // to use ToUpper() here because ToString("N") currently returns a lowercase + // hexadecimal string. Note: this extension might be improved once we move to + // .NET 5 in the WinUI 3 release, by using string.Create(...) to only + // have a single string allocation, and then using Guid.TryFormat(...) to + // serialize the guid in place over the Span starting from the second + // character. For now, this implementation is fine on UWP and still fast enough. + return $"_{guid.ToString("N").ToUpper()}"; } } } From a042ce381b522e958bf83b76fb74923025769b9c Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 5 May 2021 00:34:06 +0200 Subject: [PATCH 2/2] Fixed param name generation in ExpressionNode Fixes https://github.com/windows-toolkit/WindowsCommunityToolkit/issues/4011 --- .../Expressions/ExpressionNodes/ExpressionNode.cs | 6 +----- .../Extensions/System/GuidExtensions.cs | 2 +- .../Pipelines/PipelineBuilder.Effects.Internals.cs | 1 + .../Pipelines/PipelineBuilder.Effects.cs | 1 + .../Pipelines/PipelineBuilder.Initialization.cs | 1 + .../Pipelines/PipelineBuilder.Merge.cs | 1 + Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.cs | 1 + 7 files changed, 7 insertions(+), 6 deletions(-) rename {Microsoft.Toolkit.Uwp.UI.Media => Microsoft.Toolkit.Uwp.UI.Animations}/Extensions/System/GuidExtensions.cs (97%) diff --git a/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionNodes/ExpressionNode.cs b/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionNodes/ExpressionNode.cs index e578a450adf..51e6fbd3bfd 100644 --- a/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionNodes/ExpressionNode.cs +++ b/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionNodes/ExpressionNode.cs @@ -282,14 +282,10 @@ internal void EnsureReferenceInfo() } // Create a map to store the generated paramNames for each CompObj - uint id = 0; _compObjToParamNameMap = new Dictionary(); foreach (var compObj in compObjects) { - // compObj.ToString() will return something like "Windows.UI.Composition.SpriteVisual" - // Make it look like "SpriteVisual_1" - string paramName = compObj.ToString(); - paramName = $"{paramName.Substring(paramName.LastIndexOf('.') + 1)}_{++id}"; // make sure the created param name doesn't overwrite a custom name + string paramName = Guid.NewGuid().ToUppercaseAsciiLetters(); _compObjToParamNameMap.Add(compObj, paramName); } diff --git a/Microsoft.Toolkit.Uwp.UI.Media/Extensions/System/GuidExtensions.cs b/Microsoft.Toolkit.Uwp.UI.Animations/Extensions/System/GuidExtensions.cs similarity index 97% rename from Microsoft.Toolkit.Uwp.UI.Media/Extensions/System/GuidExtensions.cs rename to Microsoft.Toolkit.Uwp.UI.Animations/Extensions/System/GuidExtensions.cs index e096e52e0cd..6a36db86afb 100644 --- a/Microsoft.Toolkit.Uwp.UI.Media/Extensions/System/GuidExtensions.cs +++ b/Microsoft.Toolkit.Uwp.UI.Animations/Extensions/System/GuidExtensions.cs @@ -5,7 +5,7 @@ using System; using System.Diagnostics.Contracts; -namespace Microsoft.Toolkit.Uwp.UI.Media +namespace Microsoft.Toolkit.Uwp.UI.Animations { /// /// An extension for the type diff --git a/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Effects.Internals.cs b/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Effects.Internals.cs index 2a0a9960925..8a63548ca7f 100644 --- a/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Effects.Internals.cs +++ b/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Effects.Internals.cs @@ -6,6 +6,7 @@ using System.Diagnostics.Contracts; using System.Threading.Tasks; using Microsoft.Graphics.Canvas.Effects; +using Microsoft.Toolkit.Uwp.UI.Animations; using Windows.Graphics.Effects; using Windows.UI; using Windows.UI.Composition; diff --git a/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Effects.cs b/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Effects.cs index bc0980e6e35..317f17b60b7 100644 --- a/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Effects.cs +++ b/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Effects.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.Graphics.Canvas.Effects; +using Microsoft.Toolkit.Uwp.UI.Animations; using Windows.Graphics.Effects; using Windows.UI; using Windows.UI.Composition; diff --git a/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Initialization.cs b/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Initialization.cs index 92f3bb86f62..000329811d2 100644 --- a/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Initialization.cs +++ b/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Initialization.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Microsoft.Graphics.Canvas; using Microsoft.Graphics.Canvas.Effects; +using Microsoft.Toolkit.Uwp.UI.Animations; using Microsoft.Toolkit.Uwp.UI.Media.Helpers; using Microsoft.Toolkit.Uwp.UI.Media.Helpers.Cache; using Windows.Graphics.Effects; diff --git a/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Merge.cs b/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Merge.cs index b95b6b3bd99..73ea0f50a8a 100644 --- a/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Merge.cs +++ b/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Merge.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.Graphics.Canvas.Effects; +using Microsoft.Toolkit.Uwp.UI.Animations; using Windows.Graphics.Effects; using Windows.UI.Composition; using CanvasBlendEffect = Microsoft.Graphics.Canvas.Effects.BlendEffect; diff --git a/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.cs b/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.cs index 47789f26de9..3216456ec1e 100644 --- a/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.cs +++ b/Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Diagnostics.Contracts; using System.Threading.Tasks; +using Microsoft.Toolkit.Uwp.UI.Animations; using Windows.Graphics.Effects; using Windows.UI.Composition; using Windows.UI.Xaml;