From 34b5f07e9656855e20ec42c292aa0b738c3b186a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 13 Nov 2020 12:23:08 +0100 Subject: [PATCH 01/12] Added failing tests for #5027. We shouldn't subscribe to bindings until needed. --- .../Avalonia.Markup.Xaml.UnitTests.csproj | 1 + .../DynamicResourceExtensionTests.cs | 96 +++++++++++++++++++ .../Avalonia.Styling.UnitTests/StyleTests.cs | 82 ++++++++++++++++ 3 files changed, 179 insertions(+) diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/Avalonia.Markup.Xaml.UnitTests.csproj b/tests/Avalonia.Markup.Xaml.UnitTests/Avalonia.Markup.Xaml.UnitTests.csproj index ad3592294dd..b070765a602 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/Avalonia.Markup.Xaml.UnitTests.csproj +++ b/tests/Avalonia.Markup.Xaml.UnitTests/Avalonia.Markup.Xaml.UnitTests.csproj @@ -3,6 +3,7 @@ netcoreapp3.1;net47 Library true + latest diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs index 47a73cb360a..410f579a13a 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/MarkupExtensions/DynamicResourceExtensionTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using Avalonia.Controls; using Avalonia.Controls.Presenters; @@ -792,6 +793,82 @@ public void Automatically_Converts_Color_To_SolidColorBrush() Assert.Equal(0xff506070, brush.Color.ToUint32()); } + [Fact] + public void Resource_In_Non_Matching_Style_Is_Not_Resolved() + { + using var app = StyledWindow(); + + var xaml = @" + + + + + + + + + + + + + + + +"; + + var window = (Window)AvaloniaRuntimeXamlLoader.Load(xaml); + var border = window.FindControl("border"); + + Assert.Equal("bar", border.Tag); + + var resourceProvider = (TrackingResourceProvider)window.Resources.MergedDictionaries[0]; + Assert.Equal(new[] { "bar" }, resourceProvider.RequestedResources); + } + + [Fact] + public void Resource_In_Non_Active_Style_Is_Not_Resolved() + { + using var app = StyledWindow(); + + var xaml = @" + + + + + + + + + + + + + + + +"; + + var window = (Window)AvaloniaRuntimeXamlLoader.Load(xaml); + var border = window.FindControl("border"); + + Assert.Equal("bar", border.Tag); + + var resourceProvider = (TrackingResourceProvider)window.Resources.MergedDictionaries[0]; + Assert.Equal(new[] { "bar" }, resourceProvider.RequestedResources); + } + private IDisposable StyledWindow(params (string, string)[] assets) { var services = TestServices.StyledWindow.With( @@ -822,4 +899,23 @@ private Style WindowStyle() }; } } + + public class TrackingResourceProvider : IResourceProvider + { + public IResourceHost Owner { get; private set; } + public bool HasResources => true; + public List RequestedResources { get; } = new List(); + + public event EventHandler OwnerChanged; + + public void AddOwner(IResourceHost owner) => Owner = owner; + public void RemoveOwner(IResourceHost owner) => Owner = null; + + public bool TryGetResource(object key, out object value) + { + RequestedResources.Add(key); + value = key; + return true; + } + } } diff --git a/tests/Avalonia.Styling.UnitTests/StyleTests.cs b/tests/Avalonia.Styling.UnitTests/StyleTests.cs index df94887340e..61d9f961f34 100644 --- a/tests/Avalonia.Styling.UnitTests/StyleTests.cs +++ b/tests/Avalonia.Styling.UnitTests/StyleTests.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using Avalonia.Controls; +using Avalonia.Controls.Templates; using Avalonia.Data; using Avalonia.UnitTests; using Moq; @@ -217,6 +218,78 @@ public void Later_Styles_Should_Override_Earlier_3() Assert.Equal(new[] { "foodefault", "Bar" }, values); } + [Fact] + public void Template_In_Non_Matching_Style_Is_Not_Built() + { + var instantiationCount = 0; + var template = new FuncTemplate(() => + { + ++instantiationCount; + return new Class1(); + }); + + Styles styles = new Styles + { + new Style(x => x.OfType().Class("foo")) + { + Setters = + { + new Setter(Class1.ChildProperty, template), + }, + }, + + new Style(x => x.OfType()) + { + Setters = + { + new Setter(Class1.ChildProperty, template), + }, + } + }; + + var target = new Class1(); + styles.TryAttach(target, null); + + Assert.NotNull(target.Child); + Assert.Equal(1, instantiationCount); + } + + [Fact] + public void Template_In_Inactive_Style_Is_Not_Built() + { + var instantiationCount = 0; + var template = new FuncTemplate(() => + { + ++instantiationCount; + return new Class1(); + }); + + Styles styles = new Styles + { + new Style(x => x.OfType()) + { + Setters = + { + new Setter(Class1.ChildProperty, template), + }, + }, + + new Style(x => x.OfType()) + { + Setters = + { + new Setter(Class1.ChildProperty, template), + }, + } + }; + + var target = new Class1(); + styles.TryAttach(target, null); + + Assert.NotNull(target.Child); + Assert.Equal(1, instantiationCount); + } + [Fact] public void Style_Should_Detach_When_Control_Removed_From_Logical_Tree() { @@ -453,12 +526,21 @@ private class Class1 : Control public static readonly StyledProperty FooProperty = AvaloniaProperty.Register(nameof(Foo), "foodefault"); + public static readonly StyledProperty ChildProperty = + AvaloniaProperty.Register(nameof(Child)); + public string Foo { get { return GetValue(FooProperty); } set { SetValue(FooProperty, value); } } + public Class1 Child + { + get => GetValue(ChildProperty); + set => SetValue(ChildProperty, value); + } + protected override Size MeasureOverride(Size availableSize) { throw new NotImplementedException(); From 74695977123b4794e62cb188c40b9832bd3c11e4 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 16 Nov 2020 08:39:31 +0100 Subject: [PATCH 02/12] Added benchmark for fluent RepeatButton. As that's where #5027 was showing up most. --- .../Avalonia.Benchmarks.csproj | 1 + .../Themes/FluentBenchmark.cs | 74 +++++++++++++++++++ .../Avalonia.UnitTests/UnitTestApplication.cs | 2 +- 3 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 tests/Avalonia.Benchmarks/Themes/FluentBenchmark.cs diff --git a/tests/Avalonia.Benchmarks/Avalonia.Benchmarks.csproj b/tests/Avalonia.Benchmarks/Avalonia.Benchmarks.csproj index 251894e324b..e8e69efdbc5 100644 --- a/tests/Avalonia.Benchmarks/Avalonia.Benchmarks.csproj +++ b/tests/Avalonia.Benchmarks/Avalonia.Benchmarks.csproj @@ -12,6 +12,7 @@ + diff --git a/tests/Avalonia.Benchmarks/Themes/FluentBenchmark.cs b/tests/Avalonia.Benchmarks/Themes/FluentBenchmark.cs new file mode 100644 index 00000000000..3a055023715 --- /dev/null +++ b/tests/Avalonia.Benchmarks/Themes/FluentBenchmark.cs @@ -0,0 +1,74 @@ +using System; +using Avalonia.Controls; +using Avalonia.Markup.Xaml; +using Avalonia.Markup.Xaml.Styling; +using Avalonia.Platform; +using Avalonia.Shared.PlatformSupport; +using Avalonia.Styling; +using Avalonia.UnitTests; +using BenchmarkDotNet.Attributes; +using Moq; + +namespace Avalonia.Benchmarks.Themes +{ + [MemoryDiagnoser] + public class FluentBenchmark + { + private readonly IDisposable _app; + private readonly TestRoot _root; + + public FluentBenchmark() + { + _app = CreateApp(); + _root = new TestRoot(true, null) + { + Renderer = new NullRenderer() + }; + + _root.LayoutManager.ExecuteInitialLayoutPass(); + } + + public void Dispose() + { + _app.Dispose(); + } + + [Benchmark] + public void RepeatButton() + { + var button = new RepeatButton(); + _root.Child = button; + _root.LayoutManager.ExecuteLayoutPass(); + } + + private static IDisposable CreateApp() + { + var services = new TestServices( + assetLoader: new AssetLoader(), + globalClock: new MockGlobalClock(), + platform: new AppBuilder().RuntimePlatform, + renderInterface: new MockPlatformRenderInterface(), + standardCursorFactory: Mock.Of(), + styler: new Styler(), + theme: () => LoadFluentTheme(), + threadingInterface: new NullThreadingPlatform(), + fontManagerImpl: new MockFontManagerImpl(), + textShaperImpl: new MockTextShaperImpl(), + windowingPlatform: new MockWindowingPlatform()); + + return UnitTestApplication.Start(services); + } + + private static Styles LoadFluentTheme() + { + AssetLoader.RegisterResUriParsers(); + return new Styles + { + new StyleInclude(new Uri("avares://Avalonia.Benchmarks")) + { + Source = new Uri("avares://Avalonia.Themes.Fluent/Accents/FluentDark.xaml") + } + }; + } + } +} diff --git a/tests/Avalonia.UnitTests/UnitTestApplication.cs b/tests/Avalonia.UnitTests/UnitTestApplication.cs index e4a65f105dc..4e3f1ad28a4 100644 --- a/tests/Avalonia.UnitTests/UnitTestApplication.cs +++ b/tests/Avalonia.UnitTests/UnitTestApplication.cs @@ -25,6 +25,7 @@ public UnitTestApplication() : this(null) public UnitTestApplication(TestServices services) { _services = services ?? new TestServices(); + AvaloniaLocator.CurrentMutable.BindToSelf(this); RegisterServices(); } @@ -36,7 +37,6 @@ public static IDisposable Start(TestServices services = null) { var scope = AvaloniaLocator.EnterScope(); var app = new UnitTestApplication(services); - AvaloniaLocator.CurrentMutable.BindToSelf(app); Dispatcher.UIThread.UpdateServices(); return Disposable.Create(() => { From d8fbd95ef03288f3262f4de425fb1b335f17073e Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 12 Nov 2020 11:49:53 +0100 Subject: [PATCH 03/12] Don't subscribe to inner observable if not active. When subscribing to a `PropertySetterBindingInstance`, if the owner style is not active then there's no need to subscribe to the inner observable as this can cause resource lookups etc. Part of fixing #5027. --- .../Styling/PropertySetterBindingInstance.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Styling/Styling/PropertySetterBindingInstance.cs b/src/Avalonia.Styling/Styling/PropertySetterBindingInstance.cs index 929f7142bb5..a67190c8316 100644 --- a/src/Avalonia.Styling/Styling/PropertySetterBindingInstance.cs +++ b/src/Avalonia.Styling/Styling/PropertySetterBindingInstance.cs @@ -92,6 +92,7 @@ public void Activate() { if (!_isActive) { + _innerSubscription ??= _binding.Observable.Subscribe(_inner); _isActive = true; PublishNext(); } @@ -102,6 +103,8 @@ public void Deactivate() if (_isActive) { _isActive = false; + _innerSubscription?.Dispose(); + _innerSubscription = null; PublishNext(); } } @@ -148,7 +151,10 @@ void IObserver>.OnNext(BindingValue value) protected override void Subscribed() { - _innerSubscription = _binding.Observable.Subscribe(_inner); + if (_isActive) + { + _innerSubscription = _binding.Observable.Subscribe(_inner); + } } protected override void Unsubscribed() From 72a43174e18d668611d86f3a3210220643f369ce Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 14 Nov 2020 16:50:53 +0100 Subject: [PATCH 04/12] Lazily evaluate