From 27daffeb3e963c895f70f4387d71a5331b41b36c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 26 Jun 2018 02:30:47 +0200 Subject: [PATCH 1/4] Abstracted storage of styled values into ValueStore. --- src/Avalonia.Base/AvaloniaObject.cs | 124 +++------------- src/Avalonia.Base/IPriorityValueOwner.cs | 9 +- src/Avalonia.Base/PriorityValue.cs | 4 +- src/Avalonia.Base/ValueStore.cs | 134 ++++++++++++++++++ .../PriorityValueTests.cs | 4 +- 5 files changed, 161 insertions(+), 114 deletions(-) create mode 100644 src/Avalonia.Base/ValueStore.cs diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index baca1494be8..1b7bd91a8fe 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -32,8 +32,7 @@ public class AvaloniaObject : IAvaloniaObject, IAvaloniaObjectDebug, INotifyProp /// /// The set values/bindings on this object. /// - private readonly Dictionary _values = - new Dictionary(); + private readonly ValueStore _values; /// /// Maintains a list of direct property binding subscriptions so that the binding source @@ -73,6 +72,8 @@ public AvaloniaObject() { VerifyAccess(); + _values = new ValueStore(this); + void Notify(AvaloniaProperty property) { object value = property.IsDirect ? @@ -230,7 +231,7 @@ public object GetValue(AvaloniaProperty property) } else { - return GetValueInternal(property); + return _values.GetValue(property); } } @@ -257,7 +258,7 @@ public bool IsAnimating(AvaloniaProperty property) Contract.Requires(property != null); VerifyAccess(); - return _values.TryGetValue(property, out PriorityValue value) ? value.IsAnimating : false; + return _values.IsAnimating(property); } /// @@ -274,14 +275,7 @@ public bool IsSet(AvaloniaProperty property) Contract.Requires(property != null); VerifyAccess(); - PriorityValue value; - - if (_values.TryGetValue(property, out value)) - { - return value.Value != AvaloniaProperty.UnsetValue; - } - - return false; + return _values.IsSet(property); } /// @@ -369,14 +363,6 @@ public IDisposable Bind( } else { - PriorityValue v; - - if (!_values.TryGetValue(property, out v)) - { - v = CreatePriorityValue(property); - _values.Add(property, v); - } - Logger.Verbose( LogArea.Property, this, @@ -385,7 +371,7 @@ public IDisposable Bind( description, priority); - return v.Add(source, (int)priority); + return _values.AddBinding(property, source, priority); } } @@ -416,20 +402,12 @@ public IDisposable Bind( public void Revalidate(AvaloniaProperty property) { VerifyAccess(); - PriorityValue value; - - if (_values.TryGetValue(property, out value)) - { - value.Revalidate(); - } + _values.Revalidate(property); } /// - void IPriorityValueOwner.Changed(PriorityValue sender, object oldValue, object newValue) + void IPriorityValueOwner.Changed(AvaloniaProperty property, int priority, object oldValue, object newValue) { - var property = sender.Property; - var priority = (BindingPriority)sender.ValuePriority; - oldValue = (oldValue == AvaloniaProperty.UnsetValue) ? GetDefaultValue(property) : oldValue; @@ -439,7 +417,7 @@ void IPriorityValueOwner.Changed(PriorityValue sender, object oldValue, object n if (!Equals(oldValue, newValue)) { - RaisePropertyChanged(property, oldValue, newValue, priority); + RaisePropertyChanged(property, oldValue, newValue, (BindingPriority)priority); Logger.Verbose( LogArea.Property, @@ -448,14 +426,14 @@ void IPriorityValueOwner.Changed(PriorityValue sender, object oldValue, object n property, oldValue, newValue, - priority); + (BindingPriority)priority); } } /// - void IPriorityValueOwner.BindingNotificationReceived(PriorityValue sender, BindingNotification notification) + void IPriorityValueOwner.BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) { - UpdateDataValidation(sender.Property, notification); + UpdateDataValidation(property, notification); } /// @@ -468,10 +446,7 @@ Delegate[] IAvaloniaObjectDebug.GetPropertyChangedSubscribers() /// Gets all priority values set on the object. /// /// A collection of property/value tuples. - internal IDictionary GetSetValues() - { - return _values; - } + internal IDictionary GetSetValues() => _values.GetSetValues(); /// /// Forces revalidation of properties when a property value changes. @@ -660,68 +635,18 @@ private static object CastOrDefault(object value, Type type) } } - /// - /// Creates a for a . - /// - /// The property. - /// The . - private PriorityValue CreatePriorityValue(AvaloniaProperty property) - { - var validate = ((IStyledPropertyAccessor)property).GetValidationFunc(GetType()); - Func validate2 = null; - - if (validate != null) - { - validate2 = v => validate(this, v); - } - - PriorityValue result = new PriorityValue( - this, - property, - property.PropertyType, - validate2); - - return result; - } - /// /// Gets the default value for a property. /// /// The property. /// The default value. - private object GetDefaultValue(AvaloniaProperty property) + internal object GetDefaultValue(AvaloniaProperty property) { if (property.Inherits && InheritanceParent is AvaloniaObject aobj) - return aobj.GetValueInternal(property); + return aobj._values.GetValue(property); return ((IStyledPropertyAccessor) property).GetDefaultValue(GetType()); } - /// - /// Gets a value - /// without check for registered as this can slow getting the value - /// this method is intended for internal usage in AvaloniaObject only - /// it's called only after check the property is registered - /// - /// The property. - /// The value. - private object GetValueInternal(AvaloniaProperty property) - { - object result = AvaloniaProperty.UnsetValue; - PriorityValue value; - - if (_values.TryGetValue(property, out value)) - { - result = value.Value; - } - - if (result == AvaloniaProperty.UnsetValue) - { - result = GetDefaultValue(property); - } - - return result; - } - /// /// Sets the value of a direct property. /// @@ -801,22 +726,9 @@ private void SetStyledValue(AvaloniaProperty property, object value, BindingPrio originalValue, originalValue?.GetType().FullName ?? "(null)")); } - - PriorityValue v; - - if (!_values.TryGetValue(property, out v)) - { - if (value == AvaloniaProperty.UnsetValue) - { - return; - } - - v = CreatePriorityValue(property); - _values.Add(property, v); - } - + LogPropertySet(property, value, priority); - v.SetValue(value, (int)priority); + _values.AddValue(property, value, (int)priority); } /// diff --git a/src/Avalonia.Base/IPriorityValueOwner.cs b/src/Avalonia.Base/IPriorityValueOwner.cs index aeec7209204..5f63f6ef91d 100644 --- a/src/Avalonia.Base/IPriorityValueOwner.cs +++ b/src/Avalonia.Base/IPriorityValueOwner.cs @@ -13,18 +13,19 @@ internal interface IPriorityValueOwner /// /// Called when a 's value changes. /// - /// The source of the change. + /// The the property that has changed. + /// The priority of the value. /// The old value. /// The new value. - void Changed(PriorityValue sender, object oldValue, object newValue); + void Changed(AvaloniaProperty property, int priority, object oldValue, object newValue); /// /// Called when a is received by a /// . /// - /// The source of the change. + /// The the property that has changed. /// The notification. - void BindingNotificationReceived(PriorityValue sender, BindingNotification notification); + void BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification); /// /// Ensures that the current thread is the UI thread. diff --git a/src/Avalonia.Base/PriorityValue.cs b/src/Avalonia.Base/PriorityValue.cs index 9b5318083a8..c474f9098e4 100644 --- a/src/Avalonia.Base/PriorityValue.cs +++ b/src/Avalonia.Base/PriorityValue.cs @@ -281,12 +281,12 @@ private bool UpdateCore( if (notification == null || notification.HasValue) { - notify(() => Owner?.Changed(this, old, Value)); + notify(() => Owner?.Changed(Property, ValuePriority, old, Value)); } if (notification != null) { - Owner?.BindingNotificationReceived(this, notification); + Owner?.BindingNotificationReceived(Property, notification); } } else diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs new file mode 100644 index 00000000000..562cc165df2 --- /dev/null +++ b/src/Avalonia.Base/ValueStore.cs @@ -0,0 +1,134 @@ +using System; +using System.Collections.Generic; +using System.Text; +using Avalonia.Data; +using Avalonia.Utilities; + +namespace Avalonia +{ + internal class ValueStore : IPriorityValueOwner + { + private readonly AvaloniaObject _owner; + private readonly Dictionary _values = + new Dictionary(); + + public ValueStore(AvaloniaObject owner) + { + _owner = owner; + } + + public IDisposable AddBinding( + AvaloniaProperty property, + IObservable source, + BindingPriority priority) + { + if (!_values.TryGetValue(property, out PriorityValue v)) + { + v = CreatePriorityValue(property); + _values.Add(property, v); + } + + return v.Add(source, (int)priority); + } + + public void AddValue(AvaloniaProperty property, object value, int priority) + { + var originalValue = value; + + if (!TypeUtilities.TryConvertImplicit(property.PropertyType, value, out value)) + { + throw new ArgumentException(string.Format( + "Invalid value for Property '{0}': '{1}' ({2})", + property.Name, + originalValue, + originalValue?.GetType().FullName ?? "(null)")); + } + + if (!_values.TryGetValue(property, out PriorityValue v)) + { + if (value == AvaloniaProperty.UnsetValue) + { + return; + } + + v = CreatePriorityValue(property); + _values.Add(property, v); + } + + v.SetValue(value, priority); + } + + public IDictionary GetSetValues() => _values; + + public object GetValue(AvaloniaProperty property) + { + var result = AvaloniaProperty.UnsetValue; + + if (_values.TryGetValue(property, out PriorityValue value)) + { + result = value.Value; + } + + if (result == AvaloniaProperty.UnsetValue) + { + result = _owner.GetDefaultValue(property); + } + + return result; + } + + public bool IsAnimating(AvaloniaProperty property) + { + return _values.TryGetValue(property, out PriorityValue value) ? value.IsAnimating : false; + } + + public bool IsSet(AvaloniaProperty property) + { + if (_values.TryGetValue(property, out PriorityValue value)) + { + return value.Value != AvaloniaProperty.UnsetValue; + } + + return false; + } + + public void Revalidate(AvaloniaProperty property) + { + if (_values.TryGetValue(property, out PriorityValue value)) + { + value.Revalidate(); + } + } + + void IPriorityValueOwner.BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) + { + ((IPriorityValueOwner)_owner).BindingNotificationReceived(property, notification); + } + + void IPriorityValueOwner.Changed(AvaloniaProperty property, int priority, object oldValue, object newValue) + { + ((IPriorityValueOwner)_owner).Changed(property, priority, oldValue, newValue); + } + + void IPriorityValueOwner.VerifyAccess() => _owner.VerifyAccess(); + + private PriorityValue CreatePriorityValue(AvaloniaProperty property) + { + var validate = ((IStyledPropertyAccessor)property).GetValidationFunc(_owner.GetType()); + Func validate2 = null; + + if (validate != null) + { + validate2 = v => validate(_owner, v); + } + + PriorityValue result = new PriorityValue( + this, + property, + property.PropertyType, + validate2); + + return result; + } + } +} diff --git a/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs b/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs index 6ca1364f968..d6650aa1048 100644 --- a/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs +++ b/tests/Avalonia.Base.UnitTests/PriorityValueTests.cs @@ -167,7 +167,7 @@ public void Adding_Value_Should_Call_OnNext() target.Add(Single("foo"), 0); - owner.Verify(x => x.Changed(target, AvaloniaProperty.UnsetValue, "foo")); + owner.Verify(x => x.Changed(target.Property, target.ValuePriority, AvaloniaProperty.UnsetValue, "foo")); } [Fact] @@ -180,7 +180,7 @@ public void Changing_Value_Should_Call_OnNext() target.Add(subject, 0); subject.OnNext("bar"); - owner.Verify(x => x.Changed(target, "foo", "bar")); + owner.Verify(x => x.Changed(target.Property, target.ValuePriority, "foo", "bar")); } [Fact] From 8ad680187af64772253262f655a1ea3051584829 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 26 Jun 2018 19:59:01 +0200 Subject: [PATCH 2/4] Store LocalValues in value store as plain values. Instead of using a `PriorityValue`, when a property is assigned a simple `LocalValue` just store the value directly in the value store. --- src/Avalonia.Base/ValueStore.cs | 121 ++++++++++++++++++++++---------- 1 file changed, 82 insertions(+), 39 deletions(-) diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 562cc165df2..e1e399b4b31 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -1,16 +1,14 @@ using System; using System.Collections.Generic; -using System.Text; using Avalonia.Data; -using Avalonia.Utilities; namespace Avalonia { internal class ValueStore : IPriorityValueOwner { private readonly AvaloniaObject _owner; - private readonly Dictionary _values = - new Dictionary(); + private readonly Dictionary _values = + new Dictionary(); public ValueStore(AvaloniaObject owner) { @@ -22,51 +20,94 @@ public IDisposable AddBinding( IObservable source, BindingPriority priority) { - if (!_values.TryGetValue(property, out PriorityValue v)) + PriorityValue priorityValue; + + if (_values.TryGetValue(property, out var v)) + { + priorityValue = v as PriorityValue; + + if (priorityValue == null) + { + priorityValue = CreatePriorityValue(property); + priorityValue.SetValue(v, (int)BindingPriority.LocalValue); + _values[property] = priorityValue; + } + } + else { - v = CreatePriorityValue(property); - _values.Add(property, v); + priorityValue = CreatePriorityValue(property); + _values.Add(property, priorityValue); } - return v.Add(source, (int)priority); + return priorityValue.Add(source, (int)priority); } public void AddValue(AvaloniaProperty property, object value, int priority) { - var originalValue = value; + PriorityValue priorityValue; - if (!TypeUtilities.TryConvertImplicit(property.PropertyType, value, out value)) + if (_values.TryGetValue(property, out var v)) { - throw new ArgumentException(string.Format( - "Invalid value for Property '{0}': '{1}' ({2})", - property.Name, - originalValue, - originalValue?.GetType().FullName ?? "(null)")); - } + priorityValue = v as PriorityValue; - if (!_values.TryGetValue(property, out PriorityValue v)) + if (priorityValue == null) + { + if (priority == (int)BindingPriority.LocalValue) + { + _values[property] = Validate(property, value); + Changed(property, priority, v, value); + return; + } + else + { + priorityValue = CreatePriorityValue(property); + priorityValue.SetValue(v, (int)BindingPriority.LocalValue); + _values[property] = priorityValue; + } + } + } + else { if (value == AvaloniaProperty.UnsetValue) { return; } - v = CreatePriorityValue(property); - _values.Add(property, v); + if (priority == (int)BindingPriority.LocalValue) + { + _values.Add(property, Validate(property, value)); + Changed(property, priority, AvaloniaProperty.UnsetValue, value); + return; + } + else + { + priorityValue = CreatePriorityValue(property); + _values.Add(property, priorityValue); + } } - v.SetValue(value, priority); + priorityValue.SetValue(value, priority); } - public IDictionary GetSetValues() => _values; + public void BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) + { + ((IPriorityValueOwner)_owner).BindingNotificationReceived(property, notification); + } + + public void Changed(AvaloniaProperty property, int priority, object oldValue, object newValue) + { + ((IPriorityValueOwner)_owner).Changed(property, priority, oldValue, newValue); + } + + public IDictionary GetSetValues() => throw new NotImplementedException(); public object GetValue(AvaloniaProperty property) { var result = AvaloniaProperty.UnsetValue; - if (_values.TryGetValue(property, out PriorityValue value)) + if (_values.TryGetValue(property, out var value)) { - result = value.Value; + result = (value is PriorityValue priorityValue) ? priorityValue.Value : value; } if (result == AvaloniaProperty.UnsetValue) @@ -79,14 +120,14 @@ public object GetValue(AvaloniaProperty property) public bool IsAnimating(AvaloniaProperty property) { - return _values.TryGetValue(property, out PriorityValue value) ? value.IsAnimating : false; + return _values.TryGetValue(property, out var value) ? (value as PriorityValue)?.IsAnimating ?? false : false; } public bool IsSet(AvaloniaProperty property) { - if (_values.TryGetValue(property, out PriorityValue value)) + if (_values.TryGetValue(property, out var value)) { - return value.Value != AvaloniaProperty.UnsetValue; + return ((value as PriorityValue)?.Value ?? value) != AvaloniaProperty.UnsetValue; } return false; @@ -94,23 +135,13 @@ public bool IsSet(AvaloniaProperty property) public void Revalidate(AvaloniaProperty property) { - if (_values.TryGetValue(property, out PriorityValue value)) + if (_values.TryGetValue(property, out var value)) { - value.Revalidate(); + (value as PriorityValue)?.Revalidate(); } } - void IPriorityValueOwner.BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) - { - ((IPriorityValueOwner)_owner).BindingNotificationReceived(property, notification); - } - - void IPriorityValueOwner.Changed(AvaloniaProperty property, int priority, object oldValue, object newValue) - { - ((IPriorityValueOwner)_owner).Changed(property, priority, oldValue, newValue); - } - - void IPriorityValueOwner.VerifyAccess() => _owner.VerifyAccess(); + public void VerifyAccess() => _owner.VerifyAccess(); private PriorityValue CreatePriorityValue(AvaloniaProperty property) { @@ -130,5 +161,17 @@ private PriorityValue CreatePriorityValue(AvaloniaProperty property) return result; } + + private object Validate(AvaloniaProperty property, object value) + { + var validate = ((IStyledPropertyAccessor)property).GetValidationFunc(_owner.GetType()); + + if (validate != null && value != AvaloniaProperty.UnsetValue) + { + return validate(_owner, value); + } + + return value; + } } } From b00350658a5ee545516a37ab505913ec27398798 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 26 Jun 2018 20:13:31 +0200 Subject: [PATCH 3/4] Lazily create ValueStore. --- src/Avalonia.Base/AvaloniaObject.cs | 36 ++++++++++++++++++----------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 1b7bd91a8fe..ee60bf5b788 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -29,11 +29,6 @@ public class AvaloniaObject : IAvaloniaObject, IAvaloniaObjectDebug, INotifyProp /// private IAvaloniaObject _inheritanceParent; - /// - /// The set values/bindings on this object. - /// - private readonly ValueStore _values; - /// /// Maintains a list of direct property binding subscriptions so that the binding source /// doesn't get collected. @@ -51,6 +46,7 @@ public class AvaloniaObject : IAvaloniaObject, IAvaloniaObjectDebug, INotifyProp private EventHandler _propertyChanged; private DeferredSetter _directDeferredSetter; + private ValueStore _values; /// /// Delayed setter helper for direct properties. Used to fix #855. @@ -72,8 +68,6 @@ public AvaloniaObject() { VerifyAccess(); - _values = new ValueStore(this); - void Notify(AvaloniaProperty property) { object value = property.IsDirect ? @@ -229,10 +223,14 @@ public object GetValue(AvaloniaProperty property) { return ((IDirectPropertyAccessor)GetRegistered(property)).GetValue(this); } - else + else if (_values != null) { return _values.GetValue(property); } + else + { + return GetDefaultValue(property); + } } /// @@ -258,7 +256,7 @@ public bool IsAnimating(AvaloniaProperty property) Contract.Requires(property != null); VerifyAccess(); - return _values.IsAnimating(property); + return _values?.IsAnimating(property) ?? false; } /// @@ -275,7 +273,7 @@ public bool IsSet(AvaloniaProperty property) Contract.Requires(property != null); VerifyAccess(); - return _values.IsSet(property); + return _values?.IsSet(property) ?? false; } /// @@ -371,6 +369,11 @@ public IDisposable Bind( description, priority); + if (_values == null) + { + _values = new ValueStore(this); + } + return _values.AddBinding(property, source, priority); } } @@ -402,7 +405,7 @@ public IDisposable Bind( public void Revalidate(AvaloniaProperty property) { VerifyAccess(); - _values.Revalidate(property); + _values?.Revalidate(property); } /// @@ -446,7 +449,7 @@ Delegate[] IAvaloniaObjectDebug.GetPropertyChangedSubscribers() /// Gets all priority values set on the object. /// /// A collection of property/value tuples. - internal IDictionary GetSetValues() => _values.GetSetValues(); + internal IDictionary GetSetValues() => _values?.GetSetValues(); /// /// Forces revalidation of properties when a property value changes. @@ -642,7 +645,7 @@ private static object CastOrDefault(object value, Type type) /// The default value. internal object GetDefaultValue(AvaloniaProperty property) { - if (property.Inherits && InheritanceParent is AvaloniaObject aobj) + if (property.Inherits && InheritanceParent is AvaloniaObject aobj && aobj._values != null) return aobj._values.GetValue(property); return ((IStyledPropertyAccessor) property).GetDefaultValue(GetType()); } @@ -726,7 +729,12 @@ private void SetStyledValue(AvaloniaProperty property, object value, BindingPrio originalValue, originalValue?.GetType().FullName ?? "(null)")); } - + + if (_values == null) + { + _values = new ValueStore(this); + } + LogPropertySet(property, value, priority); _values.AddValue(property, value, (int)priority); } From cbc0755098ff4beefc223a051821fe5dbc7af2e2 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 26 Jun 2018 22:55:12 +0200 Subject: [PATCH 4/4] Move default value handling to AvaloniaObject. --- src/Avalonia.Base/AvaloniaObject.cs | 13 ++++++++++--- src/Avalonia.Base/ValueStore.cs | 5 ----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index ee60bf5b788..761c0618daa 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -225,7 +225,14 @@ public object GetValue(AvaloniaProperty property) } else if (_values != null) { - return _values.GetValue(property); + var result = _values.GetValue(property); + + if (result == AvaloniaProperty.UnsetValue) + { + result = GetDefaultValue(property); + } + + return result; } else { @@ -645,8 +652,8 @@ private static object CastOrDefault(object value, Type type) /// The default value. internal object GetDefaultValue(AvaloniaProperty property) { - if (property.Inherits && InheritanceParent is AvaloniaObject aobj && aobj._values != null) - return aobj._values.GetValue(property); + if (property.Inherits && InheritanceParent is AvaloniaObject aobj) + return aobj.GetValue(property); return ((IStyledPropertyAccessor) property).GetDefaultValue(GetType()); } diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index e1e399b4b31..8283edab806 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -110,11 +110,6 @@ public object GetValue(AvaloniaProperty property) result = (value is PriorityValue priorityValue) ? priorityValue.Value : value; } - if (result == AvaloniaProperty.UnsetValue) - { - result = _owner.GetDefaultValue(property); - } - return result; }