From 54a6d5d8e99ab5e42a989d27e885410cddd450d3 Mon Sep 17 00:00:00 2001 From: xiaoy312 Date: Thu, 30 Mar 2023 12:07:18 -0400 Subject: [PATCH] fix(animation): not defaulting starting value from animated value --- .../Extensions/StoryboardExtensions.cs | 63 ++++++++++++++++++ .../Given_DoubleAnimation.cs | 64 ++++++++++++++++++- src/Uno.UI/DataBinding/BindingPath.cs | 10 +++ .../Extensions/TimelineExtensions.iOSmacOS.cs | 18 ++---- src/Uno.UI/FeatureConfiguration.cs | 9 +++ .../Animators/AnimatorFactory.Android.cs | 57 +++++++++++++++-- .../Media/Animation/Timeline.animation.cs | 30 ++++++++- 7 files changed, 229 insertions(+), 22 deletions(-) create mode 100644 src/Uno.UI.RuntimeTests/Extensions/StoryboardExtensions.cs diff --git a/src/Uno.UI.RuntimeTests/Extensions/StoryboardExtensions.cs b/src/Uno.UI.RuntimeTests/Extensions/StoryboardExtensions.cs new file mode 100644 index 000000000000..bd35becf11cc --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Extensions/StoryboardExtensions.cs @@ -0,0 +1,63 @@ +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Windows.UI.Xaml; +using Windows.UI.Xaml.Media.Animation; + +namespace Uno.UI.RuntimeTests.Extensions; + +internal static class StoryboardExtensions +{ + public static void Begin(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); + } + + public static async Task RunAsync(this Storyboard storyboard, TimeSpan? timeout = null, bool throwsException = false) + { + var tcs = new TaskCompletionSource(); + void OnCompleted(object sender, object e) + { + tcs.SetResult(true); + storyboard.Completed -= OnCompleted; + } + + storyboard.Completed += OnCompleted; + storyboard.Begin(); + + if (timeout is { }) + { + if (await Task.WhenAny(tcs.Task, Task.Delay(timeout.Value)) != tcs.Task) + { + if (throwsException) + { + throw new TimeoutException($"Timeout waiting for the storyboard to complete: {timeout}ms"); + } + } + } + else + { + await tcs.Task; + } + } + + public static TTimeline BindTo(this TTimeline timeline, DependencyObject target, string property) + where TTimeline : Timeline + { + Storyboard.SetTarget(timeline, target); + Storyboard.SetTargetProperty(timeline, property); + + return timeline; + } +} diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media_Animation/Given_DoubleAnimation.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media_Animation/Given_DoubleAnimation.cs index e4fa39a2dd41..1c97feaf6d7f 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media_Animation/Given_DoubleAnimation.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Media_Animation/Given_DoubleAnimation.cs @@ -4,7 +4,10 @@ using System.Text; using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Uno.UI.RuntimeTests.Extensions; using Windows.UI; +using Windows.UI.Composition; +using Windows.UI.Xaml; using Windows.UI.Xaml.Controls; using Windows.UI.Xaml.Media; using Windows.UI.Xaml.Media.Animation; @@ -116,7 +119,7 @@ async Task WaitAndSnapshotValue(int millisecondsDelay) [TestMethod] [RunsOnUIThread] - public async void When_RepeatForever_ShouldLoop_AsdAsd() + public async Task When_RepeatForever_ShouldLoop() { var target = new Windows.UI.Xaml.Shapes.Rectangle { @@ -125,8 +128,8 @@ public async void When_RepeatForever_ShouldLoop_AsdAsd() Height = 50, }; WindowHelper.WindowContent = target; - await WindowHelper.WaitForIdle(); await WindowHelper.WaitForLoaded(target); + await WindowHelper.WaitForIdle(); var animation = new DoubleAnimation { @@ -162,5 +165,62 @@ public async void When_RepeatForever_ShouldLoop_AsdAsd() Assert.AreEqual(10d, averageIncrement, 1.5, "an rough average of increment (exluding the drop) of 10 (+-15% error margin)"); Assert.IsTrue(incrementSizes.Count(x => x > 3) > 8, $"at least 10 (-2 error margin: might miss first and/or last) sets of continuous increments that size of 4 (+-1 error margin: sliding slot): {string.Join(",", incrementSizes)}"); } + + [TestMethod] + [RunsOnUIThread] + public async Task When_StartingFrom_AnimatedValue() + { + 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(); + + // Start an animation. Its final value will serve as + // the inferred starting value for the next animation. + var animation0 = new DoubleAnimation + { + // From = should be 0 + To = 50, + Duration = new Duration(TimeSpan.FromSeconds(2)), + }.BindTo(translate, nameof(translate.Y)); + await animation0.RunAsync(timeout: animation0.Duration.TimeSpan + TimeSpan.FromSeconds(1)); + await Task.Delay(1000); + + // Start an second animation which should pick up from current animated value. + var animation1 = new DoubleAnimation + { + // From = should be 50 from animation #0 + To = -50, + Duration = new Duration(TimeSpan.FromSeconds(5)), + }.BindTo(translate, nameof(translate.Y)); + animation1.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) => +#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 + // And, when the animation is completed, this native value is reset even for HoldEnd animation. + : translate.Y; +#endif } } diff --git a/src/Uno.UI/DataBinding/BindingPath.cs b/src/Uno.UI/DataBinding/BindingPath.cs index 1d308dbc630d..27a5ca83e675 100644 --- a/src/Uno.UI/DataBinding/BindingPath.cs +++ b/src/Uno.UI/DataBinding/BindingPath.cs @@ -121,6 +121,16 @@ public IEnumerable GetPathItems() return _chain?.Flatten(i => i.Next!) ?? Array.Empty(); } + public (object DataContext, string PropertyName) GetTargetContextAndPropertyName() + { + var info = GetPathItems().Last(); + var propertyName = info.PropertyName + .Split(new[] { '.' }).Last() + .Replace("(", "").Replace(")", ""); + + return (info.DataContext, propertyName); + } + /// /// Checks the property path for members which may be shared resources (es and s) and creates a /// copy of them if need be (ie if not already copied). Intended to be used prior to animating the targeted property. diff --git a/src/Uno.UI/Extensions/TimelineExtensions.iOSmacOS.cs b/src/Uno.UI/Extensions/TimelineExtensions.iOSmacOS.cs index 6310e5ac8b63..36396140f97c 100644 --- a/src/Uno.UI/Extensions/TimelineExtensions.iOSmacOS.cs +++ b/src/Uno.UI/Extensions/TimelineExtensions.iOSmacOS.cs @@ -16,20 +16,12 @@ internal static void SetValueBypassPropagation(this Timeline timeline, object va timeline.Log().DebugFormat("Setting [{0}] to [{1} / {2}] and bypassing native propagation", value, Storyboard.GetTargetName(timeline), Storyboard.GetTargetProperty(timeline)); } - var animatedItem = timeline.PropertyInfo.GetPathItems().Last(); - - // Get the property name from the last part of the - // specified name (for dotted paths, if they exist) - var propertyName = animatedItem.PropertyName.Split(new[] { '.' }).Last().Replace("(", "").Replace(")", ""); - - var dc = animatedItem.DataContext; - using (dc != null ? - DependencyObjectStore.BypassPropagation( + var (dc, propertyName) = timeline.PropertyInfo.GetTargetContextAndPropertyName(); + using (dc != null + ? DependencyObjectStore.BypassPropagation( (DependencyObject)dc, - DependencyProperty.GetProperty(dc.GetType(), propertyName) - ) : - // DC may have been collected since it's weakly held - null + DependencyProperty.GetProperty(dc.GetType(), propertyName)) + : null // DC may have been collected since it's weakly held ) { timeline.PropertyInfo.Value = value; diff --git a/src/Uno.UI/FeatureConfiguration.cs b/src/Uno.UI/FeatureConfiguration.cs index ea580bd2a318..412613b86492 100644 --- a/src/Uno.UI/FeatureConfiguration.cs +++ b/src/Uno.UI/FeatureConfiguration.cs @@ -771,5 +771,14 @@ public static class Cursors public static bool UseHandForInteraction { get; set; } = true; #endif } + + public static class Timeline + { + /// + /// Determines if the default animation starting value + /// will be from the animated value or local value, when the From property is omitted. + /// + public static bool DefaultsStartingValueFromAnimatedValue { get; } = true; + } } } diff --git a/src/Uno.UI/UI/Xaml/Media/Animation/Animators/AnimatorFactory.Android.cs b/src/Uno.UI/UI/Xaml/Media/Animation/Animators/AnimatorFactory.Android.cs index 4c851942ceb1..f59ec69a5b80 100644 --- a/src/Uno.UI/UI/Xaml/Media/Animation/Animators/AnimatorFactory.Android.cs +++ b/src/Uno.UI/UI/Xaml/Media/Animation/Animators/AnimatorFactory.Android.cs @@ -47,10 +47,9 @@ private static IValueAnimator GetGPUAnimator(this Timeline timeline, double star // Performance : http://developer.android.com/guide/topics/graphics/hardware-accel.html#layers-anims // Properties : http://developer.android.com/guide/topics/graphics/prop-animation.html#views - var info = timeline.PropertyInfo.GetPathItems().Last(); - var target = info.DataContext; - var property = info.PropertyName.Split(new[] { '.' }).Last().Replace("(", "").Replace(")", ""); + var (target, property) = timeline.PropertyInfo.GetTargetContextAndPropertyName(); + // note: implementation below should be mirrored in TryGetNativeAnimatedValue if (target is View view) { switch (property) @@ -332,10 +331,60 @@ private static ValueAnimator GetRelativeAnimator(Java.Lang.Object target, string } /// - /// Ensures that scale value is without the android accepted values + /// Ensures that scale value is within the android accepted values /// private static double ToNativeScale(double value) => double.IsNaN(value) ? 1 : value; + /// + /// Get the underlying native property value which the target dependency property is associated with when animating. + /// + /// + /// The associated native animated value in logical unit. + /// Whether a native animation value is present and active. + /// The will be in logical unit, as the consumer of this method will have no idea which property are scaled or not. + internal static bool TryGetNativeAnimatedValue(Timeline timeline, out object value) + { + value = null; + + if (timeline.GetIsDependantAnimation() || timeline.GetIsDurationZero()) + { + return false; + } + + var (target, property) = timeline.PropertyInfo.GetTargetContextAndPropertyName(); + if (target is Transform { IsAnimating: false }) + { + // While not animating, these native properties will be reset. + // In that case, the dp actual value should be read instead (by returning false here). + return false; + } + + value = property switch + { + // note: Implementation here should be mirrored in GetGPUAnimator + nameof(FrameworkElement.Opacity) when target is View view => (double)view.Alpha, + + nameof(TranslateTransform.X) when target is TranslateTransform translate => ViewHelper.PhysicalToLogicalPixels(translate.View.TranslationX), + nameof(TranslateTransform.Y) when target is TranslateTransform translate => ViewHelper.PhysicalToLogicalPixels(translate.View.TranslationY), + nameof(RotateTransform.Angle) when target is RotateTransform rotate => (double)rotate.View.Rotation, + nameof(ScaleTransform.ScaleX) when target is ScaleTransform scale => (double)scale.View.ScaleX, + nameof(ScaleTransform.ScaleY) when target is ScaleTransform scale => (double)scale.View.ScaleY, + //nameof(SkewTransform.AngleX) when target is SkewTransform skew => ViewHelper.PhysicalToLogicalPixels(skew.View.ScaleX), // copied as is from GetGPUAnimator + //nameof(SkewTransform.AngleY) when target is SkewTransform skew => ViewHelper.PhysicalToLogicalPixels(skew.View.ScaleY), + + nameof(CompositeTransform.TranslateX) when target is CompositeTransform composite => ViewHelper.PhysicalToLogicalPixels(composite.View.TranslationX), + nameof(CompositeTransform.TranslateY) when target is CompositeTransform composite => ViewHelper.PhysicalToLogicalPixels(composite.View.TranslationY), + nameof(CompositeTransform.Rotation) when target is CompositeTransform composite => (double)composite.View.Rotation, + nameof(CompositeTransform.ScaleX) when target is CompositeTransform composite => (double)composite.View.ScaleX, + nameof(CompositeTransform.ScaleY) when target is CompositeTransform composite => (double)composite.View.ScaleY, + //nameof(CompositeTransform.SkewX) when target is CompositeTransform composite => ViewHelper.PhysicalToLogicalPixels(composite.View.ScaleX), // copied as is from GetGPUAnimator + //nameof(CompositeTransform.SkewY) when target is CompositeTransform composite => ViewHelper.PhysicalToLogicalPixels(composite.View.ScaleY), + + _ => null, + }; + + return value != null; + } } } diff --git a/src/Uno.UI/UI/Xaml/Media/Animation/Timeline.animation.cs b/src/Uno.UI/UI/Xaml/Media/Animation/Timeline.animation.cs index bb1f417bc8d6..8d5176097afb 100644 --- a/src/Uno.UI/UI/Xaml/Media/Animation/Timeline.animation.cs +++ b/src/Uno.UI/UI/Xaml/Media/Animation/Timeline.animation.cs @@ -9,6 +9,7 @@ using Uno.Foundation.Logging; using Uno.UI.DataBinding; using System.Diagnostics; +using Uno.UI; namespace Windows.UI.Xaml.Media.Animation { @@ -69,11 +70,14 @@ private TimelineState State private BindingPath PropertyInfo => _owner?.PropertyInfo; private string[] GetTraceProperties() => _owner?.GetTraceProperties(); + private void ClearValue() => _owner?.ClearValue(); private void SetValue(object value) => _owner?.SetValue(value); - private bool NeedsRepeat(Stopwatch activeDuration, int replayCount) => _owner?.NeedsRepeat(activeDuration, replayCount) ?? false; + private object GetValue() => _owner?.GetValue(); private object GetNonAnimatedValue() => _owner?.GetNonAnimatedValue(); + private bool NeedsRepeat(Stopwatch activeDuration, int replayCount) => _owner?.NeedsRepeat(activeDuration, replayCount) ?? false; + public void Begin() { if (_trace.IsEnabled) @@ -431,8 +435,9 @@ private T ComputeFromValue() } else { - var value = GetNonAnimatedValue(); - + var value = FeatureConfiguration.Timeline.DefaultsStartingValueFromAnimatedValue + ? GetValueCore() + : GetNonAnimatedValue(); if (value != null) { return AnimationOwner.Convert(value); @@ -442,6 +447,25 @@ private T ComputeFromValue() return null; } + private object GetValueCore() + { +#if !__ANDROID__ + return GetValue(); +#else + // On android, animation may target a native property implementing the behavior instead of the specified dependency property. + // When starting a new animation midst another, in order to continue from the current animated value, + // we need to retrieve the value of that native property, as reading the dp value will just give the final value. + if (AnimatorFactory.TryGetNativeAnimatedValue(_owner, out var value)) + { + return value; + } + else + { + return GetValue(); + } +#endif + } + /// /// Calculates the To value of the animation /// For simplicity, animations are based on to and from values