Browse Source

Refactor binding IValueEntry into base class.

Too many gotchas in implementing binding entries, we'll take the performance hit of a few virtual method calls.
Steven Kirk 3 years ago
parent
commit
9367f8b24d

+ 15 - 2
src/Avalonia.Base/Data/BindingValue.cs

@@ -237,6 +237,19 @@ namespace Avalonia.Data
         /// <param name="value">The untyped value.</param>
         /// <returns>The typed binding value.</returns>
         public static BindingValue<T> FromUntyped(object? value)
+        {
+            return FromUntyped(value, typeof(T));
+        }
+
+        /// <summary>
+        /// Creates a <see cref="BindingValue{T}"/> from an object, handling the special values
+        /// <see cref="AvaloniaProperty.UnsetValue"/>, <see cref="BindingOperations.DoNothing"/> and
+        /// <see cref="BindingNotification"/>.
+        /// </summary>
+        /// <param name="value">The untyped value.</param>
+        /// <param name="targetType">The runtime target type.</param>
+        /// <returns>The typed binding value.</returns>
+        public static BindingValue<T> FromUntyped(object? value, Type targetType)
         {
             if (value == AvaloniaProperty.UnsetValue)
                 return Unset;
@@ -265,13 +278,13 @@ namespace Avalonia.Data
 
             if ((type & BindingValueType.HasValue) != 0)
             {
-                if (TypeUtilities.TryConvertImplicit(typeof(T), value, out var typed))
+                if (TypeUtilities.TryConvertImplicit(targetType, value, out var typed))
                     v = (T)typed!;
                 else
                 {
                     var e = new InvalidCastException(
                         $"Unable to convert object '{value ?? "(null)"}' " +
-                        $"of type '{value?.GetType()}' to type '{typeof(T)}'.");
+                        $"of type '{value?.GetType()}' to type '{targetType}'.");
 
                     if (error is null)
                         error = e;

+ 0 - 136
src/Avalonia.Base/PropertyStore/BindingEntry.cs

@@ -1,136 +0,0 @@
-using System;
-using System.Reactive.Disposables;
-using Avalonia.Data;
-
-namespace Avalonia.PropertyStore
-{
-    internal class BindingEntry : IValueEntry,
-        IObserver<object?>,
-        IDisposable
-    {
-        private static IDisposable s_Creating = Disposable.Empty;
-        private static IDisposable s_CreatingQuiet = Disposable.Create(() => { });
-        private readonly ValueFrame _frame;
-        private IDisposable? _subscription;
-        private bool _hasValue;
-        private object? _value;
-
-        public BindingEntry(
-            ValueFrame frame,
-            AvaloniaProperty property,
-            IObservable<object?> source)
-        {
-            _frame = frame;
-            Source = source;
-            Property = property;
-        }
-
-        public bool HasValue
-        {
-            get
-            {
-                Start(produceValue: false);
-                return _hasValue;
-            }
-        }
-
-        public AvaloniaProperty Property { get; }
-        protected IObservable<object?> Source { get; }
-
-        public void Dispose()
-        {
-            Unsubscribe();
-            BindingCompleted();
-        }
-
-        public object? GetValue()
-        {
-            Start(produceValue: false);
-            if (!_hasValue)
-                throw new AvaloniaInternalException("The binding entry has no value.");
-            return _value!;
-        }
-
-        public bool TryGetValue(out object? value)
-        {
-            Start(produceValue: false);
-            value = _value;
-            return _hasValue;
-        }
-
-        public void Start() => Start(true);
-        public void OnCompleted() => BindingCompleted();
-        public void OnError(Exception error) => BindingCompleted();
-
-        public void OnNext(object? value) => SetValue(value);
-
-        public virtual void Unsubscribe()
-        {
-            _subscription?.Dispose();
-            _subscription = null;
-        }
-
-        protected virtual void Start(bool produceValue)
-        {
-            if (_subscription is not null)
-                return;
-            _subscription = produceValue ? s_Creating : s_CreatingQuiet;
-            _subscription = Source.Subscribe(this);
-        }
-
-        private void ClearValue()
-        {
-            if (_hasValue)
-            {
-                _hasValue = false;
-                _value = default;
-
-                if (_subscription is not null && _subscription != s_CreatingQuiet)
-                    _frame.Owner?.OnBindingValueCleared(Property, _frame.Priority);
-            }
-        }
-
-        private void SetValue(object? value)
-        {
-            if (_frame.Owner is null)
-                return;
-
-            if (value is BindingNotification n)
-            {
-                value = n.Value;
-                LoggingUtils.LogIfNecessary(_frame.Owner.Owner, Property, n);
-            }
-
-            if (value == AvaloniaProperty.UnsetValue)
-            {
-                ClearValue();
-            }
-            else if (value == BindingOperations.DoNothing)
-            {
-                // Do nothing!
-            }
-            else if (UntypedValueUtils.TryConvertAndValidate(Property, value, out var typedValue))
-            {
-                if (!_hasValue || !Equals(_value, typedValue))
-                {
-                    _value = typedValue;
-                    _hasValue = true;
-
-                    if (_subscription is not null && _subscription != s_CreatingQuiet)
-                        _frame.Owner?.OnBindingValueChanged(Property, _frame.Priority, typedValue);
-                }
-            }
-            else
-            {
-                ClearValue();
-                LoggingUtils.LogInvalidValue(_frame.Owner.Owner, Property, Property.PropertyType, value);
-            }
-        }
-
-        private void BindingCompleted()
-        {
-            _subscription = null;
-            _frame.OnBindingCompleted(this);
-        }
-    }
-}

+ 51 - 54
src/Avalonia.Base/PropertyStore/BindingEntry`1.cs → src/Avalonia.Base/PropertyStore/BindingEntryBase.cs

@@ -6,36 +6,35 @@ using Avalonia.Data;
 
 namespace Avalonia.PropertyStore
 {
-    internal class BindingEntry<T> : IValueEntry<T>,
-        IObserver<T>,
-        IObserver<BindingValue<T>>,
+    internal abstract class BindingEntryBase<TValue, TSource> : IValueEntry,
+        IObserver<TSource>,
+        IObserver<BindingValue<TSource>>,
         IDisposable
     {
-        private static IDisposable s_Creating = Disposable.Empty;
-        private static IDisposable s_CreatingQuiet = Disposable.Create(() => { });
+        private static IDisposable s_creating = Disposable.Empty;
+        private static IDisposable s_creatingQuiet = Disposable.Create(() => { });
         private readonly ValueFrame _frame;
-        private readonly object _source;
         private IDisposable? _subscription;
         private bool _hasValue;
-        private T? _value;
+        private TValue? _value;
 
-        public BindingEntry(
+        protected BindingEntryBase(
             ValueFrame frame,
-            StyledPropertyBase<T> property,
-            IObservable<BindingValue<T>> source)
+            AvaloniaProperty property,
+            IObservable<BindingValue<TSource>> source)
         {
             _frame = frame;
-            _source = source;
+            Source = source;
             Property = property;
         }
 
-        public BindingEntry(
+        protected BindingEntryBase(
             ValueFrame frame,
-            StyledPropertyBase<T> property,
-            IObservable<T> source)
+            AvaloniaProperty property,
+            IObservable<TSource> source)
         {
             _frame = frame;
-            _source = source;
+            Source = source;
             Property = property;
         }
 
@@ -48,8 +47,10 @@ namespace Avalonia.PropertyStore
             }
         }
 
-        public StyledPropertyBase<T> Property { get; }
+        public bool IsSubscribed => _subscription is not null;
+        public AvaloniaProperty Property { get; }
         AvaloniaProperty IValueEntry.Property => Property;
+        protected object Source { get; }
 
         public void Dispose()
         {
@@ -57,7 +58,7 @@ namespace Avalonia.PropertyStore
             BindingCompleted();
         }
 
-        public T GetValue()
+        public TValue GetValue()
         {
             Start(produceValue: false);
             if (!_hasValue)
@@ -67,7 +68,7 @@ namespace Avalonia.PropertyStore
 
         public void Start() => Start(true);
 
-        public bool TryGetValue([MaybeNullWhen(false)] out T value)
+        public bool TryGetValue([MaybeNullWhen(false)] out TValue value)
         {
             Start(produceValue: false);
             value = _value;
@@ -76,21 +77,10 @@ namespace Avalonia.PropertyStore
 
         public void OnCompleted() => BindingCompleted();
         public void OnError(Exception error) => BindingCompleted();
+        public void OnNext(TSource value) => SetValue(ConvertAndValidate(value));
+        public void OnNext(BindingValue<TSource> value) => SetValue(ConvertAndValidate(value));
 
-        public void OnNext(T value) => SetValue(value);
-
-        public void OnNext(BindingValue<T> value)
-        {
-            if (_frame.Owner is not null)
-                LoggingUtils.LogIfNecessary(_frame.Owner.Owner, Property, value);
-
-            if (value.HasValue)
-                SetValue(value.Value);
-            else
-                ClearValue();
-        }
-
-        public void Unsubscribe()
+        public virtual void Unsubscribe()
         {
             _subscription?.Dispose();
             _subscription = null;
@@ -111,6 +101,23 @@ namespace Avalonia.PropertyStore
             return _hasValue;
         }
 
+        protected abstract BindingValue<TValue> ConvertAndValidate(TSource value);
+        protected abstract BindingValue<TValue> ConvertAndValidate(BindingValue<TSource> value);
+
+        protected virtual void Start(bool produceValue)
+        {
+            if (_subscription is not null)
+                return;
+
+            _subscription = produceValue ? s_creating : s_creatingQuiet;
+            _subscription = Source switch
+            {
+                IObservable<BindingValue<TSource>> bv => bv.Subscribe(this),
+                IObservable<TSource> b => b.Subscribe(this),
+                _ => throw new AvaloniaInternalException("Unexpected binding source."),
+            };
+        }
+
         private void ClearValue()
         {
             if (_hasValue)
@@ -122,24 +129,28 @@ namespace Avalonia.PropertyStore
             }
         }
 
-        private void SetValue(T value)
+        private void SetValue(BindingValue<TValue> value)
         {
             if (_frame.Owner is null)
                 return;
 
-            if (Property.ValidateValue?.Invoke(value) != false)
+            LoggingUtils.LogIfNecessary(_frame.Owner.Owner, Property, value);
+
+            if (value.HasValue)
             {
-                if (!_hasValue || !EqualityComparer<T>.Default.Equals(_value, value))
+                if (!_hasValue || !EqualityComparer<TValue>.Default.Equals(_value, value.Value))
                 {
-                    _value = value;
+                    _value = value.Value;
                     _hasValue = true;
-                    if (_subscription is not null && _subscription != s_CreatingQuiet)
-                        _frame.Owner?.OnBindingValueChanged(Property, _frame.Priority, value);
+                    if (_subscription is not null && _subscription != s_creatingQuiet)
+                        _frame.Owner?.OnBindingValueChanged(Property, _frame.Priority, value.Value);
                 }
             }
-            else if (_subscription is not null && _subscription != s_CreatingQuiet)
+            else if (value.Type != BindingValueType.DoNothing)
             {
-                _frame.Owner?.OnBindingValueCleared(Property, _frame.Priority);
+                ClearValue();
+                if (_subscription is not null && _subscription != s_creatingQuiet)
+                    _frame.Owner?.OnBindingValueCleared(Property, _frame.Priority);
             }
         }
 
@@ -148,19 +159,5 @@ namespace Avalonia.PropertyStore
             _subscription = null;
             _frame.OnBindingCompleted(this);
         }
-
-        private void Start(bool produceValue)
-        {
-            if (_subscription is not null)
-                return;
-
-            _subscription = produceValue ? s_Creating : s_CreatingQuiet;
-            _subscription = _source switch
-            {
-                IObservable<BindingValue<T>> bv => bv.Subscribe(this),
-                IObservable<T> b => b.Subscribe(this),
-                _ => throw new AvaloniaInternalException("Unexpected binding source."),
-            };
-        }
     }
 }

+ 6 - 6
src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs

@@ -16,29 +16,29 @@ namespace Avalonia.PropertyStore
 
         public override bool IsActive => true;
 
-        public BindingEntry<T> AddBinding<T>(
+        public TypedBindingEntry<T> AddBinding<T>(
             StyledPropertyBase<T> property,
             IObservable<BindingValue<T>> source)
         {
-            var e = new BindingEntry<T>(this, property, source);
+            var e = new TypedBindingEntry<T>(this, property, source);
             Add(e);
             return e;
         }
 
-        public BindingEntry<T> AddBinding<T>(
+        public TypedBindingEntry<T> AddBinding<T>(
             StyledPropertyBase<T> property,
             IObservable<T> source)
         {
-            var e = new BindingEntry<T>(this, property, source);
+            var e = new TypedBindingEntry<T>(this, property, source);
             Add(e);
             return e;
         }
 
-        public UntypedBindingEntry<T> AddBinding<T>(
+        public SourceUntypedBindingEntry<T> AddBinding<T>(
             StyledPropertyBase<T> property,
             IObservable<object?> source)
         {
-            var e = new UntypedBindingEntry<T>(this, property, source);
+            var e = new SourceUntypedBindingEntry<T>(this, property, source);
             Add(e);
             return e;
         }

+ 36 - 0
src/Avalonia.Base/PropertyStore/SourceUntypedBindingEntry.cs

@@ -0,0 +1,36 @@
+using System;
+using Avalonia.Data;
+
+namespace Avalonia.PropertyStore
+{
+    /// <summary>
+    /// An <see cref="IValueEntry"/> that holds a binding whose source observable is untyped and
+    /// target property is typed.
+    /// </summary>
+    internal sealed class SourceUntypedBindingEntry<TTarget> : BindingEntryBase<TTarget, object?>,
+        IValueEntry<TTarget>
+    {
+        private readonly Func<TTarget, bool>? _validate;
+
+        public SourceUntypedBindingEntry(
+            ValueFrame frame, 
+            StyledPropertyBase<TTarget> property,
+            IObservable<object?> source)
+                : base(frame, property, source)
+        {
+            _validate = property.ValidateValue;
+        }
+
+        public new StyledPropertyBase<TTarget> Property => (StyledPropertyBase<TTarget>)base.Property;
+
+        protected override BindingValue<TTarget> ConvertAndValidate(object? value)
+        {
+            return UntypedValueUtils.ConvertAndValidate(value, Property.PropertyType, _validate);
+        }
+
+        protected override BindingValue<TTarget> ConvertAndValidate(BindingValue<object?> value)
+        {
+            throw new NotSupportedException();
+        }
+    }
+}

+ 52 - 0
src/Avalonia.Base/PropertyStore/TypedBindingEntry.cs

@@ -0,0 +1,52 @@
+using System;
+using Avalonia.Data;
+
+namespace Avalonia.PropertyStore
+{
+    /// <summary>
+    /// An <see cref="IValueEntry"/> that holds a binding whose source observable and target
+    /// property are both typed.
+    /// </summary>
+    internal sealed class TypedBindingEntry<T> : BindingEntryBase<T, T>, IValueEntry<T>
+    {
+        public TypedBindingEntry(
+            ValueFrame frame, 
+            StyledPropertyBase<T> property,
+            IObservable<T> source)
+                : base(frame, property, source)
+        {
+        }
+
+        public TypedBindingEntry(
+            ValueFrame frame,
+            StyledPropertyBase<T> property,
+            IObservable<BindingValue<T>> source)
+                : base(frame, property, source)
+        {
+        }
+
+        public new StyledPropertyBase<T> Property => (StyledPropertyBase<T>)base.Property;
+
+        protected override BindingValue<T> ConvertAndValidate(T value)
+        {
+            if (Property.ValidateValue?.Invoke(value) == false)
+            {
+                return BindingValue<T>.BindingError(
+                    new InvalidCastException($"'{value}' is not a valid value."));
+            }
+
+            return value;
+        }
+
+        protected override BindingValue<T> ConvertAndValidate(BindingValue<T> value)
+        {
+            if (value.HasValue && Property.ValidateValue?.Invoke(value.Value) == false)
+            {
+                return BindingValue<T>.BindingError(
+                    new InvalidCastException($"'{value.Value}' is not a valid value."));
+            }
+            
+            return value;
+        }
+    }
+}

+ 13 - 143
src/Avalonia.Base/PropertyStore/UntypedBindingEntry.cs

@@ -1,163 +1,33 @@
 using System;
-using System.Collections.Generic;
-using System.Diagnostics.CodeAnalysis;
-using System.Reactive.Disposables;
 using Avalonia.Data;
 
 namespace Avalonia.PropertyStore
 {
-    internal class UntypedBindingEntry<T> : IValueEntry<T>,
-        IObserver<object?>,
-        IDisposable
+    /// <summary>
+    /// An <see cref="IValueEntry"/> that holds a binding whose source observable and target
+    /// property are both untyped.
+    /// </summary>
+    internal class UntypedBindingEntry : BindingEntryBase<object?, object?>
     {
-        private static IDisposable s_Creating = Disposable.Empty;
-        private static IDisposable s_CreatingQuiet = Disposable.Create(() => { });
-        private readonly ValueFrame _frame;
-        private readonly IObservable<object?> _source;
-        private IDisposable? _subscription;
-        private bool _hasValue;
-        private T? _value;
+        private readonly Func<object?, bool>? _validate;
 
         public UntypedBindingEntry(
             ValueFrame frame,
-            StyledPropertyBase<T> property,
+            AvaloniaProperty property,
             IObservable<object?> source)
+            : base(frame, property, source)
         {
-            _frame = frame;
-            _source = source;
-            Property = property;
+            _validate = ((IStyledPropertyAccessor)property).ValidateValue;
         }
 
-        public bool HasValue
+        protected override BindingValue<object?> ConvertAndValidate(object? value)
         {
-            get
-            {
-                Start(produceValue: false);
-                return _hasValue;
-            }
+            return UntypedValueUtils.ConvertAndValidate(value, Property.PropertyType, _validate);
         }
 
-        public StyledPropertyBase<T> Property { get; }
-        AvaloniaProperty IValueEntry.Property => Property;
-
-        public void Dispose()
-        {
-            Unsubscribe();
-            BindingCompleted();
-        }
-
-        public T GetValue()
-        {
-            Start(produceValue: false);
-            if (!_hasValue)
-                throw new AvaloniaInternalException("The binding entry has no value.");
-            return _value!;
-        }
-
-        public void Start() => Start(true);
-
-        public bool TryGetValue([MaybeNullWhen(false)] out T value)
-        {
-            Start(produceValue: false);
-            value = _value;
-            return _hasValue;
-        }
-
-        public void OnCompleted() => BindingCompleted();
-        public void OnError(Exception error) => BindingCompleted();
-
-        public void OnNext(object? value) => SetValue(value);
-
-        public void OnNext(BindingValue<T> value)
-        {
-            if (value.HasValue)
-                SetValue(value.Value);
-            else
-                ClearValue();
-        }
-
-        public void Unsubscribe()
-        {
-            _subscription?.Dispose();
-            _subscription = null;
-        }
-
-        object? IValueEntry.GetValue()
-        {
-            Start(produceValue: false);
-            if (!_hasValue)
-                throw new AvaloniaInternalException("The BindingEntry<T> has no value.");
-            return _value!;
-        }
-
-        bool IValueEntry.TryGetValue(out object? value)
-        {
-            Start(produceValue: false);
-            value = _value;
-            return _hasValue;
-        }
-
-        private void ClearValue()
-        {
-            if (_hasValue)
-            {
-                _hasValue = false;
-                _value = default;
-
-                if (_subscription is not null && _subscription != s_CreatingQuiet)
-                    _frame.Owner?.OnBindingValueCleared(Property, _frame.Priority);
-            }
-        }
-
-        private void SetValue(object? value)
-        {
-            if (_frame.Owner is null)
-                return;
-
-            if (value is BindingNotification n)
-            {
-                value = n.Value;
-                LoggingUtils.LogIfNecessary(_frame.Owner.Owner, Property, n);
-            }
-
-            if (value == AvaloniaProperty.UnsetValue)
-            {
-                ClearValue();
-            }
-            else if (value == BindingOperations.DoNothing)
-            {
-                // Do nothing!
-            }
-            else if (UntypedValueUtils.TryConvertAndValidate(Property, value, out var typedValue))
-            {
-                if (!_hasValue || !EqualityComparer<T>.Default.Equals(_value, typedValue))
-                {
-                    _value = typedValue;
-                    _hasValue = true;
-                    
-                    if (_subscription is not null && _subscription != s_CreatingQuiet)
-                        _frame.Owner?.OnBindingValueChanged(Property, _frame.Priority, typedValue);
-                }
-            }
-            else
-            {
-                ClearValue();
-                LoggingUtils.LogInvalidValue(_frame.Owner.Owner, Property, typeof(T), value);
-            }
-        }
-
-        private void BindingCompleted()
-        {
-            _subscription = null;
-            _frame.OnBindingCompleted(this);
-        }
-
-        private void Start(bool produceValue)
+        protected override BindingValue<object?> ConvertAndValidate(BindingValue<object?> value)
         {
-            if (_subscription is not null)
-                return;
-            _subscription = produceValue ? s_Creating : s_CreatingQuiet;
-            _subscription = _source.Subscribe(this);
+            throw new NotSupportedException();
         }
     }
 }

+ 19 - 1
src/Avalonia.Base/PropertyStore/UntypedValueUtils.cs

@@ -1,10 +1,28 @@
-using System.Diagnostics.CodeAnalysis;
+using System;
+using System.Diagnostics.CodeAnalysis;
+using Avalonia.Data;
 using Avalonia.Utilities;
 
 namespace Avalonia.PropertyStore
 {
     internal static class UntypedValueUtils
     {
+        public static BindingValue<T> ConvertAndValidate<T>(
+            object? value,
+            Type targetType,
+            Func<T, bool>? validate)
+        {
+            var v = BindingValue<T>.FromUntyped(value, targetType);
+            
+            if (v.HasValue && validate?.Invoke(v.Value) == false)
+            {
+                return BindingValue<T>.BindingError(
+                    new InvalidCastException($"'{v.Value}' is not a valid value."));
+            }
+
+            return v;
+        }
+
         public static bool TryConvertAndValidate(
             AvaloniaProperty property,
             object? value,

+ 130 - 111
src/Avalonia.Base/PropertyStore/ValueStore.cs

@@ -24,6 +24,7 @@ namespace Avalonia.PropertyStore
 
         public AvaloniaObject Owner { get; }
         public ValueStore? InheritanceAncestor { get; private set; }
+        public bool IsEvaluating { get; private set; }
         public IReadOnlyList<ValueFrame> Frames => _frames;
 
         public void BeginStyling() => ++_styling;
@@ -760,159 +761,177 @@ namespace Avalonia.PropertyStore
             EffectiveValue? current,
             bool ignoreLocalValue = false)
         {
-        restart:
-            // Don't reevaluate if a styling pass is in effect, reevaluation will be done when
-            // it has finished.
-            if (_styling > 0)
-                return;
-
-            var generation = _frameGeneration;
+            IsEvaluating = true;
 
-            // Reset all non-LocalValue effective value to Unset priority.
-            if (current is not null)
+            try
             {
-                if (ignoreLocalValue || current.Priority != BindingPriority.LocalValue)
-                    current.SetPriority(BindingPriority.Unset);
-                if (ignoreLocalValue || current.BasePriority != BindingPriority.LocalValue)
-                    current.SetBasePriority(BindingPriority.Unset);
-            }
+            restart:
+                // Don't reevaluate if a styling pass is in effect, reevaluation will be done when
+                // it has finished.
+                if (_styling > 0)
+                    return;
 
-            // Iterate the frames to get the effective value.
-            for (var i = _frames.Count - 1; i >= 0; --i)
-            {
-                var frame = _frames[i];
-                var priority = frame.Priority;
+                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);
+                }
 
-                if (frame.TryGetEntry(property, out var entry) &&
-                    frame.IsActive &&
-                    entry.HasValue)
+                // Iterate the frames to get the effective value.
+                for (var i = _frames.Count - 1; i >= 0; --i)
                 {
-                    if (current is not null)
+                    var frame = _frames[i];
+                    var priority = frame.Priority;
+
+                    if (frame.TryGetEntry(property, out var entry) &&
+                        frame.IsActive &&
+                        entry.HasValue)
                     {
-                        current.SetAndRaise(this, entry, priority);
+                        if (current is not null)
+                        {
+                            current.SetAndRaise(this, entry, priority);
+                        }
+                        else
+                        {
+                            current = property.CreateEffectiveValue(Owner);
+                            AddEffectiveValue(property, current);
+                            current.SetAndRaise(this, entry, priority);
+                        }
                     }
-                    else
+
+                    if (generation != _frameGeneration)
+                        goto restart;
+
+                    if (current is not null &&
+                        current.Priority < BindingPriority.Unset &&
+                        current.BasePriority < BindingPriority.Unset)
                     {
-                        current = property.CreateEffectiveValue(Owner);
-                        AddEffectiveValue(property, current);
-                        current.SetAndRaise(this, entry, priority);
+                        UnsubscribeInactiveValues(i, property);
+                        return;
                     }
                 }
 
-                if (generation != _frameGeneration)
-                    goto restart;
-
-                if (current is not null &&
-                    current.Priority < BindingPriority.Unset &&
-                    current.BasePriority < BindingPriority.Unset)
+                if (current?.Priority == BindingPriority.Unset)
                 {
-                    UnsubscribeInactiveValues(i, property);
-                    return;
+                    if (current.BasePriority == BindingPriority.Unset)
+                    {
+                        RemoveEffectiveValue(property);
+                        current.DisposeAndRaiseUnset(this, property);
+                    }
+                    else
+                    {
+                        current.SetAndRaise(this, property, current.BaseValue, current.BasePriority);
+                    }
                 }
             }
-
-            if (current?.Priority == BindingPriority.Unset)
+            finally
             {
-                if (current.BasePriority == BindingPriority.Unset)
-                {
-                    RemoveEffectiveValue(property);
-                    current.DisposeAndRaiseUnset(this, property);
-                }
-                else
-                {
-                    current.SetAndRaise(this, property, current.BaseValue, current.BasePriority);
-                }
+                IsEvaluating = false;
             }
         }
 
         private void ReevaluateEffectiveValues()
         {
-        restart:
-            // Don't reevaluate if a styling pass is in effect, reevaluation will be done when
-            // it has finished.
-            if (_styling > 0)
-                return;
-
-            var generation = _frameGeneration;
-            var count = _effectiveValues.Count;
+            IsEvaluating = true;
 
-            // Reset all non-LocalValue effective values to Unset priority.
-            for (var i = 0; i < count; ++i)
+            try
             {
-                var e = _effectiveValues[i];
-
-                if (e.Priority != BindingPriority.LocalValue)
-                    e.SetPriority(BindingPriority.Unset);
-                if (e.BasePriority != BindingPriority.LocalValue)
-                    e.SetBasePriority(BindingPriority.Unset);
-            }
+            restart:
+                // Don't reevaluate if a styling pass is in effect, reevaluation will be done when
+                // it has finished.
+                if (_styling > 0)
+                    return;
 
-            // Iterate the frames, setting and creating effective values.
-            for (var i = _frames.Count - 1; i >= 0; --i)
-            {
-                var frame = _frames[i];
+                var generation = _frameGeneration;
+                var count = _effectiveValues.Count;
 
-                if (!frame.IsActive)
-                    continue;
+                // Reset all non-LocalValue effective values to Unset priority.
+                for (var i = 0; i < count; ++i)
+                {
+                    var e = _effectiveValues[i];
 
-                var priority = frame.Priority;
-                
-                count = frame.EntryCount;
+                    if (e.Priority != BindingPriority.LocalValue)
+                        e.SetPriority(BindingPriority.Unset);
+                    if (e.BasePriority != BindingPriority.LocalValue)
+                        e.SetBasePriority(BindingPriority.Unset);
+                }
 
-                for (var j = 0; j < count; ++j)
+                // Iterate the frames, setting and creating effective values.
+                for (var i = _frames.Count - 1; i >= 0; --i)
                 {
-                    var entry = frame.GetEntry(j);
-                    var property = entry.Property;
-                    EffectiveValue? effectiveValue;
+                    var frame = _frames[i];
 
-                    // Skip if we already have a value/base value for this property.
-                    if (_effectiveValues.TryGetValue(property, out effectiveValue) == true &&
-                        effectiveValue.BasePriority < BindingPriority.Unset)
+                    if (!frame.IsActive)
                         continue;
 
-                    if (!entry.HasValue)
-                        continue;
+                    var priority = frame.Priority;
 
-                    if (effectiveValue is not null)
-                    {
-                        effectiveValue.SetAndRaise(this, entry, priority);
-                    }
-                    else
+                    count = frame.EntryCount;
+
+                    for (var j = 0; j < count; ++j)
                     {
-                        var v = property.CreateEffectiveValue(Owner);
-                        AddEffectiveValue(property, v);
-                        v.SetAndRaise(this, entry, priority);
+                        var entry = frame.GetEntry(j);
+                        var property = entry.Property;
+                        EffectiveValue? effectiveValue;
+
+                        // Skip if we already have a value/base value for this property.
+                        if (_effectiveValues.TryGetValue(property, out effectiveValue) == true &&
+                            effectiveValue.BasePriority < BindingPriority.Unset)
+                            continue;
+
+                        if (!entry.HasValue)
+                            continue;
+
+                        if (effectiveValue is not null)
+                        {
+                            effectiveValue.SetAndRaise(this, entry, priority);
+                        }
+                        else
+                        {
+                            var v = property.CreateEffectiveValue(Owner);
+                            AddEffectiveValue(property, v);
+                            v.SetAndRaise(this, entry, priority);
+                        }
+
+                        if (generation != _frameGeneration)
+                            goto restart;
                     }
-
-                    if (generation != _frameGeneration)
-                        goto restart;
                 }
-            }
 
-            // Remove all effective values that are still unset.
-            PooledList<AvaloniaProperty>? remove = null;
+                // Remove all effective values that are still unset.
+                PooledList<AvaloniaProperty>? remove = null;
 
-            count = _effectiveValues.Count;
-
-            for (var i = 0; i < count; ++i)
-            {
-                _effectiveValues.GetKeyValue(i, out var key, out var e);
+                count = _effectiveValues.Count;
 
-                if (e.Priority == BindingPriority.Unset)
+                for (var i = 0; i < count; ++i)
                 {
-                    remove ??= new();
-                    remove.Add(key);
+                    _effectiveValues.GetKeyValue(i, out var key, out var e);
+
+                    if (e.Priority == BindingPriority.Unset)
+                    {
+                        remove ??= new();
+                        remove.Add(key);
+                    }
                 }
-            }
 
-            if (remove is not null)
-            {
-                foreach (var v in remove)
+                if (remove is not null)
                 {
-                    if (RemoveEffectiveValue(v, out var e))
-                        e.DisposeAndRaiseUnset(this, v);
+                    foreach (var v in remove)
+                    {
+                        if (RemoveEffectiveValue(v, out var e))
+                            e.DisposeAndRaiseUnset(this, v);
+                    }
+                    remove.Dispose();
                 }
-                remove.Dispose();
+            }
+            finally
+            {
+                IsEvaluating = false;
             }
         }
 

+ 18 - 8
src/Avalonia.Base/Styling/PropertySetterBindingInstance.cs

@@ -1,15 +1,13 @@
 using System;
-using System.Reactive.Linq;
 using Avalonia.Data;
 using Avalonia.PropertyStore;
 
 namespace Avalonia.Styling
 {
-    internal class PropertySetterBindingInstance : BindingEntry, ISetterInstance
+    internal class PropertySetterBindingInstance : UntypedBindingEntry, ISetterInstance
     {
         private readonly AvaloniaObject _target;
         private readonly BindingMode _mode;
-        private IDisposable? _twoWaySubscription;
 
         public PropertySetterBindingInstance(
             AvaloniaObject target,
@@ -32,19 +30,31 @@ namespace Avalonia.Styling
 
         public override void Unsubscribe()
         {
-            _twoWaySubscription?.Dispose();
+            _target.PropertyChanged -= PropertyChanged;
             base.Unsubscribe();
         }
 
         protected override void Start(bool produceValue)
         {
-            if (_mode == BindingMode.TwoWay)
+            if (!IsSubscribed)
             {
-                var observer = (IObserver<object?>)Source;
-                _twoWaySubscription = _target.GetObservable(Property).Skip(1).Subscribe(observer);
+                if (_mode == BindingMode.TwoWay)
+                {
+                    var observer = (IObserver<object?>)Source;
+                    _target.PropertyChanged += PropertyChanged;
+                }
+
+                base.Start(produceValue);
             }
+        }
 
-            base.Start(produceValue);
+        private void PropertyChanged(object? sender, AvaloniaPropertyChangedEventArgs e)
+        {
+            if (e.Property == Property && e.Priority >= BindingPriority.LocalValue)
+            {
+                //if (Frame.Owner is not null && !Frame.Owner.IsEvaluating)
+                    ((IObserver<object?>)Source).OnNext(e.NewValue);
+            }
         }
     }
 }

+ 3 - 4
tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs

@@ -901,7 +901,8 @@ namespace Avalonia.Base.UnitTests
             var target = new Class1();
             var source = new Subject<object?>();
             var called = false;
-            var expectedMessageTemplate = "Error in binding to {Target}.{Property}: expected {ExpectedType}, got {Value} ({ValueType})";
+            var expectedMessageTemplate = "Error in binding to {Target}.{Property}: {Message}";
+            var expectedMessage = "Unable to convert object 'foo' of type 'System.String' to type 'System.Double'.";
 
             LogCallback checkLogMessage = (level, area, src, mt, pv) =>
             {
@@ -911,9 +912,7 @@ namespace Avalonia.Base.UnitTests
                     src == target &&
                     pv[0].GetType() == typeof(Class1) &&
                     (AvaloniaProperty)pv[1] == Class1.QuxProperty &&
-                    (Type)pv[2] == typeof(double) &&
-                    (string)pv[3] == "foo" &&
-                    (Type)pv[4] == typeof(string))
+                    (string)pv[2] == expectedMessage)
                 {
                     called = true;
                 }