Merge pull request #8131 from MarchingCube/style-fixes-and-cleanup

Optimize memory usage of styles and fix ordering issue in devtools.

src/Avalonia.Base/DirectPropertyBase.cs

@@ -2,7 +2,6 @@
 using Avalonia.Data;
 using Avalonia.Reactive;
 using Avalonia.Styling;
-using Avalonia.Utilities;
 
 namespace Avalonia
 {
@@ -188,10 +187,10 @@ namespace Avalonia
             }
             else if (value is ITemplate template && !typeof(ITemplate).IsAssignableFrom(PropertyType))
             {
-                return new PropertySetterLazyInstance<TValue>(
+                return new PropertySetterTemplateInstance<TValue>(
                     target,
                     this,
-                    () => (TValue)template.Build());
+                    template);
             }
             else
             {

src/Avalonia.Base/StyledPropertyBase.cs

@@ -1,9 +1,7 @@
 using System;
-using System.Diagnostics;
 using Avalonia.Data;
 using Avalonia.Reactive;
 using Avalonia.Styling;
-using Avalonia.Utilities;
 
 namespace Avalonia
 {
@@ -12,7 +10,7 @@ namespace Avalonia
     /// </summary>
     public abstract class StyledPropertyBase<TValue> : AvaloniaProperty<TValue>, IStyledPropertyAccessor
     {
-        private bool _inherits;
+        private readonly bool _inherits;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="StyledPropertyBase{T}"/> class.
@@ -243,10 +241,10 @@ namespace Avalonia
             }
             else if (value is ITemplate template && !typeof(ITemplate).IsAssignableFrom(PropertyType))
             {
-                return new PropertySetterLazyInstance<TValue>(
+                return new PropertySetterTemplateInstance<TValue>(
                     target,
                     this,
-                    () => (TValue)template.Build());
+                    template);
             }
             else
             {

src/Avalonia.Base/Styling/IStyleInstance.cs

@@ -14,6 +14,11 @@ namespace Avalonia.Styling
         /// </summary>
         IStyle Source { get; }
 
+        /// <summary>
+        /// Gets a value indicating whether this style has an activator.
+        /// </summary>
+        bool HasActivator { get; }
+        
         /// <summary>
         /// Gets a value indicating whether this style is active.
         /// </summary>

src/Avalonia.Base/Styling/PropertySetterInstance.cs

@@ -44,7 +44,7 @@ namespace Avalonia.Styling
         {
             if (hasActivator)
             {
-                if (_styledProperty is object)
+                if (_styledProperty is not null)
                 {
                     _subscription = _target.Bind(_styledProperty, this, BindingPriority.StyleTrigger);
                 }
@@ -55,13 +55,15 @@ namespace Avalonia.Styling
             }
             else
             {
-                if (_styledProperty is object)
+                var target = (AvaloniaObject) _target;
+                
+                if (_styledProperty is not null)
                 {
-                    _subscription = _target.SetValue(_styledProperty!, _value, BindingPriority.Style);
+                    _subscription = target.SetValue(_styledProperty!, _value, BindingPriority.Style);
                 }
                 else
                 {
-                    _target.SetValue(_directProperty!, _value);
+                    target.SetValue(_directProperty!, _value);
                 }
             }
         }

src/Avalonia.Base/Styling/PropertySetterLazyInstance.cs → src/Avalonia.Base/Styling/PropertySetterTemplateInstance.cs

@@ -11,42 +11,42 @@ namespace Avalonia.Styling
     /// evaluated.
     /// </summary>
     /// <typeparam name="T">The target property type.</typeparam>
-    internal class PropertySetterLazyInstance<T> : SingleSubscriberObservableBase<BindingValue<T>>,
+    internal class PropertySetterTemplateInstance<T> : SingleSubscriberObservableBase<BindingValue<T>>,
         ISetterInstance
     {
         private readonly IStyleable _target;
         private readonly StyledPropertyBase<T>? _styledProperty;
         private readonly DirectPropertyBase<T>? _directProperty;
-        private readonly Func<T> _valueFactory;
+        private readonly ITemplate _template;
         private BindingValue<T> _value;
         private IDisposable? _subscription;
         private bool _isActive;
 
-        public PropertySetterLazyInstance(
+        public PropertySetterTemplateInstance(
             IStyleable target,
             StyledPropertyBase<T> property,
-            Func<T> valueFactory)
+            ITemplate template)
         {
             _target = target;
             _styledProperty = property;
-            _valueFactory = valueFactory;
+            _template = template;
         }
 
-        public PropertySetterLazyInstance(
+        public PropertySetterTemplateInstance(
             IStyleable target,
             DirectPropertyBase<T> property,
-            Func<T> valueFactory)
+            ITemplate template)
         {
             _target = target;
             _directProperty = property;
-            _valueFactory = valueFactory;
+            _template = template;
         }
 
         public void Start(bool hasActivator)
         {
             _isActive = !hasActivator;
 
-            if (_styledProperty is object)
+            if (_styledProperty is not null)
             {
                 var priority = hasActivator ? BindingPriority.StyleTrigger : BindingPriority.Style;
                 _subscription = _target.Bind(_styledProperty, this, priority);
@@ -77,7 +77,7 @@ namespace Avalonia.Styling
 
         public override void Dispose()
         {
-            if (_subscription is object)
+            if (_subscription is not null)
             {
                 var sub = _subscription;
                 _subscription = null;
@@ -85,7 +85,7 @@ namespace Avalonia.Styling
             }
             else if (_isActive)
             {
-                if (_styledProperty is object)
+                if (_styledProperty is not null)
                 {
                     _target.ClearValue(_styledProperty);
                 }
@@ -101,22 +101,21 @@ namespace Avalonia.Styling
         protected override void Subscribed() => PublishNext();
         protected override void Unsubscribed() { }
 
-        private T GetValue()
+        private void EnsureTemplate()
         {
             if (_value.HasValue)
             {
-                return _value.Value;
+                return;
             }
 
-            _value = _valueFactory();
-            return _value.Value;
+            _value = (T) _template.Build();
         }
 
         private void PublishNext()
         {
             if (_isActive)
             {
-                GetValue();
+                EnsureTemplate();
                 PublishNext(_value);
             }
             else

src/Avalonia.Base/Styling/Setter.cs

@@ -1,9 +1,7 @@
 using System;
 using Avalonia.Animation;
 using Avalonia.Data;
-using Avalonia.Data.Core;
 using Avalonia.Metadata;
-using Avalonia.Utilities;
 
 #nullable enable
 
@@ -70,12 +68,5 @@ namespace Avalonia.Styling
 
             return Property.CreateSetterInstance(target, Value);
         }
-
-        private struct SetterVisitorData
-        {
-            public IStyleable target;
-            public object? value;
-            public ISetterInstance? result;
-        }
     }
 }

src/Avalonia.Base/Styling/StyleInstance.cs

@@ -11,10 +11,10 @@ namespace Avalonia.Styling
     /// <summary>
     /// A <see cref="Style"/> which has been instanced on a control.
     /// </summary>
-    internal class StyleInstance : IStyleInstance, IStyleActivatorSink
+    internal sealed class StyleInstance : IStyleInstance, IStyleActivatorSink
     {
-        private readonly List<ISetterInstance>? _setters;
-        private readonly List<IDisposable>? _animations;
+        private readonly ISetterInstance[]? _setters;
+        private readonly IDisposable[]? _animations;
         private readonly IStyleActivator? _activator;
         private readonly Subject<bool>? _animationTrigger;
 
@@ -30,41 +30,42 @@ namespace Avalonia.Styling
             _activator = activator;
             IsActive = _activator is null;
 
-            if (setters is object)
+            if (setters is not null)
             {
                 var setterCount = setters.Count;
 
-                _setters = new List<ISetterInstance>(setterCount);
+                _setters = new ISetterInstance[setterCount];
 
                 for (var i = 0; i < setterCount; ++i)
                 {
-                    _setters.Add(setters[i].Instance(Target));
+                    _setters[i] = setters[i].Instance(Target);
                 }
             }
 
-            if (animations is object && target is Animatable animatable)
+            if (animations is not null && target is Animatable animatable)
             {
                 var animationsCount = animations.Count;
 
-                _animations = new List<IDisposable>(animationsCount);
+                _animations = new IDisposable[animationsCount];
                 _animationTrigger = new Subject<bool>();
 
                 for (var i = 0; i < animationsCount; ++i)
                 {
-                    _animations.Add(animations[i].Apply(animatable, null, _animationTrigger));
+                    _animations[i] = animations[i].Apply(animatable, null, _animationTrigger);
                 }
             }
         }
 
+        public bool HasActivator => _activator is not null;
         public bool IsActive { get; private set; }
         public IStyle Source { get; }
         public IStyleable Target { get; }
 
         public void Start()
         {
-            var hasActivator = _activator is object;
+            var hasActivator = HasActivator;
 
-            if (_setters is object)
+            if (_setters is not null)
             {
                 foreach (var setter in _setters)
                 {
@@ -76,7 +77,7 @@ namespace Avalonia.Styling
             {
                 _activator!.Subscribe(this, 0);
             }
-            else if (_animationTrigger != null)
+            else if (_animationTrigger is not null)
             {
                 _animationTrigger.OnNext(true);
             }
@@ -84,7 +85,7 @@ namespace Avalonia.Styling
 
         public void Dispose()
         {
-            if (_setters is object)
+            if (_setters is not null)
             {
                 foreach (var setter in _setters)
                 {
@@ -92,11 +93,11 @@ namespace Avalonia.Styling
                 }
             }
 
-            if (_animations is object)
+            if (_animations is not null)
             {
-                foreach (var subscripion in _animations)
+                foreach (var subscription in _animations)
                 {
-                    subscripion.Dispose();
+                    subscription.Dispose();
                 }
             }
 
@@ -111,7 +112,7 @@ namespace Avalonia.Styling
 
                 _animationTrigger?.OnNext(value);
 
-                if (_setters is object)
+                if (_setters is not null)
                 {
                     if (IsActive)
                     {

src/Avalonia.Diagnostics/Diagnostics/ViewModels/ControlDetailsViewModel.cs

@@ -60,7 +60,8 @@ namespace Avalonia.Diagnostics.ViewModels
 
                 var styleDiagnostics = styledElement.GetStyleDiagnostics();
 
-                foreach (var appliedStyle in styleDiagnostics.AppliedStyles)
+                // We need to place styles without activator first, such styles will be overwritten by ones with activators.
+                foreach (var appliedStyle in styleDiagnostics.AppliedStyles.OrderBy(s => s.HasActivator))
                 {
                     var styleSource = appliedStyle.Source;
 

tests/Avalonia.Base.UnitTests/Styling/SetterTests.cs

@@ -91,16 +91,13 @@ namespace Avalonia.Base.UnitTests.Styling
         [Fact]
         public void Setter_Should_Apply_Value_Without_Activator_With_Style_Priority()
         {
-            var control = new Mock<IStyleable>();
-            var style = Mock.Of<Style>();
+            var control = new Control();
             var setter = new Setter(TextBlock.TagProperty, "foo");
 
-            setter.Instance(control.Object).Start(false);
+            setter.Instance(control).Start(false);
 
-            control.Verify(x => x.SetValue(
-                TextBlock.TagProperty,
-                "foo",
-                BindingPriority.Style));
+            Assert.Equal("foo", control.Tag);
+            Assert.Equal(BindingPriority.Style, control.GetDiagnostic(TextBlock.TagProperty).Priority);
         }
 
         [Fact]