From bd357e08dd0c3cfe860f2b80620e01c52f15bbad Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 14 Oct 2022 18:51:10 +0200 Subject: [PATCH] Make EffectiveValue responsible for unsubscribing. --- .../PropertyStore/BindingEntryBase.cs | 2 +- .../PropertyStore/EffectiveValue.cs | 103 ++++++--- .../PropertyStore/EffectiveValue`1.cs | 208 +++++++++--------- .../PropertyStore/ImmediateValueFrame.cs | 2 +- src/Avalonia.Base/PropertyStore/ValueStore.cs | 193 +++++----------- 5 files changed, 237 insertions(+), 271 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs index 34b8048bbc..292420ff0a 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntryBase.cs @@ -143,7 +143,7 @@ namespace Avalonia.PropertyStore _value = value.Value; _hasValue = true; if (_subscription is not null && _subscription != s_creatingQuiet) - Frame.Owner?.OnBindingValueChanged(Property, Frame.Priority, value.Value); + Frame.Owner?.OnBindingValueChanged(this, Frame.Priority); } } else if (value.Type != BindingValueType.DoNothing) diff --git a/src/Avalonia.Base/PropertyStore/EffectiveValue.cs b/src/Avalonia.Base/PropertyStore/EffectiveValue.cs index 9ed4c67fd6..d5445b8f97 100644 --- a/src/Avalonia.Base/PropertyStore/EffectiveValue.cs +++ b/src/Avalonia.Base/PropertyStore/EffectiveValue.cs @@ -1,5 +1,4 @@ -using System; -using Avalonia.Data; +using Avalonia.Data; namespace Avalonia.PropertyStore { @@ -11,6 +10,9 @@ namespace Avalonia.PropertyStore /// internal abstract class EffectiveValue { + private IValueEntry? _valueEntry; + private IValueEntry? _baseValueEntry; + /// /// Gets the current effective value as a boxed value. /// @@ -33,52 +35,66 @@ namespace Avalonia.PropertyStore public BindingPriority BasePriority { get; protected set; } /// - /// Sets the value and base value, raising - /// where necessary. + /// Begins a reevaluation pass on the effective value. + /// + /// + /// Determines whether any current local value should be cleared. + /// + /// + /// This method resets the and properties + /// to Unset, pending reevaluation. + /// + public void BeginReevaluation(bool clearLocalValue = false) + { + if (clearLocalValue || Priority != BindingPriority.LocalValue) + Priority = BindingPriority.Unset; + if (clearLocalValue || BasePriority != BindingPriority.LocalValue) + BasePriority = BindingPriority.Unset; + } + + public void EndReevaluation() + { + if (Priority == BindingPriority.Unset) + { + _valueEntry?.Unsubscribe(); + _valueEntry = null; + } + + if (BasePriority == BindingPriority.Unset) + { + _baseValueEntry?.Unsubscribe(); + _baseValueEntry = null; + } + } + + /// + /// Sets the value and base value for a non-LocalValue priority, raising + /// where necessary. /// /// The associated value store. - /// The property being changed. /// The new value of the property. /// The priority of the new value. public abstract void SetAndRaise( ValueStore owner, - AvaloniaProperty property, - object? value, + IValueEntry value, BindingPriority priority); /// - /// Sets the value and base value, raising - /// where necessary. + /// Sets the value and base value for a non-LocalValue priority, raising + /// where necessary. /// /// The associated value store. - /// The property being changed. /// The new value of the property. /// The priority of the new value. /// The new base value of the property. /// The priority of the new base value. public abstract void SetAndRaise( ValueStore owner, - AvaloniaProperty property, - object? value, + IValueEntry value, BindingPriority priority, - object? baseValue, + IValueEntry baseValue, BindingPriority basePriority); - /// - /// Sets the value, raising - /// where necessary. - /// - /// The associated value store. - /// The value entry with the new value of the property. - /// The priority of the new value. - /// - /// This method does not set the base value. - /// - public abstract void SetAndRaise( - ValueStore owner, - IValueEntry entry, - BindingPriority priority); - /// /// Set the value priority, but leaves the value unchanged. /// @@ -103,6 +119,14 @@ namespace Avalonia.PropertyStore EffectiveValue? oldValue, EffectiveValue? newValue); + /// + /// Removes the current animation value and reverts to the base value, raising + /// where necessary. + /// + /// The associated value store. + /// The property being changed. + public abstract void RemoveAnimationAndRaise(ValueStore owner, AvaloniaProperty property); + /// /// Coerces the property value. /// @@ -120,5 +144,28 @@ namespace Avalonia.PropertyStore protected abstract object? GetBoxedValue(); protected abstract object? GetBoxedBaseValue(); + + protected void UpdateValueEntry(IValueEntry? entry, BindingPriority priority) + { + if (priority <= Priority && entry != _valueEntry) + { + _valueEntry?.Unsubscribe(); + _valueEntry = entry; + } + + if (priority <= BasePriority && + priority >= BindingPriority.LocalValue && + entry != _baseValueEntry) + { + _baseValueEntry?.Unsubscribe(); + _baseValueEntry = entry; + } + } + + protected void UnsubscribeValueEntries() + { + _valueEntry?.Unsubscribe(); + _baseValueEntry?.Unsubscribe(); + } } } diff --git a/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs b/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs index 50b11bab33..230ef27dcc 100644 --- a/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs +++ b/src/Avalonia.Base/PropertyStore/EffectiveValue`1.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using Avalonia.Data; +using JetBrains.Annotations; namespace Avalonia.PropertyStore { @@ -60,44 +61,117 @@ namespace Avalonia.PropertyStore public override void SetAndRaise( ValueStore owner, - AvaloniaProperty property, - object? value, + IValueEntry value, BindingPriority priority) { - // `value` should already have been converted to the correct type and - // validated by this point. - SetAndRaise(owner, (StyledPropertyBase)property, (T)value!, priority); + Debug.Assert(priority != BindingPriority.LocalValue); + UpdateValueEntry(value, priority); + SetAndRaiseCore(owner, (StyledPropertyBase)value.Property, (T)value.GetValue()!, priority); } public override void SetAndRaise( ValueStore owner, - AvaloniaProperty property, - object? value, + IValueEntry value, BindingPriority priority, - object? baseValue, + IValueEntry baseValue, BindingPriority basePriority) { - SetAndRaise(owner, (StyledPropertyBase)property, (T)value!, priority, (T)baseValue!, basePriority); + Debug.Assert(priority != BindingPriority.LocalValue); + SetAndRaiseCore(owner, (StyledPropertyBase)value.Property, (T)value.GetValue()!, priority, (T)baseValue!, basePriority); } - public override void SetAndRaise( + public void SetLocalValueAndRaise( ValueStore owner, - IValueEntry entry, - BindingPriority priority) + StyledPropertyBase property, + T value) { - var value = entry is IValueEntry typed ? typed.GetValue() : (T)entry.GetValue()!; - SetAndRaise(owner, (StyledPropertyBase)entry.Property, value, priority); + SetAndRaiseCore(owner, property, value, BindingPriority.LocalValue); } - /// - /// Sets the value and base value, raising - /// where necessary. - /// - /// The object on which to raise events. - /// The property being changed. - /// The new value of the property. - /// The priority of the new value. - public void SetAndRaise( + public bool TryGetBaseValue([MaybeNullWhen(false)] out T value) + { + value = _baseValue!; + return BasePriority != BindingPriority.Unset; + } + + public override void RaiseInheritedValueChanged( + AvaloniaObject owner, + AvaloniaProperty property, + EffectiveValue? oldValue, + EffectiveValue? newValue) + { + Debug.Assert(oldValue is not null || newValue is not null); + + var p = (StyledPropertyBase)property; + var o = oldValue is not null ? ((EffectiveValue)oldValue).Value : p.GetDefaultValue(owner.GetType()); + var n = newValue is not null ? ((EffectiveValue)newValue).Value : p.GetDefaultValue(owner.GetType()); + var priority = newValue is not null ? BindingPriority.Inherited : BindingPriority.Unset; + + if (!EqualityComparer.Default.Equals(o, n)) + { + owner.RaisePropertyChanged(p, o, n, priority, true); + } + } + + public override void RemoveAnimationAndRaise(ValueStore owner, AvaloniaProperty property) + { + Debug.Assert(Priority != BindingPriority.Animation); + Debug.Assert(BasePriority != BindingPriority.Unset); + UpdateValueEntry(null, BindingPriority.Animation); + SetAndRaiseCore(owner, (StyledPropertyBase)property, _baseValue!, BasePriority); + } + + public override void CoerceValue(ValueStore owner, AvaloniaProperty property) + { + if (_uncommon is null) + return; + SetAndRaiseCore( + owner, + (StyledPropertyBase)property, + _uncommon._uncoercedValue!, + Priority, + _uncommon._uncoercedBaseValue!, + BasePriority); + } + + public override void DisposeAndRaiseUnset(ValueStore owner, AvaloniaProperty property) + { + UnsubscribeValueEntries(); + DisposeAndRaiseUnset(owner, (StyledPropertyBase)property); + } + + public void DisposeAndRaiseUnset(ValueStore owner, StyledPropertyBase property) + { + BindingPriority priority; + T oldValue; + + if (property.Inherits && owner.TryGetInheritedValue(property, out var i)) + { + oldValue = ((EffectiveValue)i).Value; + priority = BindingPriority.Inherited; + } + else + { + oldValue = property.GetDefaultValue(owner.GetType()); + priority = BindingPriority.Unset; + } + + if (!EqualityComparer.Default.Equals(oldValue, Value)) + { + owner.Owner.RaisePropertyChanged(property, Value, oldValue, priority, true); + if (property.Inherits) + owner.OnInheritedEffectiveValueDisposed(property, Value); + } + } + + protected override object? GetBoxedValue() => Value; + + protected override object? GetBoxedBaseValue() + { + return BasePriority != BindingPriority.Unset ? _baseValue : AvaloniaProperty.UnsetValue; + } + + private void SetAndRaiseCore( ValueStore owner, StyledPropertyBase property, T value, @@ -144,17 +218,7 @@ namespace Avalonia.PropertyStore } } - /// - /// Sets the value and base value, raising - /// where necessary. - /// - /// The object on which to raise events. - /// The property being changed. - /// The new value of the property. - /// The priority of the new value. - /// The new base value of the property. - /// The priority of the new base value. - public void SetAndRaise( + private void SetAndRaiseCore( ValueStore owner, StyledPropertyBase property, T value, @@ -187,7 +251,7 @@ namespace Avalonia.PropertyStore } if (priority != BindingPriority.Unset && - (BasePriority == BindingPriority.Unset || + (BasePriority == BindingPriority.Unset || !EqualityComparer.Default.Equals(_baseValue, bv))) { _baseValue = v; @@ -212,80 +276,6 @@ namespace Avalonia.PropertyStore } } - public bool TryGetBaseValue([MaybeNullWhen(false)] out T value) - { - value = _baseValue!; - return BasePriority != BindingPriority.Unset; - } - - public override void RaiseInheritedValueChanged( - AvaloniaObject owner, - AvaloniaProperty property, - EffectiveValue? oldValue, - EffectiveValue? newValue) - { - Debug.Assert(oldValue is not null || newValue is not null); - - var p = (StyledPropertyBase)property; - var o = oldValue is not null ? ((EffectiveValue)oldValue).Value : p.GetDefaultValue(owner.GetType()); - var n = newValue is not null ? ((EffectiveValue)newValue).Value : p.GetDefaultValue(owner.GetType()); - var priority = newValue is not null ? BindingPriority.Inherited : BindingPriority.Unset; - - if (!EqualityComparer.Default.Equals(o, n)) - { - owner.RaisePropertyChanged(p, o, n, priority, true); - } - } - - public override void CoerceValue(ValueStore owner, AvaloniaProperty property) - { - if (_uncommon is null) - return; - SetAndRaise( - owner, - (StyledPropertyBase)property, - _uncommon._uncoercedValue!, - Priority, - _uncommon._uncoercedBaseValue!, - BasePriority); - } - - public override void DisposeAndRaiseUnset(ValueStore owner, AvaloniaProperty property) - { - DisposeAndRaiseUnset(owner, (StyledPropertyBase)property); - } - - public void DisposeAndRaiseUnset(ValueStore owner, StyledPropertyBase property) - { - BindingPriority priority; - T oldValue; - - if (property.Inherits && owner.TryGetInheritedValue(property, out var i)) - { - oldValue = ((EffectiveValue)i).Value; - priority = BindingPriority.Inherited; - } - else - { - oldValue = property.GetDefaultValue(owner.GetType()); - priority = BindingPriority.Unset; - } - - if (!EqualityComparer.Default.Equals(oldValue, Value)) - { - owner.Owner.RaisePropertyChanged(property, Value, oldValue, priority, true); - if (property.Inherits) - owner.OnInheritedEffectiveValueDisposed(property, Value); - } - } - - protected override object? GetBoxedValue() => Value; - - protected override object? GetBoxedBaseValue() - { - return BasePriority != BindingPriority.Unset ? _baseValue : AvaloniaProperty.UnsetValue; - } - private class UncommonFields { public Func? _coerce; diff --git a/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs b/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs index 636fea7b30..1d886e7501 100644 --- a/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs +++ b/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs @@ -41,7 +41,7 @@ namespace Avalonia.PropertyStore return e; } - public IDisposable AddValue(StyledPropertyBase property, T value) + public ImmediateValueEntry AddValue(StyledPropertyBase property, T value) { var e = new ImmediateValueEntry(this, property, value); Add(e); diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index 7fe0213148..903146ad2b 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -62,10 +62,7 @@ namespace Avalonia.PropertyStore var result = frame.AddBinding(property, source); if (effective is null || priority <= effective.Priority) - { result.Start(); - UnsubscribeInactiveValues(frameIndex, property); - } return result; } @@ -92,10 +89,7 @@ namespace Avalonia.PropertyStore var result = frame.AddBinding(property, source); if (effective is null || priority <= effective.Priority) - { result.Start(); - UnsubscribeInactiveValues(frameIndex, property); - } return result; } @@ -122,10 +116,7 @@ namespace Avalonia.PropertyStore var result = frame.AddBinding(property, source); if (effective is null || priority <= effective.Priority) - { result.Start(); - UnsubscribeInactiveValues(frameIndex, property); - } return result; } @@ -177,26 +168,43 @@ namespace Avalonia.PropertyStore throw new ArgumentException($"{value} is not a valid value for '{property.Name}."); } - IDisposable? result = null; - if (priority != BindingPriority.LocalValue) { var frame = GetOrCreateImmediateValueFrame(property, priority, out var frameIndex); - result = frame.AddValue(property, value); - UnsubscribeInactiveValues(frameIndex, property); - } + var result = frame.AddValue(property, value); - if (TryGetEffectiveValue(property, out var existing)) - { - var effective = (EffectiveValue)existing; - effective.SetAndRaise(this, property, value, priority); + if (TryGetEffectiveValue(property, out var existing)) + { + var effective = (EffectiveValue)existing; + effective.SetAndRaise(this, result, priority); + } + else + { + var defaultValue = property.GetDefaultValue(Owner.GetType()); + var effectiveValue = new EffectiveValue(Owner, property, defaultValue, BindingPriority.Unset); + AddEffectiveValue(property, effectiveValue); + effectiveValue.SetAndRaise(this, result, priority); + } + + return result; } else { - AddEffectiveValueAndRaise(property, value, priority); - } + if (TryGetEffectiveValue(property, out var existing)) + { + var effective = (EffectiveValue)existing; + effective.SetLocalValueAndRaise(this, property, value); + } + else + { + var defaultValue = property.GetDefaultValue(Owner.GetType()); + var effectiveValue = new EffectiveValue(Owner, property, defaultValue, BindingPriority.Unset); + AddEffectiveValue(property, effectiveValue); + effectiveValue.SetLocalValueAndRaise(this, property, value); + } - return result; + return null; + } } public object? GetValue(AvaloniaProperty property) @@ -349,40 +357,15 @@ namespace Avalonia.PropertyStore /// Called by non-LocalValue binding entries to re-evaluate the effective value when the /// binding produces a new value. /// - /// The bound property. + /// The binding entry. /// The priority of binding which produced a new value. - /// The new value. public void OnBindingValueChanged( - AvaloniaProperty property, - BindingPriority priority, - object? value) + IValueEntry entry, + BindingPriority priority) { Debug.Assert(priority != BindingPriority.LocalValue); - if (TryGetEffectiveValue(property, out var existing)) - { - if (priority <= existing.Priority) - ReevaluateEffectiveValue(property, existing); - } - else - { - AddEffectiveValueAndRaise(property, value, priority); - } - } - - /// - /// Called by non-LocalValue binding entries to re-evaluate the effective value when the - /// binding produces a new value. - /// - /// The bound property. - /// The priority of binding which produced a new value. - /// The new value. - public void OnBindingValueChanged( - StyledPropertyBase property, - BindingPriority priority, - T value) - { - Debug.Assert(priority != BindingPriority.LocalValue); + var property = entry.Property; if (TryGetEffectiveValue(property, out var existing)) { @@ -391,7 +374,7 @@ namespace Avalonia.PropertyStore } else { - AddEffectiveValueAndRaise(property, value, priority); + AddEffectiveValueAndRaise(property, entry, priority); } } @@ -413,8 +396,8 @@ namespace Avalonia.PropertyStore } /// - /// Called by a to re-evaluate the effective value when the - /// binding completes or terminates on error. + /// Called by a to re-evaluate the + /// effective value when the binding completes or terminates on error. /// /// The previously bound property. /// The frame which contained the binding. @@ -697,31 +680,14 @@ namespace Avalonia.PropertyStore /// event and notifies inheritance children if necessary . /// /// The property. - /// The property value. + /// The value entry. /// The value priority. - private void AddEffectiveValueAndRaise(AvaloniaProperty property, object? value, BindingPriority priority) + private void AddEffectiveValueAndRaise(AvaloniaProperty property, IValueEntry entry, BindingPriority priority) { Debug.Assert(priority < BindingPriority.Inherited); var effectiveValue = property.CreateEffectiveValue(Owner); AddEffectiveValue(property, effectiveValue); - effectiveValue.SetAndRaise(this, property, value, priority); - } - - /// - /// Adds a new effective value, raises the initial - /// event and notifies inheritance children if necessary . - /// - /// The property type. - /// The property. - /// The property value. - /// The value priority. - private void AddEffectiveValueAndRaise(StyledPropertyBase property, T value, BindingPriority priority) - { - Debug.Assert(priority < BindingPriority.Inherited); - var defaultValue = property.GetDefaultValue(Owner.GetType()); - var effectiveValue = new EffectiveValue(Owner, property, defaultValue, BindingPriority.Unset); - AddEffectiveValue(property, effectiveValue); - effectiveValue.SetAndRaise(this, property, value, priority); + effectiveValue.SetAndRaise(this, entry, priority); } private bool RemoveEffectiveValue(AvaloniaProperty property) @@ -796,14 +762,8 @@ namespace Avalonia.PropertyStore var generation = _frameGeneration; - // Reset all non-LocalValue effective value to Unset priority. - if (current is not null) - { - if (ignoreLocalValue || current.Priority != BindingPriority.LocalValue) - current.SetPriority(BindingPriority.Unset); - if (ignoreLocalValue || current.BasePriority != BindingPriority.LocalValue) - current.SetBasePriority(BindingPriority.Unset); - } + // Notify the existing effective value that reevaluation is starting. + current?.BeginReevaluation(ignoreLocalValue); // Iterate the frames to get the effective value. for (var i = _frames.Count - 1; i >= 0; --i) @@ -821,7 +781,16 @@ namespace Avalonia.PropertyStore return; } - if (foundEntry && entry!.HasValue) + // 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 > BindingPriority.Animation && priority < current.BasePriority); + + if (foundEntry && isRelevantPriority && entry!.HasValue) { if (current is not null) { @@ -838,19 +807,13 @@ namespace Avalonia.PropertyStore if (generation != _frameGeneration) goto restart; - if (current is not null && - current.Priority < BindingPriority.Unset && - current.BasePriority < BindingPriority.Unset) - { - // If the active state of one of the trailing frames has changed since - // the last read, then we need to re-evaluate the effective values of all - // properties. - if (UnsubscribeInactiveValues(i, property)) - ReevaluateEffectiveValues(); - return; - } + if (current?.Priority < BindingPriority.Unset && + current?.BasePriority < BindingPriority.Unset) + break; } + current?.EndReevaluation(); + if (current?.Priority == BindingPriority.Unset) { if (current.BasePriority == BindingPriority.Unset) @@ -860,7 +823,7 @@ namespace Avalonia.PropertyStore } else { - current.SetAndRaise(this, property, current.BaseValue, current.BasePriority); + current.RemoveAnimationAndRaise(this, property); } } } @@ -885,16 +848,9 @@ namespace Avalonia.PropertyStore var generation = _frameGeneration; var count = _effectiveValues.Count; - // Reset all non-LocalValue effective values to Unset priority. + // Notify the existing effective values that reevaluation is starting. for (var i = 0; i < count; ++i) - { - var e = _effectiveValues[i]; - - if (e.Priority != BindingPriority.LocalValue) - e.SetPriority(BindingPriority.Unset); - if (e.BasePriority != BindingPriority.LocalValue) - e.SetBasePriority(BindingPriority.Unset); - } + _effectiveValues[i].BeginReevaluation(); // Iterate the frames, setting and creating effective values. for (var i = _frames.Count - 1; i >= 0; --i) @@ -912,10 +868,9 @@ namespace Avalonia.PropertyStore { var entry = frame.GetEntry(j); var property = entry.Property; - EffectiveValue? effectiveValue; // Unsubscribe and skip if we already have a value/base value for this property. - if (_effectiveValues.TryGetValue(property, out effectiveValue) == true && + if (_effectiveValues.TryGetValue(property, out var effectiveValue) == true && effectiveValue.BasePriority < BindingPriority.Unset) { entry.Unsubscribe(); @@ -949,6 +904,7 @@ namespace Avalonia.PropertyStore for (var i = 0; i < count; ++i) { _effectiveValues.GetKeyValue(i, out var key, out var e); + e.EndReevaluation(); if (e.Priority == BindingPriority.Unset) { @@ -973,33 +929,6 @@ namespace Avalonia.PropertyStore } } - private bool UnsubscribeInactiveValues(int activeFrameIndex, AvaloniaProperty property) - { - var foundBaseValue = _frames[activeFrameIndex].Priority != BindingPriority.Animation; - var activeChanged = false; - - for (var i = activeFrameIndex - 1; i >= 0; --i) - { - var frame = _frames[i]; - - if (!foundBaseValue && frame.Priority > BindingPriority.Animation) - { - foundBaseValue = true; - continue; - } - - if ((foundBaseValue || frame.Priority <= BindingPriority.Animation) && - frame.TryGetEntryIfActive(property, out var entry, out var changed)) - { - if (changed && frame.EntryCount > 1) - activeChanged = true; - entry.Unsubscribe(); - } - } - - return activeChanged; - } - private bool TryGetEffectiveValue( AvaloniaProperty property, [NotNullWhen(true)] out EffectiveValue? value)