From 0a41ba08fd318269763385016339f69e65ba19f4 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 19 Mar 2021 12:55:58 +0100 Subject: [PATCH 01/13] Added failing batch update test. --- .../AvaloniaObjectTests_BatchUpdate.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs index 9f0e52c8d9..050fefbd53 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs @@ -449,6 +449,29 @@ namespace Avalonia.Base.UnitTests Assert.Null(raised[1].NewValue); } + [Fact] + public void Can_Set_Cleared_Value_When_Ending_Batch_Update() + { + var target = new TestClass(); + var raised = 0; + + target.Foo = "foo"; + + target.BeginBatchUpdate(); + target.ClearValue(TestClass.FooProperty); + target.PropertyChanged += (sender, e) => + { + if (e.Property == TestClass.FooProperty && e.NewValue is null) + { + target.Foo = "bar"; + ++raised; + } + }; + target.EndBatchUpdate(); + + Assert.Equal(1, raised); + } + public class TestClass : AvaloniaObject { public static readonly StyledProperty FooProperty = From f1298fb1bd5d8e91161e0e8dc5d465749470c40b Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 19 Mar 2021 13:28:35 +0100 Subject: [PATCH 02/13] Don't write new values to remove sentinels. During a batch update sentinel values are written to the value store to indicate that the value needs to be removed at the end of the update. If a new value is written to the store in the course of ending the batch update, don't update this sentinel value as the value will subsequently be lost. --- src/Avalonia.Base/ValueStore.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index e32b20cc96..470be35592 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -103,7 +103,7 @@ namespace Avalonia IDisposable? result = null; - if (_values.TryGetValue(property, out var slot)) + if (_values.TryGetValue(property, out var slot) && !IsRemoveSentinel(slot)) { result = SetExisting(slot, property, value, priority); } @@ -364,6 +364,14 @@ namespace Avalonia } } + private static bool IsRemoveSentinel(IValue value) + { + // Local value entries are optimized and contain only a single value field to save space, + // so there's no way to mark them for removal at the end of a batch update. Instead a + // ConstantValueEntry with a priority of Unset is used as a sentinel value. + return value is ConstantValueEntry t && t.Priority == BindingPriority.Unset; + } + private class BatchUpdate { private ValueStore _owner; From 03cf2c6f9f79294c3c07a82fa9a59b7e772546f2 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 19 Mar 2021 13:30:37 +0100 Subject: [PATCH 03/13] Added another failing batch update test. And a bit of a sanity check to the previous one. --- .../AvaloniaObjectTests_BatchUpdate.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs index 050fefbd53..5bf3afc9e7 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs @@ -469,6 +469,31 @@ namespace Avalonia.Base.UnitTests }; target.EndBatchUpdate(); + Assert.Equal("bar", target.Foo); + Assert.Equal(1, raised); + } + + [Fact] + public void Can_Bind_Cleared_Value_When_Ending_Batch_Update() + { + var target = new TestClass(); + var raised = 0; + + target.Foo = "foo"; + + target.BeginBatchUpdate(); + target.ClearValue(TestClass.FooProperty); + target.PropertyChanged += (sender, e) => + { + if (e.Property == TestClass.FooProperty && e.NewValue is null) + { + target.Bind(TestClass.FooProperty, new TestObservable("bar")); + ++raised; + } + }; + target.EndBatchUpdate(); + + Assert.Equal("bar", target.Foo); Assert.Equal(1, raised); } From 3afa95253f0c517e85af08bcc6d1f3ea074e9ff3 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 19 Mar 2021 13:38:29 +0100 Subject: [PATCH 04/13] Allow binding property during ending batch update. - Allow adding a binding to a cleared property while ending a batch update. Need to check that the existing value isn't a remove sentinel here, otherwise the binding will be lost. - When a binding is added during the end of a batch update, `_batchUpdate` will be non-null but newly added bindings shouldn't have `BeginBatchUpdate` called on them because no `EndBatchUpdate` will arrive (as we've already called them) - Add sanity checks to the unit test to make sure that correct notifications are raised --- src/Avalonia.Base/ValueStore.cs | 6 ++++-- .../AvaloniaObjectTests_BatchUpdate.cs | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 470be35592..9ece2b8042 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -138,7 +138,7 @@ namespace Avalonia IObservable> source, BindingPriority priority) { - if (_values.TryGetValue(property, out var slot)) + if (_values.TryGetValue(property, out var slot) && !IsRemoveSentinel(slot)) { return BindExisting(slot, property, source, priority); } @@ -338,7 +338,7 @@ namespace Avalonia private void AddValue(AvaloniaProperty property, IValue value) { _values.AddValue(property, value); - if (_batchUpdate is object && value is IBatchUpdate batch) + if (_batchUpdate?.IsBatchUpdating == true && value is IBatchUpdate batch) batch.BeginBatchUpdate(); value.Start(); } @@ -381,6 +381,8 @@ namespace Avalonia public BatchUpdate(ValueStore owner) => _owner = owner; + public bool IsBatchUpdating => _batchUpdateCount > 0; + public void Begin() { if (_batchUpdateCount++ == 0) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs index 5bf3afc9e7..036f275a71 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs @@ -478,6 +478,7 @@ namespace Avalonia.Base.UnitTests { var target = new TestClass(); var raised = 0; + var notifications = new List(); target.Foo = "foo"; @@ -490,11 +491,16 @@ namespace Avalonia.Base.UnitTests target.Bind(TestClass.FooProperty, new TestObservable("bar")); ++raised; } + + notifications.Add(e); }; target.EndBatchUpdate(); Assert.Equal("bar", target.Foo); Assert.Equal(1, raised); + Assert.Equal(2, notifications.Count); + Assert.Equal(null, notifications[0].NewValue); + Assert.Equal("bar", notifications[1].NewValue); } public class TestClass : AvaloniaObject From c6f0dfdfe3e3d359403f831e30a8017b902022d7 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 25 Mar 2021 14:18:47 +0100 Subject: [PATCH 05/13] Fix nullable warning. --- src/Avalonia.Base/Utilities/AvaloniaPropertyValueStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Base/Utilities/AvaloniaPropertyValueStore.cs b/src/Avalonia.Base/Utilities/AvaloniaPropertyValueStore.cs index 1af6f21156..c513f75962 100644 --- a/src/Avalonia.Base/Utilities/AvaloniaPropertyValueStore.cs +++ b/src/Avalonia.Base/Utilities/AvaloniaPropertyValueStore.cs @@ -94,7 +94,7 @@ namespace Avalonia.Utilities return (0, false); } - public bool TryGetValue(AvaloniaProperty property, [MaybeNull] out TValue value) + public bool TryGetValue(AvaloniaProperty property, [MaybeNullWhen(false)] out TValue value) { (int index, bool found) = TryFindEntry(property.Id); if (!found) From 5bc392ee9579e591fdba2edfd8c974483dca476b Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 25 Mar 2021 15:33:10 +0100 Subject: [PATCH 06/13] Added more failing tests. To test completing/disposing bindings during and while ending batch updates. --- .../AvaloniaObjectTests_BatchUpdate.cs | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs index 036f275a71..53ad87421e 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs @@ -53,6 +53,21 @@ namespace Avalonia.Base.UnitTests Assert.Empty(raised); } + [Fact] + public void Binding_Disposal_Should_Not_Raise_Property_Changes_During_Batch_Update() + { + var target = new TestClass(); + var observable = new TestObservable("foo"); + var raised = new List(); + + var sub = target.Bind(TestClass.FooProperty, observable, BindingPriority.LocalValue); + target.GetObservable(TestClass.FooProperty).Skip(1).Subscribe(x => raised.Add(x)); + target.BeginBatchUpdate(); + sub.Dispose(); + + Assert.Empty(raised); + } + [Fact] public void SetValue_Change_Should_Be_Raised_After_Batch_Update_1() { @@ -240,6 +255,27 @@ namespace Avalonia.Base.UnitTests Assert.Equal(BindingPriority.Unset, raised[0].Priority); } + [Fact] + public void Binding_Disposal_Should_Be_Raised_After_Batch_Update() + { + var target = new TestClass(); + var observable = new TestObservable("foo"); + var raised = new List(); + + var sub = target.Bind(TestClass.FooProperty, observable, BindingPriority.LocalValue); + target.PropertyChanged += (s, e) => raised.Add(e); + + target.BeginBatchUpdate(); + sub.Dispose(); + target.EndBatchUpdate(); + + Assert.Equal(1, raised.Count); + Assert.Null(target.Foo); + Assert.Equal("foo", raised[0].OldValue); + Assert.Null(raised[0].NewValue); + Assert.Equal(BindingPriority.Unset, raised[0].Priority); + } + [Fact] public void ClearValue_Change_Should_Be_Raised_After_Batch_Update_1() { @@ -503,6 +539,38 @@ namespace Avalonia.Base.UnitTests Assert.Equal("bar", notifications[1].NewValue); } + [Fact] + public void Can_Bind_Completed_Binding_Back_To_Original_Value_When_Ending_Batch_Update() + { + var target = new TestClass(); + var raised = 0; + var notifications = new List(); + var observable1 = new TestObservable("foo"); + var observable2 = new TestObservable("foo"); + + target.Bind(TestClass.FooProperty, observable1); + + target.BeginBatchUpdate(); + observable1.OnCompleted(); + target.PropertyChanged += (sender, e) => + { + if (e.Property == TestClass.FooProperty && e.NewValue is null) + { + target.Bind(TestClass.FooProperty, observable2); + ++raised; + } + + notifications.Add(e); + }; + target.EndBatchUpdate(); + + Assert.Equal("foo", target.Foo); + Assert.Equal(1, raised); + Assert.Equal(2, notifications.Count); + Assert.Equal(null, notifications[0].NewValue); + Assert.Equal("foo", notifications[1].NewValue); + } + public class TestClass : AvaloniaObject { public static readonly StyledProperty FooProperty = From d2c38a85266abfb7dcd2ff5c1eead6007e7b49b6 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 25 Mar 2021 15:34:04 +0100 Subject: [PATCH 07/13] Fix more batch update problems. Fixes tests in previous commit which test scenarios where bindings are completed/disposed during and when ending batch updates. --- .../PropertyStore/BindingEntry.cs | 9 ++++++--- src/Avalonia.Base/ValueStore.cs | 19 +++++++++++++------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/BindingEntry.cs b/src/Avalonia.Base/PropertyStore/BindingEntry.cs index 38c1728cd9..3e17a81dd8 100644 --- a/src/Avalonia.Base/PropertyStore/BindingEntry.cs +++ b/src/Avalonia.Base/PropertyStore/BindingEntry.cs @@ -65,7 +65,6 @@ namespace Avalonia.PropertyStore { _subscription?.Dispose(); _subscription = null; - _isSubscribed = false; OnCompleted(); } @@ -74,6 +73,7 @@ namespace Avalonia.PropertyStore var oldValue = _value; _value = default; Priority = BindingPriority.Unset; + _isSubscribed = false; _sink.Completed(Property, this, oldValue); } @@ -104,8 +104,11 @@ namespace Avalonia.PropertyStore public void Start(bool ignoreBatchUpdate) { // We can't use _subscription to check whether we're subscribed because it won't be set - // until Subscribe has finished, which will be too late to prevent reentrancy. - if (!_isSubscribed && (!_batchUpdate || ignoreBatchUpdate)) + // until Subscribe has finished, which will be too late to prevent reentrancy. In addition + // don't re-subscribe to completed/disposed bindings (indicated by Unset priority). + if (!_isSubscribed && + Priority != BindingPriority.Unset && + (!_batchUpdate || ignoreBatchUpdate)) { _isSubscribed = true; _subscription = Source.Subscribe(this); diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 9ece2b8042..8a8e9ad153 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -173,7 +173,7 @@ namespace Avalonia // During batch update values can't be removed immediately because they're needed to raise // a correctly-typed _sink.ValueChanged notification. They instead mark themselves for removal // by setting their priority to Unset. - if (_batchUpdate is null) + if (!IsBatchUpdating()) { _values.Remove(property); } @@ -285,7 +285,7 @@ namespace Avalonia else { var priorityValue = new PriorityValue(_owner, property, this, l); - if (_batchUpdate is object) + if (IsBatchUpdating()) priorityValue.BeginBatchUpdate(); result = priorityValue.SetValue(value, priority); _values.SetValue(property, priorityValue); @@ -311,7 +311,7 @@ namespace Avalonia { priorityValue = new PriorityValue(_owner, property, this, e); - if (_batchUpdate is object) + if (IsBatchUpdating()) { priorityValue.BeginBatchUpdate(); } @@ -338,7 +338,7 @@ namespace Avalonia private void AddValue(AvaloniaProperty property, IValue value) { _values.AddValue(property, value); - if (_batchUpdate?.IsBatchUpdating == true && value is IBatchUpdate batch) + if (IsBatchUpdating() && value is IBatchUpdate batch) batch.BeginBatchUpdate(); value.Start(); } @@ -364,6 +364,8 @@ namespace Avalonia } } + private bool IsBatchUpdating() => _batchUpdate?.IsBatchUpdating == true; + private static bool IsRemoveSentinel(IValue value) { // Local value entries are optimized and contain only a single value field to save space, @@ -447,9 +449,14 @@ namespace Avalonia // During batch update values can't be removed immediately because they're needed to raise // the _sink.ValueChanged notification. They instead mark themselves for removal by setting - // their priority to Unset. - if (slot.Priority == BindingPriority.Unset) + // their priority to Unset. We need to re-read the slot here because raising ValueChanged + // could have caused it to be updated. + if (values.TryGetValue(entry.property, out var updatedSlot) && + updatedSlot.Priority == BindingPriority.Unset) { + if (entry.property.Name == "Transitions" && _owner._owner.GetType().Name == "TabItem") + { + } values.Remove(entry.property); } } From eba052972683abde8f0abbfea28b418ec9091b32 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 25 Mar 2021 16:15:14 +0100 Subject: [PATCH 08/13] Added failing transitions test. --- tests/Avalonia.Animation.UnitTests/AnimatableTests.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs b/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs index 7633a761a3..93b3d2c180 100644 --- a/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs +++ b/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs @@ -330,6 +330,15 @@ namespace Avalonia.Animation.UnitTests } } + [Fact] + public void Transitions_Can_Be_Changed_To_Collection_That_Contains_The_Same_Transitions() + { + var target = CreateTarget(); + var control = CreateControl(target.Object); + + control.Transitions = new Transitions { target.Object }; + } + private static Mock CreateTarget() { return CreateTransition(Visual.OpacityProperty); From 99370bff271f70810049b46ae2032a673e5f79ea Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 25 Mar 2021 16:42:35 +0100 Subject: [PATCH 09/13] Correctly handle changing transitions. Handle changing a `Transitions` collections to new collection which contains some of the same elements. Added a comment explaining why we add new transitions before replacing old transitions; was explained in the commit message for 0694b22c51b96fb8d997ab2dc25a4d5503789a3f. --- src/Avalonia.Animation/Animatable.cs | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index 067d9f462f..b701a101be 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -2,6 +2,7 @@ using System; using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; +using System.Linq; using Avalonia.Data; #nullable enable @@ -93,16 +94,35 @@ namespace Avalonia.Animation var oldTransitions = change.OldValue.GetValueOrDefault(); var newTransitions = change.NewValue.GetValueOrDefault(); + // When transitions are replaced, we add the new transitions before removing the old + // transitions, so that when the old transition being disposed causes the value to + // change, there is a corresponding entry in `_transitionStates`. This means that we + // need to account for any transitions present in both the old and new transitions + // collections. if (newTransitions is object) { + var toAdd = (IList)newTransitions; + + if (newTransitions.Count > 0 && oldTransitions?.Count > 0) + { + toAdd = newTransitions.Except(oldTransitions).ToList(); + } + newTransitions.CollectionChanged += TransitionsCollectionChanged; - AddTransitions(newTransitions); + AddTransitions(toAdd); } if (oldTransitions is object) { + var toRemove = (IList)oldTransitions; + + if (oldTransitions.Count > 0 && newTransitions?.Count > 0) + { + toRemove = oldTransitions.Except(newTransitions).ToList(); + } + oldTransitions.CollectionChanged -= TransitionsCollectionChanged; - RemoveTransitions(oldTransitions); + RemoveTransitions(toRemove); } } else if (_transitionsEnabled && From 15a765490f7a6cb6ca3867a2e310d4d942faca70 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 25 Mar 2021 16:57:21 +0100 Subject: [PATCH 10/13] Added another failing transitions test. --- .../AnimatableTests.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs b/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs index 93b3d2c180..b01fb70f58 100644 --- a/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs +++ b/tests/Avalonia.Animation.UnitTests/AnimatableTests.cs @@ -339,6 +339,28 @@ namespace Avalonia.Animation.UnitTests control.Transitions = new Transitions { target.Object }; } + [Fact] + public void Transitions_Can_Re_Set_During_Batch_Update() + { + var target = CreateTarget(); + var control = CreateControl(target.Object); + + // Assigning and then clearing Transitions ensures we have a transition state + // collection created. + control.Transitions = null; + + control.BeginBatchUpdate(); + + // Setting opacity then Transitions means that we receive the Transitions change + // after the Opacity change when EndBatchUpdate is called. + control.Opacity = 0.5; + control.Transitions = new Transitions { target.Object }; + + // Which means that the transition state hasn't been initialized with the new + // Transitions when the Opacity change notification gets raised here. + control.EndBatchUpdate(); + } + private static Mock CreateTarget() { return CreateTransition(Visual.OpacityProperty); From 66cd58566c83e045f2f63c7f044cff528a2aa773 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 25 Mar 2021 18:09:35 +0100 Subject: [PATCH 11/13] Handle changing transitions during batch update. Fixes failing test from previous commit. --- src/Avalonia.Animation/Animatable.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Animation/Animatable.cs b/src/Avalonia.Animation/Animatable.cs index b701a101be..a415046513 100644 --- a/src/Avalonia.Animation/Animatable.cs +++ b/src/Avalonia.Animation/Animatable.cs @@ -135,9 +135,9 @@ namespace Avalonia.Animation { var transition = Transitions[i]; - if (transition.Property == change.Property) + if (transition.Property == change.Property && + _transitionState.TryGetValue(transition, out var state)) { - var state = _transitionState[transition]; var oldValue = state.BaseValue; var newValue = GetAnimationBaseValue(transition.Property); From f713af3a7c6282f9face859ce5b6cdf57d228e8f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 25 Mar 2021 18:14:31 +0100 Subject: [PATCH 12/13] Make handling of sentinel values consistent. --- .../PropertyStore/ConstantValueEntry.cs | 9 ++++++- src/Avalonia.Base/ValueStore.cs | 27 ++++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/ConstantValueEntry.cs b/src/Avalonia.Base/PropertyStore/ConstantValueEntry.cs index 600d725187..d39fc3bb1e 100644 --- a/src/Avalonia.Base/PropertyStore/ConstantValueEntry.cs +++ b/src/Avalonia.Base/PropertyStore/ConstantValueEntry.cs @@ -6,12 +6,19 @@ using Avalonia.Data; namespace Avalonia.PropertyStore { + /// + /// Represents an untyped interface to . + /// + internal interface IConstantValueEntry : IPriorityValueEntry, IDisposable + { + } + /// /// Stores a value with a priority in a or /// . /// /// The property type. - internal class ConstantValueEntry : IPriorityValueEntry, IDisposable + internal class ConstantValueEntry : IPriorityValueEntry, IConstantValueEntry { private IValueSink _sink; private Optional _value; diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 8a8e9ad153..17b3c7f7f5 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using Avalonia.Data; using Avalonia.PropertyStore; using Avalonia.Utilities; @@ -56,7 +57,7 @@ namespace Avalonia public bool IsAnimating(AvaloniaProperty property) { - if (_values.TryGetValue(property, out var slot)) + if (TryGetValue(property, out var slot)) { return slot.Priority < BindingPriority.LocalValue; } @@ -66,7 +67,7 @@ namespace Avalonia public bool IsSet(AvaloniaProperty property) { - if (_values.TryGetValue(property, out var slot)) + if (TryGetValue(property, out var slot)) { return slot.GetValue().HasValue; } @@ -79,7 +80,7 @@ namespace Avalonia BindingPriority maxPriority, out T value) { - if (_values.TryGetValue(property, out var slot)) + if (TryGetValue(property, out var slot)) { var v = ((IValue)slot).GetValue(maxPriority); @@ -103,7 +104,7 @@ namespace Avalonia IDisposable? result = null; - if (_values.TryGetValue(property, out var slot) && !IsRemoveSentinel(slot)) + if (TryGetValue(property, out var slot)) { result = SetExisting(slot, property, value, priority); } @@ -138,7 +139,7 @@ namespace Avalonia IObservable> source, BindingPriority priority) { - if (_values.TryGetValue(property, out var slot) && !IsRemoveSentinel(slot)) + if (TryGetValue(property, out var slot)) { return BindExisting(slot, property, source, priority); } @@ -160,7 +161,7 @@ namespace Avalonia public void ClearLocalValue(StyledPropertyBase property) { - if (_values.TryGetValue(property, out var slot)) + if (TryGetValue(property, out var slot)) { if (slot is PriorityValue p) { @@ -198,7 +199,7 @@ namespace Avalonia public void CoerceValue(StyledPropertyBase property) { - if (_values.TryGetValue(property, out var slot)) + if (TryGetValue(property, out var slot)) { if (slot is PriorityValue p) { @@ -209,7 +210,7 @@ namespace Avalonia public Diagnostics.AvaloniaPropertyValue? GetDiagnostic(AvaloniaProperty property) { - if (_values.TryGetValue(property, out var slot)) + if (TryGetValue(property, out var slot)) { var slotValue = slot.GetValue(); return new Diagnostics.AvaloniaPropertyValue( @@ -242,6 +243,7 @@ namespace Avalonia IPriorityValueEntry entry, Optional oldValue) { + // We need to include remove sentinels here so call `_values.TryGetValue` directly. if (_values.TryGetValue(property, out var slot) && slot == entry) { if (_batchUpdate is null) @@ -366,12 +368,17 @@ namespace Avalonia private bool IsBatchUpdating() => _batchUpdate?.IsBatchUpdating == true; - private static bool IsRemoveSentinel(IValue value) + private bool TryGetValue(AvaloniaProperty property, [MaybeNullWhen(false)] out IValue value) + { + return _values.TryGetValue(property, out value) && !IsRemoveSentinel(value); + } + + private static bool IsRemoveSentinel(IValue value) { // Local value entries are optimized and contain only a single value field to save space, // so there's no way to mark them for removal at the end of a batch update. Instead a // ConstantValueEntry with a priority of Unset is used as a sentinel value. - return value is ConstantValueEntry t && t.Priority == BindingPriority.Unset; + return value is IConstantValueEntry t && t.Priority == BindingPriority.Unset; } private class BatchUpdate From f325da4f4db7b3f45ab48d4cd3881056d6f5ab64 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 25 Mar 2021 22:11:07 +0100 Subject: [PATCH 13/13] Remove debugging code. --- src/Avalonia.Base/ValueStore.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 17b3c7f7f5..495f13e1a9 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -461,9 +461,6 @@ namespace Avalonia if (values.TryGetValue(entry.property, out var updatedSlot) && updatedSlot.Priority == BindingPriority.Unset) { - if (entry.property.Name == "Transitions" && _owner._owner.GetType().Name == "TabItem") - { - } values.Remove(entry.property); } }