From c64cc02ec68d5d5bdecc64e9001d2c09691cc43b Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Wed, 28 Aug 2019 14:06:00 +0800 Subject: [PATCH 1/8] Move transitions initializer to setter. --- src/Avalonia.Animation/Animatable.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index 3a3d00b94a..89bcd2efa5 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -44,6 +44,10 @@ namespace Avalonia.Animation public Transitions Transitions { get + { + return _transitions; + } + set { if (_transitions == null) _transitions = new Transitions(); @@ -51,10 +55,6 @@ namespace Avalonia.Animation if (_previousTransitions == null) _previousTransitions = new Dictionary(); - return _transitions; - } - set - { SetAndRaise(TransitionsProperty, ref _transitions, value); } } From 87245c90b46684a01c0c9283a40af4180002eae4 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Wed, 28 Aug 2019 14:20:43 +0800 Subject: [PATCH 2/8] Fixes: part 2 of n. --- src/Avalonia.Animation/Animatable.cs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index 89bcd2efa5..09991a5990 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -49,10 +49,10 @@ namespace Avalonia.Animation } set { - if (_transitions == null) - _transitions = new Transitions(); + if (value is null) + return; - if (_previousTransitions == null) + if (_previousTransitions is null) _previousTransitions = new Dictionary(); SetAndRaise(TransitionsProperty, ref _transitions, value); @@ -66,19 +66,18 @@ namespace Avalonia.Animation /// The event args. protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs e) { - if (e.Priority != BindingPriority.Animation && Transitions != null && _previousTransitions != null) - { - var match = Transitions.FirstOrDefault(x => x.Property == e.Property); + if (Transitions is null || e.Priority == BindingPriority.Animation) return; + + var match = Transitions.FirstOrDefault(x => x.Property == e.Property); - if (match != null) - { - if (_previousTransitions.TryGetValue(e.Property, out var dispose)) - dispose.Dispose(); + if (match != null) + { + if (_previousTransitions.TryGetValue(e.Property, out var dispose)) + dispose.Dispose(); - var instance = match.Apply(this, Clock ?? Avalonia.Animation.Clock.GlobalClock, e.OldValue, e.NewValue); + var instance = match.Apply(this, Clock ?? Avalonia.Animation.Clock.GlobalClock, e.OldValue, e.NewValue); - _previousTransitions[e.Property] = instance; - } + _previousTransitions[e.Property] = instance; } } } From d176b1d7dc10e22137a21b7e2cf1836ec4cd2e27 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Thu, 29 Aug 2019 12:34:03 +0800 Subject: [PATCH 3/8] Fix unit tests. --- src/Avalonia.Animation/Animatable.cs | 4 ++-- tests/Avalonia.Animation.UnitTests/TransitionsTests.cs | 4 ++-- tests/Avalonia.LeakTests/TransitionTests.cs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index 09991a5990..5ff3b17fb5 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -35,7 +35,7 @@ namespace Avalonia.Animation (o, v) => o.Transitions = v); private Transitions _transitions; - + private bool _isTransitionsSet = false; private Dictionary _previousTransitions; /// @@ -66,7 +66,7 @@ namespace Avalonia.Animation /// The event args. protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs e) { - if (Transitions is null || e.Priority == BindingPriority.Animation) return; + if (_transitions is null || _previousTransitions is null || e.Priority == BindingPriority.Animation) return; var match = Transitions.FirstOrDefault(x => x.Property == e.Property); diff --git a/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs b/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs index f1b4b0d071..a8efc2c8ae 100644 --- a/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs +++ b/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs @@ -23,7 +23,7 @@ namespace Avalonia.Animation.UnitTests { var border = new Border { - Transitions = + Transitions = new Transitions { new DoubleTransition { @@ -51,7 +51,7 @@ namespace Avalonia.Animation.UnitTests { var border = new Border { - Transitions = + Transitions = new Transitions { new DoubleTransition { diff --git a/tests/Avalonia.LeakTests/TransitionTests.cs b/tests/Avalonia.LeakTests/TransitionTests.cs index c7add1fe11..699dec7229 100644 --- a/tests/Avalonia.LeakTests/TransitionTests.cs +++ b/tests/Avalonia.LeakTests/TransitionTests.cs @@ -27,7 +27,7 @@ namespace Avalonia.LeakTests { var border = new Border { - Transitions = + Transitions = new Transitions { new DoubleTransition { From 64fdb5828ee6943a5add0f2877bcb29a1de32361 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Mon, 9 Sep 2019 02:02:54 +0800 Subject: [PATCH 4/8] un-LINQ the Transition property matching codepath. --- src/Avalonia.Animation/Animatable.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index 5ff3b17fb5..3f548d207d 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -68,16 +68,19 @@ namespace Avalonia.Animation { if (_transitions is null || _previousTransitions is null || e.Priority == BindingPriority.Animation) return; - var match = Transitions.FirstOrDefault(x => x.Property == e.Property); - - if (match != null) + // PERF-SENSITIVE: Called on every property change. Don't use LINQ here (too many allocations). + foreach (var transition in Transitions) { - if (_previousTransitions.TryGetValue(e.Property, out var dispose)) - dispose.Dispose(); + if (transition.Property == e.Property) + { + if (_previousTransitions.TryGetValue(e.Property, out var dispose)) + dispose.Dispose(); - var instance = match.Apply(this, Clock ?? Avalonia.Animation.Clock.GlobalClock, e.OldValue, e.NewValue); + var instance = transition.Apply(this, Clock ?? Avalonia.Animation.Clock.GlobalClock, e.OldValue, e.NewValue); - _previousTransitions[e.Property] = instance; + _previousTransitions[e.Property] = instance; + return; + } } } } From b8e4909b104423cb7163b3fbb8ea75b8c466c997 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Mon, 9 Sep 2019 02:18:47 +0800 Subject: [PATCH 5/8] Restore Lazy Init to avoid API break. --- src/Avalonia.Animation/Animatable.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index 3f548d207d..68ca03f910 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -45,15 +45,16 @@ namespace Avalonia.Animation { get { + if (_transitions is null) + _transitions = new Transitions(); + + if (_previousTransitions is null) + _previousTransitions = new Dictionary(); + return _transitions; } set { - if (value is null) - return; - - if (_previousTransitions is null) - _previousTransitions = new Dictionary(); SetAndRaise(TransitionsProperty, ref _transitions, value); } From 3549f00fcba37fffe35c827fc20ded88743f8250 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Mon, 9 Sep 2019 02:19:25 +0800 Subject: [PATCH 6/8] Revert unit test changes. --- tests/Avalonia.Animation.UnitTests/TransitionsTests.cs | 4 ++-- tests/Avalonia.LeakTests/TransitionTests.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs b/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs index a8efc2c8ae..4ebe472125 100644 --- a/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs +++ b/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs @@ -23,7 +23,7 @@ namespace Avalonia.Animation.UnitTests { var border = new Border { - Transitions = new Transitions + Transitions = { new DoubleTransition { @@ -51,7 +51,7 @@ namespace Avalonia.Animation.UnitTests { var border = new Border { - Transitions = new Transitions + Transitions = { new DoubleTransition { diff --git a/tests/Avalonia.LeakTests/TransitionTests.cs b/tests/Avalonia.LeakTests/TransitionTests.cs index 699dec7229..5ab8c5c0dd 100644 --- a/tests/Avalonia.LeakTests/TransitionTests.cs +++ b/tests/Avalonia.LeakTests/TransitionTests.cs @@ -27,7 +27,7 @@ namespace Avalonia.LeakTests { var border = new Border { - Transitions = new Transitions + Transitions = { new DoubleTransition { From d368d9675a17beb045e1e94dce880ee2f0b45042 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Mon, 9 Sep 2019 14:25:59 +0800 Subject: [PATCH 7/8] Address review. --- src/Avalonia.Animation/Animatable.cs | 2 +- tests/Avalonia.Animation.UnitTests/TransitionsTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index 68ca03f910..2c321b8b28 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -35,7 +35,7 @@ namespace Avalonia.Animation (o, v) => o.Transitions = v); private Transitions _transitions; - private bool _isTransitionsSet = false; + private Dictionary _previousTransitions; /// diff --git a/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs b/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs index 4ebe472125..f1b4b0d071 100644 --- a/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs +++ b/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs @@ -23,7 +23,7 @@ namespace Avalonia.Animation.UnitTests { var border = new Border { - Transitions = + Transitions = { new DoubleTransition { @@ -51,7 +51,7 @@ namespace Avalonia.Animation.UnitTests { var border = new Border { - Transitions = + Transitions = { new DoubleTransition { From 9970bf67223624c33d751f3aa7c9f8726aed7d0c Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Mon, 9 Sep 2019 14:38:53 +0800 Subject: [PATCH 8/8] Address review pt. 2 --- tests/Avalonia.LeakTests/TransitionTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.LeakTests/TransitionTests.cs b/tests/Avalonia.LeakTests/TransitionTests.cs index 5ab8c5c0dd..c7add1fe11 100644 --- a/tests/Avalonia.LeakTests/TransitionTests.cs +++ b/tests/Avalonia.LeakTests/TransitionTests.cs @@ -27,7 +27,7 @@ namespace Avalonia.LeakTests { var border = new Border { - Transitions = + Transitions = { new DoubleTransition {