From 6b0ded4fc2866e4c7ef38c702281d1e03a833e5e Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Tue, 14 May 2019 16:42:03 +0800 Subject: [PATCH 01/10] Track old `TransitionInstances` and dispose when its invalid. Fixes #2485 --- src/Avalonia.Animation/Animatable.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index 4b0f76c5d5..445c490b4b 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -7,7 +7,7 @@ using System.Linq; using System.Reactive.Linq; using Avalonia.Collections; using Avalonia.Data; -using Avalonia.Animation.Animators; +using Avalonia.Animation.Animators; namespace Avalonia.Animation { @@ -36,6 +36,9 @@ namespace Avalonia.Animation private Transitions _transitions; + private Dictionary _previousTransitions + = new Dictionary(); + /// /// Gets or sets the property transitions for the control. /// @@ -58,7 +61,12 @@ namespace Avalonia.Animation if (match != null) { - match.Apply(this, Clock ?? Avalonia.Animation.Clock.GlobalClock, e.OldValue, e.NewValue); + if (_previousTransitions.TryGetValue(e.Property, out var dispose)) + dispose.Dispose(); + + var instance = match.Apply(this, Clock ?? Avalonia.Animation.Clock.GlobalClock, e.OldValue, e.NewValue); + + _previousTransitions[e.Property] = instance; } } } From ffe56b55bcf00ec9b10a6262643b73e8ed0d3830 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Tue, 14 May 2019 16:44:38 +0800 Subject: [PATCH 02/10] Fix #2490 --- src/Avalonia.Animation/TransitionInstance.cs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Animation/TransitionInstance.cs b/src/Avalonia.Animation/TransitionInstance.cs index eff2c4e9f3..fe8103adb2 100644 --- a/src/Avalonia.Animation/TransitionInstance.cs +++ b/src/Avalonia.Animation/TransitionInstance.cs @@ -30,13 +30,22 @@ namespace Avalonia.Animation { var interpVal = (double)t.Ticks / _duration.Ticks; - if (interpVal > 1d || interpVal < 0d) + // Clamp interpolation value. + if (interpVal >= 1d) { + PublishNext(1d); PublishCompleted(); - return; } - - PublishNext(interpVal); + // Cut-off when interpolation value is negative. + else if (interpVal < 0d) + { + PublishNext(0d); + PublishCompleted(); + } + else + { + PublishNext(interpVal); + } } protected override void Unsubscribed() From 20284403816cb3b6a988958ffffdb87b7bb476bf Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Tue, 14 May 2019 16:48:55 +0800 Subject: [PATCH 03/10] Speedup Sidebar hover transitions. --- samples/ControlCatalog/SideBar.xaml | 2 +- samples/RenderDemo/SideBar.xaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/ControlCatalog/SideBar.xaml b/samples/ControlCatalog/SideBar.xaml index 3bae7edb00..3047b1e519 100644 --- a/samples/ControlCatalog/SideBar.xaml +++ b/samples/ControlCatalog/SideBar.xaml @@ -56,7 +56,7 @@ - + diff --git a/samples/RenderDemo/SideBar.xaml b/samples/RenderDemo/SideBar.xaml index 3af90f1844..e37b9bb5fc 100644 --- a/samples/RenderDemo/SideBar.xaml +++ b/samples/RenderDemo/SideBar.xaml @@ -47,7 +47,7 @@ - + From 3b93e72d45c11bed994142c37da525b763f633c0 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Tue, 14 May 2019 20:24:09 +0800 Subject: [PATCH 04/10] Dont initialize transition instance dictionary on objects without Transitions on it. --- src/Avalonia.Animation/Animatable.cs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index 445c490b4b..3a3d00b94a 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -36,16 +36,27 @@ namespace Avalonia.Animation private Transitions _transitions; - private Dictionary _previousTransitions - = new Dictionary(); + private Dictionary _previousTransitions; /// /// Gets or sets the property transitions for the control. /// public Transitions Transitions { - get { return _transitions ?? (_transitions = new Transitions()); } - set { SetAndRaise(TransitionsProperty, ref _transitions, value); } + get + { + if (_transitions == null) + _transitions = new Transitions(); + + if (_previousTransitions == null) + _previousTransitions = new Dictionary(); + + return _transitions; + } + set + { + SetAndRaise(TransitionsProperty, ref _transitions, value); + } } /// @@ -55,7 +66,7 @@ namespace Avalonia.Animation /// The event args. protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs e) { - if (e.Priority != BindingPriority.Animation && Transitions != null) + if (e.Priority != BindingPriority.Animation && Transitions != null && _previousTransitions != null) { var match = Transitions.FirstOrDefault(x => x.Property == e.Property); From 4a386d1b84e879bec43f89c084b2ed073c4f4484 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 14 May 2019 14:36:59 +0200 Subject: [PATCH 05/10] Added skipped failing leak test for transitions. It's leaking on master too, so address this separately. --- .../Properties/AssemblyInfo.cs | 5 +- tests/Avalonia.LeakTests/TransitionTests.cs | 60 +++++++++++++++++++ tests/Avalonia.UnitTests/MockGlobalClock.cs | 10 ++++ tests/Avalonia.UnitTests/TestServices.cs | 6 ++ .../Avalonia.UnitTests/UnitTestApplication.cs | 2 + 5 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 tests/Avalonia.LeakTests/TransitionTests.cs create mode 100644 tests/Avalonia.UnitTests/MockGlobalClock.cs diff --git a/src/Avalonia.Animation/Properties/AssemblyInfo.cs b/src/Avalonia.Animation/Properties/AssemblyInfo.cs index 985a8e5bfe..eb38a66a84 100644 --- a/src/Avalonia.Animation/Properties/AssemblyInfo.cs +++ b/src/Avalonia.Animation/Properties/AssemblyInfo.cs @@ -3,7 +3,10 @@ using Avalonia.Metadata; using System.Reflection; +using System.Runtime.CompilerServices; [assembly: XmlnsDefinition("https://github.com/avaloniaui", "Avalonia.Animation")] [assembly: XmlnsDefinition("https://github.com/avaloniaui", "Avalonia.Animation.Easings")] -[assembly: XmlnsDefinition("https://github.com/avaloniaui", "Avalonia.Animation.Animators")] \ No newline at end of file +[assembly: XmlnsDefinition("https://github.com/avaloniaui", "Avalonia.Animation.Animators")] + +[assembly: InternalsVisibleTo("Avalonia.LeakTests")] diff --git a/tests/Avalonia.LeakTests/TransitionTests.cs b/tests/Avalonia.LeakTests/TransitionTests.cs new file mode 100644 index 0000000000..c7add1fe11 --- /dev/null +++ b/tests/Avalonia.LeakTests/TransitionTests.cs @@ -0,0 +1,60 @@ +using System; +using Avalonia.Animation; +using Avalonia.Controls; +using Avalonia.UnitTests; +using JetBrains.dotMemoryUnit; +using Xunit; +using Xunit.Abstractions; + +namespace Avalonia.LeakTests +{ + [DotMemoryUnit(FailIfRunWithoutSupport = false)] + public class TransitionTests + { + public TransitionTests(ITestOutputHelper atr) + { + DotMemoryUnitTestOutput.SetOutputMethod(atr.WriteLine); + } + + [Fact(Skip = "TODO: Fix this leak")] + public void Transition_On_StyledProperty_Is_Freed() + { + var clock = new MockGlobalClock(); + + using (UnitTestApplication.Start(new TestServices(globalClock: clock))) + { + Func run = () => + { + var border = new Border + { + Transitions = + { + new DoubleTransition + { + Duration = TimeSpan.FromSeconds(1), + Property = Border.OpacityProperty, + } + } + }; + + border.Opacity = 0; + + clock.Pulse(TimeSpan.FromSeconds(0)); + clock.Pulse(TimeSpan.FromSeconds(0.5)); + + Assert.Equal(0.5, border.Opacity); + + clock.Pulse(TimeSpan.FromSeconds(1)); + + Assert.Equal(0, border.Opacity); + return border; + }; + + var result = run(); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + } + } + } +} diff --git a/tests/Avalonia.UnitTests/MockGlobalClock.cs b/tests/Avalonia.UnitTests/MockGlobalClock.cs new file mode 100644 index 0000000000..b53e5acc01 --- /dev/null +++ b/tests/Avalonia.UnitTests/MockGlobalClock.cs @@ -0,0 +1,10 @@ +using System; +using Avalonia.Animation; + +namespace Avalonia.UnitTests +{ + public class MockGlobalClock : ClockBase, IGlobalClock + { + public new void Pulse(TimeSpan systemTime) => base.Pulse(systemTime); + } +} diff --git a/tests/Avalonia.UnitTests/TestServices.cs b/tests/Avalonia.UnitTests/TestServices.cs index d68f1d167a..f7a878feba 100644 --- a/tests/Avalonia.UnitTests/TestServices.cs +++ b/tests/Avalonia.UnitTests/TestServices.cs @@ -16,6 +16,7 @@ using System.Reactive.Concurrency; using System.Collections.Generic; using Avalonia.Controls; using System.Reflection; +using Avalonia.Animation; namespace Avalonia.UnitTests { @@ -58,6 +59,7 @@ namespace Avalonia.UnitTests public TestServices( IAssetLoader assetLoader = null, IFocusManager focusManager = null, + IGlobalClock globalClock = null, IInputManager inputManager = null, Func keyboardDevice = null, IKeyboardNavigationHandler keyboardNavigation = null, @@ -75,6 +77,7 @@ namespace Avalonia.UnitTests { AssetLoader = assetLoader; FocusManager = focusManager; + GlobalClock = globalClock; InputManager = inputManager; KeyboardDevice = keyboardDevice; KeyboardNavigation = keyboardNavigation; @@ -93,6 +96,7 @@ namespace Avalonia.UnitTests public IAssetLoader AssetLoader { get; } public IInputManager InputManager { get; } public IFocusManager FocusManager { get; } + public IGlobalClock GlobalClock { get; } public Func KeyboardDevice { get; } public IKeyboardNavigationHandler KeyboardNavigation { get; } public Func MouseDevice { get; } @@ -109,6 +113,7 @@ namespace Avalonia.UnitTests public TestServices With( IAssetLoader assetLoader = null, IFocusManager focusManager = null, + IGlobalClock globalClock = null, IInputManager inputManager = null, Func keyboardDevice = null, IKeyboardNavigationHandler keyboardNavigation = null, @@ -127,6 +132,7 @@ namespace Avalonia.UnitTests return new TestServices( assetLoader: assetLoader ?? AssetLoader, focusManager: focusManager ?? FocusManager, + globalClock: globalClock ?? GlobalClock, inputManager: inputManager ?? InputManager, keyboardDevice: keyboardDevice ?? KeyboardDevice, keyboardNavigation: keyboardNavigation ?? KeyboardNavigation, diff --git a/tests/Avalonia.UnitTests/UnitTestApplication.cs b/tests/Avalonia.UnitTests/UnitTestApplication.cs index 4802278c1e..3578471397 100644 --- a/tests/Avalonia.UnitTests/UnitTestApplication.cs +++ b/tests/Avalonia.UnitTests/UnitTestApplication.cs @@ -12,6 +12,7 @@ using Avalonia.Threading; using System.Reactive.Disposables; using System.Reactive.Concurrency; using Avalonia.Input.Platform; +using Avalonia.Animation; namespace Avalonia.UnitTests { @@ -52,6 +53,7 @@ namespace Avalonia.UnitTests AvaloniaLocator.CurrentMutable .Bind().ToConstant(Services.AssetLoader) .Bind().ToConstant(Services.FocusManager) + .Bind().ToConstant(Services.GlobalClock) .BindToSelf(this) .Bind().ToConstant(Services.InputManager) .Bind().ToConstant(Services.KeyboardDevice?.Invoke()) From e06a9dffc32733a58c7dcfa5ba2e8e0b7f49561c Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Tue, 14 May 2019 21:19:35 +0800 Subject: [PATCH 06/10] Partially revert Transition interpolation clamp. --- src/Avalonia.Animation/TransitionInstance.cs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/Avalonia.Animation/TransitionInstance.cs b/src/Avalonia.Animation/TransitionInstance.cs index fe8103adb2..39dc36aa33 100644 --- a/src/Avalonia.Animation/TransitionInstance.cs +++ b/src/Avalonia.Animation/TransitionInstance.cs @@ -31,17 +31,11 @@ namespace Avalonia.Animation var interpVal = (double)t.Ticks / _duration.Ticks; // Clamp interpolation value. - if (interpVal >= 1d) + if (interpVal >= 1d | (interpVal < 0d)) { PublishNext(1d); PublishCompleted(); } - // Cut-off when interpolation value is negative. - else if (interpVal < 0d) - { - PublishNext(0d); - PublishCompleted(); - } else { PublishNext(interpVal); From b94f5975eb7d0f338b2983628df175a975230b9e Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Tue, 14 May 2019 21:21:19 +0800 Subject: [PATCH 07/10] Remove extra space. --- src/Avalonia.Animation/TransitionInstance.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Animation/TransitionInstance.cs b/src/Avalonia.Animation/TransitionInstance.cs index 39dc36aa33..ebb6e2ca68 100644 --- a/src/Avalonia.Animation/TransitionInstance.cs +++ b/src/Avalonia.Animation/TransitionInstance.cs @@ -31,7 +31,7 @@ namespace Avalonia.Animation var interpVal = (double)t.Ticks / _duration.Ticks; // Clamp interpolation value. - if (interpVal >= 1d | (interpVal < 0d)) + if (interpVal >= 1d | (interpVal < 0d)) { PublishNext(1d); PublishCompleted(); From 26b8fd6a056a44b2c69e9a463c45e1971eae4782 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Tue, 14 May 2019 21:31:08 +0800 Subject: [PATCH 08/10] Add transitions unit test. --- src/Avalonia.Animation/TransitionInstance.cs | 2 +- .../AnimationTransitionsTest.cs | 73 +++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 tests/Avalonia.Animation.UnitTests/AnimationTransitionsTest.cs diff --git a/src/Avalonia.Animation/TransitionInstance.cs b/src/Avalonia.Animation/TransitionInstance.cs index ebb6e2ca68..10ea6bf523 100644 --- a/src/Avalonia.Animation/TransitionInstance.cs +++ b/src/Avalonia.Animation/TransitionInstance.cs @@ -31,7 +31,7 @@ namespace Avalonia.Animation var interpVal = (double)t.Ticks / _duration.Ticks; // Clamp interpolation value. - if (interpVal >= 1d | (interpVal < 0d)) + if (interpVal >= 1d | interpVal < 0d) { PublishNext(1d); PublishCompleted(); diff --git a/tests/Avalonia.Animation.UnitTests/AnimationTransitionsTest.cs b/tests/Avalonia.Animation.UnitTests/AnimationTransitionsTest.cs new file mode 100644 index 0000000000..9014c40299 --- /dev/null +++ b/tests/Avalonia.Animation.UnitTests/AnimationTransitionsTest.cs @@ -0,0 +1,73 @@ +using System; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Avalonia.Animation; +using Avalonia.Controls; +using Avalonia.Styling; +using Avalonia.UnitTests; +using Avalonia.Data; +using Xunit; +using Avalonia.Animation.Easings; + +namespace Avalonia.Animation.UnitTests +{ + public class AnimationTransitionsTests + { + [Fact] + public void Check_Transitions_Interpolation_Negative_Bounds_Clamp() + { + var clock = new MockGlobalClock(); + + using (UnitTestApplication.Start(new TestServices(globalClock: clock))) + { + var border = new Border + { + Transitions = + { + new DoubleTransition + { + Duration = TimeSpan.FromSeconds(1), + Property = Border.OpacityProperty, + } + } + }; + + border.Opacity = 0; + + clock.Pulse(TimeSpan.FromSeconds(0)); + clock.Pulse(TimeSpan.FromSeconds(-0.5)); + + Assert.Equal(0, border.Opacity); + } + } + + [Fact] + public void Check_Transitions_Interpolation_Positive_Bounds_Clamp() + { + var clock = new MockGlobalClock(); + + using (UnitTestApplication.Start(new TestServices(globalClock: clock))) + { + var border = new Border + { + Transitions = + { + new DoubleTransition + { + Duration = TimeSpan.FromSeconds(1), + Property = Border.OpacityProperty, + } + } + }; + + border.Opacity = 0; + + clock.Pulse(TimeSpan.FromSeconds(0)); + clock.Pulse(TimeSpan.FromMilliseconds(1001)); + + Assert.Equal(0, border.Opacity); + } + } + } +} From 66371bfa477770423fcb2b0c82bc22bac95899ed Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Tue, 14 May 2019 21:46:28 +0800 Subject: [PATCH 09/10] Rename to TransitionsTest --- .../{AnimationTransitionsTest.cs => TransitionsTest.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/Avalonia.Animation.UnitTests/{AnimationTransitionsTest.cs => TransitionsTest.cs} (97%) diff --git a/tests/Avalonia.Animation.UnitTests/AnimationTransitionsTest.cs b/tests/Avalonia.Animation.UnitTests/TransitionsTest.cs similarity index 97% rename from tests/Avalonia.Animation.UnitTests/AnimationTransitionsTest.cs rename to tests/Avalonia.Animation.UnitTests/TransitionsTest.cs index 9014c40299..8f2ccb9cad 100644 --- a/tests/Avalonia.Animation.UnitTests/AnimationTransitionsTest.cs +++ b/tests/Avalonia.Animation.UnitTests/TransitionsTest.cs @@ -12,7 +12,7 @@ using Avalonia.Animation.Easings; namespace Avalonia.Animation.UnitTests { - public class AnimationTransitionsTests + public class TransitionsTest { [Fact] public void Check_Transitions_Interpolation_Negative_Bounds_Clamp() From 066f608ca27ddd76ab5fdf9a95c309c0763fc966 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Wed, 15 May 2019 20:44:12 +0800 Subject: [PATCH 10/10] Rename to plural form. --- .../{TransitionsTest.cs => TransitionsTests.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/Avalonia.Animation.UnitTests/{TransitionsTest.cs => TransitionsTests.cs} (98%) diff --git a/tests/Avalonia.Animation.UnitTests/TransitionsTest.cs b/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs similarity index 98% rename from tests/Avalonia.Animation.UnitTests/TransitionsTest.cs rename to tests/Avalonia.Animation.UnitTests/TransitionsTests.cs index 8f2ccb9cad..f1b4b0d071 100644 --- a/tests/Avalonia.Animation.UnitTests/TransitionsTest.cs +++ b/tests/Avalonia.Animation.UnitTests/TransitionsTests.cs @@ -12,7 +12,7 @@ using Avalonia.Animation.Easings; namespace Avalonia.Animation.UnitTests { - public class TransitionsTest + public class TransitionsTests { [Fact] public void Check_Transitions_Interpolation_Negative_Bounds_Clamp()