Addressed all review notes by sir @grokys .

build/Base.props

@@ -1,6 +1,5 @@
 <Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <ItemGroup>
     <PackageReference Include="System.ValueTuple" Version="4.3.1" />
-    <!-- <PackageReference Include="System.Numerics.Vectors" Version="4.5.0-preview2-26406-04"/> -->
   </ItemGroup>
 </Project>

src/Avalonia.Animation/Animatable.cs

@@ -23,7 +23,7 @@ namespace Avalonia.Animation
         /// </summary>
         public Animatable()
         {
-            Transitions = new Transitions.Transitions();
+            Transitions = new Transitions();
             AnimatableTimer = Timing.AnimationStateTimer
                                 .Select(p =>
                                 {

src/Avalonia.Animation/IKeyFrames.cs

@@ -7,7 +7,7 @@ namespace Avalonia.Animation
     /// <summary>
     /// Interface for Keyframe group object
     /// </summary>
-    public interface IKeyFrames
+    internal interface IKeyFrames
     {
         /// <summary>
         /// Applies the current KeyFrame group to the specified control.

src/Avalonia.Animation/KeyFrame.cs

@@ -64,7 +64,4 @@ namespace Avalonia.Animation
         public object Value { get; set; }
 
     }
-
-
-
 }

src/Avalonia.Animation/KeyFramesStateMachine`1.cs

@@ -11,7 +11,7 @@ namespace Avalonia.Animation
     {
         object _lastInterpValue;
         object _firstKFValue;
-        
+
         private ulong _delayTotalFrameCount,
             _durationTotalFrameCount,
             _delayFrameCount,
@@ -33,18 +33,19 @@ namespace Avalonia.Animation
         internal bool _unsubscribe = false;
         private IObserver<object> _targetObserver;
 
+        [Flags]
         private enum KeyFramesStates
         {
-            INITIALIZE,
-            DO_DELAY,
-            DO_RUN,
-            RUN_FORWARDS,
-            RUN_BACKWARDS,
-            RUN_APPLYVALUE,
-            RUN_COMPLETE,
-            PAUSE,
-            STOP,
-            DISPOSED
+            Initialize,
+            DoDelay,
+            DoRun,
+            RunForwards,
+            RunBackwards,
+            RunApplyValue,
+            RunComplete,
+            Pause,
+            Stop,
+            Disposed
         }
 
         public void Initialize(Animation animation, Animatable control, KeyFrames<T> keyframes)
@@ -82,24 +83,14 @@ namespace Avalonia.Animation
                     break;
             }
 
-            switch (animation.PlaybackDirection)
-            {
-                case PlaybackDirection.Reverse:
-                case PlaybackDirection.AlternateReverse:
-                    _isReversed = true;
-                    break;
-                default:
-                    _isReversed = false;
-                    break;
-            }
-
+            _isReversed = (animation.PlaybackDirection & PlaybackDirection.Reverse) != 0;
             _animationDirection = _targetAnimation.PlaybackDirection;
             _fillMode = _targetAnimation.FillMode;
 
             if (_durationTotalFrameCount > 0)
-                _currentState = KeyFramesStates.DO_DELAY;
+                _currentState = KeyFramesStates.DoDelay;
             else
-                _currentState = KeyFramesStates.DO_RUN;
+                _currentState = KeyFramesStates.DoRun;
 
         }
 
@@ -124,21 +115,21 @@ namespace Avalonia.Animation
                 _gotFirstKFValue = true;
             }
 
-            if (_currentState == KeyFramesStates.DISPOSED)
+            if (_currentState == KeyFramesStates.Disposed)
                 throw new InvalidProgramException("This KeyFrames Animation is already disposed.");
 
             if (_playState == PlayState.Stop)
-                _currentState = KeyFramesStates.STOP;
+                _currentState = KeyFramesStates.Stop;
 
             // Save state and pause the machine
-            if (_playState == PlayState.Pause && _currentState != KeyFramesStates.PAUSE)
+            if (_playState == PlayState.Pause && _currentState != KeyFramesStates.Pause)
             {
                 _savedState = _currentState;
-                _currentState = KeyFramesStates.PAUSE;
+                _currentState = KeyFramesStates.Pause;
             }
 
             // Resume the previous state
-            if (_playState != PlayState.Pause && _currentState == KeyFramesStates.PAUSE)
+            if (_playState != PlayState.Pause && _currentState == KeyFramesStates.Pause)
                 _currentState = _savedState;
 
             double _tempDuration = 0d, _easedTime;
@@ -146,7 +137,7 @@ namespace Avalonia.Animation
         checkstate:
             switch (_currentState)
             {
-                case KeyFramesStates.DO_DELAY:
+                case KeyFramesStates.DoDelay:
 
                     if (_fillMode == FillMode.Backward
                      || _fillMode == FillMode.Both)
@@ -163,60 +154,60 @@ namespace Avalonia.Animation
 
                     if (_delayFrameCount > _delayTotalFrameCount)
                     {
-                        _currentState = KeyFramesStates.DO_RUN;
+                        _currentState = KeyFramesStates.DoRun;
                         goto checkstate;
                     }
                     _delayFrameCount++;
 
                     break;
 
-                case KeyFramesStates.DO_RUN:
+                case KeyFramesStates.DoRun:
 
                     if (_isReversed)
-                        _currentState = KeyFramesStates.RUN_BACKWARDS;
+                        _currentState = KeyFramesStates.RunBackwards;
                     else
-                        _currentState = KeyFramesStates.RUN_FORWARDS;
+                        _currentState = KeyFramesStates.RunForwards;
 
                     goto checkstate;
 
-                case KeyFramesStates.RUN_FORWARDS:
+                case KeyFramesStates.RunForwards:
 
                     if (_durationFrameCount > _durationTotalFrameCount)
                     {
-                        _currentState = KeyFramesStates.RUN_COMPLETE;
+                        _currentState = KeyFramesStates.RunComplete;
                         goto checkstate;
                     }
 
                     _tempDuration = (double)_durationFrameCount / _durationTotalFrameCount;
-                    _currentState = KeyFramesStates.RUN_APPLYVALUE;
+                    _currentState = KeyFramesStates.RunApplyValue;
 
                     goto checkstate;
 
-                case KeyFramesStates.RUN_BACKWARDS:
+                case KeyFramesStates.RunBackwards:
 
                     if (_durationFrameCount > _durationTotalFrameCount)
                     {
-                        _currentState = KeyFramesStates.RUN_COMPLETE;
+                        _currentState = KeyFramesStates.RunComplete;
                         goto checkstate;
                     }
 
                     _tempDuration = (double)(_durationTotalFrameCount - _durationFrameCount) / _durationTotalFrameCount;
-                    _currentState = KeyFramesStates.RUN_APPLYVALUE;
+                    _currentState = KeyFramesStates.RunApplyValue;
 
                     goto checkstate;
 
-                case KeyFramesStates.RUN_APPLYVALUE:
+                case KeyFramesStates.RunApplyValue:
 
                     _easedTime = _targetAnimation.Easing.Ease(_tempDuration);
 
                     _durationFrameCount++;
                     _lastInterpValue = Interpolator(_easedTime);
                     _targetObserver.OnNext(_lastInterpValue);
-                    _currentState = KeyFramesStates.DO_RUN;
+                    _currentState = KeyFramesStates.DoRun;
 
                     break;
 
-                case KeyFramesStates.RUN_COMPLETE:
+                case KeyFramesStates.RunComplete:
 
                     if (_checkLoopAndRepeat)
                     {
@@ -225,17 +216,17 @@ namespace Avalonia.Animation
 
                         if (_isLooping)
                         {
-                            _currentState = KeyFramesStates.DO_RUN;
+                            _currentState = KeyFramesStates.DoRun;
                         }
                         else if (_isRepeating)
                         {
                             if (_currentIteration >= _repeatCount)
                             {
-                                _currentState = KeyFramesStates.STOP;
+                                _currentState = KeyFramesStates.Stop;
                             }
                             else
                             {
-                                _currentState = KeyFramesStates.DO_RUN;
+                                _currentState = KeyFramesStates.DoRun;
                             }
                             _currentIteration++;
                         }
@@ -247,14 +238,15 @@ namespace Avalonia.Animation
                         break;
                     }
 
-                    _currentState = KeyFramesStates.STOP;
+                    _currentState = KeyFramesStates.Stop;
                     goto checkstate;
 
-                case KeyFramesStates.STOP:
+                case KeyFramesStates.Stop:
+
                     if (_fillMode == FillMode.Forward
                      || _fillMode == FillMode.Both)
                     {
-                        _targetControl.SetValue(_parent.Property, _lastInterpValue, BindingPriority.Animation);
+                        _targetControl.SetValue(_parent.Property, _lastInterpValue, BindingPriority.LocalValue);
                     }
                     _targetObserver.OnCompleted();
                     break;
@@ -267,10 +259,12 @@ namespace Avalonia.Animation
             _targetObserver = observer;
             return this;
         }
+
         public void Dispose()
         {
             _unsubscribe = true;
-            _currentState = KeyFramesStates.DISPOSED;
+            _currentState = KeyFramesStates.Disposed;
         }
+        
     }
 }

src/Avalonia.Animation/KeyFrames`1.cs

@@ -17,17 +17,17 @@ namespace Avalonia.Animation
     public abstract class KeyFrames<T> : AvaloniaList<KeyFrame>, IKeyFrames
     {
         
-        private bool _isVerfifiedAndConverted;
-
         /// <summary>
-        /// Target property.
+        /// List of type-converted keyframes.
         /// </summary>
-        public AvaloniaProperty Property { get; set; }
+        private Dictionary<double, T> _convertedKeyframes = new Dictionary<double, T>();
+
+        private bool _isVerfifiedAndConverted;
 
         /// <summary>
-        /// List of type-converted keyframes.
+        /// Gets or sets the target property for the keyframe.
         /// </summary>
-        public Dictionary<double, T> ConvertedKeyframes = new Dictionary<double, T>();
+        public AvaloniaProperty Property { get; set; }
 
         /// <inheritdoc/>
         public virtual IDisposable Apply(Animation animation, Animatable control, IObservable<bool> obsMatch)
@@ -47,38 +47,38 @@ namespace Avalonia.Animation
 
         /// <summary>
         /// Get the nearest pair of cue-time ordered keyframes 
-        /// according to the given time parameter that is relative to
+        /// according to the given time parameter that is relative to the
         /// total animation time and the normalized intra-keyframe pair time 
         /// (i.e., the normalized time between the selected keyframes, relative to the
         /// time parameter).
         /// </summary>
         /// <param name="t">The time parameter, relative to the total animation time</param>
-        public (double IntraKFTime, KeyFramePair<T> KFPair) GetKFPairAndIntraKFTime(double t)
+        protected (double IntraKFTime, KeyFramePair<T> KFPair) GetKFPairAndIntraKFTime(double t)
         {
             KeyValuePair<double, T> firstCue, lastCue;
-            int kvCount = ConvertedKeyframes.Count();
+            int kvCount = _convertedKeyframes.Count();
             if (kvCount > 2)
             {
                 if (DoubleUtils.AboutEqual(t, 0.0) || t < 0.0)
                 {
-                    firstCue = ConvertedKeyframes.First();
-                    lastCue = ConvertedKeyframes.Skip(1).First();
+                    firstCue = _convertedKeyframes.First();
+                    lastCue = _convertedKeyframes.Skip(1).First();
                 }
                 else if (DoubleUtils.AboutEqual(t, 1.0) || t > 1.0)
                 {
-                    firstCue = ConvertedKeyframes.Skip(kvCount - 2).First();
-                    lastCue = ConvertedKeyframes.Last();
+                    firstCue = _convertedKeyframes.Skip(kvCount - 2).First();
+                    lastCue = _convertedKeyframes.Last();
                 }
                 else
                 {
-                    firstCue = ConvertedKeyframes.Where(j => j.Key <= t).Last();
-                    lastCue = ConvertedKeyframes.Where(j => j.Key >= t).First();
+                    firstCue = _convertedKeyframes.Where(j => j.Key <= t).Last();
+                    lastCue = _convertedKeyframes.Where(j => j.Key >= t).First();
                 }
             }
             else
             {
-                firstCue = ConvertedKeyframes.First();
-                lastCue = ConvertedKeyframes.Last();
+                firstCue = _convertedKeyframes.First();
+                lastCue = _convertedKeyframes.Last();
             }
 
             double t0 = firstCue.Key;
@@ -91,7 +91,7 @@ namespace Avalonia.Animation
         /// <summary>
         /// Runs the KeyFrames Animation.
         /// </summary>
-        public IDisposable RunKeyFrames(Animation animation, Animatable control)
+        private IDisposable RunKeyFrames(Animation animation, Animatable control)
         {
             var _kfStateMach = new KeyFramesStateMachine<T>();
             _kfStateMach.Initialize(animation, control, this);
@@ -109,12 +109,7 @@ namespace Avalonia.Animation
         /// <summary>
         /// Interpolates a value given the desired time.
         /// </summary>
-        public abstract T DoInterpolation(double time);
-
-
-        // public abstract IObservable<T> DoInterpolation(IObservable<double> timer, Animation animation,
-        //                                                Animatable control);
-
+        protected abstract T DoInterpolation(double time);
 
         /// <summary>
         /// Verifies and converts keyframe values according to this class's target type.
@@ -144,11 +139,11 @@ namespace Avalonia.Animation
                     _normalizedCue = new Cue(k.KeyTime.Ticks / animation.Duration.Ticks);
                 }
 
-                ConvertedKeyframes.Add(_normalizedCue.CueValue, convertedValue);
+                _convertedKeyframes.Add(_normalizedCue.CueValue, convertedValue);
 
             }
 
-            SortKeyFrameCues(ConvertedKeyframes);
+            SortKeyFrameCues(_convertedKeyframes);
             _isVerfifiedAndConverted = true;
 
         }
@@ -158,9 +153,8 @@ namespace Avalonia.Animation
             bool hasStartKey, hasEndKey;
             hasStartKey = hasEndKey = false;
 
-            // this can be optional later, by making the default start/end keyframes
-            // to have a neutral value (a.k.a. the value prior to the animation).
-            foreach (var converted in ConvertedKeyframes.Keys)
+            // Make start and end keyframe mandatory.
+            foreach (var converted in _convertedKeyframes.Keys)
             {
                 if (DoubleUtils.AboutEqual(converted, 0.0))
                 {
@@ -177,8 +171,8 @@ namespace Avalonia.Animation
                     ($"{this.GetType().Name} must have a starting (0% cue) and ending (100% cue) keyframe.");
 
             // Sort Cues, in case users don't order it by themselves.
-            ConvertedKeyframes = ConvertedKeyframes.OrderBy(p => p.Key)
-                                                   .ToDictionary((k) => k.Key, (v) => v.Value);
+            _convertedKeyframes = _convertedKeyframes.OrderBy(p => p.Key)
+                                                     .ToDictionary((k) => k.Key, (v) => v.Value);
         }
 
     }

src/Avalonia.Styling/Styling/Style.cs

@@ -88,11 +88,8 @@ namespace Avalonia.Styling
             {
                 return _animations ?? (_animations = new List<IAnimation>());
             }
-            set
-            {
-                _animations = value;
-            }
         }
+
         /// <inheritdoc/>
         IResourceNode IResourceNode.ResourceParent => _parent;
 
@@ -118,8 +115,7 @@ namespace Avalonia.Styling
 
                     foreach (var animation in Animations)
                     {
-                        // TODO: Needs more work in passing the appropriate
-                        //       observable.
+
                         IObservable<bool> obsMatch = match.ObservableResult;
 
                         if (match.ImmediateResult == true)

src/Avalonia.Visuals/Animation/TransformKeyFrames.cs

@@ -11,7 +11,7 @@ using Avalonia.Media;
 namespace Avalonia.Animation
 {
     /// <summary>
-    /// Key frames that handles <see cref="double"/> properties.
+    /// Keyframes that handles <see cref="Transform"/> properties.
     /// </summary>
     public class TransformKeyFrames : KeyFrames<double>
     {
@@ -29,7 +29,7 @@ namespace Avalonia.Animation
 
                 if (childKeyFrames == null)
                 {
-                    InitializeInternalDoubleKeyFrames();
+                    InitializeChildKeyFrames();
                 }
 
                 // It's a transform object so let's target that.
@@ -49,6 +49,7 @@ namespace Avalonia.Animation
                     }
                 }
 
+                //TODO: determine correctly when to throw this error.
                 //throw new InvalidOperationException($"TransformKeyFrame hasn't found an appropriate Transform object with type {Property.OwnerType} in target {control}.");
 
                 return null;
@@ -56,11 +57,11 @@ namespace Avalonia.Animation
             }
             else
             {
-                throw new Exception($"Unsupported property {Property}");
+                throw new Exception($"Cannot apply keyframe: property {Property} is not a Transform");
             }
         }
 
-        void InitializeInternalDoubleKeyFrames()
+        void InitializeChildKeyFrames()
         {
             childKeyFrames = new DoubleKeyFrames();
 
@@ -73,6 +74,6 @@ namespace Avalonia.Animation
         }
 
         /// <inheritdocs/>
-        public override double DoInterpolation(double time) => 0;
+        protected override double DoInterpolation(double time) => 0;
     }
 }

src/Avalonia.Visuals/Media/Color.cs

@@ -81,32 +81,6 @@ namespace Avalonia.Media
                 (byte)(value & 0xff)
             );
         }
-
-        // /// <summary>
-        // /// Creates a <see cref="Color"/> from a <see cref="Vector4"/>.
-        // /// </summary>
-        // /// <param name="value">The <see cref="Vector4"/> value.</param>
-        // /// <param name="normalized">True if the color vector is normalized floats, false if it's bytes (0-255)</param>
-        // /// <returns>The color.</returns>
-        // public static Color FromVector4(Vector4 value, bool normalized)
-        // {
-        //     if (normalized)
-        //     {
-        //         return new Color(
-        //             (byte)(value.W * 255),
-        //             (byte)(value.X * 255),
-        //             (byte)(value.Y * 255),
-        //             (byte)(value.Z * 255)
-        //         );
-        //     } else {
-        //         return new Color(
-        //             (byte)(value.W),
-        //             (byte)(value.X),
-        //             (byte)(value.Y),
-        //             (byte)(value.Z)
-        //         );
-        //     }
-        // }
         
         /// <summary>
         /// Parses a color string.

src/Avalonia.Visuals/Media/TransformGroup.cs

@@ -18,23 +18,13 @@ namespace Avalonia.Media
         public TransformGroup()
         {
             Children = new Transforms();
-            Children.CollectionChanged += (_, e) =>
+            Children.ResetBehavior = ResetBehavior.Remove;
+            Children.CollectionChanged += delegate
             {
-                switch (e.Action)
-                {
-                    case NotifyCollectionChangedAction.Add:
-                        foreach (Transform tr in e.NewItems)
-                        {
-                            tr.Changed += ChildTransform_Changed;
-                        }
-                        break;
-                    case NotifyCollectionChangedAction.Remove:
-                        foreach (Transform tr in e.OldItems)
-                        {
-                            tr.Changed -= ChildTransform_Changed;
-                        }
-                        break;
-                }
+                Children.ForEachItem(
+                    (tr) => tr.Changed += ChildTransform_Changed,
+                    (tr) => tr.Changed -= ChildTransform_Changed,
+                    () => { });
             };
         }