diff --git a/src/Avalonia.Base/PropertyStore/InheritanceFrame.cs b/src/Avalonia.Base/PropertyStore/InheritanceFrame.cs deleted file mode 100644 index 3df42ee366..0000000000 --- a/src/Avalonia.Base/PropertyStore/InheritanceFrame.cs +++ /dev/null @@ -1,34 +0,0 @@ -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; - -namespace Avalonia.PropertyStore -{ - internal class InheritanceFrame : Dictionary - { - public InheritanceFrame(ValueStore owner, InheritanceFrame? parent = null) - { - Owner = owner; - Parent = parent; - } - - public ValueStore Owner { get; } - public InheritanceFrame? Parent { get; private set; } - - public bool TryGetFromThisOrAncestor(AvaloniaProperty property, [NotNullWhen(true)] out EffectiveValue? value) - { - var frame = this; - - while (frame is object) - { - if (frame.TryGetValue(property, out value)) - return true; - frame = frame.Parent; - } - - value = default; - return false; - } - - public void SetParent(InheritanceFrame? value) => Parent = value; - } -} diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index 4db4b141f1..aa9d7525a3 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -2,7 +2,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Runtime.CompilerServices; using Avalonia.Collections.Pooled; using Avalonia.Data; using Avalonia.Diagnostics; @@ -14,14 +13,15 @@ namespace Avalonia.PropertyStore { private readonly List _frames = new(); private Dictionary? _localValueBindings; - private InheritanceFrame? _inheritanceFrame; private Dictionary? _effectiveValues; + private int _inheritedValueCount; private int _frameGeneration; private int _styling; public ValueStore(AvaloniaObject owner) => Owner = owner; public AvaloniaObject Owner { get; } + public ValueStore? InheritanceAncestor { get; private set; } public IReadOnlyList Frames => _frames; public void BeginStyling() => ++_styling; @@ -160,7 +160,7 @@ namespace Avalonia.PropertyStore { if (_effectiveValues is not null && _effectiveValues.TryGetValue(property, out var v)) return v.Value; - if (_inheritanceFrame is not null && _inheritanceFrame.TryGetFromThisOrAncestor(property, out v)) + if (property.Inherits && TryGetInheritedValue(property, out v)) return v.Value; return GetDefaultValue(property); @@ -170,7 +170,7 @@ namespace Avalonia.PropertyStore { if (_effectiveValues is not null && _effectiveValues.TryGetValue(property, out var v)) return ((EffectiveValue)v).Value; - if (_inheritanceFrame is not null && _inheritanceFrame.TryGetFromThisOrAncestor(property, out v)) + if (property.Inherits && TryGetInheritedValue(property, out v)) return ((EffectiveValue)v).Value; return property.GetDefaultValue(Owner.GetType()); } @@ -203,41 +203,57 @@ namespace Avalonia.PropertyStore public void SetInheritanceParent(AvaloniaObject? oldParent, AvaloniaObject? newParent) { var values = DictionaryPool.Get(); - var oldInheritanceFrame = oldParent?.GetValueStore()._inheritanceFrame; - var newInheritanceFrame = newParent?.GetValueStore().OnBecameInheritanceParent(); + var oldAncestor = InheritanceAncestor; + var newAncestor = newParent?.GetValueStore(); - // The old and new parents are the same, nothing to do here. - if (oldInheritanceFrame == newInheritanceFrame) + if (newAncestor?._inheritedValueCount == 0) + newAncestor = newAncestor.InheritanceAncestor; + + // The old and new inheritance ancestors are the same, nothing to do here. + if (oldAncestor == newAncestor) return; - // First get the old values from the old inheritance parent. - var f = oldInheritanceFrame; + // First get the old values from the old inheritance ancestor. + var f = oldAncestor; while (f is not null) { - foreach (var i in f) + Debug.Assert(f._effectiveValues is not null); + + if (f._effectiveValues is not null) { - values.TryAdd(i.Key, new(i.Value)); + foreach (var i in f._effectiveValues) + { + if (i.Key.Inherits) + values.TryAdd(i.Key, new(i.Value)); + } } - f = f.Parent; + + f = f.InheritanceAncestor; } - f = newInheritanceFrame; + f = newAncestor; - // Get the new values from the new inheritance parent. + // Get the new values from the new inheritance ancestor. while (f is not null) { - foreach (var i in f) + Debug.Assert(f._effectiveValues is not null); + + foreach (var i in f._effectiveValues) { - if (values.TryGetValue(i.Key, out var existing)) - values[i.Key] = existing.WithNewValue(i.Value); - else - values.Add(i.Key, new(null, i.Value)); + if (i.Key.Inherits) + { + if (values.TryGetValue(i.Key, out var existing)) + values[i.Key] = existing.WithNewValue(i.Value); + else + values.Add(i.Key, new(null, i.Value)); + } } - f = f.Parent; + + f = f.InheritanceAncestor; } - ParentInheritanceFrameChanged(newInheritanceFrame); + OnInheritanceAncestorChanged(newAncestor); // Raise PropertyChanged events where necessary on this object and inheritance children. foreach (var i in values) @@ -257,27 +273,6 @@ namespace Avalonia.PropertyStore ReevaluateEffectiveValues(); } - /// - /// Called by an inheritance child to notify the value store that it has become an - /// inheritance parent. Creates and returns an inheritance frame if necessary. - /// - /// - public InheritanceFrame? OnBecameInheritanceParent() - { - if (_inheritanceFrame is not null) - return _inheritanceFrame; - if (_effectiveValues is null) - return null; - - foreach (var i in _effectiveValues) - { - if (i.Key.Inherits) - return GetOrCreateInheritanceFrame(true); - } - - return null; - } - /// /// Called by non-LocalValue binding entries to re-evaluate the effective value when the /// binding produces a new value. @@ -362,6 +357,32 @@ namespace Avalonia.PropertyStore } } + /// + /// Called by the parent value store when its inheritance ancestor changes. + /// + /// The new inheritance ancestor. + public void OnInheritanceAncestorChanged(ValueStore? ancestor) + { + if (ancestor != this) + { + InheritanceAncestor = ancestor; + if (_inheritedValueCount > 0) + return; + } + + var children = Owner.GetInheritanceChildren(); + + if (children is null) + return; + + var count = children.Count; + + for (var i = 0; i < count; ++i) + { + children[i].GetValueStore().OnInheritanceAncestorChanged(ancestor); + } + } + /// /// Called by when an property with inheritance enabled /// changes its value on this value store. @@ -370,7 +391,7 @@ namespace Avalonia.PropertyStore /// The old value of the property. /// The effective value instance. public void OnInheritedEffectiveValueChanged( - StyledPropertyBase property, + StyledPropertyBase property, T oldValue, EffectiveValue value) { @@ -378,20 +399,14 @@ namespace Avalonia.PropertyStore var children = Owner.GetInheritanceChildren(); - // If we have children or an existing inheritance frame, then make sure it's owned and - // set the value. If we have no children and no inheritance frame then it will be - // created when it's needed. - if (children is not null || _inheritanceFrame is not null) - GetOrCreateInheritanceFrame(true)[property] = value; + if (children is null) + return; - if (children is not null) - { - var count = children.Count; + var count = children.Count; - for (var i = 0; i < count; ++i) - { - children[i].GetValueStore().OnParentInheritedValueChanged(property, oldValue, value.Value); - } + for (var i = 0; i < count; ++i) + { + children[i].GetValueStore().OnAncestorInheritedValueChanged(property, oldValue, value.Value); } } @@ -404,12 +419,6 @@ namespace Avalonia.PropertyStore public void OnInheritedEffectiveValueDisposed(StyledPropertyBase property, T oldValue) { Debug.Assert(property.Inherits); - Debug.Assert(_inheritanceFrame is null || _inheritanceFrame.Owner == this); - - if (_inheritanceFrame is null || _inheritanceFrame.Owner != this) - return; - - _inheritanceFrame.Remove(property); var children = Owner.GetInheritanceChildren(); @@ -420,7 +429,7 @@ namespace Avalonia.PropertyStore for (var i = 0; i < count; ++i) { - children[i].GetValueStore().OnParentInheritedValueChanged(property, oldValue, defaultValue); + children[i].GetValueStore().OnAncestorInheritedValueChanged(property, oldValue, defaultValue); } } } @@ -443,16 +452,20 @@ namespace Avalonia.PropertyStore } } - public void OnParentInheritedValueChanged( + /// + /// Called when an inherited property changes on the value store of the inheritance ancestor. + /// + /// The property type. + /// The property. + /// The old value of the property. + /// The new value of the property. + public void OnAncestorInheritedValueChanged( StyledPropertyBase property, T oldValue, T newValue) { Debug.Assert(property.Inherits); - // Ensure the inheritance frame is created. - GetOrCreateInheritanceFrame(false); - // If the inherited value is set locally, propagation stops here. if (_effectiveValues is not null && _effectiveValues.ContainsKey(property)) return; @@ -473,7 +486,7 @@ namespace Avalonia.PropertyStore for (var i = 0; i < count; ++i) { - children[i].GetValueStore().OnParentInheritedValueChanged(property, oldValue, newValue); + children[i].GetValueStore().OnAncestorInheritedValueChanged(property, oldValue, newValue); } } @@ -535,33 +548,6 @@ namespace Avalonia.PropertyStore frame.SetOwner(this); } - private InheritanceFrame GetOrCreateInheritanceFrame(bool owned) - { - if (_inheritanceFrame is null) - { - var parentFrame = Owner.InheritanceParent?.GetValueStore()._inheritanceFrame; - - _inheritanceFrame = owned || parentFrame is null ? - new(this, parentFrame) : - parentFrame; - - if (_effectiveValues is not null) - { - foreach (var i in _effectiveValues) - { - if (i.Key.Inherits) - _inheritanceFrame[i.Key] = i.Value; - } - } - } - else if (owned && _inheritanceFrame.Owner != this) - { - _inheritanceFrame = new(this, _inheritanceFrame); - } - - return _inheritanceFrame; - } - private ImmediateValueFrame GetOrCreateImmediateValueFrame( AvaloniaProperty property, BindingPriority priority) @@ -600,11 +586,20 @@ namespace Avalonia.PropertyStore } else { - _effectiveValues?.Remove(property); + RemoveEffectiveValue(property); current.DisposeAndRaiseUnset(this, property); } } + private void AddEffectiveValue(AvaloniaProperty property, EffectiveValue effectiveValue) + { + _effectiveValues ??= new(); + _effectiveValues.Add(property, effectiveValue); + + if (property.Inherits && _inheritedValueCount++ == 0) + OnInheritanceAncestorChanged(this); + } + /// /// Adds a new effective value, raises the initial /// event and notifies inheritance children if necessary . @@ -617,8 +612,7 @@ namespace Avalonia.PropertyStore { Debug.Assert(priority < BindingPriority.Inherited); var effectiveValue = property.CreateEffectiveValue(Owner); - _effectiveValues ??= new(); - _effectiveValues.Add(property, effectiveValue); + AddEffectiveValue(property, effectiveValue); effectiveValue.SetAndRaise(this, property, value, priority); } @@ -635,11 +629,35 @@ namespace Avalonia.PropertyStore Debug.Assert(priority < BindingPriority.Inherited); var defaultValue = property.GetDefaultValue(Owner.GetType()); var effectiveValue = new EffectiveValue(defaultValue, BindingPriority.Unset); - _effectiveValues ??= new(); - _effectiveValues.Add(property, effectiveValue); + AddEffectiveValue(property, effectiveValue); effectiveValue.SetAndRaise(this, property, value, priority); } + private bool RemoveEffectiveValue(AvaloniaProperty property) + { + if (_effectiveValues is not null && _effectiveValues.Remove(property)) + { + if (property.Inherits && --_inheritedValueCount == 0) + OnInheritanceAncestorChanged(InheritanceAncestor?.InheritanceAncestor); + return true; + } + + return false; + } + + private bool RemoveEffectiveValue(AvaloniaProperty property, [NotNullWhen(true)] out EffectiveValue? result) + { + if (_effectiveValues is not null && _effectiveValues.Remove(property, out result)) + { + if (property.Inherits && --_inheritedValueCount == 0) + OnInheritanceAncestorChanged(InheritanceAncestor?.InheritanceAncestor); + return true; + } + + result = null; + return false; + } + /// /// Evaluates the current value and base value for a property based on the current frames and optionally /// local values. Does not evaluate inherited values. @@ -761,30 +779,6 @@ namespace Avalonia.PropertyStore } } - private void ParentInheritanceFrameChanged(InheritanceFrame? parent) - { - if (_inheritanceFrame?.Owner == this) - { - _inheritanceFrame.SetParent(parent); - } - else if (_inheritanceFrame != parent) - { - _inheritanceFrame = parent; - - var children = Owner.GetInheritanceChildren(); - - if (children is null) - return; - - var count = children.Count; - - for (var i = 0; i < count; ++i) - { - children[i].GetValueStore().ParentInheritanceFrameChanged(parent); - } - } - } - private void ReevaluateEffectiveValues() { restart: @@ -841,8 +835,7 @@ namespace Avalonia.PropertyStore else { var v = property.CreateEffectiveValue(Owner); - _effectiveValues ??= new(); - _effectiveValues.Add(property, v); + AddEffectiveValue(property, v); v.SetAndRaise(this, entry, priority); } @@ -871,7 +864,7 @@ namespace Avalonia.PropertyStore { foreach (var v in remove) { - if (_effectiveValues.Remove(v, out var e)) + if (RemoveEffectiveValue(v, out var e)) e.DisposeAndRaiseUnset(this, v); } remove.Dispose(); @@ -890,6 +883,25 @@ namespace Avalonia.PropertyStore return false; } + private bool TryGetInheritedValue( + AvaloniaProperty property, + [NotNullWhen(true)] out EffectiveValue? result) + { + Debug.Assert(property.Inherits); + + var i = InheritanceAncestor; + + while (i is not null) + { + if (i.TryGetEffectiveValue(property, out result)) + return true; + i = i.InheritanceAncestor; + } + + result = null; + return false; + } + private EffectiveValue? GetEffectiveValue(AvaloniaProperty property) { if (_effectiveValues is not null && _effectiveValues.TryGetValue(property, out var value)) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs index 556440178d..09832e2c1d 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs @@ -146,6 +146,28 @@ namespace Avalonia.Base.UnitTests Assert.Equal("changed", child.GetValue(AttachedOwner.AttachedProperty)); } + [Fact] + public void Clearing_Value_In_InheritanceParent_Raises_PropertyChanged() + { + bool raised = false; + + Class1 parent = new Class1(); + parent.SetValue(Class1.BazProperty, "changed"); + + Class2 child = new Class2 { Parent = parent }; + + child.PropertyChanged += (s, e) => + raised = s == child && + e.Property == Class1.BazProperty && + (string)e.OldValue == "changed" && + (string)e.NewValue == "bazdefault"; + + parent.ClearValue(Class1.BazProperty); + + Assert.True(raised); + Assert.Equal("bazdefault", child.GetValue(Class1.BazProperty)); + } + [Fact] public void PropertyChanged_Is_Raised_In_Parent_Before_Child() { diff --git a/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Inheritance.cs b/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Inheritance.cs new file mode 100644 index 0000000000..549cd6fa73 --- /dev/null +++ b/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Inheritance.cs @@ -0,0 +1,117 @@ +using Xunit; + +#nullable enable + +namespace Avalonia.Base.UnitTests.PropertyStore +{ + public class ValueStoreTests_Inheritance + { + [Fact] + public void InheritanceAncestor_Is_Initially_Null() + { + var parent = new Class1(); + var child = new Class1 { Parent = parent }; + var grandchild = new Class1 { Parent = child }; + + Assert.Null(parent.GetValueStore().InheritanceAncestor); + Assert.Null(child.GetValueStore().InheritanceAncestor); + Assert.Null(grandchild.GetValueStore().InheritanceAncestor); + } + + [Fact] + public void Setting_Value_In_Parent_Updates_InheritanceAncestor() + { + var parent = new Class1(); + var child = new Class1 { Parent = parent }; + var grandchild = new Class1 { Parent = child }; + + parent.Foo = "changed"; + + var parentStore = parent.GetValueStore(); + Assert.Null(parentStore.InheritanceAncestor); + Assert.Same(parentStore, child.GetValueStore().InheritanceAncestor); + Assert.Same(parentStore, grandchild.GetValueStore().InheritanceAncestor); + } + + [Fact] + public void Setting_Value_In_Parent_Doesnt_Update_Grandchild_InheritanceAncestor_If_Child_Has_Value_Set() + { + var parent = new Class1(); + var child = new Class1 { Parent = parent }; + var grandchild = new Class1 { Parent = child }; + + child.Foo = "foochanged"; + parent.Foo = "changed"; + + var parentStore = parent.GetValueStore(); + Assert.Null(parentStore.InheritanceAncestor); + Assert.Same(parentStore, child.GetValueStore().InheritanceAncestor); + Assert.Same(child.GetValueStore(), grandchild.GetValueStore().InheritanceAncestor); + } + + [Fact] + public void Clearing_Value_In_Parent_Updates_InheritanceAncestor() + { + var parent = new Class1(); + var child = new Class1 { Parent = parent }; + var grandchild = new Class1 { Parent = child }; + + parent.Foo = "changed"; + parent.ClearValue(Class1.FooProperty); + + Assert.Null(parent.GetValueStore().InheritanceAncestor); + Assert.Null(child.GetValueStore().InheritanceAncestor); + Assert.Null(grandchild.GetValueStore().InheritanceAncestor); + } + + [Fact] + public void Clear_Value_In_Parent_Doesnt_Update_Grandchild_InheritanceAncestor_If_Child_Has_Value_Set() + { + var parent = new Class1(); + var child = new Class1 { Parent = parent }; + var grandchild = new Class1 { Parent = child }; + + child.Foo = "foochanged"; + parent.Foo = "changed"; + parent.ClearValue(Class1.FooProperty); + + Assert.Null(parent.GetValueStore().InheritanceAncestor); + Assert.Null(child.GetValueStore().InheritanceAncestor); + Assert.Same(child.GetValueStore(), grandchild.GetValueStore().InheritanceAncestor); + } + + [Fact] + public void Adding_Child_Sets_InheritanceAncestor() + { + var parent = new Class1(); + var child = new Class1(); + var grandchild = new Class1 { Parent = child }; + + parent.Foo = "changed"; + child.Parent = parent; + + var parentStore = parent.GetValueStore(); + Assert.Null(parentStore.InheritanceAncestor); + Assert.Same(parentStore, child.GetValueStore().InheritanceAncestor); + Assert.Same(parentStore, grandchild.GetValueStore().InheritanceAncestor); + } + + private class Class1 : AvaloniaObject + { + public static readonly StyledProperty FooProperty = + AvaloniaProperty.Register("Foo", "foodefault", inherits: true); + + public string Foo + { + get => GetValue(FooProperty); + set => SetValue(FooProperty, value); + } + + public Class1? Parent + { + get { return (Class1?)InheritanceParent; } + set { InheritanceParent = value; } + } + } + } +}