Skip to content

Commit c9895b2

Browse files
Arlodotexemichael-hawker
authored andcommitted
Fixed an issue where expression nodes created with custom param names were incorrectly cleared during internal cleanup
1 parent 06aec3e commit c9895b2

File tree

3 files changed

+32
-26
lines changed

3 files changed

+32
-26
lines changed

Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ExpressionFunctions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1199,11 +1199,11 @@ internal static T Function<T>(ExpressionNodeType nodeType, params ExpressionNode
11991199
where T : ExpressionNode
12001200
{
12011201
T newNode = ExpressionNode.CreateExpressionNode<T>();
1202+
newNode.NodeType = nodeType;
12021203

1203-
(newNode as ExpressionNode).NodeType = nodeType;
12041204
foreach (var param in expressionFunctionParams)
12051205
{
1206-
(newNode as ExpressionNode).Children.Add(param);
1206+
newNode.Children.Add(param);
12071207
}
12081208

12091209
return newNode;

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

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace Microsoft.Toolkit.Uwp.UI.Animations.Expressions
1717
public abstract class ExpressionNode : IDisposable
1818
{
1919
private List<ReferenceInfo> _objRefList = null;
20-
private Dictionary<CompositionObject, string> _compObjToParamNameMap = null;
20+
private Dictionary<CompositionObject, string> _compObjToNodeNameMap = null;
2121
private Dictionary<string, object> _constParamMap = new Dictionary<string, object>(StringComparer.CurrentCultureIgnoreCase);
2222

2323
/// <summary>
@@ -144,7 +144,7 @@ public void SetMatrix4x4Parameter(string parameterName, Matrix4x4 value)
144144
public void Dispose()
145145
{
146146
_objRefList = null;
147-
_compObjToParamNameMap = null;
147+
this._compObjToNodeNameMap = null;
148148
_constParamMap = null;
149149
Subchannels = null;
150150
PropertyName = null;
@@ -227,16 +227,16 @@ internal static T CreateValueKeyword<T>(ValueKeywordKind keywordKind)
227227
{
228228
T node = CreateExpressionNode<T>();
229229

230-
(node as ExpressionNode).ParamName = null;
230+
node.ParamName = null;
231231

232232
switch (keywordKind)
233233
{
234234
case ValueKeywordKind.CurrentValue:
235-
(node as ExpressionNode).NodeType = ExpressionNodeType.CurrentValueProperty;
235+
node.NodeType = ExpressionNodeType.CurrentValueProperty;
236236
break;
237237

238238
case ValueKeywordKind.StartingValue:
239-
(node as ExpressionNode).NodeType = ExpressionNodeType.StartingValueProperty;
239+
node.NodeType = ExpressionNodeType.StartingValueProperty;
240240
break;
241241

242242
default:
@@ -263,11 +263,11 @@ internal string ToExpressionString()
263263
/// <summary>
264264
/// Clears the reference information.
265265
/// </summary>
266-
/// <exception cref="System.Exception">Reference and paramName can't both be null</exception>
266+
/// <exception cref="Exception">Reference and paramName can't both be null</exception>
267267
internal void ClearReferenceInfo()
268268
{
269269
_objRefList = null;
270-
ParamName = null;
270+
this.NodeName = null;
271271
foreach (var child in Children)
272272
{
273273
child.ClearReferenceInfo();
@@ -277,7 +277,7 @@ internal void ClearReferenceInfo()
277277
/// <summary>
278278
/// Ensures the reference information.
279279
/// </summary>
280-
/// <exception cref="System.Exception">Reference and paramName can't both be null</exception>
280+
/// <exception cref="Exception">Reference and paramName can't both be null</exception>
281281
internal void EnsureReferenceInfo()
282282
{
283283
if (_objRefList == null)
@@ -290,42 +290,42 @@ internal void EnsureReferenceInfo()
290290
HashSet<CompositionObject> compObjects = new HashSet<CompositionObject>();
291291
foreach (var refNode in referenceNodes)
292292
{
293-
if ((refNode.Reference != null) && (refNode.GetReferenceParamString() == null))
293+
if ((refNode.Reference != null) && (refNode.GetReferenceNodeString() == null))
294294
{
295295
compObjects.Add(refNode.Reference);
296296
}
297297
}
298298

299299
// Create a map to store the generated paramNames for each CompObj
300-
_compObjToParamNameMap = new Dictionary<CompositionObject, string>();
300+
this._compObjToNodeNameMap = new Dictionary<CompositionObject, string>();
301301
var paramCount = 0u;
302302
foreach (var compObj in compObjects)
303303
{
304-
string paramName = CreateUniqueParamNameFromIndex(paramCount++);
304+
string nodeName = !string.IsNullOrWhiteSpace(ParamName) ? ParamName : CreateUniqueNodeNameFromIndex(paramCount++);
305305

306-
_compObjToParamNameMap.Add(compObj, paramName);
306+
this._compObjToNodeNameMap.Add(compObj, nodeName);
307307
}
308308

309309
// Go through all reference nodes again to create our full list of referenceInfo. This time, if
310310
// the param name is null, look it up from our new map and store it.
311311
_objRefList = new List<ReferenceInfo>();
312312
foreach (var refNode in referenceNodes)
313313
{
314-
string paramName = refNode.GetReferenceParamString();
314+
string nodeName = refNode.GetReferenceNodeString();
315315

316-
if ((refNode.Reference == null) && (paramName == null))
316+
if ((refNode.Reference == null) && (nodeName == null))
317317
{
318318
// This can't happen - if the ref is null it must be because it's a named param
319-
throw new Exception("Reference and paramName can't both be null");
319+
throw new Exception($"{nameof(refNode.Reference)} and {nameof(nodeName)} can't both be null");
320320
}
321321

322-
if (paramName == null)
322+
if (nodeName == null)
323323
{
324-
paramName = _compObjToParamNameMap[refNode.Reference];
324+
nodeName = this._compObjToNodeNameMap[refNode.Reference];
325325
}
326326

327-
_objRefList.Add(new ReferenceInfo(paramName, refNode.Reference));
328-
refNode.ParamName = paramName;
327+
_objRefList.Add(new ReferenceInfo(nodeName, refNode.Reference));
328+
refNode.NodeName = nodeName;
329329
}
330330
}
331331

@@ -335,7 +335,7 @@ internal void EnsureReferenceInfo()
335335
// important in this context as the only critical property to maintain is to have
336336
// a unique mapping to each input value to the resulting sequence of letters.
337337
[SkipLocalsInit]
338-
static unsafe string CreateUniqueParamNameFromIndex(uint i)
338+
static unsafe string CreateUniqueNodeNameFromIndex(uint i)
339339
{
340340
const int alphabetLength = 'Z' - 'A' + 1;
341341

@@ -592,7 +592,7 @@ private string ToExpressionStringInternal()
592592
throw new Exception("References cannot have children");
593593
}
594594

595-
ret = (this as ReferenceNode).GetReferenceParamString();
595+
ret = (this as ReferenceNode).GetReferenceNodeString();
596596
}
597597
else if (NodeType == ExpressionNodeType.ReferenceProperty)
598598
{
@@ -700,11 +700,17 @@ public ReferenceInfo(string paramName, CompositionObject compObj)
700700
internal List<ExpressionNode> Children { get; set; } = new List<ExpressionNode>();
701701

702702
/// <summary>
703-
/// Gets or sets the name of the parameter.
703+
/// Gets or sets the user-defined name of the parameter.
704704
/// </summary>
705705
/// <value>The name of the parameter.</value>
706706
internal string ParamName { get; set; }
707707

708+
/// <summary>
709+
/// Gets or sets the unique name for the expression node. Can be user-defined or generated.
710+
/// </summary>
711+
/// <value>The name of the parameter.</value>
712+
internal string NodeName { get; set; }
713+
708714
/// <summary>
709715
/// Gets or sets the expression animation.
710716
/// </summary>

Microsoft.Toolkit.Uwp.UI.Animations/Expressions/ReferenceNodes/ReferenceNode.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,15 +125,15 @@ public Matrix4x4Node GetMatrix4x4Property(string propertyName)
125125
/// Gets the reference parameter string.
126126
/// </summary>
127127
/// <returns>System.String.</returns>
128-
internal string GetReferenceParamString()
128+
internal string GetReferenceNodeString()
129129
{
130130
if (NodeType == ExpressionNodeType.TargetReference)
131131
{
132132
return "this.target";
133133
}
134134
else
135135
{
136-
return ParamName;
136+
return NodeName;
137137
}
138138
}
139139

0 commit comments

Comments
 (0)