From e7aaf8a7f7120c971da687613aaf136b5469ae1e Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 28 Mar 2023 14:45:19 +0200 Subject: [PATCH 1/2] Ensure that the default value can be coerced. When the coercion function on a property results in the default value being coerced, ensure that state is recorded in the value store. --- src/Avalonia.Base/AvaloniaProperty.cs | 6 + src/Avalonia.Base/DirectPropertyBase.cs | 5 + .../PropertyStore/EffectiveValue.cs | 49 ++++++- .../PropertyStore/EffectiveValue`1.cs | 42 ++++-- src/Avalonia.Base/PropertyStore/ValueStore.cs | 52 ++++++-- src/Avalonia.Base/StyledProperty.cs | 5 + .../AvaloniaObjectTests_Coercion.cs | 125 +++++++++++++++++- .../AvaloniaPropertyTests.cs | 5 + 8 files changed, 260 insertions(+), 29 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaProperty.cs b/src/Avalonia.Base/AvaloniaProperty.cs index e98d9f0517..c57131f7b5 100644 --- a/src/Avalonia.Base/AvaloniaProperty.cs +++ b/src/Avalonia.Base/AvaloniaProperty.cs @@ -499,6 +499,12 @@ namespace Avalonia /// The object instance. internal abstract void RouteClearValue(AvaloniaObject o); + /// + /// Routes an untyped CoerceValue call on a property with its default value to a typed call. + /// + /// The object instance. + internal abstract void RouteCoerceDefaultValue(AvaloniaObject o); + /// /// Routes an untyped GetValue call to a typed call. /// diff --git a/src/Avalonia.Base/DirectPropertyBase.cs b/src/Avalonia.Base/DirectPropertyBase.cs index 94dfaaab01..7e5a962157 100644 --- a/src/Avalonia.Base/DirectPropertyBase.cs +++ b/src/Avalonia.Base/DirectPropertyBase.cs @@ -117,6 +117,11 @@ namespace Avalonia o.ClearValue(this); } + internal override void RouteCoerceDefaultValue(AvaloniaObject o) + { + // Do nothing. + } + /// internal override object? RouteGetValue(AvaloniaObject o) { diff --git a/src/Avalonia.Base/PropertyStore/EffectiveValue.cs b/src/Avalonia.Base/PropertyStore/EffectiveValue.cs index 7e9f9ae9ba..a00a51e694 100644 --- a/src/Avalonia.Base/PropertyStore/EffectiveValue.cs +++ b/src/Avalonia.Base/PropertyStore/EffectiveValue.cs @@ -36,12 +36,23 @@ namespace Avalonia.PropertyStore /// public IValueEntry? BaseValueEntry { get; private set; } + /// + /// Gets a value indicating whether the property has a coercion function. + /// + public bool HasCoercion { get; protected set; } + /// /// Gets a value indicating whether the was overridden by a call to /// . /// public bool IsOverridenCurrentValue { get; set; } + /// + /// Gets a value indicating whether the is the result of the + /// + /// + public bool IsCoercedDefaultValue { get; set; } + /// /// Begins a reevaluation pass on the effective value. /// @@ -63,10 +74,33 @@ namespace Avalonia.PropertyStore /// /// Ends a reevaluation pass on the effective value. /// + /// The associated value store. + /// The property being reevaluated. /// - /// This method unsubscribes from any unused value entries. + /// Handles coercing the default value if necessary. /// - public void EndReevaluation() + public void EndReevaluation(ValueStore owner, AvaloniaProperty property) + { + if (Priority == BindingPriority.Unset && HasCoercion) + CoerceDefaultValueAndRaise(owner, property); + } + + /// + /// Gets a value indicating whether the effective value represents the default value of the + /// property and can be removed. + /// + /// True if the effective value van be removed; otherwise false. + public bool CanRemove() + { + return Priority == BindingPriority.Unset && + !IsOverridenCurrentValue && + !IsCoercedDefaultValue; + } + + /// + /// Unsubscribes from any unused value entries. + /// + public void UnsubscribeIfNecessary() { if (Priority == BindingPriority.Unset) { @@ -130,6 +164,17 @@ namespace Avalonia.PropertyStore /// The property being cleared. public abstract void DisposeAndRaiseUnset(ValueStore owner, AvaloniaProperty property); + /// + /// Coerces the default value, raising + /// where necessary. + /// + /// The associated value store. + /// The property being coerced. + protected abstract void CoerceDefaultValueAndRaise(ValueStore owner, AvaloniaProperty property); + + /// + /// Gets the current effective value as a boxed value. + /// protected abstract object? GetBoxedValue(); protected void UpdateValueEntry(IValueEntry? entry, BindingPriority priority) diff --git a/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs b/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs index 330034f51d..369cd3ea63 100644 --- a/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs +++ b/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using Avalonia.Data; -using static Avalonia.Rendering.Composition.Animations.PropertySetSnapshot; namespace Avalonia.PropertyStore { @@ -33,19 +32,16 @@ namespace Avalonia.PropertyStore if (_metadata.CoerceValue is { } coerce) { + HasCoercion = true; _uncommon = new() { _coerce = coerce, _uncoercedValue = value, _uncoercedBaseValue = value, }; - - Value = coerce(owner, value); - } - else - { - Value = value; } + + Value = value; } /// @@ -61,7 +57,7 @@ namespace Avalonia.PropertyStore Debug.Assert(priority != BindingPriority.LocalValue); UpdateValueEntry(value, priority); - SetAndRaiseCore(owner, (StyledProperty)value.Property, GetValue(value), priority, false); + SetAndRaiseCore(owner, (StyledProperty)value.Property, GetValue(value), priority); if (priority > BindingPriority.LocalValue && value.GetDataValidationState(out var state, out var error)) @@ -75,7 +71,7 @@ namespace Avalonia.PropertyStore StyledProperty property, T value) { - SetAndRaiseCore(owner, property, value, BindingPriority.LocalValue, false); + SetAndRaiseCore(owner, property, value, BindingPriority.LocalValue); } public void SetCurrentValueAndRaise( @@ -83,8 +79,15 @@ namespace Avalonia.PropertyStore StyledProperty property, T value) { - IsOverridenCurrentValue = true; - SetAndRaiseCore(owner, property, value, Priority, true); + SetAndRaiseCore(owner, property, value, Priority, isOverriddenCurrentValue: true); + } + + public void SetCoercedDefaultValueAndRaise( + ValueStore owner, + StyledProperty property, + T value) + { + SetAndRaiseCore(owner, property, value, Priority, isCoercedDefaultValue: true); } public bool TryGetBaseValue([MaybeNullWhen(false)] out T value) @@ -117,7 +120,7 @@ namespace Avalonia.PropertyStore Debug.Assert(Priority != BindingPriority.Animation); Debug.Assert(BasePriority != BindingPriority.Unset); UpdateValueEntry(null, BindingPriority.Animation); - SetAndRaiseCore(owner, (StyledProperty)property, _baseValue!, BasePriority, false); + SetAndRaiseCore(owner, (StyledProperty)property, _baseValue!, BasePriority); } public override void CoerceValue(ValueStore owner, AvaloniaProperty property) @@ -168,6 +171,17 @@ namespace Avalonia.PropertyStore } } + protected override void CoerceDefaultValueAndRaise(ValueStore owner, AvaloniaProperty property) + { + Debug.Assert(_uncommon?._coerce is not null); + Debug.Assert(Priority == BindingPriority.Unset); + + var coercedDefaultValue = _uncommon!._coerce!(owner.Owner, _metadata.DefaultValue); + + if (!EqualityComparer.Default.Equals(_metadata.DefaultValue, coercedDefaultValue)) + SetCoercedDefaultValueAndRaise(owner, (StyledProperty)property, coercedDefaultValue); + } + protected override object? GetBoxedValue() => Value; private static T GetValue(IValueEntry entry) @@ -183,7 +197,8 @@ namespace Avalonia.PropertyStore StyledProperty property, T value, BindingPriority priority, - bool isOverriddenCurrentValue) + bool isOverriddenCurrentValue = false, + bool isCoercedDefaultValue = false) { var oldValue = Value; var valueChanged = false; @@ -191,6 +206,7 @@ namespace Avalonia.PropertyStore var v = value; IsOverridenCurrentValue = isOverriddenCurrentValue; + IsCoercedDefaultValue = isCoercedDefaultValue; if (_uncommon?._coerce is { } coerce) v = coerce(owner.Owner, value); diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index af31459a98..0ef14272b4 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -259,6 +259,27 @@ namespace Avalonia.PropertyStore { if (_effectiveValues.TryGetValue(property, out var v)) v.CoerceValue(this, property); + else + property.RouteCoerceDefaultValue(Owner); + } + + public void CoerceDefaultValue(StyledProperty property) + { + var metadata = property.GetMetadata(Owner.GetType()); + + if (metadata.CoerceValue is null) + return; + + var coercedDefaultValue = metadata.CoerceValue(Owner, metadata.DefaultValue); + + if (EqualityComparer.Default.Equals(metadata.DefaultValue, coercedDefaultValue)) + return; + + // We have a situation where the default value isn't valid according to the coerce + // function. In this case, we need to create an EffectiveValue entry. + var effectiveValue = CreateEffectiveValue(property); + AddEffectiveValue(property, effectiveValue); + effectiveValue.SetCoercedDefaultValueAndRaise(this, property, coercedDefaultValue); } public Optional GetBaseValue(StyledProperty property) @@ -838,20 +859,25 @@ namespace Avalonia.PropertyStore goto restart; } - if (current?.Priority == BindingPriority.Unset) + if (current is not null) { - if (current.BasePriority == BindingPriority.Unset) - { - RemoveEffectiveValue(property); - current.DisposeAndRaiseUnset(this, property); - } - else + current.EndReevaluation(this, property); + + if (current.CanRemove()) { - current.RemoveAnimationAndRaise(this, property); + if (current.BasePriority == BindingPriority.Unset) + { + RemoveEffectiveValue(property); + current.DisposeAndRaiseUnset(this, property); + } + else + { + current.RemoveAnimationAndRaise(this, property); + } } - } - current?.EndReevaluation(); + current.UnsubscribeIfNecessary(); + } } finally { @@ -923,7 +949,9 @@ namespace Avalonia.PropertyStore { _effectiveValues.GetKeyValue(i, out var key, out var e); - if (e.Priority == BindingPriority.Unset && !e.IsOverridenCurrentValue) + e.EndReevaluation(this, key); + + if (e.CanRemove()) { RemoveEffectiveValue(key, i); e.DisposeAndRaiseUnset(this, key); @@ -932,7 +960,7 @@ namespace Avalonia.PropertyStore break; } - e.EndReevaluation(); + e.UnsubscribeIfNecessary(); } } finally diff --git a/src/Avalonia.Base/StyledProperty.cs b/src/Avalonia.Base/StyledProperty.cs index e1b88cde49..5cb330eda9 100644 --- a/src/Avalonia.Base/StyledProperty.cs +++ b/src/Avalonia.Base/StyledProperty.cs @@ -176,6 +176,11 @@ namespace Avalonia o.ClearValue(this); } + internal override void RouteCoerceDefaultValue(AvaloniaObject o) + { + o.GetValueStore().CoerceDefaultValue(this); + } + /// internal override object? RouteGetValue(AvaloniaObject o) { diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Coercion.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Coercion.cs index 2154bb63d0..d53c055639 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Coercion.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Coercion.cs @@ -1,7 +1,10 @@ using System; using System.Collections.Generic; using System.Reactive.Subjects; +using Avalonia.Controls; using Avalonia.Data; +using Avalonia.Styling; +using Avalonia.UnitTests; using Xunit; namespace Avalonia.Base.UnitTests @@ -182,11 +185,103 @@ namespace Avalonia.Base.UnitTests Assert.Equal(-150, target.Foo); } + [Fact] + public void Default_Value_Can_Be_Coerced() + { + var target = new Class1(); + var raised = 0; + + target.MinFoo = 20; + + target.PropertyChanged += (s, e) => + { + Assert.Equal(Class1.FooProperty, e.Property); + Assert.Equal(11, e.OldValue); + Assert.Equal(20, e.NewValue); + Assert.Equal(BindingPriority.Unset, e.Priority); + ++raised; + }; + + target.CoerceValue(Class1.FooProperty); + + Assert.Equal(20, target.Foo); + Assert.Equal(1, raised); + } + + [Fact] + public void ClearValue_Respects_Coerced_Default_Value() + { + var target = new Class1(); + var raised = 0; + + target.Foo = 30; + target.MinFoo = 20; + + target.PropertyChanged += (s, e) => + { + Assert.Equal(Class1.FooProperty, e.Property); + Assert.Equal(30, e.OldValue); + Assert.Equal(20, e.NewValue); + Assert.Equal(BindingPriority.Unset, e.Priority); + ++raised; + }; + + target.ClearValue(Class1.FooProperty); + + Assert.Equal(20, target.Foo); + Assert.Equal(1, raised); + } + + [Fact] + public void Deactivating_Style_Respects_Coerced_Default_Value() + { + var target = new Control1 + { + MinFoo = 20, + }; + + var root = new TestRoot + { + Styles = + { + new Style(x => x.OfType().Class("foo")) + { + Setters = + { + new Setter(Control1.FooProperty, 50), + }, + }, + }, + Child = target, + }; + + var raised = 0; + + target.Classes.Add("foo"); + root.LayoutManager.ExecuteInitialLayoutPass(); + + Assert.Equal(50, target.Foo); + + target.PropertyChanged += (s, e) => + { + Assert.Equal(Control1.FooProperty, e.Property); + Assert.Equal(50, e.OldValue); + Assert.Equal(20, e.NewValue); + Assert.Equal(BindingPriority.Unset, e.Priority); + ++raised; + }; + + target.Classes.Remove("foo"); + + Assert.Equal(20, target.Foo); + Assert.Equal(1, raised); + } + private class Class1 : AvaloniaObject { public static readonly StyledProperty FooProperty = AvaloniaProperty.Register( - "Qux", + "Foo", defaultValue: 11, coerce: CoerceFoo); @@ -215,13 +310,15 @@ namespace Avalonia.Base.UnitTests set => SetValue(InheritedProperty, value); } + public int MinFoo { get; set; } = 0; public int MaxFoo { get; set; } = 100; public List CoreChanges { get; } = new(); public static int CoerceFoo(AvaloniaObject instance, int value) { - return Math.Min(((Class1)instance).MaxFoo, value); + var o = (Class1)instance; + return Math.Clamp(value, o.MinFoo, o.MaxFoo); } protected override void OnPropertyChangedCore(AvaloniaPropertyChangedEventArgs change) @@ -266,5 +363,29 @@ namespace Avalonia.Base.UnitTests return -value; } } + + private class Control1 : Control + { + public static readonly StyledProperty FooProperty = + AvaloniaProperty.Register( + "Foo", + defaultValue: 11, + coerce: CoerceFoo); + + public int Foo + { + get => GetValue(FooProperty); + set => SetValue(FooProperty, value); + } + + public int MinFoo { get; set; } = 0; + public int MaxFoo { get; set; } = 100; + + public static int CoerceFoo(AvaloniaObject instance, int value) + { + var o = (Control1)instance; + return Math.Clamp(value, o.MinFoo, o.MaxFoo); + } + } } } diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs index bde750efdc..e44c15d962 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs @@ -179,6 +179,11 @@ namespace Avalonia.Base.UnitTests throw new NotImplementedException(); } + internal override void RouteCoerceDefaultValue(AvaloniaObject o) + { + throw new NotImplementedException(); + } + internal override object RouteGetValue(AvaloniaObject o) { throw new NotImplementedException(); From 964f30e883b31d8f8825e64f68d49d3fecf57475 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 28 Mar 2023 14:50:10 +0200 Subject: [PATCH 2/2] Describe an edge-case. --- .../AvaloniaObjectTests_Coercion.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Coercion.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Coercion.cs index d53c055639..fe4262331f 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Coercion.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Coercion.cs @@ -277,6 +277,21 @@ namespace Avalonia.Base.UnitTests Assert.Equal(1, raised); } + [Fact] + public void If_Initial_State_Has_Coerced_Default_Value_Then_CoerceValue_Must_Be_Called() + { + // This test is just explicitly describing an edge-case. If the initial state of the + // object results in a coerced property value then CoerceValue must be called before + // coercion takes effect. Confirmed as matching the behavior of WPF. + var target = new Class3(); + + Assert.Equal(11, target.Foo); + + target.CoerceValue(Class3.FooProperty); + + Assert.Equal(50, target.Foo); + } + private class Class1 : AvaloniaObject { public static readonly StyledProperty FooProperty = @@ -364,6 +379,28 @@ namespace Avalonia.Base.UnitTests } } + private class Class3: AvaloniaObject + { + public static readonly StyledProperty FooProperty = + AvaloniaProperty.Register( + "Foo", + defaultValue: 11, + coerce: CoerceFoo); + + public int Foo + { + get => GetValue(FooProperty); + set => SetValue(FooProperty, value); + } + + + public static int CoerceFoo(AvaloniaObject instance, int value) + { + var o = (Class3)instance; + return Math.Clamp(value, 50, 100); + } + } + private class Control1 : Control { public static readonly StyledProperty FooProperty =