Skip to content

Commit 11c75f8

Browse files
author
Simeon
authored
Fix some missing simplifications in Composition expressions. (#429)
* Fix some missing simplifications in Composition expressions. The logic for determining whether an expression could be simplified was sometimes wrong because it ended up hitting an overridden equality operator which compared the already-simplified values. Also fixed the test script to look at last write time when determining which is the latest build of LottieGen.exe to run. We were looking at creation time, but it turns out that when the compiler builds, it doesn't cause the creation time to be updated if the output file previously existed. Our test script could accidentally use a release build when the latest build was actually a debug build, resulting in the test running on an old build. * Replace the IsAtomic bool with a more generalized Precedence concept. Without this, it was getting unwieldy to handle all the cases of precedence. Now we're removing parentheses more aggressively, so the expressions will be more efficient to parse. * Eliminate some unnecessary whitespace in Min and Max expression functions.
1 parent 12fbb26 commit 11c75f8

File tree

11 files changed

+278
-171
lines changed

11 files changed

+278
-171
lines changed

source/WinCompData/Expressions/Boolean.cs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ public Asserted(string text)
3131
_text = text;
3232
}
3333

34-
/// <inheritdoc/>
35-
protected override bool IsAtomic => true;
34+
internal override Precedence Precedence => Precedence.Atomic;
3635

3736
/// <inheritdoc/>
3837
// We don't actually know the operation count because the text could
@@ -78,9 +77,9 @@ protected override Boolean Simplify()
7877
return new Literal(literalLeft.Value < literalRight.Value);
7978
}
8079

81-
return left != Left || right != Right
82-
? new LessThan(left, right)
83-
: this;
80+
return ReferenceEquals(left, Left) && ReferenceEquals(right, Right)
81+
? this
82+
: new LessThan(left, right);
8483
}
8584

8685
/// <inheritdoc/>
@@ -97,8 +96,7 @@ public Literal(bool value)
9796
Value = value;
9897
}
9998

100-
/// <inheritdoc/>
101-
protected override bool IsAtomic => true;
99+
internal override Precedence Precedence => Precedence.Atomic;
102100

103101
/// <inheritdoc/>
104102
public override int OperationsCount => 0;

source/WinCompData/Expressions/Color.cs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ public Asserted(string text)
2525
_text = text;
2626
}
2727

28-
/// <inheritdoc/>
29-
protected override bool IsAtomic => true;
28+
internal override Precedence Precedence => Precedence.Atomic;
3029

3130
/// <inheritdoc/>
3231
// We don't actually know the operation count because the text could
@@ -58,8 +57,7 @@ public Constructed(Scalar r, Scalar g, Scalar b, Scalar a)
5857

5958
public Scalar A { get; }
6059

61-
/// <inheritdoc/>
62-
protected override bool IsAtomic => true;
60+
internal override Precedence Precedence => Precedence.Atomic;
6361

6462
/// <inheritdoc/>
6563
public override int OperationsCount => R.OperationsCount + G.OperationsCount + B.OperationsCount + A.OperationsCount;
@@ -72,9 +70,13 @@ protected override Color Simplify()
7270
var b = B.Simplified;
7371
var a = A.Simplified;
7472

75-
return r != R || g != G || b != B || a != A
76-
? new Constructed(r, g, b, a)
77-
: this;
73+
return
74+
ReferenceEquals(r, R) &&
75+
ReferenceEquals(g, G) &&
76+
ReferenceEquals(b, B) &&
77+
ReferenceEquals(a, A)
78+
? this
79+
: new Constructed(r, g, b, a);
7880
}
7981

8082
/// <inheritdoc/>

source/WinCompData/Expressions/Expression.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,11 @@ private protected Expression()
7171
public abstract string ToText();
7272

7373
/// <summary>
74-
/// Gets a value indicating whether the string form of the expression can be unambigiously
75-
/// parsed without parentheses.
74+
/// Gets a value indicating the relative precedence of the string form of the expression.
75+
/// This is used to determine whether parentheses are necessary in order to
76+
/// override the left-to-right evaluation of the parent expression.
7677
/// </summary>
77-
protected virtual bool IsAtomic => false;
78+
internal virtual Precedence Precedence => Precedence.Unknown;
7879

7980
/// <summary>
8081
/// The number of operations in this <see cref="Expression"/>. An operation is something that
@@ -83,7 +84,7 @@ private protected Expression()
8384
public abstract int OperationsCount { get; }
8485

8586
protected static string Parenthesize(Expression expression)
86-
=> expression.IsAtomic
87+
=> expression.Precedence == Precedence.Atomic
8788
? expression.ToText()
8889
: $"({expression.ToText()})";
8990

source/WinCompData/Expressions/Matrix3x2.cs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ internal Asserted(string text)
5757
_text = text;
5858
}
5959

60-
/// <inheritdoc/>
61-
protected override bool IsAtomic => true;
60+
internal override Precedence Precedence => Precedence.Atomic;
6261

6362
/// <inheritdoc/>
6463
// We don't actually know the operation count because the text could
@@ -108,18 +107,18 @@ protected override Matrix3x2 Simplify()
108107
var m32 = _32.Simplified;
109108

110109
return
111-
m11 != _11 || m12 != _12 ||
112-
m21 != _21 || m22 != _22 ||
113-
m31 != _31 || m32 != _32
114-
? new Constructed(m11, m12, m21, m22, m31, m32)
115-
: this;
110+
ReferenceEquals(m11, _11) && ReferenceEquals(m12, _12) &&
111+
ReferenceEquals(m21, _21) && ReferenceEquals(m22, _22) &&
112+
ReferenceEquals(m31, _31) && ReferenceEquals(m32, _32)
113+
? this
114+
: new Constructed(m11, m12, m21, m22, m31, m32);
116115
}
117116

118117
/// <inheritdoc/>
119118
protected override string CreateExpressionText()
120119
=> $"Matrix3x2({Parenthesize(_11)},{Parenthesize(_12)},{Parenthesize(_21)},{Parenthesize(_22)},{Parenthesize(_31)},{Parenthesize(_32)})";
121120

122-
protected override bool IsAtomic => true;
121+
internal override Precedence Precedence => Precedence.Atomic;
123122
}
124123

125124
Scalar Channel(string channelName) => Expressions.Scalar.Channel(this, channelName);
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
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+
namespace Microsoft.Toolkit.Uwp.UI.Lottie.WinCompData.Expressions
6+
{
7+
/// <summary>
8+
/// Determines the relative precedence of an operation in the string
9+
/// form of an expression. For example, multiplication has higher
10+
/// precedence than addition.
11+
/// </summary>
12+
enum Precedence : uint
13+
{
14+
Unknown = 0,
15+
Subtraction,
16+
Addition,
17+
Multiplication,
18+
Division,
19+
Atomic,
20+
}
21+
}

0 commit comments

Comments
 (0)