From 273124603f184b8de42a22067b7dfc560f7d9792 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 21 Nov 2022 15:03:33 +0100 Subject: [PATCH 1/5] Added benchmark for changing control theme. --- .../Styling/ControlTheme_Change.cs | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 tests/Avalonia.Benchmarks/Styling/ControlTheme_Change.cs diff --git a/tests/Avalonia.Benchmarks/Styling/ControlTheme_Change.cs b/tests/Avalonia.Benchmarks/Styling/ControlTheme_Change.cs new file mode 100644 index 0000000000..627edfdeb6 --- /dev/null +++ b/tests/Avalonia.Benchmarks/Styling/ControlTheme_Change.cs @@ -0,0 +1,149 @@ +using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using Avalonia.Controls; +using Avalonia.Media; +using Avalonia.Styling; +using Avalonia.UnitTests; +using BenchmarkDotNet.Attributes; + +namespace Avalonia.Benchmarks.Styling +{ + [MemoryDiagnoser] + public class ControlTheme_Change : IDisposable + { + private readonly IDisposable _app; + private readonly TestRoot _root; + private readonly TextBox _control; + private readonly ControlTheme _theme1; + private readonly ControlTheme _theme2; + + public ControlTheme_Change() + { + _app = UnitTestApplication.Start( + TestServices.StyledWindow.With( + renderInterface: new NullRenderingPlatform(), + threadingInterface: new NullThreadingPlatform())); + + // Simulate an application with a lot of styles by creating a tree of nested panels, + // each with a bunch of styles applied. + var (rootPanel, leafPanel) = CreateNestedPanels(10); + + // We're benchmarking how long it takes to switch control theme on a TextBox in this + // situation. + var baseTheme = (ControlTheme)Application.Current.FindResource(typeof(TextBox)) ?? + throw new Exception("Base TextBox theme not found."); + + _theme1 = new ControlTheme(typeof(TextBox)) + { + BasedOn = baseTheme, + Setters = { new Setter(TextBox.BackgroundProperty, Brushes.Red) }, + }; + + _theme2 = new ControlTheme(typeof(TextBox)) + { + BasedOn = baseTheme, + Setters = { new Setter(TextBox.BackgroundProperty, Brushes.Green) }, + }; + + _control = new TextBox { Theme = _theme1 }; + leafPanel.Children.Add(_control); + + _root = new TestRoot(true, rootPanel) + { + Renderer = new NullRenderer(), + }; + + _root.LayoutManager.ExecuteInitialLayoutPass(); + } + + [Benchmark] + [MethodImpl(MethodImplOptions.NoInlining)] + public void Change_ControlTheme() + { + if (_control.Background != Brushes.Red) + throw new Exception("Invalid benchmark state"); + + _control.Theme = _theme2; + _root.LayoutManager.ExecuteLayoutPass(); + + if (_control.Background != Brushes.Green) + throw new Exception("Invalid benchmark state"); + + _control.Theme = _theme1; + _root.LayoutManager.ExecuteLayoutPass(); + + if (_control.Background != Brushes.Red) + throw new Exception("Invalid benchmark state"); + } + + public void Dispose() + { + _app.Dispose(); + } + + private static (Panel, Panel) CreateNestedPanels(int count) + { + var root = new Panel(); + var last = root; + + for (var i = 0; i < count; ++i) + { + var panel = new Panel(); + panel.Styles.AddRange(CreateStyles()); + last.Children.Add(panel); + last = panel; + } + + return (root, last); + } + + private static IEnumerable CreateStyles() + { + var types = new[] + { + typeof(Border), + typeof(Button), + typeof(ButtonSpinner), + typeof(Carousel), + typeof(CheckBox), + typeof(ComboBox), + typeof(ContentControl), + typeof(Expander), + typeof(ItemsControl), + typeof(Label), + typeof(ListBox), + typeof(ProgressBar), + typeof(RadioButton), + typeof(RepeatButton), + typeof(ScrollViewer), + typeof(Slider), + typeof(Spinner), + typeof(SplitView), + typeof(TextBox), + typeof(ToggleSwitch), + typeof(TreeView), + typeof(Viewbox), + typeof(Window), + }; + + foreach (var type in types) + { + yield return new Style(x => x.OfType(type)) + { + Setters = { new Setter(Control.TagProperty, type.Name) } + }; + + yield return new Style(x => x.OfType(type).Class("foo")) + { + Setters = { new Setter(Control.TagProperty, type.Name + " foo") } + }; + + yield return new Style(x => x.OfType(type).Class("bar")) + { + Setters = { new Setter(Control.TagProperty, type.Name + " bar") } + }; + } + } + } +} From 326dac232899a17d7f6ae8852c81357282af0cfa Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 18 Nov 2022 12:02:32 +0100 Subject: [PATCH 2/5] Refactored how we switch control themes. Instead of simply wiping all control themes and styles that are applied to a control, we can now just remove the `ValueFrame`s which relate to the control theme that was changed. To do this, added `ValueFrame.FramePriority` which encodes both the `BindingPriority` and source of the frame (style, control theme, templated parent control theme). --- src/Avalonia.Base/Layout/Layoutable.cs | 7 + .../PropertyStore/FramePriority.cs | 36 +++ .../PropertyStore/ImmediateValueFrame.cs | 2 +- src/Avalonia.Base/PropertyStore/ValueFrame.cs | 20 +- src/Avalonia.Base/PropertyStore/ValueStore.cs | 33 ++- src/Avalonia.Base/StyledElement.cs | 95 ++++---- src/Avalonia.Base/Styling/ControlTheme.cs | 7 +- src/Avalonia.Base/Styling/Style.cs | 5 +- src/Avalonia.Base/Styling/StyleBase.cs | 6 +- src/Avalonia.Base/Styling/StyleInstance.cs | 13 +- src/Avalonia.Base/Styling/Styles.cs | 3 +- .../Primitives/TemplatedControl.cs | 59 ++--- .../Animation/AnimatableTests.cs | 3 +- .../PropertyStore/ValueStoreTests_Frames.cs | 2 +- .../Styling/SetterTests.cs | 3 +- .../Styling/StyleTests.cs | 17 +- .../Styling/StyledElementTests_Theming.cs | 218 +++++++++++++++++- .../Styling/Style_Activation.cs | 3 +- .../Styling/Style_Apply.cs | 3 +- .../Styling/Style_ClassSelector.cs | 7 +- .../Styling/Style_NonActive.cs | 3 +- .../StyleTests.cs | 3 +- 22 files changed, 410 insertions(+), 138 deletions(-) create mode 100644 src/Avalonia.Base/PropertyStore/FramePriority.cs diff --git a/src/Avalonia.Base/Layout/Layoutable.cs b/src/Avalonia.Base/Layout/Layoutable.cs index 527b63292d..d09d1dc8d2 100644 --- a/src/Avalonia.Base/Layout/Layoutable.cs +++ b/src/Avalonia.Base/Layout/Layoutable.cs @@ -1,5 +1,6 @@ using System; using Avalonia.Logging; +using Avalonia.Styling; using Avalonia.VisualTree; #nullable enable @@ -795,6 +796,12 @@ namespace Avalonia.Layout base.OnVisualParentChanged(oldParent, newParent); } + private protected override void OnControlThemeChanged() + { + base.OnControlThemeChanged(); + InvalidateMeasure(); + } + /// /// Called when the layout manager raises a LayoutUpdated event. /// diff --git a/src/Avalonia.Base/PropertyStore/FramePriority.cs b/src/Avalonia.Base/PropertyStore/FramePriority.cs new file mode 100644 index 0000000000..950a8375f2 --- /dev/null +++ b/src/Avalonia.Base/PropertyStore/FramePriority.cs @@ -0,0 +1,36 @@ +using System.Diagnostics; +using Avalonia.Data; + +namespace Avalonia.PropertyStore +{ + internal enum FramePriority : sbyte + { + Animation, + AnimationTemplatedParentTheme, + AnimationTheme, + StyleTrigger, + StyleTriggerTemplatedParentTheme, + StyleTriggerTheme, + Template, + TemplateTemplatedParentTheme, + TemplateTheme, + Style, + StyleTemplatedParentTheme, + StyleTheme, + } + + internal static class FramePriorityExtensions + { + public static FramePriority ToFramePriority(this BindingPriority priority, FrameType type = FrameType.Style) + { + Debug.Assert(priority != BindingPriority.LocalValue); + var p = (int)(priority > 0 ? priority : priority + 1); + return (FramePriority)(p * 3 + (int)type); + } + + public static bool IsType(this FramePriority priority, FrameType type) + { + return (FrameType)((int)priority % 3) == type; + } + } +} diff --git a/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs b/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs index 1d886e7501..756ab7aadf 100644 --- a/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs +++ b/src/Avalonia.Base/PropertyStore/ImmediateValueFrame.cs @@ -10,8 +10,8 @@ namespace Avalonia.PropertyStore internal class ImmediateValueFrame : ValueFrame { public ImmediateValueFrame(BindingPriority priority) + : base(priority, FrameType.Style) { - Priority = priority; } public TypedBindingEntry AddBinding( diff --git a/src/Avalonia.Base/PropertyStore/ValueFrame.cs b/src/Avalonia.Base/PropertyStore/ValueFrame.cs index 5ada4b3c84..7a9d1bb13a 100644 --- a/src/Avalonia.Base/PropertyStore/ValueFrame.cs +++ b/src/Avalonia.Base/PropertyStore/ValueFrame.cs @@ -1,13 +1,18 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using Avalonia.Data; using Avalonia.Utilities; -using static Avalonia.Rendering.Composition.Animations.PropertySetSnapshot; namespace Avalonia.PropertyStore { + internal enum FrameType + { + Style, + TemplatedParentTheme, + Theme, + } + internal abstract class ValueFrame { private List? _entries; @@ -15,11 +20,18 @@ namespace Avalonia.PropertyStore private ValueStore? _owner; private bool _isShared; + protected ValueFrame(BindingPriority priority, FrameType type) + { + Priority = priority; + FramePriority = priority.ToFramePriority(type); + } + public int EntryCount => _index.Count; public bool IsActive => GetIsActive(out _); public ValueStore? Owner => !_isShared ? _owner : throw new AvaloniaInternalException("Cannot get owner for shared ValueFrame"); - public BindingPriority Priority { get; protected set; } + public BindingPriority Priority { get; } + public FramePriority FramePriority { get; } public bool Contains(AvaloniaProperty property) => _index.ContainsKey(property); diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index d858e30212..e14d018564 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/src/Avalonia.Base/PropertyStore/ValueStore.cs @@ -4,8 +4,8 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using Avalonia.Data; using Avalonia.Diagnostics; -using Avalonia.Logging; using Avalonia.Utilities; +using static Avalonia.Rendering.Composition.Animations.PropertySetSnapshot; namespace Avalonia.PropertyStore { @@ -580,6 +580,29 @@ namespace Avalonia.PropertyStore return false; } + public void RemoveFrames(FrameType type) + { + var removed = false; + + for (var i = _frames.Count - 1; i >= 0; --i) + { + var frame = _frames[i]; + + if (frame.FramePriority.IsType(type)) + { + _frames.RemoveAt(i); + frame.Dispose(); + removed = true; + } + } + + if (removed) + { + ++_frameGeneration; + ReevaluateEffectiveValues(); + } + } + public AvaloniaPropertyValue GetDiagnostic(AvaloniaProperty property) { object? value; @@ -612,7 +635,7 @@ namespace Avalonia.PropertyStore { Debug.Assert(!_frames.Contains(frame)); - var index = BinarySearchFrame(frame.Priority); + var index = BinarySearchFrame(frame.FramePriority); _frames.Insert(index, frame); ++_frameGeneration; frame.SetOwner(this); @@ -626,7 +649,7 @@ namespace Avalonia.PropertyStore { Debug.Assert(priority != BindingPriority.LocalValue); - var index = BinarySearchFrame(priority); + var index = BinarySearchFrame(priority.ToFramePriority()); if (index > 0 && _frames[index - 1] is ImmediateValueFrame f && f.Priority == priority && @@ -914,7 +937,7 @@ namespace Avalonia.PropertyStore } } - private int BinarySearchFrame(BindingPriority priority) + private int BinarySearchFrame(FramePriority priority) { var lo = 0; var hi = _frames.Count - 1; @@ -923,7 +946,7 @@ namespace Avalonia.PropertyStore while (lo <= hi) { var i = lo + ((hi - lo) >> 1); - var order = priority - _frames[i].Priority; + var order = priority - _frames[i].FramePriority; if (order <= 0) { diff --git a/src/Avalonia.Base/StyledElement.cs b/src/Avalonia.Base/StyledElement.cs index c72f398fd9..33bca9b0ab 100644 --- a/src/Avalonia.Base/StyledElement.cs +++ b/src/Avalonia.Base/StyledElement.cs @@ -3,6 +3,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; using System.ComponentModel; +using System.Diagnostics; using System.Linq; using Avalonia.Animation; using Avalonia.Collections; @@ -11,6 +12,7 @@ using Avalonia.Data; using Avalonia.Diagnostics; using Avalonia.Logging; using Avalonia.LogicalTree; +using Avalonia.PropertyStore; using Avalonia.Styling; namespace Avalonia @@ -69,10 +71,10 @@ namespace Avalonia private IAvaloniaList? _logicalChildren; private IResourceDictionary? _resources; private Styles? _styles; - private bool _styled; + private bool _stylesApplied; + private bool _themeApplied; private ITemplatedControl? _templatedParent; private bool _dataContextUpdating; - private bool _hasPromotedTheme; private ControlTheme? _implicitTheme; /// @@ -141,7 +143,7 @@ namespace Avalonia set { - if (_styled) + if (_stylesApplied) { throw new InvalidOperationException("Cannot set Name : styled element already styled."); } @@ -353,31 +355,31 @@ namespace Avalonia /// public bool ApplyStyling() { - if (_initCount == 0 && !_styled) + if (_initCount == 0 && (!_stylesApplied || !_themeApplied)) { - var hasPromotedTheme = _hasPromotedTheme; - GetValueStore().BeginStyling(); try { - ApplyControlTheme(); - ApplyStyles(this); + if (!_themeApplied) + { + ApplyControlTheme(); + _themeApplied = true; + } + + if (!_stylesApplied) + { + ApplyStyles(this); + _stylesApplied = true; + } } finally { - _styled = true; GetValueStore().EndStyling(); } - - if (hasPromotedTheme) - { - _hasPromotedTheme = false; - ClearValue(ThemeProperty); - } } - return _styled; + return _stylesApplied; } /// @@ -615,31 +617,25 @@ namespace Avalonia if (change.Property == ThemeProperty) { - var (oldValue, newValue) = change.GetOldAndNewValue(); - - // Changing the theme detaches all styles, meaning that if the theme property was - // set via a style, it will get cleared! To work around this, if the value was - // applied at less than local value priority then promote the value to local value - // priority until styling is re-applied. - if (change.Priority > BindingPriority.LocalValue) - { - Theme = newValue; - _hasPromotedTheme = true; - } - else if (_hasPromotedTheme && change.Priority == BindingPriority.LocalValue) - { - _hasPromotedTheme = false; - } - - InvalidateStyles(); - - if (oldValue is not null) - DetachControlThemeFromTemplateChildren(oldValue); + OnControlThemeChanged(); + _themeApplied = false; } } - internal virtual void DetachControlThemeFromTemplateChildren(ControlTheme theme) + private protected virtual void OnControlThemeChanged() { + var values = GetValueStore(); + values.BeginStyling(); + try { values.RemoveFrames(FrameType.Theme); } + finally { values.EndStyling(); } + } + + internal virtual void OnTemplatedParentControlThemeChanged() + { + var values = GetValueStore(); + values.BeginStyling(); + try { values.RemoveFrames(FrameType.TemplatedParentTheme); } + finally { values.EndStyling(); } } internal ControlTheme? GetEffectiveTheme() @@ -736,26 +732,28 @@ namespace Avalonia var theme = GetEffectiveTheme(); if (theme is not null) - ApplyControlTheme(theme); + ApplyControlTheme(theme, FrameType.Theme); if (TemplatedParent is StyledElement styleableParent && styleableParent.GetEffectiveTheme() is { } parentTheme) { - ApplyControlTheme(parentTheme); + ApplyControlTheme(parentTheme, FrameType.TemplatedParentTheme); } } - private void ApplyControlTheme(ControlTheme theme) + private void ApplyControlTheme(ControlTheme theme, FrameType type) { + Debug.Assert(type is FrameType.Theme or FrameType.TemplatedParentTheme); + if (theme.BasedOn is ControlTheme basedOn) - ApplyControlTheme(basedOn); + ApplyControlTheme(basedOn, type); - theme.TryAttach(this, null); + theme.TryAttach(this, type); if (theme.HasChildren) { foreach (var child in theme.Children) - ApplyStyle(child, null); + ApplyStyle(child, null, type); } } @@ -768,17 +766,17 @@ namespace Avalonia if (host.IsStylesInitialized) { foreach (var style in host.Styles) - ApplyStyle(style, host); + ApplyStyle(style, host, FrameType.Style); } } - private void ApplyStyle(IStyle style, IStyleHost? host) + private void ApplyStyle(IStyle style, IStyleHost? host, FrameType type) { if (style is Style s) - s.TryAttach(this, host); + s.TryAttach(this, host, type); foreach (var child in style.Children) - ApplyStyle(child, host); + ApplyStyle(child, host, type); } private void OnAttachedToLogicalTreeCore(LogicalTreeAttachmentEventArgs e) @@ -895,6 +893,7 @@ namespace Avalonia for (var i = valueStore.Frames.Count - 1; i >= 0; --i) { if (valueStore.Frames[i] is StyleInstance si && + si.Source is not ControlTheme && (styles is null || styles.Contains(si.Source))) { valueStore.RemoveFrame(si); @@ -902,7 +901,7 @@ namespace Avalonia } valueStore.EndStyling(); - _styled = false; + _stylesApplied = false; } private void InvalidateStylesOnThisAndDescendents() diff --git a/src/Avalonia.Base/Styling/ControlTheme.cs b/src/Avalonia.Base/Styling/ControlTheme.cs index 2971703c95..5fc900d2cb 100644 --- a/src/Avalonia.Base/Styling/ControlTheme.cs +++ b/src/Avalonia.Base/Styling/ControlTheme.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using Avalonia.PropertyStore; namespace Avalonia.Styling @@ -36,8 +37,10 @@ namespace Avalonia.Styling throw new InvalidOperationException("ControlThemes cannot be added as a nested style."); } - internal override SelectorMatchResult TryAttach(IStyleable target, object? host) + internal SelectorMatchResult TryAttach(IStyleable target, FrameType type) { + Debug.Assert(type is FrameType.Theme or FrameType.TemplatedParentTheme); + _ = target ?? throw new ArgumentNullException(nameof(target)); if (TargetType is null) @@ -45,7 +48,7 @@ namespace Avalonia.Styling if (HasSettersOrAnimations && TargetType.IsAssignableFrom(target.StyleKey)) { - Attach(target, null); + Attach(target, null, type); return SelectorMatchResult.AlwaysThisType; } diff --git a/src/Avalonia.Base/Styling/Style.cs b/src/Avalonia.Base/Styling/Style.cs index aad91824d3..15d8a9fe2e 100644 --- a/src/Avalonia.Base/Styling/Style.cs +++ b/src/Avalonia.Base/Styling/Style.cs @@ -1,4 +1,5 @@ using System; +using Avalonia.PropertyStore; namespace Avalonia.Styling { @@ -58,7 +59,7 @@ namespace Avalonia.Styling base.SetParent(parent); } - internal override SelectorMatchResult TryAttach(IStyleable target, object? host) + internal SelectorMatchResult TryAttach(IStyleable target, object? host, FrameType type) { _ = target ?? throw new ArgumentNullException(nameof(target)); @@ -73,7 +74,7 @@ namespace Avalonia.Styling if (match.IsMatch) { - Attach(target, match.Activator); + Attach(target, match.Activator, type); } result = match.Result; diff --git a/src/Avalonia.Base/Styling/StyleBase.cs b/src/Avalonia.Base/Styling/StyleBase.cs index dba80df2e5..83fcf04d2f 100644 --- a/src/Avalonia.Base/Styling/StyleBase.cs +++ b/src/Avalonia.Base/Styling/StyleBase.cs @@ -92,9 +92,7 @@ namespace Avalonia.Styling return false; } - internal abstract SelectorMatchResult TryAttach(IStyleable target, object? host); - - internal ValueFrame Attach(IStyleable target, IStyleActivator? activator) + internal ValueFrame Attach(IStyleable target, IStyleActivator? activator, FrameType type) { if (target is not AvaloniaObject ao) throw new InvalidOperationException("Styles can only be applied to AvaloniaObjects."); @@ -109,7 +107,7 @@ namespace Avalonia.Styling { var canShareInstance = activator is null; - instance = new StyleInstance(this, activator); + instance = new StyleInstance(this, activator, type); if (_setters is not null) { diff --git a/src/Avalonia.Base/Styling/StyleInstance.cs b/src/Avalonia.Base/Styling/StyleInstance.cs index 2d7c695b32..4985aa16c7 100644 --- a/src/Avalonia.Base/Styling/StyleInstance.cs +++ b/src/Avalonia.Base/Styling/StyleInstance.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Reactive.Subjects; using Avalonia.Animation; using Avalonia.Data; @@ -27,10 +26,13 @@ namespace Avalonia.Styling private List? _animations; private Subject? _animationTrigger; - public StyleInstance(IStyle style, IStyleActivator? activator) + public StyleInstance( + IStyle style, + IStyleActivator? activator, + FrameType type) + : base(GetPriority(activator), type) { _activator = activator; - Priority = activator is object ? BindingPriority.StyleTrigger : BindingPriority.Style; Source = style; } @@ -99,5 +101,10 @@ namespace Avalonia.Styling hasChanged = _isActive != previous; return _isActive; } + + private static BindingPriority GetPriority(IStyleActivator? activator) + { + return activator is not null ? BindingPriority.StyleTrigger : BindingPriority.Style; + } } } diff --git a/src/Avalonia.Base/Styling/Styles.cs b/src/Avalonia.Base/Styling/Styles.cs index 76271b9748..f22bbc0eae 100644 --- a/src/Avalonia.Base/Styling/Styles.cs +++ b/src/Avalonia.Base/Styling/Styles.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Collections.Specialized; using Avalonia.Collections; using Avalonia.Controls; +using Avalonia.PropertyStore; namespace Avalonia.Styling { @@ -233,7 +234,7 @@ namespace Avalonia.Styling { if (s is not Style style) continue; - var r = style.TryAttach(target, host); + var r = style.TryAttach(target, host, FrameType.Style); if (r > result) result = r; } diff --git a/src/Avalonia.Controls/Primitives/TemplatedControl.cs b/src/Avalonia.Controls/Primitives/TemplatedControl.cs index 80151fbfb3..17d90e6dd0 100644 --- a/src/Avalonia.Controls/Primitives/TemplatedControl.cs +++ b/src/Avalonia.Controls/Primitives/TemplatedControl.cs @@ -5,6 +5,7 @@ using Avalonia.Interactivity; using Avalonia.Logging; using Avalonia.LogicalTree; using Avalonia.Media; +using Avalonia.PropertyStore; using Avalonia.Styling; using Avalonia.VisualTree; @@ -395,56 +396,36 @@ namespace Avalonia.Controls.Primitives } } - internal override void DetachControlThemeFromTemplateChildren(ControlTheme theme) + private protected override void OnControlThemeChanged() { - static ControlTheme? GetControlTheme(StyleBase style) - { - var s = style; + base.OnControlThemeChanged(); - while (s is not null) + var count = VisualChildren.Count; + for (var i = 0; i < count; ++i) + { + if (VisualChildren[i] is StyledElement child && + child.TemplatedParent == this) { - if (s is ControlTheme c) - return c; - s = s.Parent as StyleBase; + child.OnTemplatedParentControlThemeChanged(); } - - return null; } + } - static void Detach(Visual control, ITemplatedControl templatedParent, ControlTheme theme) - { - var valueStore = control.GetValueStore(); - var count = valueStore.Frames.Count; - - if (control != templatedParent) - { - valueStore.BeginStyling(); - - for (var i = count - 1; i >= 0; --i) - { - if (valueStore.Frames[i] is StyleInstance si && - si.Source is StyleBase style && - GetControlTheme(style) == theme) - { - valueStore.RemoveFrame(si); - } - } - - valueStore.EndStyling(); - } + internal override void OnTemplatedParentControlThemeChanged() + { + base.OnTemplatedParentControlThemeChanged(); - var children = ((IVisual)control).VisualChildren; - count = children.Count; + var count = VisualChildren.Count; + var templatedParent = TemplatedParent; - for (var i = 0; i < count; i++) + for (var i = 0; i < count; ++i) + { + if (VisualChildren[i] is TemplatedControl child && + child.TemplatedParent == templatedParent) { - if (children[i] is Visual v && - v.TemplatedParent == templatedParent) - Detach(v, templatedParent, theme); + child.OnTemplatedParentControlThemeChanged(); } } - - Detach(this, this, theme); } } } diff --git a/tests/Avalonia.Base.UnitTests/Animation/AnimatableTests.cs b/tests/Avalonia.Base.UnitTests/Animation/AnimatableTests.cs index 668b8a875c..dec813b3b0 100644 --- a/tests/Avalonia.Base.UnitTests/Animation/AnimatableTests.cs +++ b/tests/Avalonia.Base.UnitTests/Animation/AnimatableTests.cs @@ -5,6 +5,7 @@ using Avalonia.Controls.Shapes; using Avalonia.Data; using Avalonia.Layout; using Avalonia.Media; +using Avalonia.PropertyStore; using Avalonia.Styling; using Avalonia.UnitTests; using Moq; @@ -435,7 +436,7 @@ namespace Avalonia.Base.UnitTests.Animation } }; - style.TryAttach(control, control); + style.TryAttach(control, control, FrameType.Style); // Which means that the transition state hasn't been initialized with the new // Transitions when the Opacity change notification gets raised here. diff --git a/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Frames.cs b/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Frames.cs index bb726a1d63..3a307447ac 100644 --- a/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Frames.cs +++ b/tests/Avalonia.Base.UnitTests/PropertyStore/ValueStoreTests_Frames.cs @@ -117,7 +117,7 @@ namespace Avalonia.Base.UnitTests.PropertyStore private static StyleInstance InstanceStyle(Style style, StyledElement target) { - var result = new StyleInstance(style, null); + var result = new StyleInstance(style, null, FrameType.Style); foreach (var setter in style.Setters) result.Add(setter.Instance(result, target)); diff --git a/tests/Avalonia.Base.UnitTests/Styling/SetterTests.cs b/tests/Avalonia.Base.UnitTests/Styling/SetterTests.cs index dc31d3d3ec..18f572dedc 100644 --- a/tests/Avalonia.Base.UnitTests/Styling/SetterTests.cs +++ b/tests/Avalonia.Base.UnitTests/Styling/SetterTests.cs @@ -6,6 +6,7 @@ using Avalonia.Controls.Templates; using Avalonia.Data; using Avalonia.Data.Converters; using Avalonia.Media; +using Avalonia.PropertyStore; using Avalonia.Styling; using Avalonia.UnitTests; using Moq; @@ -503,7 +504,7 @@ namespace Avalonia.Base.UnitTests.Styling private void Apply(Style style, Control control) { - style.TryAttach(control, null); + style.TryAttach(control, null, FrameType.Style); } private void Apply(Setter setter, Control control) diff --git a/tests/Avalonia.Base.UnitTests/Styling/StyleTests.cs b/tests/Avalonia.Base.UnitTests/Styling/StyleTests.cs index e7ecba61a7..a318a8d76a 100644 --- a/tests/Avalonia.Base.UnitTests/Styling/StyleTests.cs +++ b/tests/Avalonia.Base.UnitTests/Styling/StyleTests.cs @@ -5,6 +5,7 @@ using Avalonia.Base.UnitTests.Animation; using Avalonia.Controls; using Avalonia.Controls.Templates; using Avalonia.Data; +using Avalonia.PropertyStore; using Avalonia.Styling; using Avalonia.UnitTests; using Moq; @@ -27,7 +28,7 @@ namespace Avalonia.Base.UnitTests.Styling var target = new Class1(); - style.TryAttach(target, null); + style.TryAttach(target, null, FrameType.Style); Assert.Equal("Foo", target.Foo); } @@ -45,7 +46,7 @@ namespace Avalonia.Base.UnitTests.Styling var target = new Class1(); - style.TryAttach(target, null); + style.TryAttach(target, null, FrameType.Style); Assert.Equal("foodefault", target.Foo); target.Classes.Add("foo"); Assert.Equal("Foo", target.Foo); @@ -66,7 +67,7 @@ namespace Avalonia.Base.UnitTests.Styling var target = new Class1(); - style.TryAttach(target, target); + style.TryAttach(target, target, FrameType.Style); Assert.Equal("Foo", target.Foo); } @@ -92,7 +93,7 @@ namespace Avalonia.Base.UnitTests.Styling var target = new Class1(); var other = new Class1(); - style.TryAttach(target, other); + style.TryAttach(target, other, FrameType.Style); Assert.Equal("foodefault", target.Foo); } @@ -113,7 +114,7 @@ namespace Avalonia.Base.UnitTests.Styling Foo = "Original", }; - style.TryAttach(target, null); + style.TryAttach(target, null, FrameType.Style); Assert.Equal("Original", target.Foo); } @@ -577,7 +578,7 @@ namespace Avalonia.Base.UnitTests.Styling Child = border = new Border(), }; - style.TryAttach(border, null); + style.TryAttach(border, null, FrameType.Style); Assert.Equal(new Thickness(4), border.BorderThickness); root.Child = null; @@ -761,7 +762,7 @@ namespace Avalonia.Base.UnitTests.Styling var target = new Class1(); - style.TryAttach(target, null); + style.TryAttach(target, null, FrameType.Style); Assert.Equal(1, target.Classes.ListenerCount); @@ -874,7 +875,7 @@ namespace Avalonia.Base.UnitTests.Styling var clock = new TestClock(); var target = new Class1 { Clock = clock }; - style.TryAttach(target, null); + style.TryAttach(target, null, FrameType.Style); Assert.Equal(0.0, target.Double); diff --git a/tests/Avalonia.Base.UnitTests/Styling/StyledElementTests_Theming.cs b/tests/Avalonia.Base.UnitTests/Styling/StyledElementTests_Theming.cs index a1dac931ce..b5524affb7 100644 --- a/tests/Avalonia.Base.UnitTests/Styling/StyledElementTests_Theming.cs +++ b/tests/Avalonia.Base.UnitTests/Styling/StyledElementTests_Theming.cs @@ -23,7 +23,7 @@ public class StyledElementTests_Theming Assert.Null(target.Template); - var root = CreateRoot(target); + CreateRoot(target); Assert.NotNull(target.Template); var border = Assert.IsType(target.VisualChild); @@ -43,7 +43,7 @@ public class StyledElementTests_Theming Assert.Null(target.Template); - var root = CreateRoot(target); + CreateRoot(target); Assert.NotNull(target.Template); var border = Assert.IsType(target.VisualChild); @@ -57,7 +57,7 @@ public class StyledElementTests_Theming public void Theme_Is_Detached_When_Theme_Property_Cleared() { var target = CreateTarget(); - var root = CreateRoot(target); + CreateRoot(target); Assert.NotNull(target.Template); @@ -66,7 +66,47 @@ public class StyledElementTests_Theming } [Fact] - public void Theme_Is_Detached_From_Template_Controls_When_Theme_Property_Cleared() + public void Setting_Explicit_Theme_Detaches_Default_Theme() + { + var target = new ThemedControl(); + var root = new TestRoot + { + Resources = { { typeof(ThemedControl), CreateTheme() } }, + Child = target, + }; + + root.LayoutManager.ExecuteInitialLayoutPass(); + + Assert.Equal("theme", target.Tag); + + target.Theme = new ControlTheme(typeof(ThemedControl)) + { + Setters = + { + new Setter(ThemedControl.BackgroundProperty, Brushes.Yellow), + } + }; + + root.LayoutManager.ExecuteLayoutPass(); + + Assert.Null(target.Tag); + Assert.Equal(Brushes.Yellow, target.Background); + } + + [Fact] + public void Unrelated_Styles_Are_Not_Detached_When_Theme_Property_Cleared() + { + var target = CreateTarget(); + CreateRoot(target, createAdditionalStyles: true); + + Assert.Equal("style", target.Tag); + + target.Theme = null; + Assert.Equal("style", target.Tag); + } + + [Fact] + public void TemplatedParent_Theme_Is_Detached_From_Template_Controls_When_Theme_Property_Cleared() { var theme = new ControlTheme { @@ -93,10 +133,115 @@ public class StyledElementTests_Theming target.Theme = null; - Assert.IsType(target.VisualChild); + Assert.Same(canvas, target.VisualChild); Assert.Null(canvas.Background); } + [Fact] + public void Primary_Theme_Is_Not_Detached_From_Template_Controls_When_Theme_Property_Cleared() + { + var templatedParentTheme = new ControlTheme + { + TargetType = typeof(ThemedControl), + Children = + { + new Style(x => x.Nesting().Template().OfType - internal class ImmediateValueFrame : ValueFrame + internal sealed class ImmediateValueFrame : ValueFrame { public ImmediateValueFrame(BindingPriority priority) : base(priority, FrameType.Style) diff --git a/src/Avalonia.Base/PropertyStore/ValueStore.cs b/src/Avalonia.Base/PropertyStore/ValueStore.cs index e14d018564..92e5288255 100644 --- a/src/Avalonia.Base/PropertyStore/ValueStore.cs +++ b/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 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(); diff --git a/src/Avalonia.Base/StyledElement.cs b/src/Avalonia.Base/StyledElement.cs index 33bca9b0ab..187edd8335 100644 --- a/src/Avalonia.Base/StyledElement.cs +++ b/src/Avalonia.Base/StyledElement.cs @@ -382,11 +382,6 @@ namespace Avalonia return _stylesApplied; } - /// - /// Detaches all styles from the element and queues a restyle. - /// - 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 styles) { - InvalidateStylesOnThisAndDescendents(); + if (HasSettersOrAnimations(styles)) + InvalidateStyles(recurse: true); } void IStyleHost.StylesRemoved(IReadOnlyList 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? styles = null) + private void DetachStyles(IReadOnlyList