From 229c8b7f12762ce4f37c241c13c15e50bde83c1b Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Fri, 29 May 2020 00:36:15 +0200 Subject: [PATCH 1/3] Benchmark and simple optimization for INPC accessor. --- .../Plugins/InpcPropertyAccessorPlugin.cs | 8 ++- .../Data/PropertyAccessorBenchmarks.cs | 57 +++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 tests/Avalonia.Benchmarks/Data/PropertyAccessorBenchmarks.cs diff --git a/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs b/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs index f2ed86d2aa..84ef0fb695 100644 --- a/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs +++ b/src/Avalonia.Base/Data/Core/Plugins/InpcPropertyAccessorPlugin.cs @@ -1,7 +1,5 @@ using System; using System.ComponentModel; -using System.Linq; -using System.Reactive.Linq; using System.Reflection; using Avalonia.Utilities; @@ -31,7 +29,11 @@ namespace Avalonia.Data.Core.Plugins Contract.Requires(propertyName != null); reference.TryGetTarget(out object instance); - var p = instance.GetType().GetRuntimeProperties().FirstOrDefault(x => x.Name == propertyName); + + const BindingFlags bindingFlags = BindingFlags.NonPublic | BindingFlags.Public | + BindingFlags.Static | BindingFlags.Instance; + + var p = instance.GetType().GetProperty(propertyName, bindingFlags); if (p != null) { diff --git a/tests/Avalonia.Benchmarks/Data/PropertyAccessorBenchmarks.cs b/tests/Avalonia.Benchmarks/Data/PropertyAccessorBenchmarks.cs new file mode 100644 index 0000000000..5c2502422a --- /dev/null +++ b/tests/Avalonia.Benchmarks/Data/PropertyAccessorBenchmarks.cs @@ -0,0 +1,57 @@ +using System; +using System.ComponentModel; +using System.Runtime.CompilerServices; +using Avalonia.Data.Core.Plugins; +using BenchmarkDotNet.Attributes; +using JetBrains.Annotations; + +namespace Avalonia.Benchmarks.Data +{ + [MemoryDiagnoser, InProcess] + public class PropertyAccessorBenchmarks + { + private readonly InpcPropertyAccessorPlugin _plugin = new InpcPropertyAccessorPlugin(); + private readonly TestObject _targetStrongRef = new TestObject(); + private readonly WeakReference _targetWeakRef; + + public PropertyAccessorBenchmarks() + { + _targetWeakRef = new WeakReference(_targetStrongRef); + } + + [Benchmark] + public void InpcAccessor() + { + _plugin.Start(_targetWeakRef, nameof(TestObject.Test)); + } + + private class TestObject : INotifyPropertyChanged + { + private string _test; + + public string Test + { + get => _test; + set + { + if (_test == value) + { + return; + } + + _test = value; + + OnPropertyChanged(); + } + } + + public event PropertyChangedEventHandler PropertyChanged; + + [NotifyPropertyChangedInvocator] + protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null) + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); + } + } + } +} From 6239e4f5e303f84e2b68976080258266bbc39faa Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 1 Jun 2020 12:08:52 +0200 Subject: [PATCH 2/3] Added failing test for #4059. --- .../AnimatableTests.cs | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs b/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs index b5c61883e7..784f40fe1f 100644 --- a/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs +++ b/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs @@ -2,6 +2,7 @@ using Avalonia.Controls; using Avalonia.Data; using Avalonia.Layout; +using Avalonia.Media; using Avalonia.Styling; using Avalonia.UnitTests; using Moq; @@ -265,6 +266,70 @@ namespace Avalonia.Animation.UnitTests } } + [Fact] + public void Replacing_Transitions_During_Animation_Does_Not_Throw_KeyNotFound() + { + // Issue #4059 + using (UnitTestApplication.Start(TestServices.RealStyler)) + { + Border target; + var clock = new TestClock(); + var root = new TestRoot + { + Clock = clock, + Styles = + { + new Style(x => x.OfType()) + { + Setters = + { + new Setter(Border.TransitionsProperty, + new Transitions + { + new DoubleTransition + { + Property = Border.OpacityProperty, + Duration = TimeSpan.FromSeconds(1), + }, + }), + }, + }, + new Style(x => x.OfType().Class("foo")) + { + Setters = + { + new Setter(Border.TransitionsProperty, + new Transitions + { + new DoubleTransition + { + Property = Border.OpacityProperty, + Duration = TimeSpan.FromSeconds(1), + }, + }), + new Setter(Border.OpacityProperty, 0.0), + }, + }, + }, + Child = target = new Border + { + Background = Brushes.Red, + } + }; + + root.Measure(Size.Infinity); + root.Arrange(new Rect(root.DesiredSize)); + + target.Classes.Add("foo"); + clock.Step(TimeSpan.FromSeconds(0)); + clock.Step(TimeSpan.FromSeconds(0.5)); + + Assert.Equal(0.5, target.Opacity); + + target.Classes.Remove("foo"); + } + } + private static Mock CreateTarget() { return CreateTransition(Visual.OpacityProperty); From 0694b22c51b96fb8d997ab2dc25a4d5503789a3f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 1 Jun 2020 13:50:29 +0200 Subject: [PATCH 3/3] Fix KeyNotFound exception in Animatable. When transitions are replaced, add the new transitions before removing the old transitions, so that when the old transition being disposed causes the value to change, there is a corresponding entry in `_transitionStates`. Also search the `Transitions` collection from last to first to find a matching transition, so that later added transitions have priority. Fixes #4059 --- src/Avalonia.Animation/Animatable.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index 9e9b84537b..067d9f462f 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -93,17 +93,17 @@ namespace Avalonia.Animation var oldTransitions = change.OldValue.GetValueOrDefault(); var newTransitions = change.NewValue.GetValueOrDefault(); - if (oldTransitions is object) - { - oldTransitions.CollectionChanged -= TransitionsCollectionChanged; - RemoveTransitions(oldTransitions); - } - if (newTransitions is object) { newTransitions.CollectionChanged += TransitionsCollectionChanged; AddTransitions(newTransitions); } + + if (oldTransitions is object) + { + oldTransitions.CollectionChanged -= TransitionsCollectionChanged; + RemoveTransitions(oldTransitions); + } } else if (_transitionsEnabled && Transitions is object && @@ -111,8 +111,10 @@ namespace Avalonia.Animation !change.Property.IsDirect && change.Priority > BindingPriority.Animation) { - foreach (var transition in Transitions) + for (var i = Transitions.Count -1; i >= 0; --i) { + var transition = Transitions[i]; + if (transition.Property == change.Property) { var state = _transitionState[transition];