Skip to content

Commit 529624c

Browse files
author
msftbot[bot]
authored
Fixed ExpressionNode.EnsureReferenceInfo() crash with .NET Native (#4014)
## Closes #4011 <!-- Add the relevant issue number after the "#" mentioned above (for ex: Fixes #1234) which will automatically close the issue once the PR is merged. --> <!-- Add a brief overview here of the feature/bug & fix. --> ## PR Type What kind of change does this PR introduce? <!-- Please uncomment one or more that apply to this PR. --> - Bugfix <!-- - Feature --> <!-- - Code style update (formatting) --> <!-- - Refactoring (no functional changes, no api changes) --> <!-- - Build or CI related changes --> <!-- - Documentation content changes --> <!-- - Sample app changes --> <!-- - Other... Please describe: --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying, or link to a relevant issue. --> Crash with .NET Native, documented in the related issue. ## What is the new behavior? <!-- Describe how was this issue resolved or changed? --> The logic has been changed to not rely on APIs that might have a behavioral change under .NET Native. ## PR Checklist Please check if your PR fulfills the following requirements: - [X] Tested code with current [supported SDKs](../readme.md#supported) - [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link -->~~ - [ ] ~~Sample in sample app has been added / updated (for bug fixes / features)~~ - [ ] ~~Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~ - [X] New major technical changes in the toolkit have or will be added to the [Wiki](https://github.com/windows-toolkit/WindowsCommunityToolkit/wiki) e.g. build changes, source generators, testing infrastructure, sample creation changes, etc... - [X] Tests for the changes have been added (for bug fixes / features) (if applicable) - [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*) - [X] Contains **NO** breaking changes
2 parents 6961808 + a042ce3 commit 529624c

File tree

8 files changed

+42
-35
lines changed

8 files changed

+42
-35
lines changed

Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionNodes/ExpressionNode.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,14 +282,10 @@ internal void EnsureReferenceInfo()
282282
}
283283

284284
// Create a map to store the generated paramNames for each CompObj
285-
uint id = 0;
286285
_compObjToParamNameMap = new Dictionary<CompositionObject, string>();
287286
foreach (var compObj in compObjects)
288287
{
289-
// compObj.ToString() will return something like "Windows.UI.Composition.SpriteVisual"
290-
// Make it look like "SpriteVisual_1"
291-
string paramName = compObj.ToString();
292-
paramName = $"{paramName.Substring(paramName.LastIndexOf('.') + 1)}_{++id}"; // make sure the created param name doesn't overwrite a custom name
288+
string paramName = Guid.NewGuid().ToUppercaseAsciiLetters();
293289

294290
_compObjToParamNameMap.Add(compObj, paramName);
295291
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Diagnostics.Contracts;
7+
8+
namespace Microsoft.Toolkit.Uwp.UI.Animations
9+
{
10+
/// <summary>
11+
/// An extension <see langword="class"/> for the <see cref="Guid"/> type
12+
/// </summary>
13+
internal static class GuidExtensions
14+
{
15+
/// <summary>
16+
/// Returns a <see cref="string"/> representation of a <see cref="Guid"/> only made of uppercase letters
17+
/// </summary>
18+
/// <param name="guid">The input <see cref="Guid"/> to process</param>
19+
/// <returns>A <see cref="string"/> representation of <paramref name="guid"/> only made up of letters in the [A-Z] range</returns>
20+
[Pure]
21+
public static string ToUppercaseAsciiLetters(in this Guid guid)
22+
{
23+
// Composition IDs must only be composed of characters in the [A-Z0-9_] set,
24+
// and also have the restriction that the initial character cannot be a digit.
25+
// Because of this, we need to prepend an underscore to a serialized guid to
26+
// avoid cases where the first character is a digit. Additionally, we're forced
27+
// to use ToUpper() here because ToString("N") currently returns a lowercase
28+
// hexadecimal string. Note: this extension might be improved once we move to
29+
// .NET 5 in the WinUI 3 release, by using string.Create<TState>(...) to only
30+
// have a single string allocation, and then using Guid.TryFormat(...) to
31+
// serialize the guid in place over the Span<char> starting from the second
32+
// character. For now, this implementation is fine on UWP and still fast enough.
33+
return $"_{guid.ToString("N").ToUpper()}";
34+
}
35+
}
36+
}

Microsoft.Toolkit.Uwp.UI.Media/Extensions/System/GuidExtensions.cs

Lines changed: 0 additions & 30 deletions
This file was deleted.

Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Effects.Internals.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Diagnostics.Contracts;
77
using System.Threading.Tasks;
88
using Microsoft.Graphics.Canvas.Effects;
9+
using Microsoft.Toolkit.Uwp.UI.Animations;
910
using Windows.Graphics.Effects;
1011
using Windows.UI;
1112
using Windows.UI.Composition;

Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Effects.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Linq;
99
using System.Threading.Tasks;
1010
using Microsoft.Graphics.Canvas.Effects;
11+
using Microsoft.Toolkit.Uwp.UI.Animations;
1112
using Windows.Graphics.Effects;
1213
using Windows.UI;
1314
using Windows.UI.Composition;

Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Initialization.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Threading.Tasks;
99
using Microsoft.Graphics.Canvas;
1010
using Microsoft.Graphics.Canvas.Effects;
11+
using Microsoft.Toolkit.Uwp.UI.Animations;
1112
using Microsoft.Toolkit.Uwp.UI.Media.Helpers;
1213
using Microsoft.Toolkit.Uwp.UI.Media.Helpers.Cache;
1314
using Windows.Graphics.Effects;

Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.Merge.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Linq;
99
using System.Threading.Tasks;
1010
using Microsoft.Graphics.Canvas.Effects;
11+
using Microsoft.Toolkit.Uwp.UI.Animations;
1112
using Windows.Graphics.Effects;
1213
using Windows.UI.Composition;
1314
using CanvasBlendEffect = Microsoft.Graphics.Canvas.Effects.BlendEffect;

Microsoft.Toolkit.Uwp.UI.Media/Pipelines/PipelineBuilder.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Collections.Generic;
77
using System.Diagnostics.Contracts;
88
using System.Threading.Tasks;
9+
using Microsoft.Toolkit.Uwp.UI.Animations;
910
using Windows.Graphics.Effects;
1011
using Windows.UI.Composition;
1112
using Windows.UI.Xaml;

0 commit comments

Comments
 (0)