Browse Source

Refactored style invalidation/removal.

Steven Kirk 3 years ago
parent
commit
e074a70187

+ 2 - 2
src/Avalonia.Base/Layout/Layoutable.cs

@@ -720,9 +720,9 @@ namespace Avalonia.Layout
             return finalSize;
         }
 
-        protected sealed override void InvalidateStyles()
+        internal sealed override void InvalidateStyles(bool recurse)
         {
-            base.InvalidateStyles();
+            base.InvalidateStyles(recurse);
             InvalidateMeasure();
         }
 

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

@@ -7,7 +7,7 @@ namespace Avalonia.PropertyStore
     /// Holds values in a <see cref="ValueStore"/> set by one of the SetValue or AddBinding
     /// overloads with non-LocalValue priority.
     /// </summary>
-    internal class ImmediateValueFrame : ValueFrame
+    internal sealed class ImmediateValueFrame : ValueFrame
     {
         public ImmediateValueFrame(BindingPriority priority)
             : base(priority, FrameType.Style)

+ 27 - 1
src/Avalonia.Base/PropertyStore/ValueStore.cs

@@ -2,8 +2,10 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
+using System.Linq;
 using Avalonia.Data;
 using Avalonia.Diagnostics;
+using Avalonia.Styling;
 using Avalonia.Utilities;
 using static Avalonia.Rendering.Composition.Animations.PropertySetSnapshot;
 
@@ -588,7 +590,31 @@ namespace Avalonia.PropertyStore
             {
                 var frame = _frames[i];
 
-                if (frame.FramePriority.IsType(type))
+                if (frame is not ImmediateValueFrame && frame.FramePriority.IsType(type))
+                {
+                    _frames.RemoveAt(i);
+                    frame.Dispose();
+                    removed = true;
+                }
+            }
+
+            if (removed)
+            {
+                ++_frameGeneration;
+                ReevaluateEffectiveValues();
+            }
+        }
+
+
+        public void RemoveFrames(IReadOnlyList<IStyle> styles)
+        {
+            var removed = false;
+
+            for (var i = _frames.Count - 1; i >= 0; --i)
+            {
+                var frame = _frames[i];
+
+                if (frame is StyleInstance style && styles.Contains(style.Source))
                 {
                     _frames.RemoveAt(i);
                     frame.Dispose();

+ 70 - 49
src/Avalonia.Base/StyledElement.cs

@@ -382,11 +382,6 @@ namespace Avalonia
             return _stylesApplied;
         }
 
-        /// <summary>
-        /// Detaches all styles from the element and queues a restyle.
-        /// </summary>
-        protected virtual void InvalidateStyles() => DetachStyles();
-
         protected void InitializeIfNeeded()
         {
             if (_initCount == 0 && !IsInitialized)
@@ -508,17 +503,16 @@ namespace Avalonia
             };
         }
 
-        void IStyleable.DetachStyles() => DetachStyles();
-
         void IStyleHost.StylesAdded(IReadOnlyList<IStyle> styles)
         {
-            InvalidateStylesOnThisAndDescendents();
+            if (HasSettersOrAnimations(styles))
+                InvalidateStyles(recurse: true);
         }
 
         void IStyleHost.StylesRemoved(IReadOnlyList<IStyle> styles)
         {
-            var allStyles = RecurseStyles(styles);
-            DetachStylesFromThisAndDescendents(allStyles);
+            if (FlattenStyles(styles) is { } allStyles)
+                DetachStyles(allStyles);
         }
 
         protected virtual void LogicalChildrenCollectionChanged(object? sender, NotifyCollectionChangedEventArgs e)
@@ -663,6 +657,23 @@ namespace Avalonia
             return null;
         }
 
+        internal virtual void InvalidateStyles(bool recurse)
+        {
+            var values = GetValueStore();
+            values.BeginStyling();
+            try { values.RemoveFrames(FrameType.Style); }
+            finally { values.EndStyling(); }
+
+            _stylesApplied = false;
+
+            if (recurse && GetInheritanceChildren() is { } children)
+            {
+                var childCount = children.Count;
+                for (var i = 0; i < childCount; ++i)
+                    (children[i] as StyledElement)?.InvalidateStyles(recurse);
+            }
+        }
+
         private static void DataContextNotifying(IAvaloniaObject o, bool updateStarted)
         {
             if (o is StyledElement element)
@@ -822,7 +833,7 @@ namespace Avalonia
             {
                 _logicalRoot = null;
                 _implicitTheme = null;
-                DetachStyles();
+                InvalidateStyles(recurse: false);
                 OnDetachedFromLogicalTree(e);
                 DetachedFromLogicalTree?.Invoke(this, e);
 
@@ -884,71 +895,81 @@ namespace Avalonia
             }
         }
 
-        private void DetachStyles(IReadOnlyList<StyleBase>? styles = null)
+        private void DetachStyles(IReadOnlyList<Style> styles)
         {
-            var valueStore = GetValueStore();
-
-            valueStore.BeginStyling();
+            var values = GetValueStore();
+            values.BeginStyling();
+            try { values.RemoveFrames(styles); }
+            finally { values.EndStyling(); }
 
-            for (var i = valueStore.Frames.Count - 1; i >= 0; --i)
+            if (_logicalChildren is not null)
             {
-                if (valueStore.Frames[i] is StyleInstance si &&
-                    si.Source is not ControlTheme &&
-                    (styles is null || styles.Contains(si.Source)))
+                var childCount = _logicalChildren.Count;
+
+                for (var i = 0; i < childCount; ++i)
                 {
-                    valueStore.RemoveFrame(si);
+                    (_logicalChildren[i] as StyledElement)?.DetachStyles(styles);
                 }
             }
-
-            valueStore.EndStyling();
-            _stylesApplied = false;
         }
 
-        private void InvalidateStylesOnThisAndDescendents()
+        private void NotifyResourcesChanged(
+            ResourcesChangedEventArgs? e = null,
+            bool propagate = true)
         {
-            InvalidateStyles();
-
-            if (_logicalChildren is object)
+            if (ResourcesChanged is object)
             {
-                var childCount = _logicalChildren.Count;
+                e ??= ResourcesChangedEventArgs.Empty;
+                ResourcesChanged(this, e);
+            }
 
-                for (var i = 0; i < childCount; ++i)
-                {
-                    (_logicalChildren[i] as StyledElement)?.InvalidateStylesOnThisAndDescendents();
-                }
+            if (propagate)
+            {
+                e ??= ResourcesChangedEventArgs.Empty;
+                NotifyChildResourcesChanged(e);
             }
         }
 
-        private void DetachStylesFromThisAndDescendents(IReadOnlyList<StyleBase> styles)
+        private static IReadOnlyList<Style>? FlattenStyles(IReadOnlyList<IStyle> styles)
         {
-            DetachStyles(styles);
+            List<Style>? result = null;
 
-            if (_logicalChildren is object)
+            static void FlattenStyle(IStyle style, ref List<Style>? result)
             {
-                var childCount = _logicalChildren.Count;
+                if (style is Style s)
+                    (result ??= new()).Add(s);
+                FlattenStyles(style.Children, ref result);
+            }
 
-                for (var i = 0; i < childCount; ++i)
-                {
-                    (_logicalChildren[i] as StyledElement)?.DetachStylesFromThisAndDescendents(styles);
-                }
+            static void FlattenStyles(IReadOnlyList<IStyle> styles, ref List<Style>? result)
+            {
+                var count = styles.Count;
+                for (var i = 0; i < count; ++i)
+                    FlattenStyle(styles[i], ref result);
             }
+
+            FlattenStyles(styles, ref result);
+            return result;
         }
 
-        private void NotifyResourcesChanged(
-            ResourcesChangedEventArgs? e = null,
-            bool propagate = true)
+        private static bool HasSettersOrAnimations(IReadOnlyList<IStyle> styles)
         {
-            if (ResourcesChanged is object)
+            static bool StyleHasSettersOrAnimations(IStyle style)
             {
-                e ??= ResourcesChangedEventArgs.Empty;
-                ResourcesChanged(this, e);
+                if (style is StyleBase s && s.HasSettersOrAnimations)
+                    return true;
+                return HasSettersOrAnimations(style.Children);
             }
 
-            if (propagate)
+            var count = styles.Count;
+
+            for (var i = 0; i < count; ++i)
             {
-                e ??= ResourcesChangedEventArgs.Empty;
-                NotifyChildResourcesChanged(e);
+                if (StyleHasSettersOrAnimations(styles[i]))
+                    return true;
             }
+
+            return false;
         }
 
         private static IReadOnlyList<StyleBase> RecurseStyles(IReadOnlyList<IStyle> styles)

+ 0 - 2
src/Avalonia.Base/Styling/IStyleable.cs

@@ -24,7 +24,5 @@ namespace Avalonia.Styling
         /// Gets the template parent of this element if the control comes from a template.
         /// </summary>
         ITemplatedControl? TemplatedParent { get; }
-
-        void DetachStyles();
     }
 }

+ 45 - 18
tests/Avalonia.Base.UnitTests/Styling/StyleTests.cs

@@ -618,15 +618,15 @@ namespace Avalonia.Base.UnitTests.Styling
             var root = new TestRoot
             {
                 Styles =
+                {
+                    new Style(x => x.OfType<Border>())
                     {
-                        new Style(x => x.OfType<Border>())
+                        Setters =
                         {
-                            Setters =
-                            {
-                                new Setter(Border.BorderThicknessProperty, new Thickness(4)),
-                            }
+                            new Setter(Border.BorderThicknessProperty, new Thickness(4)),
                         }
-                    },
+                    }
+                },
                 Child = border,
             };
 
@@ -636,9 +636,9 @@ namespace Avalonia.Base.UnitTests.Styling
             root.Styles.Add(new Style(x => x.OfType<Border>())
             {
                 Setters =
-                    {
-                        new Setter(Border.BorderThicknessProperty, new Thickness(6)),
-                    }
+                {
+                    new Setter(Border.BorderThicknessProperty, new Thickness(6)),
+                }
             });
 
             root.Measure(Size.Infinity);
@@ -652,18 +652,18 @@ namespace Avalonia.Base.UnitTests.Styling
             var root = new TestRoot
             {
                 Styles =
+                {
+                    new Styles
                     {
-                        new Styles
+                        new Style(x => x.OfType<Border>())
                         {
-                            new Style(x => x.OfType<Border>())
+                            Setters =
                             {
-                                Setters =
-                                {
-                                    new Setter(Border.BorderThicknessProperty, new Thickness(4)),
-                                }
+                                new Setter(Border.BorderThicknessProperty, new Thickness(4)),
                             }
                         }
-                    },
+                    }
+                },
                 Child = border,
             };
 
@@ -750,7 +750,34 @@ namespace Avalonia.Base.UnitTests.Styling
         }
 
         [Fact]
-        public void DetachStyles_Should_Detach_Activator()
+        public void Adding_Style_With_No_Setters_Or_Animations_Should_Not_Invalidate_Styles()
+        {
+            var border = new Border();
+            var root = new TestRoot
+            {
+                Styles =
+                    {
+                        new Style(x => x.OfType<Border>())
+                        {
+                            Setters =
+                            {
+                                new Setter(Border.BorderThicknessProperty, new Thickness(4)),
+                            }
+                        }
+                    },
+                Child = border,
+            };
+
+            root.Measure(Size.Infinity);
+            Assert.Equal(new Thickness(4), border.BorderThickness);
+
+            root.Styles.Add(new Style(x => x.OfType<Border>()));
+
+            Assert.Equal(new Thickness(4), border.BorderThickness);
+        }
+
+        [Fact]
+        public void Invalidating_Styles_Should_Detach_Activator()
         {
             Style style = new Style(x => x.OfType<Class1>().Class("foo"))
             {
@@ -766,7 +793,7 @@ namespace Avalonia.Base.UnitTests.Styling
 
             Assert.Equal(1, target.Classes.ListenerCount);
 
-            ((IStyleable)target).DetachStyles();
+            target.InvalidateStyles(recurse: false);
 
             Assert.Equal(0, target.Classes.ListenerCount);
         }

+ 1 - 1
tests/Avalonia.Benchmarks/Styling/Style_Apply_Detach_Complex.cs

@@ -45,7 +45,7 @@ namespace Avalonia.Benchmarks.Styling
             if ((string)_control.Tag != "TextBox")
                 throw new Exception("Invalid benchmark state");
 
-            ((IStyleable)_control).DetachStyles();
+            _control.InvalidateStyles(true);
 
             if (_control.Tag is not null)
                 throw new Exception("Invalid benchmark state");

+ 1 - 1
tests/Avalonia.Benchmarks/Styling/Style_ClassSelector.cs

@@ -77,7 +77,7 @@ namespace Avalonia.Benchmarks.Styling
         {
             public static readonly StyledProperty<string?> StringProperty =
                 AvaloniaProperty.Register<TestClass, string?>("String");
-            public void DetachStyles() => InvalidateStyles();
+            public void DetachStyles() => InvalidateStyles(recurse: true);
         }
 
         private class TestClass2 : Control