fix(animation): setting Local value should clear Animation value in filling state

src/Uno.UI.RuntimeTests/Extensions/StoryboardExtensions.cs → src/Uno.UI.RuntimeTests/Extensions/TimelineExtensions.cs

@@ -8,20 +8,11 @@
 
 namespace Uno.UI.RuntimeTests.Extensions;
 
-internal static class StoryboardExtensions
+internal static class TimelineExtensions
 {
-	public static void Begin(this Timeline timeline)
+	public static Storyboard ToStoryboard(this Timeline timeline)
 	{
-		var storyboard = new Storyboard { Children = { timeline } };
-
-		storyboard.Begin();
-	}
-
-	public static async Task RunAsync(this Timeline timeline, TimeSpan? timeout, bool throwsException = false)
-	{
-		var storyboard = new Storyboard { Children = { timeline } };
-
-		await storyboard.RunAsync(timeout, throwsException);
+		return new Storyboard { Children = { timeline } };
 	}
 
 	public static async Task RunAsync(this Storyboard storyboard, TimeSpan? timeout = null, bool throwsException = false)

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media_Animation/Given_DoubleAnimation.cs

@@ -149,7 +149,7 @@ public async Task When_RepeatForever_ShouldLoop()
 				RepeatBehavior = RepeatBehavior.Forever,
 				Duration = TimeSpan.FromMilliseconds(500 * TimeResolutionScaling),
 			}.BindTo(target, nameof(Rectangle.Width));
-			animation.Begin();
+			animation.ToStoryboard().Begin();
 
 			// In an ideal world, the measurements would be [0 or 50,10,20,30,40] repeated 10 times.
 			var list = new List<double>();
@@ -185,7 +185,7 @@ public async Task When_RepeatForever_ShouldLoop()
 
 		[TestMethod]
 		[RunsOnUIThread]
-		public async Task When_StartingFrom_AnimatedValue()
+		public async Task When_StartingFrom_AnimatedValue() // value from completed(filling) animation
 		{
 			var translate = new TranslateTransform();
 			var border = new Border()
@@ -208,7 +208,7 @@ public async Task When_StartingFrom_AnimatedValue()
 				To = 50,
 				Duration = new Duration(TimeSpan.FromSeconds(2)),
 			}.BindTo(translate, nameof(translate.Y));
-			await animation0.RunAsync(timeout: animation0.Duration.TimeSpan + TimeSpan.FromSeconds(1));
+			await animation0.ToStoryboard().RunAsync(timeout: animation0.Duration.TimeSpan + TimeSpan.FromSeconds(1));
 			await Task.Delay(1000);
 
 			// Start an second animation which should pick up from current animated value.
@@ -218,24 +218,127 @@ public async Task When_StartingFrom_AnimatedValue()
 				To = -50,
 				Duration = new Duration(TimeSpan.FromSeconds(5)),
 			}.BindTo(translate, nameof(translate.Y));
-			animation1.Begin();
+			animation1.ToStoryboard().Begin();
 			await Task.Delay(125);
 
 			// ~125ms into a 5s animation where the value is animating from 50 to -50,
 			// the value should be still positive.
-			// note: On android, the value will be scaled by ViewHelper.Scale, but the assertion will still hold true.
 			var y = GetTranslateY(translate, isStillAnimating: true);
 			Assert.IsTrue(y > 0, $"Expecting Translate.Y to be still positive: {y}");
 		}
 
-		private double GetTranslateY(TranslateTransform translate, bool isStillAnimating = false) =>
+		[TestMethod]
+		[RunsOnUIThread]
+		public async Task When_StartingFrom_AnimatingValue() // value from mid animation
+		{
+			var translate = new TranslateTransform();
+			var border = new Border()
+			{
+				Background = new SolidColorBrush(Colors.Pink),
+				Margin = new Thickness(0, 50, 0, 0),
+				Width = 50,
+				Height = 50,
+				RenderTransform = translate,
+			};
+			WindowHelper.WindowContent = border;
+			await WindowHelper.WaitForLoaded(border);
+			await WindowHelper.WaitForIdle();
+
+			translate.Y = 50;
+			await WindowHelper.WaitForIdle();
+			await Task.Delay(1000);
+			var threshold = GetTranslateY(translate); // snapshot the value
+
+			// Start an animation. Its animating value will serve as
+			// the inferred starting value for the next animation.
+			var animation0 = new DoubleAnimation
+			{
+				From = 100,
+				To = 105,
+				Duration = new Duration(TimeSpan.FromSeconds(5)),
+			}.BindTo(translate, nameof(translate.Y));
+			animation0.ToStoryboard().Begin();
+			await Task.Delay(125);
+
+			// Start an second animation which should pick up from current animating value.
+			var animation1 = new DoubleAnimation
+			{
+				// From = should be around 100~105 from animation #0
+				To = 50,
+				Duration = new Duration(TimeSpan.FromSeconds(5)),
+			}.BindTo(translate, nameof(translate.Y));
+			animation1.ToStoryboard().Begin();
+			await Task.Delay(125);
+
+			var value = GetTranslateY(translate, isStillAnimating: true);
+			if (value is double y)
+			{
+				// Animation #1 should be animating from around[100~105] to 50, and not from 0 (unanimated Local value).
+				Assert.IsTrue(y > 50, $"Expecting Translate.Y to be still positive: {y}");
+			}
+			else
+			{
+				Assert.Fail($"Translate.Y is not a double: value={value}");
+			}
+		}
+
+		[TestMethod]
+		[RunsOnUIThread]
+		public Task When_OverridingFillingValue_WithLocalValue() =>
+			When_OverridingFillingValue_WithLocalValue_Impl(skipToFill: false);
+
+		[TestMethod]
+		[RunsOnUIThread]
+		public Task When_OverridingFillingValue_WithLocalValue_Skipped() =>
+			When_OverridingFillingValue_WithLocalValue_Impl(skipToFill: true);
+
+		public async Task When_OverridingFillingValue_WithLocalValue_Impl(bool skipToFill)
+		{
+			var translate = new TranslateTransform();
+			var border = new Border()
+			{
+				Background = new SolidColorBrush(Colors.Pink),
+				Margin = new Thickness(0, 50, 0, 0),
+				Width = 50,
+				Height = 50,
+				RenderTransform = translate,
+			};
+			WindowHelper.WindowContent = border;
+			await WindowHelper.WaitForLoaded(border);
+			await WindowHelper.WaitForIdle();
+
+			// Animate the value to fill.
+			var animation0 = new DoubleAnimation
+			{
+				To = 100,
+				Duration = new Duration(TimeSpan.FromSeconds(1)),
+			}.BindTo(translate, nameof(translate.Y));
+			if (skipToFill)
+			{
+				animation0.ToStoryboard().SkipToFill();
+			}
+			else
+			{
+				await animation0.ToStoryboard().RunAsync();
+			}
+			var beforeValue = translate.Y;
+
+			// Set a new local value
+			translate.Y = 312.0;
+			var afterValue = translate.Y;
+
+			Assert.AreEqual(beforeValue, 100.0, "before: Should be animated to 100");
+			Assert.AreEqual(afterValue, 312.0, "after: Should be set to 312");
+		}
+
+		private static double GetTranslateY(TranslateTransform translate, bool isStillAnimating = false) =>
 #if !__ANDROID__
 			translate.Y;
 #else
 			isStillAnimating
 				// On android, animation may target a native property implementing the behavior instead of the specified dependency property.
 				// We need to retrieve the value of that native property, as reading the dp value will just give the final value.
-				? (double)translate.View.TranslationY
+				? ViewHelper.PhysicalToLogicalPixels((double)translate.View.TranslationY)
 				// And, when the animation is completed, this native value is reset even for HoldEnd animation.
 				: translate.Y;
 #endif

src/Uno.UI/DataBinding/BindingPath.cs

@@ -16,6 +16,7 @@
 using Windows.UI.Xaml;
 using Windows.UI.Xaml.Data;
 using Windows.UI.Xaml.Media;
+using System.Diagnostics.CodeAnalysis;
 
 namespace Uno.UI.DataBinding
 {
@@ -236,6 +237,14 @@ internal void SetLocalValue(object value)
 			}
 		}
 
+		internal void SetAnimationFillingValue(object value)
+		{
+			if (!_disposed)
+			{
+				_value?.SetAnimationFillingValue(value);
+			}
+		}
+
 		/// <summary>
 		/// Clears the value of the current precedence.
 		/// </summary>
@@ -249,6 +258,13 @@ public void ClearValue()
 				_value?.ClearValue();
 			}
 		}
+		public void ClearAnimationFillingValue()
+		{
+			if (!_disposed)
+			{
+				_value?.ClearAnimationFillingValue();
+			}
+		}
 
 		public Type? ValueType => _value?.PropertyType;
 
@@ -489,7 +505,9 @@ private sealed class BindingItem : IBindingItem, IDisposable
 			private ValueGetterHandler? _substituteValueGetter;
 			private ValueSetterHandler? _valueSetter;
 			private ValueSetterHandler? _localValueSetter;
+			private ValueSetterHandler? _animationFillingValueSetter;
 			private ValueUnsetterHandler? _valueUnsetter;
+			private ValueUnsetterHandler? _animationFillingValueUnsetter;
 
 			private Type? _dataContextType;
 
@@ -580,6 +598,12 @@ public void SetLocalValue(object value)
 				SetSourceValue(_localValueSetter!, value);
 			}
 
+			public void SetAnimationFillingValue(object value)
+			{
+				BuildAnimationFillingValueSetter();
+				SetSourceValue(_animationFillingValueSetter!, value);
+			}
+
 			public Type? PropertyType
 			{
 				get
@@ -696,14 +720,15 @@ private void BuildValueSetter()
 			{
 				if (_valueSetter == null && _dataContextType != null)
 				{
+					_valueSetter = BindingPropertyHelper.GetValueSetter(
+						_dataContextType,
+						PropertyName,
+						convert: true,
+						precedence: _precedence ?? DependencyPropertyValuePrecedences.Local
+					);
 					if (_precedence == null)
 					{
-						BuildLocalValueSetter();
-						_valueSetter = _localValueSetter;
-					}
-					else
-					{
-						_valueSetter = BindingPropertyHelper.GetValueSetter(_dataContextType, PropertyName, convert: true, precedence: _precedence.Value);
+						_localValueSetter = _valueSetter;
 					}
 				}
 			}
@@ -712,7 +737,25 @@ private void BuildLocalValueSetter()
 			{
 				if (_localValueSetter == null && _dataContextType != null)
 				{
-					_localValueSetter = BindingPropertyHelper.GetValueSetter(_dataContextType, PropertyName, convert: true);
+					_localValueSetter = BindingPropertyHelper.GetValueSetter(
+						_dataContextType,
+						PropertyName,
+						convert: true,
+						precedence: DependencyPropertyValuePrecedences.Local
+					);
+				}
+			}
+
+			private void BuildAnimationFillingValueSetter()
+			{
+				if (_animationFillingValueSetter == null && _dataContextType != null)
+				{
+					_animationFillingValueSetter = BindingPropertyHelper.GetValueSetter(
+						_dataContextType,
+						PropertyName,
+						convert: true,
+						precedence: DependencyPropertyValuePrecedences.FillingAnimations
+					);
 				}
 			}
 
@@ -838,6 +881,30 @@ internal void ClearValue()
 				}
 			}
 
+			private void BuildAnimationFillingValueUnsetter()
+			{
+				if (_animationFillingValueUnsetter == null && _dataContextType != null)
+				{
+					_animationFillingValueUnsetter = BindingPropertyHelper.GetValueUnsetter(
+						_dataContextType,
+						PropertyName,
+						precedence: DependencyPropertyValuePrecedences.FillingAnimations
+					);
+				}
+			}
+
+			internal void ClearAnimationFillingValue()
+			{
+				BuildAnimationFillingValueUnsetter();
+
+				// Capture the datacontext before the call to avoid a race condition with the GC.
+				var dataContext = DataContext;
+				if (dataContext != null)
+				{
+					_animationFillingValueUnsetter!(dataContext);
+				}
+			}
+
 			private void RaiseValueChanged(object? newValue)
 			{
 				ValueChangedListener?.OnValueChanged(newValue);

src/Uno.UI/DataBinding/BindingPropertyHelper.cs

@@ -49,7 +49,7 @@ internal static partial class BindingPropertyHelper
 		//          those. If this situation changes, we could remove the associated code and 
 		//          revert to memoized Funcs.
 		//
-		private static Dictionary<CachedTuple<Type, String, DependencyPropertyValuePrecedences?, bool>, ValueGetterHandler> _getValueGetter = new Dictionary<CachedTuple<Type, string, DependencyPropertyValuePrecedences?, bool>, ValueGetterHandler>(CachedTuple<Type, String, DependencyPropertyValuePrecedences?, bool>.Comparer);
+		private static Dictionary<CachedTuple<Type, string, DependencyPropertyValuePrecedences?, bool>, ValueGetterHandler> _getValueGetter = new Dictionary<CachedTuple<Type, string, DependencyPropertyValuePrecedences?, bool>, ValueGetterHandler>(CachedTuple<Type, String, DependencyPropertyValuePrecedences?, bool>.Comparer);
 		private static Dictionary<CachedTuple<Type, string, bool, DependencyPropertyValuePrecedences>, ValueSetterHandler> _getValueSetter = new Dictionary<CachedTuple<Type, string, bool, DependencyPropertyValuePrecedences>, ValueSetterHandler>(CachedTuple<Type, string, bool, DependencyPropertyValuePrecedences>.Comparer);
 		private static Dictionary<CachedTuple<Type, string, DependencyPropertyValuePrecedences>, ValueGetterHandler> _getPrecedenceSpecificValueGetter = new Dictionary<CachedTuple<Type, string, DependencyPropertyValuePrecedences>, ValueGetterHandler>(CachedTuple<Type, string, DependencyPropertyValuePrecedences>.Comparer);
 		private static Dictionary<CachedTuple<Type, string, DependencyPropertyValuePrecedences>, ValueGetterHandler> _getSubstituteValueGetter = new Dictionary<CachedTuple<Type, string, DependencyPropertyValuePrecedences>, ValueGetterHandler>(CachedTuple<Type, string, DependencyPropertyValuePrecedences>.Comparer);
@@ -1132,7 +1132,7 @@ private static ValueUnsetterHandler InternalGetValueUnsetter(Type type, string p
 		{
 			if (type == typeof(UnsetValue))
 			{
-				return _ => { };
+				return UnsetValueUnsetter;
 			}
 
 			property = SanitizePropertyName(type, property);
@@ -1250,6 +1250,8 @@ private static object UnsetValueGetter(object unused)
 
 		private static void UnsetValueSetter(object unused, object? unused2) { }
 
+		private static void UnsetValueUnsetter(object unused) { }
+
 		/// <summary>
 		/// Determines if the type can be provided by the MetadataProvider
 		/// </summary>

src/Uno.UI/UI/Xaml/DependencyPropertyDetails.cs

@@ -130,8 +130,33 @@ private void SetValueFull(object? value, DependencyPropertyValuePrecedences prec
 				return;
 			}
 
-			// If we were unsetting the current highest precedence value, we need to find the next highest
-			if (valueIsUnsetValue && precedence == _highestPrecedence)
+			// On Windows, explicitly calling SetValue(dp, DP.UnsetValue) or ClearValue(dp) doesnt clear the animation value.
+			// (note: Both the methods target local value)
+			// This means that we should not be handling those special case in here. Instead,
+			// when Timeline clears the animation value, it should also clears the filling animation value at the same time.
+			// ---
+			// Clear the animated value, when we are setting a local value to a property
+			// with an animated value from the filling part of an HoldEnd animation.
+			// note: There is no equivalent block in SetValueFast, as its condition would never be satisfied:
+			// _stack would've been materialized if the property had been animated.
+			bool forceUpdatePrecedence = false;
+			if (!valueIsUnsetValue &&
+				_highestPrecedence == DependencyPropertyValuePrecedences.FillingAnimations &&
+				(precedence is DependencyPropertyValuePrecedences.Local or DependencyPropertyValuePrecedences.Animations))
+			{
+				stackAlias[(int)DependencyPropertyValuePrecedences.FillingAnimations] = UnsetValue.Instance;
+				if (precedence is DependencyPropertyValuePrecedences.Local)
+				{
+					stackAlias[(int)DependencyPropertyValuePrecedences.Animations] = UnsetValue.Instance;
+				}
+
+				forceUpdatePrecedence = true;
+			}
+
+			// Update highest precedence, when the current highest value was unset or
+			// when animation value was overridden by local value.
+			if ((valueIsUnsetValue && precedence == _highestPrecedence) ||
+				forceUpdatePrecedence)
 			{
 				// Start from current precedence and find next highest
 				for (int i = (int)precedence; i < (int)DependencyPropertyValuePrecedences.DefaultValue; i++)

src/Uno.UI/UI/Xaml/DependencyPropertyValuePrecedences.cs

@@ -11,6 +11,12 @@ public enum DependencyPropertyValuePrecedences : int
 		/// </summary>
 		Coercion = 0,
 
+		/// <summary>
+		/// Defined when filling from a HoldEnd animation
+		/// </summary>
+		/// <remarks>Set the animation or local value will clear this value.</remarks>
+		FillingAnimations,
+
 		/// <summary>
 		/// Defined by animation storyboards
 		/// </summary>