From e045893d92416044281084253268ffededf64ec1 Mon Sep 17 00:00:00 2001 From: Arlo Godfrey Date: Tue, 6 Sep 2022 14:04:59 -0500 Subject: [PATCH] Fixed an issue where expression nodes created with custom param names were incorrectly cleared during internal cleanup --- .../Expressions/ExpressionFunctions.cs | 4 +- .../ExpressionNodes/ExpressionNode.cs | 50 +++++++++++-------- .../ReferenceNodes/ReferenceNode.cs | 4 +- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionFunctions.cs b/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionFunctions.cs index b0f7b69b40d..9cb0b1d14ba 100644 --- a/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionFunctions.cs +++ b/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionFunctions.cs @@ -1199,11 +1199,11 @@ internal static T Function(ExpressionNodeType nodeType, params ExpressionNode where T : ExpressionNode { T newNode = ExpressionNode.CreateExpressionNode(); + newNode.NodeType = nodeType; - (newNode as ExpressionNode).NodeType = nodeType; foreach (var param in expressionFunctionParams) { - (newNode as ExpressionNode).Children.Add(param); + newNode.Children.Add(param); } return newNode; diff --git a/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionNodes/ExpressionNode.cs b/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionNodes/ExpressionNode.cs index e864b725a03..b8d72b60941 100644 --- a/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionNodes/ExpressionNode.cs +++ b/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionNodes/ExpressionNode.cs @@ -17,7 +17,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Animations.Expressions public abstract class ExpressionNode : IDisposable { private List _objRefList = null; - private Dictionary _compObjToParamNameMap = null; + private Dictionary _compObjToNodeNameMap = null; private Dictionary _constParamMap = new Dictionary(StringComparer.CurrentCultureIgnoreCase); /// @@ -144,7 +144,7 @@ public void SetMatrix4x4Parameter(string parameterName, Matrix4x4 value) public void Dispose() { _objRefList = null; - _compObjToParamNameMap = null; + this._compObjToNodeNameMap = null; _constParamMap = null; Subchannels = null; PropertyName = null; @@ -227,16 +227,16 @@ internal static T CreateValueKeyword(ValueKeywordKind keywordKind) { T node = CreateExpressionNode(); - (node as ExpressionNode).ParamName = null; + node.ParamName = null; switch (keywordKind) { case ValueKeywordKind.CurrentValue: - (node as ExpressionNode).NodeType = ExpressionNodeType.CurrentValueProperty; + node.NodeType = ExpressionNodeType.CurrentValueProperty; break; case ValueKeywordKind.StartingValue: - (node as ExpressionNode).NodeType = ExpressionNodeType.StartingValueProperty; + node.NodeType = ExpressionNodeType.StartingValueProperty; break; default: @@ -263,11 +263,11 @@ internal string ToExpressionString() /// /// Clears the reference information. /// - /// Reference and paramName can't both be null + /// Reference and paramName can't both be null internal void ClearReferenceInfo() { _objRefList = null; - ParamName = null; + this.NodeName = null; foreach (var child in Children) { child.ClearReferenceInfo(); @@ -277,7 +277,7 @@ internal void ClearReferenceInfo() /// /// Ensures the reference information. /// - /// Reference and paramName can't both be null + /// Reference and paramName can't both be null internal void EnsureReferenceInfo() { if (_objRefList == null) @@ -290,20 +290,20 @@ internal void EnsureReferenceInfo() HashSet compObjects = new HashSet(); foreach (var refNode in referenceNodes) { - if ((refNode.Reference != null) && (refNode.GetReferenceParamString() == null)) + if ((refNode.Reference != null) && (refNode.GetReferenceNodeString() == null)) { compObjects.Add(refNode.Reference); } } // Create a map to store the generated paramNames for each CompObj - _compObjToParamNameMap = new Dictionary(); + this._compObjToNodeNameMap = new Dictionary(); var paramCount = 0u; foreach (var compObj in compObjects) { - string paramName = CreateUniqueParamNameFromIndex(paramCount++); + string nodeName = !string.IsNullOrWhiteSpace(ParamName) ? ParamName : CreateUniqueNodeNameFromIndex(paramCount++); - _compObjToParamNameMap.Add(compObj, paramName); + this._compObjToNodeNameMap.Add(compObj, nodeName); } // Go through all reference nodes again to create our full list of referenceInfo. This time, if @@ -311,21 +311,21 @@ internal void EnsureReferenceInfo() _objRefList = new List(); foreach (var refNode in referenceNodes) { - string paramName = refNode.GetReferenceParamString(); + string nodeName = refNode.GetReferenceNodeString(); - if ((refNode.Reference == null) && (paramName == null)) + if ((refNode.Reference == null) && (nodeName == null)) { // This can't happen - if the ref is null it must be because it's a named param - throw new Exception("Reference and paramName can't both be null"); + throw new Exception($"{nameof(refNode.Reference)} and {nameof(nodeName)} can't both be null"); } - if (paramName == null) + if (nodeName == null) { - paramName = _compObjToParamNameMap[refNode.Reference]; + nodeName = this._compObjToNodeNameMap[refNode.Reference]; } - _objRefList.Add(new ReferenceInfo(paramName, refNode.Reference)); - refNode.ParamName = paramName; + _objRefList.Add(new ReferenceInfo(nodeName, refNode.Reference)); + refNode.NodeName = nodeName; } } @@ -335,7 +335,7 @@ internal void EnsureReferenceInfo() // important in this context as the only critical property to maintain is to have // a unique mapping to each input value to the resulting sequence of letters. [SkipLocalsInit] - static unsafe string CreateUniqueParamNameFromIndex(uint i) + static unsafe string CreateUniqueNodeNameFromIndex(uint i) { const int alphabetLength = 'Z' - 'A' + 1; @@ -592,7 +592,7 @@ private string ToExpressionStringInternal() throw new Exception("References cannot have children"); } - ret = (this as ReferenceNode).GetReferenceParamString(); + ret = (this as ReferenceNode).GetReferenceNodeString(); } else if (NodeType == ExpressionNodeType.ReferenceProperty) { @@ -700,11 +700,17 @@ public ReferenceInfo(string paramName, CompositionObject compObj) internal List Children { get; set; } = new List(); /// - /// Gets or sets the name of the parameter. + /// Gets or sets the user-defined name of the parameter. /// /// The name of the parameter. internal string ParamName { get; set; } + /// + /// Gets or sets the unique name for the expression node. Can be user-defined or generated. + /// + /// The name of the parameter. + internal string NodeName { get; set; } + /// /// Gets or sets the expression animation. /// diff --git a/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ReferenceNodes/ReferenceNode.cs b/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ReferenceNodes/ReferenceNode.cs index fb95338d1e7..da8a8c874c4 100644 --- a/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ReferenceNodes/ReferenceNode.cs +++ b/Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ReferenceNodes/ReferenceNode.cs @@ -125,7 +125,7 @@ public Matrix4x4Node GetMatrix4x4Property(string propertyName) /// Gets the reference parameter string. /// /// System.String. - internal string GetReferenceParamString() + internal string GetReferenceNodeString() { if (NodeType == ExpressionNodeType.TargetReference) { @@ -133,7 +133,7 @@ internal string GetReferenceParamString() } else { - return ParamName; + return NodeName; } }