diff --git a/src/Uno.UI/DataBinding/BindingPath.BindingItem.cs b/src/Uno.UI/DataBinding/BindingPath.BindingItem.cs index 2a2c00001f90..3b4b42876b86 100644 --- a/src/Uno.UI/DataBinding/BindingPath.BindingItem.cs +++ b/src/Uno.UI/DataBinding/BindingPath.BindingItem.cs @@ -12,10 +12,8 @@ namespace Uno.UI.DataBinding { internal partial class BindingPath { - private sealed class BindingItem : IBindingItem, IDisposable + private sealed class BindingItem : IBindingItem, IDisposable, IEquatable { - private delegate void PropertyChangedHandler(object? previousValue, object? newValue, bool shouldRaiseValueChanged); - private ManagedWeakReference? _dataContextWeakStorage; private Flags _flags; @@ -540,6 +538,35 @@ public void NewValue() public void NewValue(DependencyObject dependencyObject, DependencyPropertyChangedEventArgs args) => NewValue(); } + + /// + /// This is defined so that 2 BindingItems are equal if they refer to the same property on the same object + /// even if they're a part of 2 different "chains". + /// + public bool Equals(BindingItem? that) + { + return that != null + && this.PropertyType == that.PropertyType + && !DependencyObjectStore.AreDifferent(this.DataContext, that.DataContext) + && ComparePropertyName(this.PropertyName, that.PropertyName); + + // This is a naive comparison that most definitely doesn't match WinUI, but it should be good enough + // for almost all cases. + static bool ComparePropertyName(string name1, string name2) + { + if (name1.StartsWith('[')) + { + // for indexers, we look for an identical match. + return name1 == name2; + } + else + { + // e.g. "(Microsoft.UI.Xaml.Controls.Border.Background)" and "Background" should match. + return name1.Replace(")", "").Replace("(", "").Split(':', '.')[^1] == + name2.Replace(")", "").Replace("(", "").Split(':', '.')[^1]; + } + } + } } } } diff --git a/src/Uno.UI/DataBinding/BindingPath.cs b/src/Uno.UI/DataBinding/BindingPath.cs index 50c89788c63a..a6d27ea1a571 100644 --- a/src/Uno.UI/DataBinding/BindingPath.cs +++ b/src/Uno.UI/DataBinding/BindingPath.cs @@ -23,7 +23,7 @@ namespace Uno.UI.DataBinding { [DebuggerDisplay("Path={_path} DataContext={_dataContextWeakStorage?.Target}")] - internal partial class BindingPath : IDisposable, IEquatable + internal partial class BindingPath : IDisposable { private static List _propertyChangedHandlers = new List(2); private readonly string _path; @@ -317,11 +317,26 @@ internal void OnValueChanged(object? o) Dispose(false); } - public bool Equals(BindingPath? that) + public bool PrefixOfOrEqualTo(BindingPath? that) { - return that != null - && this.Path == that.Path - && !DependencyObjectStore.AreDifferent(this.DataContext, that.DataContext); + if (that == null || DependencyObjectStore.AreDifferent(this.DataContext, that.DataContext)) + { + return false; + } + + var currentLeft = this._chain; + var currentRight = that._chain; + while (currentLeft != null) + { + if (!currentLeft.Equals(currentRight)) + { + return false; + } + currentLeft = currentLeft.Next; + currentRight = currentRight.Next; + } + + return true; } // We define Equals to be based on Path, so `RuntimeHelpers.GetHashCode(path)` doesn't work, diff --git a/src/Uno.UI/UI/Xaml/VisualStateGroup.cs b/src/Uno.UI/UI/Xaml/VisualStateGroup.cs index 4a8b4f37a697..210590b93d28 100644 --- a/src/Uno.UI/UI/Xaml/VisualStateGroup.cs +++ b/src/Uno.UI/UI/Xaml/VisualStateGroup.cs @@ -9,6 +9,7 @@ using Microsoft.UI.Xaml.Markup; using Uno.UI; using Windows.Foundation.Collections; +using System.Buffers; using Uno.UI.DataBinding; using static Microsoft.UI.Xaml.Media.Animation.Timeline.TimelineState; @@ -23,14 +24,6 @@ namespace Microsoft.UI.Xaml [ContentProperty(Name = "States")] public sealed partial class VisualStateGroup : DependencyObject { - /// - /// Reusable HashSet to check properties set by the current state and not set by the next state - /// in . Since changing visual states should happen on the UI thread, there - /// are no concurrency problems. Consider this not a part of the state. Use it instead of - /// new HashSet() and clear before each use. - /// - private static HashSet _propertiesNotToClear; - /// /// The xaml scope in force at the time the VisualStateGroup was created. /// @@ -387,13 +380,15 @@ void ApplyTargetStateSetters() // WinUI rolls back properties that are set by the current state that won't be set by the // setters of the next state and won't be set by the nextAnimation. // It doesn't matter how it's set by the current state (setters or transition or animation) or - // how it will possibly be set by the new state. Simply put, if the ne doesn't touch + // how it will possibly be set by the new state. Simply put, if the next state (or transition) doesn't touch // the property in any way, roll it back. Similarly, if the animation of the new state // doesn't set a property but it's set in the transition, it should be cleared after the // transition. // In other words, one can think of it as actually manipulating 3 states // "Active" State (Setter & runningAnimation) --> Transition (if exists) --> New State (Setter & target.animation) // Moving from one to the next rolls back the properties of the one before it. + // Note that "touching the property" also includes modifying a subproperty. Meaning, if the previous state sets + // the property Prop and the "next state" sets "Prop.SubProp", then we don't roll back Prop. /*****************************************************************/ private static void ClearPropertiesNotSetInNextState( IFrameworkElement element, @@ -407,13 +402,14 @@ private static void ClearPropertiesNotSetInNextState( return; } - (_propertiesNotToClear ??= new HashSet(new PathComparer())).Clear(); + var propertiesNotToClear = ArrayPool.Shared.Rent((nextSetters?.Count ?? 0) + (nextAnimation?.Children.Items.Count ?? 0)); + var propsLength = 0; if (nextSetters is { }) { foreach (var setter in nextSetters.OfType()) { - _propertiesNotToClear.Add(setter.TryGetOrCreateBindingPath(element)); + propertiesNotToClear[propsLength++] = setter.TryGetOrCreateBindingPath(element); } } @@ -421,7 +417,7 @@ private static void ClearPropertiesNotSetInNextState( { foreach (var item in nextAnimation.Children.Items) { - _propertiesNotToClear.Add(item.PropertyInfo); + propertiesNotToClear[propsLength++] = item.PropertyInfo; } } @@ -430,7 +426,7 @@ private static void ClearPropertiesNotSetInNextState( foreach (var setter in prevSetters.OfType()) { // Setters with a null path are always cleared. - if (setter.TryGetOrCreateBindingPath(element) is not { } path || !_propertiesNotToClear.Contains(path)) + if (setter.TryGetOrCreateBindingPath(element) is not { } prevPath || !propertiesNotToClear.Any(path => prevPath.PrefixOfOrEqualTo(path))) { setter.ClearValue(); } @@ -442,7 +438,7 @@ private static void ClearPropertiesNotSetInNextState( foreach (var item in prevAnimation.Children.Items) { // Animations with a null path are always cleared. - if (item.PropertyInfo is not { } path || !_propertiesNotToClear.Contains(path)) + if (item.PropertyInfo is not { } prevPath || !propertiesNotToClear.Any(path => prevPath.PrefixOfOrEqualTo(path))) { item.PropertyInfo.ClearValue(); } @@ -595,13 +591,5 @@ private VisualState GetActiveTrigger() public override string ToString() => Name ?? $""; - - private class PathComparer : EqualityComparer - { - public override bool Equals(BindingPath path1, BindingPath path2) - => (path1 != null && path1.Equals(path2)) || (path1 == null && path2 == null); - - public override int GetHashCode(BindingPath path) => path.GetHashCode(); - } } }