Merge pull request #10695 from AvaloniaUI/fixes/10655-setcurrentvalue-with-style-2

Fix current values being overridden.

src/Avalonia.Base/PropertyStore/EffectiveValue.cs

@@ -54,9 +54,9 @@ namespace Avalonia.PropertyStore
         /// </remarks>
         public void BeginReevaluation(bool clearLocalValue = false)
         {
-            if (clearLocalValue || Priority != BindingPriority.LocalValue)
+            if (clearLocalValue || (Priority != BindingPriority.LocalValue && !IsOverridenCurrentValue))
                 Priority = BindingPriority.Unset;
-            if (clearLocalValue || BasePriority != BindingPriority.LocalValue)
+            if (clearLocalValue || (BasePriority != BindingPriority.LocalValue && !IsOverridenCurrentValue))
                 BasePriority = BindingPriority.Unset;
         }
 

src/Avalonia.Base/PropertyStore/ValueStore.cs

@@ -395,7 +395,7 @@ namespace Avalonia.PropertyStore
             if (TryGetEffectiveValue(property, out var existing))
             {
                 if (priority <= existing.BasePriority)
-                    ReevaluateEffectiveValue(property, existing);
+                    ReevaluateEffectiveValue(property, existing, changedValueEntry: entry);
             }
             else
             {
@@ -774,6 +774,7 @@ namespace Avalonia.PropertyStore
         private void ReevaluateEffectiveValue(
             AvaloniaProperty property,
             EffectiveValue? current,
+            IValueEntry? changedValueEntry = null,
             bool ignoreLocalValue = false)
         {
             ++_isEvaluating;
@@ -796,6 +797,12 @@ namespace Avalonia.PropertyStore
                 {
                     var frame = _frames[i];
                     var priority = frame.Priority;
+
+                    // Exit early if the current EffectiveValue has higher priority than this frame.
+                    if (current?.Priority < priority && current?.BasePriority < priority)
+                        break;
+
+                    // Try to get an entry from the frame for the property we're reevaluating.
                     var foundEntry = frame.TryGetEntryIfActive(property, out var entry, out var activeChanged);
                     
                     // If the active state of the frame has changed since the last read, and
@@ -803,20 +810,17 @@ namespace Avalonia.PropertyStore
                     // effective values of all properties.
                     if (activeChanged && frame.EntryCount > 1)
                     {
-                        ReevaluateEffectiveValues();
+                        ReevaluateEffectiveValues(changedValueEntry);
                         return;
                     }
 
-                    // We're interested in the value if:
-                    // - There is no current effective value, or
-                    // - The value's priority is higher than the current effective value's priority, or
-                    // - The value is a non-animation value and its priority is higher than the current
-                    //   effective value's base priority
-                    var isRelevantPriority = current is null ||
-                        (priority < current.Priority && priority < current.BasePriority) ||
-                        (priority > BindingPriority.Animation && priority < current.BasePriority);
-
-                    if (foundEntry && isRelevantPriority && entry!.HasValue)
+                    // If the frame has an entry for this property with a higher priority than the
+                    // current effective value (and that entry has a value), then we have a new 
+                    // value for the property. Note that the check for entry.HasValue must be 
+                    // evaluated last as it can cause bindings to be subscribed.
+                    if (foundEntry &&
+                        HasHigherPriority(entry!, priority, current, changedValueEntry) && 
+                        entry!.HasValue)
                     {
                         if (current is not null)
                         {
@@ -832,10 +836,6 @@ namespace Avalonia.PropertyStore
 
                     if (generation != _frameGeneration)
                         goto restart;
-
-                    if (current?.Priority < BindingPriority.Unset &&
-                        current?.BasePriority < BindingPriority.Unset)
-                        break;
                 }
 
                 if (current?.Priority == BindingPriority.Unset)
@@ -859,7 +859,7 @@ namespace Avalonia.PropertyStore
             }
         }
 
-        private void ReevaluateEffectiveValues()
+        private void ReevaluateEffectiveValues(IValueEntry? changedValueEntry = null)
         {
             ++_isEvaluating;
 
@@ -894,10 +894,9 @@ namespace Avalonia.PropertyStore
                     {
                         var entry = frame.GetEntry(j);
                         var property = entry.Property;
-
-                        // Skip if we already have a value/base value for this property.
-                        if (_effectiveValues.TryGetValue(property, out var effectiveValue) &&
-                            effectiveValue.BasePriority < BindingPriority.Unset)
+                        _effectiveValues.TryGetValue(property, out var effectiveValue);
+                        
+                        if (!HasHigherPriority(entry, priority, effectiveValue, changedValueEntry))
                             continue;
 
                         if (!entry.HasValue)
@@ -942,6 +941,37 @@ namespace Avalonia.PropertyStore
             }
         }
 
+        private static bool HasHigherPriority(
+            IValueEntry entry,
+            BindingPriority entryPriority,
+            EffectiveValue? current,
+            IValueEntry? changedValueEntry)
+        {
+            // Set the value if: there is no current effective value; or
+            if (current is null)
+                return true; 
+
+            // The value's priority is higher than the current effective value's priority; or
+            if (entryPriority < current.Priority && entryPriority < current.BasePriority)
+                return true;
+
+            // - The value's priority is equal to the current effective value's priority
+            // - But the effective value was set via SetCurrentValue
+            // - As long as the SetCurrentValue wasn't overriding the value from the value entry under consideration
+            // - Or if it was, the value entry under consideration has changed; or
+            if (entryPriority == current.Priority &&
+                current.IsOverridenCurrentValue &&
+                (current.ValueEntry != entry || entry == changedValueEntry))
+                return true;
+
+            // The value is a non-animation value and its priority is higher than the current effective value's base
+            // priority.
+            if (entryPriority > BindingPriority.Animation && entryPriority < current.BasePriority)
+                return true;
+
+            return false;
+        }
+
         private bool TryGetEffectiveValue(
             AvaloniaProperty property, 
             [NotNullWhen(true)] out EffectiveValue? value)

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_SetCurrentValue.cs

@@ -1,4 +1,6 @@
 using System;
+using System.Reactive.Concurrency;
+using System.Reactive.Subjects;
 using Avalonia.Controls;
 using Avalonia.Data;
 using Avalonia.Diagnostics;
@@ -351,6 +353,134 @@ namespace Avalonia.Base.UnitTests
             Assert.Equal("inheriteddefault", target.Inherited);
         }
 
+        [Fact]
+        public void SetCurrent_Value_Persists_When_Toggling_Style_3()
+        {
+            var target = new Class1();
+            var root = new TestRoot(target)
+            {
+                Styles =
+                {
+                    new Style(x => x.OfType<Class1>().Class("foo"))
+                    {
+                        Setters =
+                        {
+                            new Setter(Class1.BarProperty, "bar"),
+                            new Setter(Class1.InheritedProperty, "inherited"),
+                        },
+                    }
+                }
+            };
+
+            root.LayoutManager.ExecuteInitialLayoutPass();
+
+            target.SetValue(Class1.FooProperty, "not current", BindingPriority.Template);
+            target.SetCurrentValue(Class1.FooProperty, "current");
+
+            Assert.Equal("current", target.Foo);
+            Assert.Equal("bardefault", target.Bar);
+            Assert.Equal("inheriteddefault", target.Inherited);
+
+            target.Classes.Add("foo");
+
+            Assert.Equal("current", target.Foo);
+            Assert.Equal("bar", target.Bar);
+            Assert.Equal("inherited", target.Inherited);
+
+            target.Classes.Remove("foo");
+
+            Assert.Equal("current", target.Foo);
+            Assert.Equal("bardefault", target.Bar);
+            Assert.Equal("inheriteddefault", target.Inherited);
+        }
+
+        [Theory]
+        [InlineData(BindingPriority.LocalValue)]
+        [InlineData(BindingPriority.Style)]
+        [InlineData(BindingPriority.Animation)]
+        public void CurrentValue_Is_Replaced_By_Binding_Value(BindingPriority priority)
+        {
+            var target = new Class1();
+            var source = new BehaviorSubject<string>("initial");
+
+            target.Bind(Class1.FooProperty, source, priority);
+            target.SetCurrentValue(Class1.FooProperty, "current");
+            source.OnNext("new");
+            
+            Assert.Equal("new", target.Foo);
+        }
+
+        [Fact]
+        public void CurrentValue_Is_Replaced_By_New_Style_Activation_1()
+        {
+            var target = new Class1();
+            var root = new TestRoot(target)
+            {
+                Styles =
+                {
+                    new Style(x => x.OfType<Class1>().Class("foo"))
+                    {
+                        Setters =
+                        {
+                            new Setter(Class1.FooProperty, "initial"),
+                            new Setter(Class1.BarProperty, "bar"),
+                        },
+                    },
+                    new Style(x => x.OfType<Class1>().Class("bar"))
+                    {
+                        Setters =
+                        {
+                            new Setter(Class1.FooProperty, "new"),
+                            new Setter(Class1.BarProperty, "baz"),
+                        },
+                    },                }
+            };
+
+            root.LayoutManager.ExecuteInitialLayoutPass();
+
+            target.Classes.Add("foo");
+            Assert.Equal("initial", target.Foo);
+
+            target.SetCurrentValue(Class1.FooProperty, "current");
+            target.Classes.Add("bar");
+
+            Assert.Equal("new", target.Foo);
+        }
+
+        [Fact]
+        public void CurrentValue_Is_Replaced_By_New_Style_Activation_2()
+        {
+            var target = new Class1();
+            var root = new TestRoot(target)
+            {
+                Styles =
+                {
+                    new Style(x => x.OfType<Class1>().Class("foo"))
+                    {
+                        Setters =
+                        {
+                            new Setter(Class1.FooProperty, "foo"),
+                        },
+                    },
+                    new Style(x => x.OfType<Class1>().Class("foo"))
+                    {
+                        Setters =
+                        {
+                            new Setter(Class1.BarProperty, "bar"),
+                        },
+                    },
+                }
+            };
+
+            root.LayoutManager.ExecuteInitialLayoutPass();
+
+            target.SetValue(Class1.FooProperty, "template", BindingPriority.Template);
+            target.SetCurrentValue(Class1.FooProperty, "current");
+
+            target.Classes.Add("foo");
+            Assert.Equal("foo", target.Foo);
+        }
+
         private BindingPriority GetPriority(AvaloniaObject target, AvaloniaProperty property)
         {
             return target.GetDiagnostic(property).Priority;
@@ -383,5 +513,23 @@ namespace Avalonia.Base.UnitTests
                 return Math.Min(value, ((Class1)sender).CoerceMax);
             }
         }
+
+        private class ViewModel : NotifyingBase
+        {
+            private string _value;
+
+            public string Value
+            {
+                get => _value;
+                set
+                {
+                    if (_value != value)
+                    {
+                        _value = value;
+                        RaisePropertyChanged();
+                    }
+                }
+            }
+        }
     }
 }