Fix Animation.FillMode when cue isn't 0% or 100%

src/Avalonia.Base/Animation/Animatable.cs

@@ -17,8 +17,8 @@ public class Animatable : AvaloniaObject
         /// <summary>
         /// Defines the <see cref="Clock"/> property.
         /// </summary>
-        internal static readonly StyledProperty<IClock> ClockProperty =
-            AvaloniaProperty.Register<Animatable, IClock>(nameof(Clock), inherits: true);
+        internal static readonly StyledProperty<IClock?> ClockProperty =
+            AvaloniaProperty.Register<Animatable, IClock?>(nameof(Clock), inherits: true);
 
         /// <summary>
         /// Defines the <see cref="Transitions"/> property.
@@ -36,7 +36,7 @@ public class Animatable : AvaloniaObject
         /// <summary>
         /// Gets or sets the clock which controls the animations on the control.
         /// </summary>
-        internal IClock Clock
+        internal IClock? Clock
         {
             get => GetValue(ClockProperty);
             set => SetValue(ClockProperty, value);

src/Avalonia.Base/Animation/Animation.cs

@@ -234,6 +234,8 @@ internal static (Type Type, Func<IAnimator> Factory)? GetAnimator(IAnimationSett
                 }
             }
 
+            animatorKeyFrames.Sort(static (x, y) => x.Cue.CueValue.CompareTo(y.Cue.CueValue));
+
             var newAnimatorInstances = new List<IAnimator>();
 
             foreach (var handler in handlerList)
@@ -247,9 +249,22 @@ internal static (Type Type, Func<IAnimator> Factory)? GetAnimator(IAnimationSett
             {
                 var animator = newAnimatorInstances.First(a => a.GetType() == keyframe.AnimatorType &&
                                                              a.Property == keyframe.Property);
+
+                if (animator.Count == 0 && FillMode is FillMode.Backward or FillMode.Both)
+                    keyframe.FillBefore = true;
+
                 animator.Add(keyframe);
             }
 
+            if (FillMode is FillMode.Forward or FillMode.Both)
+            {
+                foreach (var newAnimatorInstance in newAnimatorInstances)
+                {
+                    if (newAnimatorInstance.Count > 0)
+                        newAnimatorInstance[newAnimatorInstance.Count - 1].FillAfter = true;
+                }
+            }
+
             return (newAnimatorInstances, subscriptions);
         }
 

src/Avalonia.Base/Animation/AnimatorKeyFrame.cs

@@ -16,19 +16,6 @@ internal class AnimatorKeyFrame : AvaloniaObject
         public static readonly DirectProperty<AnimatorKeyFrame, object?> ValueProperty =
             AvaloniaProperty.RegisterDirect<AnimatorKeyFrame, object?>(nameof(Value), k => k.Value, (k, v) => k.Value = v);
 
-        public AnimatorKeyFrame()
-        {
-
-        }
-
-        public AnimatorKeyFrame(Type? animatorType, Func<IAnimator>? animatorFactory, Cue cue)
-        {
-            AnimatorType = animatorType;
-            AnimatorFactory = animatorFactory;
-            Cue = cue;
-            KeySpline = null;
-        }
-
         public AnimatorKeyFrame(Type? animatorType, Func<IAnimator>? animatorFactory, Cue cue, KeySpline? keySpline)
         {
             AnimatorType = animatorType;
@@ -37,11 +24,12 @@ public AnimatorKeyFrame(Type? animatorType, Func<IAnimator>? animatorFactory, Cu
             KeySpline = keySpline;
         }
 
-        internal bool isNeutral;
         public Type? AnimatorType { get; }
         public Func<IAnimator>? AnimatorFactory { get; }
         public Cue Cue { get; }
         public KeySpline? KeySpline { get; }
+        public bool FillBefore { get; set; }
+        public bool FillAfter { get; set; }
         public AvaloniaProperty? Property { get; private set; }
 
         private object? _value;

src/Avalonia.Base/Animation/Animators/Animator`1.cs

@@ -1,7 +1,5 @@
 using System;
-using System.Collections.Generic;
-using System.Linq;
-using Avalonia.Animation.Utils;
+using System.Diagnostics;
 using Avalonia.Collections;
 using Avalonia.Data;
 using Avalonia.Reactive;
@@ -13,94 +11,72 @@ namespace Avalonia.Animation.Animators
     /// </summary>
     internal abstract class Animator<T> : AvaloniaList<AnimatorKeyFrame>, IAnimator
     {
-        /// <summary>
-        /// List of type-converted keyframes.
-        /// </summary>
-        private readonly List<AnimatorKeyFrame> _convertedKeyframes = new List<AnimatorKeyFrame>();
-
-        private bool _isVerifiedAndConverted;
-
         /// <summary>
         /// Gets or sets the target property for the keyframe.
         /// </summary>
         public AvaloniaProperty? Property { get; set; }
 
-        public Animator()
-        {
-            // Invalidate keyframes when changed.
-            this.CollectionChanged += delegate { _isVerifiedAndConverted = false; };
-        }
-
         /// <inheritdoc/>
         public virtual IDisposable? Apply(Animation animation, Animatable control, IClock? clock, IObservable<bool> match, Action? onComplete)
         {
-            if (!_isVerifiedAndConverted)
-                VerifyConvertKeyFrames();
-
             var subject = new DisposeAnimationInstanceSubject<T>(this, animation, control, clock, onComplete);
             return new CompositeDisposable(match.Subscribe(subject), subject);
         }
 
         protected T InterpolationHandler(double animationTime, T neutralValue)
         {
-            AnimatorKeyFrame firstKeyframe, lastKeyframe;
+            if (Count == 0)
+                return neutralValue;
+
+            var (beforeKeyFrame, afterKeyFrame) = FindKeyFrames(animationTime);
 
-            int kvCount = _convertedKeyframes.Count;
-            if (kvCount > 2)
+            double beforeTime, afterTime;
+            T beforeValue, afterValue;
+
+            if (beforeKeyFrame is null)
             {
-                if (animationTime <= 0.0)
-                {
-                    firstKeyframe = _convertedKeyframes[0];
-                    lastKeyframe = _convertedKeyframes[1];
-                }
-                else if (animationTime >= 1.0)
-                {
-                    firstKeyframe = _convertedKeyframes[_convertedKeyframes.Count - 2];
-                    lastKeyframe = _convertedKeyframes[_convertedKeyframes.Count - 1];
-                }
-                else
-                {
-                    int index = FindClosestBeforeKeyFrame(animationTime);
-                    firstKeyframe = _convertedKeyframes[index];
-                    lastKeyframe = _convertedKeyframes[index + 1];
-                }
+                beforeTime = 0.0;
+                beforeValue = afterKeyFrame is { FillBefore: true, Value: T fillValue } ? fillValue : neutralValue;
             }
             else
             {
-                firstKeyframe = _convertedKeyframes[0];
-                lastKeyframe = _convertedKeyframes[1];
+                beforeTime = beforeKeyFrame.Cue.CueValue;
+                beforeValue = beforeKeyFrame.Value is T value ? value : neutralValue;
             }
 
-            double t0 = firstKeyframe.Cue.CueValue;
-            double t1 = lastKeyframe.Cue.CueValue;
-
-            double progress = (animationTime - t0) / (t1 - t0);
-
-            T oldValue, newValue;
-
-            if (!firstKeyframe.isNeutral && firstKeyframe.Value is T firstKeyframeValue)
-                oldValue = firstKeyframeValue;
+            if (afterKeyFrame is null)
+            {
+                afterTime = 1.0;
+                afterValue = beforeKeyFrame is { FillAfter: true, Value: T fillValue } ? fillValue : neutralValue;
+            }
             else
-                oldValue = neutralValue;
+            {
+                afterTime = afterKeyFrame.Cue.CueValue;
+                afterValue = afterKeyFrame.Value is T value ? value : neutralValue;
+            }
 
-            if (!lastKeyframe.isNeutral && lastKeyframe.Value is T lastKeyframeValue)
-                newValue = lastKeyframeValue;
-            else
-                newValue = neutralValue;
+            var progress = (animationTime - beforeTime) / (afterTime - beforeTime);
 
-            if (lastKeyframe.KeySpline != null)
-                progress = lastKeyframe.KeySpline.GetSplineProgress(progress);
+            if (afterKeyFrame?.KeySpline is { } keySpline)
+                progress = keySpline.GetSplineProgress(progress);
 
-            return Interpolate(progress, oldValue, newValue);
+            return Interpolate(progress, beforeValue, afterValue);
         }
 
-        private int FindClosestBeforeKeyFrame(double time)
+        private (AnimatorKeyFrame? Before, AnimatorKeyFrame? After) FindKeyFrames(double time)
         {
-            for (int i = 0; i < _convertedKeyframes.Count; i++)
-                if (_convertedKeyframes[i].Cue.CueValue > time)
-                    return i - 1;
+            Debug.Assert(Count >= 1);
 
-            throw new Exception("Index time is out of keyframe time range.");
+            for (var i = 0; i < Count; i++)
+            {
+                var keyFrame = this[i];
+                var keyFrameTime = keyFrame.Cue.CueValue;
+
+                if (time < keyFrameTime || keyFrameTime == 1.0)
+                    return (i > 0 ? this[i - 1] : null, keyFrame);
+            }
+
+            return (this[Count - 1], null);
         }
 
         public virtual IDisposable BindAnimation(Animatable control, IObservable<T> instance)
@@ -123,60 +99,13 @@ internal IDisposable Run(Animation animation, Animatable control, IClock? clock,
                 clock ?? control.Clock ?? Clock.GlobalClock,
                 onComplete,
                 InterpolationHandler);
-            
+
             return BindAnimation(control, instance);
         }
 
         /// <summary>
         /// Interpolates in-between two key values given the desired progress time.
         /// </summary>
         public abstract T Interpolate(double progress, T oldValue, T newValue);
-
-        private void VerifyConvertKeyFrames()
-        {
-            foreach (AnimatorKeyFrame keyframe in this)
-            {
-                _convertedKeyframes.Add(keyframe);
-            }
-
-            AddNeutralKeyFramesIfNeeded();
-
-            _isVerifiedAndConverted = true;
-        }
-
-        private void AddNeutralKeyFramesIfNeeded()
-        {
-            bool hasStartKey, hasEndKey;
-            hasStartKey = hasEndKey = false;
-
-            // Check if there's start and end keyframes.
-            foreach (var frame in _convertedKeyframes)
-            {
-                if (frame.Cue.CueValue == 0.0d)
-                {
-                    hasStartKey = true;
-                }
-                else if (frame.Cue.CueValue == 1.0d)
-                {
-                    hasEndKey = true;
-                }
-            }
-
-            if (!hasStartKey || !hasEndKey)
-                AddNeutralKeyFrames(hasStartKey, hasEndKey);
-        }
-
-        private void AddNeutralKeyFrames(bool hasStartKey, bool hasEndKey)
-        {
-            if (!hasStartKey)
-            {
-                _convertedKeyframes.Insert(0, new AnimatorKeyFrame(null, null, new Cue(0.0d)) { Value = default(T), isNeutral = true });
-            }
-
-            if (!hasEndKey)
-            {
-                _convertedKeyframes.Add(new AnimatorKeyFrame(null, null, new Cue(1.0d)) { Value = default(T), isNeutral = true });
-            }
-        }
     }
 }

src/Avalonia.Base/Animation/Animators/BaseBrushAnimator.cs

@@ -86,14 +86,18 @@ private bool TryCreateGradientAnimator([NotNullWhen(true)] out IAnimator? animat
                 {
                     gradientAnimator.Add(new AnimatorKeyFrame(typeof(GradientBrushAnimator), () => new GradientBrushAnimator(), keyframe.Cue, keyframe.KeySpline)
                     {
-                        Value = GradientBrushAnimator.ConvertSolidColorBrushToGradient(firstGradient, solidColorBrush)
+                        Value = GradientBrushAnimator.ConvertSolidColorBrushToGradient(firstGradient, solidColorBrush),
+                        FillBefore = keyframe.FillBefore,
+                        FillAfter = keyframe.FillAfter
                     });
                 }
                 else if (keyframe.Value is IGradientBrush)
                 {
                     gradientAnimator.Add(new AnimatorKeyFrame(typeof(GradientBrushAnimator), () => new GradientBrushAnimator(), keyframe.Cue, keyframe.KeySpline)
                     {
-                        Value = keyframe.Value
+                        Value = keyframe.Value,
+                        FillBefore = keyframe.FillBefore,
+                        FillAfter = keyframe.FillAfter
                     });
                 }
                 else
@@ -118,7 +122,9 @@ private bool TryCreateSolidColorBrushAnimator([NotNullWhen(true)] out IAnimator?
                 {
                     solidColorBrushAnimator.Add(new AnimatorKeyFrame(typeof(ISolidColorBrushAnimator), () => new ISolidColorBrushAnimator(), keyframe.Cue, keyframe.KeySpline)
                     {
-                        Value = keyframe.Value
+                        Value = keyframe.Value,
+                        FillBefore = keyframe.FillBefore,
+                        FillAfter = keyframe.FillAfter
                     });
                 }
                 else
@@ -149,7 +155,9 @@ private bool TryCreateCustomRegisteredAnimator([NotNullWhen(true)] out IAnimator
                         {
                             animator.Add(new AnimatorKeyFrame(animatorType, animatorFactory, keyframe.Cue, keyframe.KeySpline)
                             {
-                                Value = keyframe.Value
+                                Value = keyframe.Value,
+                                FillBefore = keyframe.FillBefore,
+                                FillAfter = keyframe.FillAfter
                             });
                         }
 

src/Avalonia.Base/Media/Effects/EffectAnimator.cs

@@ -38,7 +38,9 @@ private bool TryCreateAnimator<TAnimator, TInterface>([NotNullWhen(true)] out IA
                 createdAnimator.Add(new AnimatorKeyFrame(typeof(TAnimator), () => new TAnimator(), keyFrame.Cue,
                     keyFrame.KeySpline)
                 {
-                    Value = keyFrame.Value
+                    Value = keyFrame.Value,
+                    FillBefore = keyFrame.FillBefore,
+                    FillAfter = keyFrame.FillAfter
                 });
             }
             else

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

@@ -126,18 +126,16 @@ public void Invalid_Values_In_Animation_Should_Not_Crash_Animations(object inval
 
             var rect = new Rectangle() { Width = 11, };
 
-            var originalValue = rect.Width;
-
             var clock = new TestClock();
             animation.RunAsync(rect, clock);
 
             clock.Step(TimeSpan.Zero);
-            Assert.Equal(rect.Width, 1);
+            Assert.Equal(1, rect.Width);
             clock.Step(TimeSpan.FromSeconds(2));
-            Assert.Equal(rect.Width, 2);
+            Assert.Equal(2, rect.Width);
             clock.Step(TimeSpan.FromSeconds(3));
             //here we have invalid value so value should be expected and set to initial original value
-            Assert.Equal(rect.Width, originalValue);
+            Assert.Equal(11, rect.Width);
         }
 
         [Fact]