Merge pull request #2517 from AvaloniaUI/fixes/transitions-disposal

Fixes for Transitions + Sidebar hover effect speedup.

samples/ControlCatalog/SideBar.xaml

@@ -56,7 +56,7 @@
         <Setter Property="Opacity" Value="0.5"/>
         <Setter Property="Transitions">
             <Transitions>
-                <DoubleTransition Property="Opacity" Duration="0:0:0.5"/>
+                <DoubleTransition Property="Opacity" Duration="0:0:0.150"/>
             </Transitions>
         </Setter>
     </Style>

samples/RenderDemo/SideBar.xaml

@@ -47,7 +47,7 @@
         <Setter Property="Opacity" Value="0.5"/>
         <Setter Property="Transitions">
             <Transitions>
-                <DoubleTransition Property="Opacity" Duration="0:0:0.5"/>
+                <DoubleTransition Property="Opacity" Duration="0:0:0.150"/>
             </Transitions>
         </Setter>
     </Style>

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,13 +36,27 @@ namespace Avalonia.Animation
 
         private Transitions _transitions;
 
+        private Dictionary<AvaloniaProperty, IDisposable> _previousTransitions;
+
         /// <summary>
         /// Gets or sets the property transitions for the control.
         /// </summary>
         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<AvaloniaProperty, IDisposable>();
+
+                return _transitions;
+            }
+            set
+            {
+                SetAndRaise(TransitionsProperty, ref _transitions, value);
+            }
         }
 
         /// <summary>
@@ -52,13 +66,18 @@ namespace Avalonia.Animation
         /// <param name="e">The event args.</param>
         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);
 
                 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;
                 }
             }
         }

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")]
+[assembly: XmlnsDefinition("https://github.com/avaloniaui", "Avalonia.Animation.Animators")]
+
+[assembly: InternalsVisibleTo("Avalonia.LeakTests")]

src/Avalonia.Animation/TransitionInstance.cs

@@ -30,13 +30,16 @@ namespace Avalonia.Animation
         {
             var interpVal = (double)t.Ticks / _duration.Ticks;
 
-            if (interpVal > 1d || interpVal < 0d)
+            // Clamp interpolation value.
+            if (interpVal >= 1d | interpVal < 0d)
             {
+                PublishNext(1d);
                 PublishCompleted();
-                return;
             }
-
-            PublishNext(interpVal);
+            else
+            {
+                PublishNext(interpVal);
+            }
         }
 
         protected override void Unsubscribed()

tests/Avalonia.Animation.UnitTests/TransitionsTests.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 TransitionsTests
+    {
+        [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);
+            }
+        }
+    }
+}

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<Border> 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<TransitionInstance>()).ObjectsCount));
+            }
+        }
+    }
+}

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);
+    }
+}

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<IKeyboardDevice> 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<IKeyboardDevice> KeyboardDevice { get; }
         public IKeyboardNavigationHandler KeyboardNavigation { get; }
         public Func<IMouseDevice> 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<IKeyboardDevice> 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,

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<IAssetLoader>().ToConstant(Services.AssetLoader)
                 .Bind<IFocusManager>().ToConstant(Services.FocusManager)
+                .Bind<IGlobalClock>().ToConstant(Services.GlobalClock)
                 .BindToSelf<IGlobalStyles>(this)
                 .Bind<IInputManager>().ToConstant(Services.InputManager)
                 .Bind<IKeyboardDevice>().ToConstant(Services.KeyboardDevice?.Invoke())