Disable Animations processing when Visual's not visible redux (#20820)

* Add failing tests for animations visibility behavior

* Dont trigger InternalStep when IsEffectivelyVisible is false.

* Fix a 6 year old mistake.

Dont spawn a new DoubleAnimator, instead just parse the target Transform object and do a SetValue call for the interpolated animation value.

* add comment and remove redundant else

* use LightweightSubject to avoid hammering the binding system with extra value frames from SetValue as @grokys suggested.

* whitespace

* remove unused namespace

* remove extra fields

* Implement animation pause on invisible and dispose on visual tree detach

* Implement animation pause on invisible and dispose on visual tree detach

* add tests

* check pause state and combo with IEV

* fix review comment

src/Avalonia.Base/Animation/AnimationInstance`1.cs

@@ -35,6 +35,8 @@ namespace Avalonia.Animation
         private readonly IClock _baseClock;
         private IClock? _clock;
         private EventHandler<AvaloniaPropertyChangedEventArgs>? _propertyChangedDelegate;
+        private EventHandler? _visibilityChangedHandler;
+        private EventHandler<VisualTreeAttachmentEventArgs>? _detachedHandler;
 
         public AnimationInstance(Animation animation, Animatable control, Animator<T> animator, IClock baseClock, Action? OnComplete, Func<double, T, T> Interpolator)
         {
@@ -80,11 +82,34 @@ namespace Avalonia.Animation
 
         protected override void Unsubscribed()
         {
+            // Guard against reentrancy: DoComplete() can trigger Unsubscribed() via the
+            // _onCompleteAction disposal chain, and then PublishCompleted() calls it again.
+            var timerSub = _timerSub;
+            _timerSub = null;
+            if (timerSub is null)
+                return;
+
             // Animation may have been stopped before it has finished.
             ApplyFinalFill();
 
             _targetControl.PropertyChanged -= _propertyChangedDelegate;
-            _timerSub?.Dispose();
+            timerSub.Dispose();
+
+            if (_targetControl is Visual visual)
+            {
+                if (_visibilityChangedHandler is not null)
+                {
+                    visual.IsEffectivelyVisibleChanged -= _visibilityChangedHandler;
+                    _visibilityChangedHandler = null;
+                }
+
+                if (_detachedHandler is not null)
+                {
+                    visual.DetachedFromVisualTree -= _detachedHandler;
+                    _detachedHandler = null;
+                }
+            }
+
             _clock!.PlayState = PlayState.Stop;
         }
 
@@ -92,6 +117,35 @@ namespace Avalonia.Animation
         {
             _clock = new Clock(_baseClock);
             _timerSub = _clock.Subscribe(Step);
+
+            if (_targetControl is Visual visual)
+            {
+                _visibilityChangedHandler = (_, _) =>
+                {
+                    if (_clock is null || _clock.PlayState == PlayState.Stop)
+                        return;
+                    if (visual.IsEffectivelyVisible)
+                    {
+                        if (_clock.PlayState == PlayState.Pause)
+                            _clock.PlayState = PlayState.Run;
+                    }
+                    else
+                    {
+                        if (_clock.PlayState == PlayState.Run)
+                            _clock.PlayState = PlayState.Pause;
+                    }
+                };
+                visual.IsEffectivelyVisibleChanged += _visibilityChangedHandler;
+
+                // If already invisible when animation starts, pause immediately.
+                if (!visual.IsEffectivelyVisible)
+                    _clock.PlayState = PlayState.Pause;
+
+                // Stop and dispose the animation when detached from the visual tree.
+                _detachedHandler = (_, _) => DoComplete();
+                visual.DetachedFromVisualTree += _detachedHandler;
+            }
+
             _propertyChangedDelegate ??= ControlPropertyChanged;
             _targetControl.PropertyChanged += _propertyChangedDelegate;
             UpdateNeutralValue();
@@ -101,6 +155,9 @@ namespace Avalonia.Animation
         {
             try
             {
+                if (_clock?.PlayState == PlayState.Pause)
+                    return;
+                
                 InternalStep(frameTick); 
             }
             catch (Exception e)

src/Avalonia.Base/Visual.cs

@@ -208,6 +208,11 @@ namespace Avalonia
         /// </summary>
         public bool IsEffectivelyVisible { get; private set; } = true;
 
+        /// <summary>
+        /// Raised when <see cref="IsEffectivelyVisible"/> changes.
+        /// </summary>
+        internal event EventHandler? IsEffectivelyVisibleChanged;
+
         /// <summary>
         /// Updates the <see cref="IsEffectivelyVisible"/> property based on the parent's
         /// <see cref="IsEffectivelyVisible"/>.
@@ -221,6 +226,7 @@ namespace Avalonia
                 return;
 
             IsEffectivelyVisible = isEffectivelyVisible;
+            IsEffectivelyVisibleChanged?.Invoke(this, EventArgs.Empty);
 
             // PERF-SENSITIVE: This is called on entire hierarchy and using foreach or LINQ
             // will cause extra allocations and overhead.

tests/Avalonia.Base.UnitTests/Animation/AnimationIterationTests.cs

@@ -94,6 +94,256 @@ namespace Avalonia.Base.UnitTests.Animation
             Assert.Equal(border.Width, 100d);
         }
         
+        [Fact]
+        public void Pause_Animation_When_IsEffectivelyVisible_Is_False()
+        {
+            var keyframe1 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 200d), }, Cue = new Cue(1d)
+            };
+            var keyframe2 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 150d), }, Cue = new Cue(0.5d)
+            };
+
+            var keyframe3 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 100d), }, Cue = new Cue(0d)
+            };
+
+            var animation = new Animation()
+            {
+                Duration = TimeSpan.FromSeconds(3),
+                Delay = TimeSpan.FromSeconds(3),
+                DelayBetweenIterations = TimeSpan.FromSeconds(3),
+                IterationCount = new IterationCount(2),
+                Children = { keyframe1, keyframe2, keyframe3 }
+            };
+
+            var border = new Border() { Height = 100d, Width = 100d };
+
+            var clock = new TestClock();
+            var animationRun = animation.RunAsync(border, clock, TestContext.Current.CancellationToken);
+
+            border.Measure(Size.Infinity);
+            border.Arrange(new Rect(border.DesiredSize));
+
+            clock.Step(TimeSpan.Zero);
+
+            clock.Step(TimeSpan.FromSeconds(0));
+            Assert.Equal(100d, border.Width);
+
+            // Hide the border — this should pause the animation clock.
+            border.IsVisible = false;
+
+            clock.Step(TimeSpan.FromSeconds(4.5));
+
+            // Width should not change while invisible (animation is paused).
+            Assert.Equal(100d, border.Width);
+
+            // Show the border — animation resumes from where it left off.
+            border.IsVisible = true;
+
+            // The pause absorbed 4.5s of wall-clock time, so internal time = wall - 4.5.
+            // To reach internal time 6s (end of iteration 1, cue 1.0 -> width 200):
+            // wall = 4.5 + 6 = 10.5
+            clock.Step(TimeSpan.FromSeconds(10.5));
+            Assert.Equal(200d, border.Width);
+
+            // To complete the animation (internal time 14s triggers trailing delay of iter 2):
+            // wall = 4.5 + 14 = 18.5
+            clock.Step(TimeSpan.FromSeconds(18.5));
+            Assert.True(animationRun.Status == TaskStatus.RanToCompletion);
+            Assert.Equal(100d, border.Width);
+        }
+        
+        [Fact]
+        public void Pause_Animation_When_IsEffectivelyVisible_Is_False_Nested()
+        {
+            var keyframe1 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 200d), }, Cue = new Cue(1d)
+            };
+            var keyframe2 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 150d), }, Cue = new Cue(0.5d)
+            };
+
+            var keyframe3 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 100d), }, Cue = new Cue(0d)
+            };
+
+            var animation = new Animation()
+            {
+                Duration = TimeSpan.FromSeconds(3),
+                Delay = TimeSpan.FromSeconds(3),
+                DelayBetweenIterations = TimeSpan.FromSeconds(3),
+                IterationCount = new IterationCount(2),
+                Children = { keyframe1, keyframe2, keyframe3 }
+            };
+
+            var border = new Border() { Height = 100d, Width = 100d };
+
+            var borderParent = new Border { Child = border };
+
+            var clock = new TestClock();
+            var animationRun = animation.RunAsync(border, clock, TestContext.Current.CancellationToken);
+
+            border.Measure(Size.Infinity);
+            border.Arrange(new Rect(border.DesiredSize));
+
+            clock.Step(TimeSpan.Zero);
+
+            clock.Step(TimeSpan.FromSeconds(0));
+            Assert.Equal(100d, border.Width);
+
+            // Hide the parent — this makes border.IsEffectivelyVisible false,
+            // which should pause the animation clock.
+            borderParent.IsVisible = false;
+
+            clock.Step(TimeSpan.FromSeconds(4.5));
+
+            // Width should not change while parent is invisible (animation is paused).
+            Assert.Equal(100d, border.Width);
+
+            // Show the parent — animation resumes from where it left off.
+            borderParent.IsVisible = true;
+
+            // The pause absorbed 4.5s of wall-clock time, so internal time = wall - 4.5.
+            // To reach internal time 6s (end of iteration 1, cue 1.0 -> width 200):
+            // wall = 4.5 + 6 = 10.5
+            clock.Step(TimeSpan.FromSeconds(10.5));
+            Assert.Equal(200d, border.Width);
+
+            // To complete the animation (internal time 14s triggers trailing delay of iter 2):
+            // wall = 4.5 + 14 = 18.5
+            clock.Step(TimeSpan.FromSeconds(18.5));
+            Assert.True(animationRun.Status == TaskStatus.RanToCompletion);
+            Assert.Equal(100d, border.Width);
+        }
+        
+        [Fact]
+        public void Stop_And_Dispose_Animation_When_Detached_From_Visual_Tree()
+        {
+            var keyframe1 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 200d), }, Cue = new Cue(1d)
+            };
+            var keyframe2 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 100d), }, Cue = new Cue(0d)
+            };
+            var animation = new Animation()
+            {
+                Duration = TimeSpan.FromSeconds(10),
+                IterationCount = new IterationCount(1),
+                Children = { keyframe2, keyframe1 }
+            };
+            var border = new Border() { Height = 100d, Width = 50d };
+            var root = new TestRoot(border);
+            var clock = new TestClock();
+            var animationRun = animation.RunAsync(border, clock, TestContext.Current.CancellationToken);
+            clock.Step(TimeSpan.Zero);
+            clock.Step(TimeSpan.FromSeconds(0));
+            Assert.False(animationRun.IsCompleted);
+
+            // Detach from visual tree
+            root.Child = null;
+
+            // Animation should be completed/disposed
+            Assert.True(animationRun.IsCompleted);
+
+            // Further clock ticks should not affect the border
+            var widthAfterDetach = border.Width;
+            clock.Step(TimeSpan.FromSeconds(5));
+            clock.Step(TimeSpan.FromSeconds(10));
+            Assert.Equal(widthAfterDetach, border.Width);
+        }
+
+        [Fact]
+        public void Pause_Animation_When_Control_Starts_Invisible()
+        {
+            var keyframe1 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 200d), }, Cue = new Cue(1d)
+            };
+            var keyframe2 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 100d), }, Cue = new Cue(0d)
+            };
+            var animation = new Animation()
+            {
+                Duration = TimeSpan.FromSeconds(3),
+                IterationCount = new IterationCount(1),
+                Children = { keyframe2, keyframe1 }
+            };
+
+            var border = new Border() { Height = 100d, Width = 100d, IsVisible = false };
+            var clock = new TestClock();
+            var animationRun = animation.RunAsync(border, clock, TestContext.Current.CancellationToken);
+
+            // Clock ticks while invisible should not advance the animation.
+            clock.Step(TimeSpan.Zero);
+            clock.Step(TimeSpan.FromSeconds(1));
+            clock.Step(TimeSpan.FromSeconds(2));
+            Assert.Equal(100d, border.Width);
+            Assert.False(animationRun.IsCompleted);
+
+            // Make visible — animation starts from the beginning.
+            border.IsVisible = true;
+
+            // The pause absorbed 2s of wall-clock time, so to reach internal time 3s:
+            // wall = 2 + 3 = 5
+            clock.Step(TimeSpan.FromSeconds(5));
+            Assert.True(animationRun.IsCompleted);
+            Assert.Equal(100d, border.Width);
+        }
+
+        [Fact]
+        public void Animation_Plays_Correctly_After_Reattach()
+        {
+            var keyframe1 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 200d), }, Cue = new Cue(1d)
+            };
+            var keyframe2 = new KeyFrame()
+            {
+                Setters = { new Setter(Layoutable.WidthProperty, 100d), }, Cue = new Cue(0d)
+            };
+            var animation = new Animation()
+            {
+                Duration = TimeSpan.FromSeconds(5),
+                IterationCount = new IterationCount(1),
+                FillMode = FillMode.Forward,
+                Children = { keyframe2, keyframe1 }
+            };
+
+            var border = new Border() { Height = 100d, Width = 50d };
+            var root = new TestRoot(border);
+            var clock = new TestClock();
+            var animationRun = animation.RunAsync(border, clock, TestContext.Current.CancellationToken);
+
+            clock.Step(TimeSpan.Zero);
+            Assert.False(animationRun.IsCompleted);
+
+            // Detach — animation completes.
+            root.Child = null;
+            Assert.True(animationRun.IsCompleted);
+
+            // Reattach and start a fresh animation.
+            root.Child = border;
+            var clock2 = new TestClock();
+            var animationRun2 = animation.RunAsync(border, clock2, TestContext.Current.CancellationToken);
+
+            clock2.Step(TimeSpan.Zero);
+            Assert.False(animationRun2.IsCompleted);
+
+            clock2.Step(TimeSpan.FromSeconds(5));
+            Assert.True(animationRun2.IsCompleted);
+            Assert.Equal(200d, border.Width);
+        }
+
         [Fact]
         public void Check_FillModes_Start_and_End_Values_if_Retained()
         {