From 302bf55b8a8827350795cef4945b41ffd980eb30 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 20 Jan 2019 01:15:53 +0100 Subject: [PATCH 01/21] Added failing tests for #1099. --- .../AvaloniaObjectTests_Inheritance.cs | 16 ++++++++++ .../ContentPresenterTests_InTemplate.cs | 28 +++++++++++++++- .../ContentPresenterTests_Standalone.cs | 32 ++++++++++++++++++- 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs index ef6e03a60b..c465db50fb 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Inheritance.cs @@ -1,6 +1,7 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using System.Collections.Generic; using Xunit; namespace Avalonia.Base.UnitTests @@ -115,6 +116,21 @@ namespace Avalonia.Base.UnitTests Assert.True(raised); } + [Fact] + public void PropertyChanged_Is_Raised_In_Parent_Before_Child() + { + var parent = new Class1(); + var child = new Class2 { Parent = parent }; + var result = new List(); + + parent.PropertyChanged += (s, e) => result.Add(parent); + child.PropertyChanged += (s, e) => result.Add(child); + + parent.SetValue(Class1.BazProperty, "changed"); + + Assert.Equal(new[] { parent, child }, result); + } + private class Class1 : AvaloniaObject { public static readonly StyledProperty FooProperty = diff --git a/tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_InTemplate.cs b/tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_InTemplate.cs index a524ca3e89..708e934214 100644 --- a/tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_InTemplate.cs +++ b/tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_InTemplate.cs @@ -4,6 +4,7 @@ using System.Linq; using Avalonia.Controls.Presenters; using Avalonia.Controls.Templates; +using Avalonia.Data; using Avalonia.LogicalTree; using Avalonia.UnitTests; using Avalonia.VisualTree; @@ -266,6 +267,31 @@ namespace Avalonia.Controls.UnitTests.Presenters Assert.IsType(target.Child); } + + [Fact] + public void Should_Not_Bind_Old_Child_To_New_DataContext() + { + // Test for issue #1099. + var textBlock = new TextBlock + { + [!TextBlock.TextProperty] = new Binding(), + }; + + var (target, host) = CreateTarget(); + host.DataTemplates.Add(new FuncDataTemplate(x => textBlock)); + host.DataTemplates.Add(new FuncDataTemplate(x => new Canvas())); + + target.Content = "foo"; + Assert.Same(textBlock, target.Child); + + textBlock.PropertyChanged += (s, e) => + { + Assert.NotEqual(e.NewValue, "42"); + }; + + target.Content = 42; + } + (ContentPresenter presenter, ContentControl templatedParent) CreateTarget() { var templatedParent = new ContentControl @@ -288,4 +314,4 @@ namespace Avalonia.Controls.UnitTests.Presenters public IControl Child { get; set; } } } -} \ No newline at end of file +} diff --git a/tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_Standalone.cs b/tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_Standalone.cs index 9d65f2cba7..2facee16b7 100644 --- a/tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_Standalone.cs +++ b/tests/Avalonia.Controls.UnitTests/Presenters/ContentPresenterTests_Standalone.cs @@ -14,6 +14,7 @@ using System.Linq; using Xunit; using Avalonia.Rendering; using Avalonia.Media; +using Avalonia.Data; namespace Avalonia.Controls.UnitTests.Presenters { @@ -204,7 +205,6 @@ namespace Avalonia.Controls.UnitTests.Presenters Assert.NotEqual(foo, logicalChildren.First()); } - [Fact] public void Changing_Background_Brush_Color_Should_Invalidate_Visual() { @@ -221,5 +221,35 @@ namespace Avalonia.Controls.UnitTests.Presenters renderer.Verify(x => x.AddDirty(target), Times.Once); } + + [Fact] + public void Should_Not_Bind_Old_Child_To_New_DataContext() + { + // Test for issue #1099. + var textBlock = new TextBlock + { + [!TextBlock.TextProperty] = new Binding(), + }; + + var target = new ContentPresenter() + { + DataTemplates = + { + new FuncDataTemplate(x => textBlock), + new FuncDataTemplate(x => new Canvas()), + }, + }; + + var root = new TestRoot(target); + target.Content = "foo"; + Assert.Same(textBlock, target.Child); + + textBlock.PropertyChanged += (s, e) => + { + Assert.NotEqual(e.NewValue, "42"); + }; + + target.Content = 42; + } } } From f75f85af2bd6d81b2deb9f571a2abd8e823441a3 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sun, 20 Jan 2019 01:19:25 +0100 Subject: [PATCH 02/21] Fix order of inherited property changed events. Previously `PropertyChanged` was raised on the child before the parent because the inherited value change was being notified by listening to the parent `PropertyChanged` event, and this event handler was added first. Added an `InheritablePropertyChanged` event which will be called only after all other property changed events have been raised and only for inheritable properties. --- src/Avalonia.Base/AvaloniaObject.cs | 37 +++++++++---------- src/Avalonia.Base/IAvaloniaObject.cs | 7 +++- .../SelectorTests_Child.cs | 1 + .../SelectorTests_Descendent.cs | 1 + .../TestControlBase.cs | 1 + .../TestTemplatedControl.cs | 1 + 6 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 7e8d733f1b..28148f6568 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -22,27 +22,11 @@ namespace Avalonia /// public class AvaloniaObject : IAvaloniaObject, IAvaloniaObjectDebug, INotifyPropertyChanged { - /// - /// The parent object that inherited values are inherited from. - /// private IAvaloniaObject _inheritanceParent; - - /// - /// Maintains a list of direct property binding subscriptions so that the binding source - /// doesn't get collected. - /// private List _directBindings; - - /// - /// Event handler for implementation. - /// private PropertyChangedEventHandler _inpcChanged; - - /// - /// Event handler for implementation. - /// private EventHandler _propertyChanged; - + private EventHandler _inheritablePropertyChanged; private ValueStore _values; private ValueStore Values => _values ?? (_values = new ValueStore(this)); @@ -98,6 +82,15 @@ namespace Avalonia remove { _inpcChanged -= value; } } + /// + /// Raised when an inheritable value changes on this object. + /// + event EventHandler IAvaloniaObject.InheritablePropertyChanged + { + add { _inheritablePropertyChanged += value; } + remove { _inheritablePropertyChanged -= value; } + } + /// /// Gets or sets the parent object that inherited values /// are inherited from. @@ -118,8 +111,9 @@ namespace Avalonia { if (_inheritanceParent != null) { - _inheritanceParent.PropertyChanged -= ParentPropertyChanged; + _inheritanceParent.InheritablePropertyChanged -= ParentPropertyChanged; } + var properties = AvaloniaPropertyRegistry.Instance.GetRegistered(this) .Concat(AvaloniaPropertyRegistry.Instance.GetRegisteredAttached(this.GetType())); var inherited = (from property in properties @@ -144,7 +138,7 @@ namespace Avalonia if (_inheritanceParent != null) { - _inheritanceParent.PropertyChanged += ParentPropertyChanged; + _inheritanceParent.InheritablePropertyChanged += ParentPropertyChanged; } } } @@ -509,6 +503,11 @@ namespace Avalonia PropertyChangedEventArgs e2 = new PropertyChangedEventArgs(property.Name); _inpcChanged(this, e2); } + + if (property.Inherits) + { + _inheritablePropertyChanged?.Invoke(this, e); + } } finally { diff --git a/src/Avalonia.Base/IAvaloniaObject.cs b/src/Avalonia.Base/IAvaloniaObject.cs index c11f8ada7e..5a3829167a 100644 --- a/src/Avalonia.Base/IAvaloniaObject.cs +++ b/src/Avalonia.Base/IAvaloniaObject.cs @@ -16,6 +16,11 @@ namespace Avalonia /// event EventHandler PropertyChanged; + /// + /// Raised when an inheritable value changes on this object. + /// + event EventHandler InheritablePropertyChanged; + /// /// Gets a value. /// @@ -97,4 +102,4 @@ namespace Avalonia IObservable source, BindingPriority priority = BindingPriority.LocalValue); } -} \ No newline at end of file +} diff --git a/tests/Avalonia.Styling.UnitTests/SelectorTests_Child.cs b/tests/Avalonia.Styling.UnitTests/SelectorTests_Child.cs index 0561837ad1..c6eeb1ec0e 100644 --- a/tests/Avalonia.Styling.UnitTests/SelectorTests_Child.cs +++ b/tests/Avalonia.Styling.UnitTests/SelectorTests_Child.cs @@ -89,6 +89,7 @@ namespace Avalonia.Styling.UnitTests } public event EventHandler PropertyChanged; + public event EventHandler InheritablePropertyChanged; public event EventHandler AttachedToLogicalTree; public event EventHandler DetachedFromLogicalTree; diff --git a/tests/Avalonia.Styling.UnitTests/SelectorTests_Descendent.cs b/tests/Avalonia.Styling.UnitTests/SelectorTests_Descendent.cs index 56dad13186..aef539becd 100644 --- a/tests/Avalonia.Styling.UnitTests/SelectorTests_Descendent.cs +++ b/tests/Avalonia.Styling.UnitTests/SelectorTests_Descendent.cs @@ -119,6 +119,7 @@ namespace Avalonia.Styling.UnitTests } public event EventHandler PropertyChanged; + public event EventHandler InheritablePropertyChanged; public event EventHandler AttachedToLogicalTree; public event EventHandler DetachedFromLogicalTree; diff --git a/tests/Avalonia.Styling.UnitTests/TestControlBase.cs b/tests/Avalonia.Styling.UnitTests/TestControlBase.cs index 82be755a39..d9fabf6f5d 100644 --- a/tests/Avalonia.Styling.UnitTests/TestControlBase.cs +++ b/tests/Avalonia.Styling.UnitTests/TestControlBase.cs @@ -19,6 +19,7 @@ namespace Avalonia.Styling.UnitTests #pragma warning disable CS0067 // Event not used public event EventHandler PropertyChanged; + public event EventHandler InheritablePropertyChanged; #pragma warning restore CS0067 public string Name { get; set; } diff --git a/tests/Avalonia.Styling.UnitTests/TestTemplatedControl.cs b/tests/Avalonia.Styling.UnitTests/TestTemplatedControl.cs index 03b2f03bf2..e92ac36e8f 100644 --- a/tests/Avalonia.Styling.UnitTests/TestTemplatedControl.cs +++ b/tests/Avalonia.Styling.UnitTests/TestTemplatedControl.cs @@ -12,6 +12,7 @@ namespace Avalonia.Styling.UnitTests public abstract class TestTemplatedControl : ITemplatedControl, IStyleable { public event EventHandler PropertyChanged; + public event EventHandler InheritablePropertyChanged; public abstract Classes Classes { From f54e48d9a2027ec491c2aeab0a59465f0c525ea8 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 22 Jan 2019 18:44:32 +0100 Subject: [PATCH 03/21] Added ContentPresenter.ChildChanging. And listen for this event in `ContentControlMixin` in order to remove the logical child before setting the `DataContext`. Fixes #1099 --- .../Mixins/ContentControlMixin.cs | 29 ++++++++++--- .../Presenters/ContentPresenter.cs | 42 +++++++++++++++---- .../Presenters/IContentPresenter.cs | 15 ++++++- 3 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/Avalonia.Controls/Mixins/ContentControlMixin.cs b/src/Avalonia.Controls/Mixins/ContentControlMixin.cs index c4da00f5d0..25b29e37e6 100644 --- a/src/Avalonia.Controls/Mixins/ContentControlMixin.cs +++ b/src/Avalonia.Controls/Mixins/ContentControlMixin.cs @@ -19,8 +19,8 @@ namespace Avalonia.Controls.Mixins /// /// The adds behavior to a control which acts as a content /// control such as and . It - /// updates keeps the control's logical children in sync with the content being displayed by - /// the control. + /// keeps the control's logical children in sync with the content being displayed by the + /// control. /// public class ContentControlMixin { @@ -49,25 +49,42 @@ namespace Avalonia.Controls.Mixins Contract.Requires(content != null); Contract.Requires(logicalChildrenSelector != null); + void ChildChanging(object s, AvaloniaPropertyChangedEventArgs e) + { + if (s is IControl sender && sender?.TemplatedParent is TControl parent) + { + UpdateLogicalChild( + sender, + logicalChildrenSelector(parent), + e.OldValue, + null); + } + } + void TemplateApplied(object s, RoutedEventArgs ev) { if (s is TControl sender) { var e = (TemplateAppliedEventArgs)ev; - var presenter = (IControl)e.NameScope.Find(presenterName); + var presenter = e.NameScope.Find(presenterName) as IContentPresenter; if (presenter != null) { presenter.ApplyTemplate(); var logicalChildren = logicalChildrenSelector(sender); - var subscription = presenter + var subscription = new CompositeDisposable(); + + presenter.ChildChanging += ChildChanging; + subscription.Add(Disposable.Create(() => presenter.ChildChanging -= ChildChanging)); + + subscription.Add(presenter .GetPropertyChangedObservable(ContentPresenter.ChildProperty) .Subscribe(c => UpdateLogicalChild( sender, logicalChildren, - c.OldValue, - c.NewValue)); + null, + c.NewValue))); UpdateLogicalChild( sender, diff --git a/src/Avalonia.Controls/Presenters/ContentPresenter.cs b/src/Avalonia.Controls/Presenters/ContentPresenter.cs index 83d8616e90..49f268c128 100644 --- a/src/Avalonia.Controls/Presenters/ContentPresenter.cs +++ b/src/Avalonia.Controls/Presenters/ContentPresenter.cs @@ -5,6 +5,7 @@ using System; using Avalonia.Controls.Primitives; using Avalonia.Controls.Templates; using Avalonia.Controls.Utils; +using Avalonia.Data; using Avalonia.Layout; using Avalonia.LogicalTree; using Avalonia.Media; @@ -82,6 +83,7 @@ namespace Avalonia.Controls.Presenters private IControl _child; private bool _createdChild; + EventHandler _childChanging; private IDataTemplate _dataTemplate; private readonly BorderRenderHelper _borderRenderer = new BorderRenderHelper(); @@ -188,6 +190,13 @@ namespace Avalonia.Controls.Presenters set { SetValue(PaddingProperty, value); } } + /// + event EventHandler IContentPresenter.ChildChanging + { + add => _childChanging += value; + remove => _childChanging -= value; + } + /// public sealed override void ApplyTemplate() { @@ -215,9 +224,30 @@ namespace Avalonia.Controls.Presenters var newChild = CreateChild(); // Remove the old child if we're not recycling it. - if (oldChild != null && newChild != oldChild) + if (newChild != oldChild) { - VisualChildren.Remove(oldChild); + if (oldChild != null) + { + VisualChildren.Remove(oldChild); + } + + if (oldChild?.Parent == this) + { + // If we're the child's parent then the presenter isn't in a ContentControl's + // template. + LogicalChildren.Remove(oldChild); + } + else + { + // If we're in a ContentControl's template then invoke ChildChanging to let + // ContentControlMixin handle removing the logical child. + _childChanging?.Invoke(this, new AvaloniaPropertyChangedEventArgs( + this, + ChildProperty, + oldChild, + newChild, + BindingPriority.LocalValue)); + } } // Set the DataContext if the data isn't a control. @@ -241,11 +271,9 @@ namespace Avalonia.Controls.Presenters Child = newChild; - if (oldChild?.Parent == this) - { - LogicalChildren.Remove(oldChild); - } - + // If we're in a ContentControl's template then the child's parent will have been + // set by ContentControlMixin in response to Child changing. If not, then we're + // standalone and should make the control our own logical child. if (newChild.Parent == null && TemplatedParent == null) { LogicalChildren.Add(newChild); diff --git a/src/Avalonia.Controls/Presenters/IContentPresenter.cs b/src/Avalonia.Controls/Presenters/IContentPresenter.cs index 3b8039f33c..78bffec93b 100644 --- a/src/Avalonia.Controls/Presenters/IContentPresenter.cs +++ b/src/Avalonia.Controls/Presenters/IContentPresenter.cs @@ -1,6 +1,8 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using System; +using Avalonia.Controls.Mixins; using Avalonia.Controls.Primitives; namespace Avalonia.Controls.Presenters @@ -20,5 +22,16 @@ namespace Avalonia.Controls.Presenters /// Gets or sets the content to be displayed by the presenter. /// object Content { get; set; } + + /// + /// Raised when property is about to change. + /// + /// + /// This event should be raised after the child has been removed from the visual tree, + /// but before the property has changed. It is intended for consumption + /// by in order to update the host control's logical + /// children. + /// + event EventHandler ChildChanging; } -} \ No newline at end of file +} From fafa6952570d264ad41d181d2dde6b527519f29c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 22 Jan 2019 19:13:16 +0100 Subject: [PATCH 04/21] Use recommended ContentControl pattern. --- src/Avalonia.Controls/ContentControl.cs | 14 ++++++- .../Primitives/HeaderedContentControl.cs | 37 ++++++++++++++++++- .../Primitives/HeaderedItemsControl.cs | 21 ++++++++--- ...ol.cs => HeaderedSelectingItemsControl.cs} | 21 ++++++++--- .../HeaderedItemsControlTests .cs | 2 +- tests/Avalonia.UnitTests/TestTemplatedRoot.cs | 5 ++- 6 files changed, 85 insertions(+), 15 deletions(-) rename src/Avalonia.Controls/Primitives/{HeaderedSelectingControl.cs => HeaderedSelectingItemsControl.cs} (71%) diff --git a/src/Avalonia.Controls/ContentControl.cs b/src/Avalonia.Controls/ContentControl.cs index 6da6da54a5..16f17ae1bd 100644 --- a/src/Avalonia.Controls/ContentControl.cs +++ b/src/Avalonia.Controls/ContentControl.cs @@ -97,7 +97,19 @@ namespace Avalonia.Controls /// void IContentPresenterHost.RegisterContentPresenter(IContentPresenter presenter) { - Presenter = presenter; + RegisterContentPresenter(presenter); + } + + /// + /// Called when an is registered with the control. + /// + /// The presenter. + protected virtual void RegisterContentPresenter(IContentPresenter presenter) + { + if (presenter.Name == "PART_ContentPresenter") + { + Presenter = presenter; + } } } } diff --git a/src/Avalonia.Controls/Primitives/HeaderedContentControl.cs b/src/Avalonia.Controls/Primitives/HeaderedContentControl.cs index 7a46e0f776..98476c9c94 100644 --- a/src/Avalonia.Controls/Primitives/HeaderedContentControl.cs +++ b/src/Avalonia.Controls/Primitives/HeaderedContentControl.cs @@ -1,6 +1,8 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using Avalonia.Controls.Mixins; +using Avalonia.Controls.Presenters; using Avalonia.Controls.Templates; namespace Avalonia.Controls.Primitives @@ -20,7 +22,18 @@ namespace Avalonia.Controls.Primitives /// Defines the property. /// public static readonly StyledProperty HeaderTemplateProperty = - AvaloniaProperty.Register(nameof(HeaderTemplate)); + AvaloniaProperty.Register(nameof(HeaderTemplate)); + + /// + /// Initializes static members of the class. + /// + static HeaderedContentControl() + { + ContentControlMixin.Attach( + HeaderProperty, + x => x.LogicalChildren, + "PART_HeaderPresenter"); + } /// /// Gets or sets the header content. @@ -29,7 +42,16 @@ namespace Avalonia.Controls.Primitives { get { return GetValue(HeaderProperty); } set { SetValue(HeaderProperty, value); } - } + } + + /// + /// Gets the header presenter from the control's template. + /// + public IContentPresenter HeaderPresenter + { + get; + private set; + } /// /// Gets or sets the data template used to display the header content of the control. @@ -39,5 +61,16 @@ namespace Avalonia.Controls.Primitives get { return GetValue(HeaderTemplateProperty); } set { SetValue(HeaderTemplateProperty, value); } } + + /// + protected override void RegisterContentPresenter(IContentPresenter presenter) + { + base.RegisterContentPresenter(presenter); + + if (presenter.Name == "PART_HeaderPresenter") + { + HeaderPresenter = presenter; + } + } } } diff --git a/src/Avalonia.Controls/Primitives/HeaderedItemsControl.cs b/src/Avalonia.Controls/Primitives/HeaderedItemsControl.cs index c5aa73e56a..bda426c23b 100644 --- a/src/Avalonia.Controls/Primitives/HeaderedItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/HeaderedItemsControl.cs @@ -9,7 +9,7 @@ namespace Avalonia.Controls.Primitives /// /// Represents an with a related header. /// - public class HeaderedItemsControl : ItemsControl + public class HeaderedItemsControl : ItemsControl, IContentPresenterHost { /// /// Defines the property. @@ -40,17 +40,28 @@ namespace Avalonia.Controls.Primitives /// /// Gets the header presenter from the control's template. /// - public ContentPresenter HeaderPresenter + public IContentPresenter HeaderPresenter { get; private set; } /// - protected override void OnTemplateApplied(TemplateAppliedEventArgs e) + void IContentPresenterHost.RegisterContentPresenter(IContentPresenter presenter) { - HeaderPresenter = e.NameScope.Find("PART_HeaderPresenter"); - base.OnTemplateApplied(e); + RegisterContentPresenter(presenter); + } + + /// + /// Called when an is registered with the control. + /// + /// The presenter. + protected virtual void RegisterContentPresenter(IContentPresenter presenter) + { + if (presenter.Name == "PART_HeaderPresenter") + { + HeaderPresenter = presenter; + } } } } diff --git a/src/Avalonia.Controls/Primitives/HeaderedSelectingControl.cs b/src/Avalonia.Controls/Primitives/HeaderedSelectingItemsControl.cs similarity index 71% rename from src/Avalonia.Controls/Primitives/HeaderedSelectingControl.cs rename to src/Avalonia.Controls/Primitives/HeaderedSelectingItemsControl.cs index 87bb079ae7..d59be66b2b 100644 --- a/src/Avalonia.Controls/Primitives/HeaderedSelectingControl.cs +++ b/src/Avalonia.Controls/Primitives/HeaderedSelectingItemsControl.cs @@ -9,7 +9,7 @@ namespace Avalonia.Controls.Primitives /// /// Represents a with a related header. /// - public class HeaderedSelectingItemsControl : SelectingItemsControl + public class HeaderedSelectingItemsControl : SelectingItemsControl, IContentPresenterHost { /// /// Defines the property. @@ -40,17 +40,28 @@ namespace Avalonia.Controls.Primitives /// /// Gets the header presenter from the control's template. /// - public ContentPresenter HeaderPresenter + public IContentPresenter HeaderPresenter { get; private set; } /// - protected override void OnTemplateApplied(TemplateAppliedEventArgs e) + void IContentPresenterHost.RegisterContentPresenter(IContentPresenter presenter) { - base.OnTemplateApplied(e); - HeaderPresenter = e.NameScope.Find("PART_HeaderPresenter"); + RegisterContentPresenter(presenter); + } + + /// + /// Called when an is registered with the control. + /// + /// The presenter. + protected virtual void RegisterContentPresenter(IContentPresenter presenter) + { + if (presenter.Name == "PART_HeaderPresenter") + { + HeaderPresenter = presenter; + } } } } diff --git a/tests/Avalonia.Controls.UnitTests/HeaderedItemsControlTests .cs b/tests/Avalonia.Controls.UnitTests/HeaderedItemsControlTests .cs index 570d619963..66789ef874 100644 --- a/tests/Avalonia.Controls.UnitTests/HeaderedItemsControlTests .cs +++ b/tests/Avalonia.Controls.UnitTests/HeaderedItemsControlTests .cs @@ -37,7 +37,7 @@ namespace Avalonia.Controls.UnitTests target.Header = "Foo"; target.ApplyTemplate(); - target.HeaderPresenter.UpdateChild(); + ((ContentPresenter)target.HeaderPresenter).UpdateChild(); var child = target.HeaderPresenter.Child; diff --git a/tests/Avalonia.UnitTests/TestTemplatedRoot.cs b/tests/Avalonia.UnitTests/TestTemplatedRoot.cs index 5d42699d3f..ef49bd2f5c 100644 --- a/tests/Avalonia.UnitTests/TestTemplatedRoot.cs +++ b/tests/Avalonia.UnitTests/TestTemplatedRoot.cs @@ -18,7 +18,10 @@ namespace Avalonia.UnitTests public TestTemplatedRoot() { - Template = new FuncControlTemplate(x => new ContentPresenter()); + Template = new FuncControlTemplate(x => new ContentPresenter + { + Name = "PART_ContentPresenter", + }); } public event EventHandler Registered From 870a2f365f6409a8940616ccad5baa36dc067886 Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Tue, 12 Feb 2019 16:45:04 +0200 Subject: [PATCH 05/21] layoutmanager/listbox/vistualization/scroll issue unit test --- .../ListBoxTests.cs | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index b6f7c9ec96..6939c6a081 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -267,6 +267,71 @@ namespace Avalonia.Controls.UnitTests Assert.True(true); } + [Fact] + public void LayoutManager_Should_Measure_Arrange_All() + { + var virtualizationMode = ItemVirtualizationMode.Simple; + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var items = new AvaloniaList(Enumerable.Range(1, 7).Select(v => v.ToString())); + + var wnd = new Window() { SizeToContent = SizeToContent.WidthAndHeight }; + + wnd.IsVisible = true; + + var target = new ListBox(); + + wnd.Content = target; + + var lm = wnd.LayoutManager; + + target.Height = 110; + target.Width = 50; + target.DataContext = items; + target.VirtualizationMode = virtualizationMode; + + target.ItemTemplate = new FuncDataTemplate(c => + { + var tb = new TextBlock() { Height = 10, Width = 30 }; + tb.Bind(TextBlock.TextProperty, new Data.Binding()); + return tb; + }, true); + + lm.ExecuteInitialLayoutPass(wnd); + + target.Items = items; + + lm.ExecuteLayoutPass(); + + items.Insert(3, "3+"); + lm.ExecuteLayoutPass(); + + items.Insert(4, "4+"); + lm.ExecuteLayoutPass(); + + //RESET + items.Clear(); + foreach (var i in Enumerable.Range(1, 7)) + { + items.Add(i.ToString()); + } + + //working bit better with this line no outof memory or remaining to arrange/measure ??? + //lm.ExecuteLayoutPass(); + + items.Insert(2, "2+"); + + lm.ExecuteLayoutPass(); + + var flags = System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic; + var toMeasure = lm.GetType().GetField("_toMeasure", flags).GetValue(lm) as System.Collections.Generic.IEnumerable; + var toArrange = lm.GetType().GetField("_toArrange", flags).GetValue(lm) as System.Collections.Generic.IEnumerable; + + Assert.Equal(0, toMeasure.Count()); + Assert.Equal(0, toArrange.Count()); + } + } + private FuncControlTemplate ListBoxTemplate() { return new FuncControlTemplate(parent => From d53dbe9416775f46f62b3e8a0dff832cc5f7c148 Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Tue, 19 Feb 2019 14:29:44 +0200 Subject: [PATCH 06/21] add some more layout manager tests for potential issues with infinity loop and proper work --- .../LayoutManagerTests.cs | 119 ++++++++++++++++-- 1 file changed, 110 insertions(+), 9 deletions(-) diff --git a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs index b0e8a3780e..10006609aa 100644 --- a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs +++ b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs @@ -1,11 +1,10 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using System.Collections.Generic; +using System.Linq; using Avalonia.Controls; -using Avalonia.UnitTests; -using System; using Xunit; -using System.Collections.Generic; namespace Avalonia.Layout.UnitTests { @@ -74,7 +73,6 @@ namespace Avalonia.Layout.UnitTests } }; - var order = new List(); Size MeasureOverride(ILayoutable control, Size size) { @@ -110,7 +108,6 @@ namespace Avalonia.Layout.UnitTests } }; - var order = new List(); Size MeasureOverride(ILayoutable control, Size size) { @@ -196,9 +193,9 @@ namespace Avalonia.Layout.UnitTests Width = 100, Height = 100, }; - + var arrangeSize = default(Size); - + root.DoArrangeOverride = (_, s) => { arrangeSize = s; @@ -207,7 +204,7 @@ namespace Avalonia.Layout.UnitTests root.LayoutManager.ExecuteInitialLayoutPass(root); Assert.Equal(new Size(100, 100), arrangeSize); - + root.Width = 120; root.LayoutManager.ExecuteLayoutPass(); @@ -238,7 +235,111 @@ namespace Avalonia.Layout.UnitTests border.Height = 100; root.LayoutManager.ExecuteLayoutPass(); - Assert.Equal(new Size(100, 100), panel.DesiredSize); + Assert.Equal(new Size(100, 100), panel.DesiredSize); + } + + [Fact] + public void LayoutManager_Should_Prevent_Infinity_Loop_On_Measure() + { + var control = new LayoutTestControl(); + var root = new LayoutTestRoot { Child = control }; + + root.LayoutManager.ExecuteInitialLayoutPass(root); + control.Measured = false; + + int cnt = 0; + int maxcnt = 100; + control.DoMeasureOverride = (l, s) => + { + //emulate a problem in the logic of a control that triggers + //invalidate measure during measure + //it can lead to infinity loop in layoutmanager + if (++cnt < maxcnt) + { + control.InvalidateMeasure(); + } + + return new Size(100, 100); + }; + + control.InvalidateMeasure(); + + root.LayoutManager.ExecuteLayoutPass(); + + Assert.True(cnt < 100); + } + + [Fact] + public void LayoutManager_Should_Prevent_Infinity_Loop_On_Arrange() + { + var control = new LayoutTestControl(); + var root = new LayoutTestRoot { Child = control }; + + root.LayoutManager.ExecuteInitialLayoutPass(root); + control.Arranged = false; + + int cnt = 0; + int maxcnt = 100; + control.DoArrangeOverride = (l, s) => + { + //emulate a problem in the logic of a control that triggers + //invalidate measure during arrange + //it can lead to infinity loop in layoutmanager + if (++cnt < maxcnt) + { + control.InvalidateArrange(); + } + + return new Size(100, 100); + }; + + control.InvalidateArrange(); + + root.LayoutManager.ExecuteLayoutPass(); + + Assert.True(cnt < 100); + } + + [Fact] + public void LayoutManager_Should_Properly_Arrange_Visuals_Even_There_Are_Issues_With_Previous_Arranged() + { + var nonArrageableTargets = Enumerable.Range(1, 10).Select(_ => new LayoutTestControl()).ToArray(); + var targets = Enumerable.Range(1, 10).Select(_ => new LayoutTestControl()).ToArray(); + + StackPanel panel; + + var root = new LayoutTestRoot + { + Child = panel = new StackPanel() + }; + + panel.Children.AddRange(nonArrageableTargets); + panel.Children.AddRange(targets); + + root.LayoutManager.ExecuteInitialLayoutPass(root); + + foreach (var c in panel.Children.OfType()) + { + c.Measured = c.Arranged = false; + c.InvalidateMeasure(); + } + + foreach (var c in nonArrageableTargets) + { + c.DoArrangeOverride = (l, s) => + { + //emulate a problem in the logic of a control that triggers + //invalidate measure during arrange + c.InvalidateMeasure(); + return new Size(100, 100); + }; + } + + root.LayoutManager.ExecuteLayoutPass(); + + //altough nonArrageableTargets has rubbish logic and can't be measured/arranged properly + //layoutmanager should process properly other visuals + Assert.All(targets, c => Assert.True(c.Arranged)); } } } From 48ad6babdabfd02a9338aa16fde1195ce36d3ef8 Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Tue, 19 Feb 2019 14:31:01 +0200 Subject: [PATCH 07/21] improve test --- tests/Avalonia.Controls.UnitTests/ListBoxTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index 6939c6a081..343d8d41f3 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -321,6 +321,9 @@ namespace Avalonia.Controls.UnitTests items.Insert(2, "2+"); + lm.ExecuteLayoutPass(); + //after few more layout cycles layoutmanager shouldn't hold any more visual for measure/arrange + lm.ExecuteLayoutPass(); lm.ExecuteLayoutPass(); var flags = System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic; From 8c0746132349abf93caa8a479967603535e7e255 Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Mon, 18 Feb 2019 17:19:12 +0200 Subject: [PATCH 08/21] LayoutManager InfinityLoop protection and safety improvements --- src/Avalonia.Layout/LayoutManager.cs | 93 ++++++++++++++++++++++++++-- 1 file changed, 89 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Layout/LayoutManager.cs b/src/Avalonia.Layout/LayoutManager.cs index f3540ea631..3d390f46e6 100644 --- a/src/Avalonia.Layout/LayoutManager.cs +++ b/src/Avalonia.Layout/LayoutManager.cs @@ -2,7 +2,9 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; +using System.Collections; using System.Collections.Generic; +using System.Linq; using Avalonia.Logging; using Avalonia.Threading; @@ -13,8 +15,85 @@ namespace Avalonia.Layout /// public class LayoutManager : ILayoutManager { - private readonly Queue _toMeasure = new Queue(); - private readonly Queue _toArrange = new Queue(); + private class LayoutQueue : IReadOnlyCollection + { + private class Info + { + public bool Active; + public int Count; + } + + public LayoutQueue(Func shouldEnqueue) + { + _shouldEnqueue = shouldEnqueue; + } + + private Func _shouldEnqueue; + private Queue _inner = new Queue(); + private Dictionary _loopQueueInfo = new Dictionary(); + private int _maxEnqueueCountPerLoop = 1; + + public int Count => _inner.Count; + + public IEnumerator GetEnumerator() => (_inner as IEnumerable).GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => _inner.GetEnumerator(); + + public T Dequeue() + { + var result = _inner.Dequeue(); + + if (_loopQueueInfo.TryGetValue(result, out var info)) + { + info.Active = false; + } + + return result; + } + + public void Enqueue(T item) + { + if (!_loopQueueInfo.TryGetValue(item, out var info)) + { + _loopQueueInfo[item] = info = new Info(); + } + + if (!info.Active && info.Count < _maxEnqueueCountPerLoop) + { + _inner.Enqueue(item); + info.Active = true; + info.Count++; + } + } + + public void BeginLoop(int maxEnqueueCountPerLoop) + { + _maxEnqueueCountPerLoop = maxEnqueueCountPerLoop; + } + + public void EndLoop() + { + var notfinalized = _loopQueueInfo.Where(v => v.Value.Count == _maxEnqueueCountPerLoop).ToArray(); + + _loopQueueInfo.Clear(); + + //prevent layout cycle but add to next layout the non arranged/measured items that might have caused cycle + //one more time as a final attempt + foreach (var item in notfinalized) + { + if (_shouldEnqueue(item.Key)) + { + item.Value.Active = true; + item.Value.Count++; + _loopQueueInfo[item.Key] = item.Value; + _inner.Enqueue(item.Key); + } + } + } + } + + private readonly LayoutQueue _toMeasure = new LayoutQueue(v => !v.IsMeasureValid); + private readonly LayoutQueue _toArrange = new LayoutQueue(v => !v.IsArrangeValid); private bool _queued; private bool _running; @@ -80,6 +159,9 @@ namespace Avalonia.Layout var stopwatch = new System.Diagnostics.Stopwatch(); stopwatch.Start(); + _toMeasure.BeginLoop(MaxPasses); + _toArrange.BeginLoop(MaxPasses); + try { for (var pass = 0; pass < MaxPasses; ++pass) @@ -98,6 +180,9 @@ namespace Avalonia.Layout _running = false; } + _toMeasure.EndLoop(); + _toArrange.EndLoop(); + stopwatch.Stop(); Logger.Information(LogArea.Layout, this, "Layout pass finished in {Time}", stopwatch.Elapsed); } @@ -112,7 +197,7 @@ namespace Avalonia.Layout Arrange(root); // Running the initial layout pass may have caused some control to be invalidated - // so run a full layout pass now (this usually due to scrollbars; its not known + // so run a full layout pass now (this usually due to scrollbars; its not known // whether they will need to be shown until the layout pass has run and if the // first guess was incorrect the layout will need to be updated). ExecuteLayoutPass(); @@ -133,7 +218,7 @@ namespace Avalonia.Layout private void ExecuteArrangePass() { - while (_toArrange.Count > 0 && _toMeasure.Count == 0) + while (_toArrange.Count > 0) { var control = _toArrange.Dequeue(); From b97b3c59353dd4af4765a1077d8cfc79a731c8ee Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Tue, 19 Feb 2019 17:55:33 +0200 Subject: [PATCH 09/21] make internal stored Info a strcut to optimize memory (pr note) --- src/Avalonia.Layout/LayoutManager.cs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/Avalonia.Layout/LayoutManager.cs b/src/Avalonia.Layout/LayoutManager.cs index 3d390f46e6..ea84b58cbd 100644 --- a/src/Avalonia.Layout/LayoutManager.cs +++ b/src/Avalonia.Layout/LayoutManager.cs @@ -17,7 +17,7 @@ namespace Avalonia.Layout { private class LayoutQueue : IReadOnlyCollection { - private class Info + private struct Info { public bool Active; public int Count; @@ -46,6 +46,7 @@ namespace Avalonia.Layout if (_loopQueueInfo.TryGetValue(result, out var info)) { info.Active = false; + _loopQueueInfo[result] = info; } return result; @@ -53,16 +54,12 @@ namespace Avalonia.Layout public void Enqueue(T item) { - if (!_loopQueueInfo.TryGetValue(item, out var info)) - { - _loopQueueInfo[item] = info = new Info(); - } + _loopQueueInfo.TryGetValue(item, out var info); if (!info.Active && info.Count < _maxEnqueueCountPerLoop) { _inner.Enqueue(item); - info.Active = true; - info.Count++; + _loopQueueInfo[item] = new Info() { Active = true, Count = info.Count + 1 }; } } @@ -83,9 +80,7 @@ namespace Avalonia.Layout { if (_shouldEnqueue(item.Key)) { - item.Value.Active = true; - item.Value.Count++; - _loopQueueInfo[item.Key] = item.Value; + _loopQueueInfo[item.Key] = new Info() { Active = true, Count = item.Value.Count + 1 }; _inner.Enqueue(item.Key); } } From 6d7d051f3e717d46dde6bce80dd480190df0994c Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Tue, 19 Feb 2019 18:31:06 +0200 Subject: [PATCH 10/21] extract layoutqueue to separate file --- src/Avalonia.Layout/LayoutManager.cs | 75 -------------------------- src/Avalonia.Layout/LayoutQueue.cs | 79 ++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 75 deletions(-) create mode 100644 src/Avalonia.Layout/LayoutQueue.cs diff --git a/src/Avalonia.Layout/LayoutManager.cs b/src/Avalonia.Layout/LayoutManager.cs index ea84b58cbd..45efccc1fa 100644 --- a/src/Avalonia.Layout/LayoutManager.cs +++ b/src/Avalonia.Layout/LayoutManager.cs @@ -2,9 +2,6 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Collections; -using System.Collections.Generic; -using System.Linq; using Avalonia.Logging; using Avalonia.Threading; @@ -15,78 +12,6 @@ namespace Avalonia.Layout /// public class LayoutManager : ILayoutManager { - private class LayoutQueue : IReadOnlyCollection - { - private struct Info - { - public bool Active; - public int Count; - } - - public LayoutQueue(Func shouldEnqueue) - { - _shouldEnqueue = shouldEnqueue; - } - - private Func _shouldEnqueue; - private Queue _inner = new Queue(); - private Dictionary _loopQueueInfo = new Dictionary(); - private int _maxEnqueueCountPerLoop = 1; - - public int Count => _inner.Count; - - public IEnumerator GetEnumerator() => (_inner as IEnumerable).GetEnumerator(); - - IEnumerator IEnumerable.GetEnumerator() => _inner.GetEnumerator(); - - public T Dequeue() - { - var result = _inner.Dequeue(); - - if (_loopQueueInfo.TryGetValue(result, out var info)) - { - info.Active = false; - _loopQueueInfo[result] = info; - } - - return result; - } - - public void Enqueue(T item) - { - _loopQueueInfo.TryGetValue(item, out var info); - - if (!info.Active && info.Count < _maxEnqueueCountPerLoop) - { - _inner.Enqueue(item); - _loopQueueInfo[item] = new Info() { Active = true, Count = info.Count + 1 }; - } - } - - public void BeginLoop(int maxEnqueueCountPerLoop) - { - _maxEnqueueCountPerLoop = maxEnqueueCountPerLoop; - } - - public void EndLoop() - { - var notfinalized = _loopQueueInfo.Where(v => v.Value.Count == _maxEnqueueCountPerLoop).ToArray(); - - _loopQueueInfo.Clear(); - - //prevent layout cycle but add to next layout the non arranged/measured items that might have caused cycle - //one more time as a final attempt - foreach (var item in notfinalized) - { - if (_shouldEnqueue(item.Key)) - { - _loopQueueInfo[item.Key] = new Info() { Active = true, Count = item.Value.Count + 1 }; - _inner.Enqueue(item.Key); - } - } - } - } - private readonly LayoutQueue _toMeasure = new LayoutQueue(v => !v.IsMeasureValid); private readonly LayoutQueue _toArrange = new LayoutQueue(v => !v.IsArrangeValid); private bool _queued; diff --git a/src/Avalonia.Layout/LayoutQueue.cs b/src/Avalonia.Layout/LayoutQueue.cs new file mode 100644 index 0000000000..ce40fdde49 --- /dev/null +++ b/src/Avalonia.Layout/LayoutQueue.cs @@ -0,0 +1,79 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; + +namespace Avalonia.Layout +{ + internal class LayoutQueue : IReadOnlyCollection + { + private struct Info + { + public bool Active; + public int Count; + } + + public LayoutQueue(Func shouldEnqueue) + { + _shouldEnqueue = shouldEnqueue; + } + + private Func _shouldEnqueue; + private Queue _inner = new Queue(); + private Dictionary _loopQueueInfo = new Dictionary(); + private int _maxEnqueueCountPerLoop = 1; + + public int Count => _inner.Count; + + public IEnumerator GetEnumerator() => (_inner as IEnumerable).GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => _inner.GetEnumerator(); + + public T Dequeue() + { + var result = _inner.Dequeue(); + + if (_loopQueueInfo.TryGetValue(result, out var info)) + { + info.Active = false; + _loopQueueInfo[result] = info; + } + + return result; + } + + public void Enqueue(T item) + { + _loopQueueInfo.TryGetValue(item, out var info); + + if (!info.Active && info.Count < _maxEnqueueCountPerLoop) + { + _inner.Enqueue(item); + _loopQueueInfo[item] = new Info() { Active = true, Count = info.Count + 1 }; + } + } + + public void BeginLoop(int maxEnqueueCountPerLoop) + { + _maxEnqueueCountPerLoop = maxEnqueueCountPerLoop; + } + + public void EndLoop() + { + var notfinalized = _loopQueueInfo.Where(v => v.Value.Count == _maxEnqueueCountPerLoop).ToArray(); + + _loopQueueInfo.Clear(); + + //prevent layout cycle but add to next layout the non arranged/measured items that might have caused cycle + //one more time as a final attempt + foreach (var item in notfinalized) + { + if (_shouldEnqueue(item.Key)) + { + _loopQueueInfo[item.Key] = new Info() { Active = true, Count = item.Value.Count + 1 }; + _inner.Enqueue(item.Key); + } + } + } + } +} From a4132890bf6a7003cc7faac4090cc90896881488 Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Tue, 19 Feb 2019 18:32:15 +0200 Subject: [PATCH 11/21] add unit tests for layout queue --- .../Properties/AssemblyInfo.cs | 4 + .../LayoutQueueTests.cs | 196 ++++++++++++++++++ 2 files changed, 200 insertions(+) create mode 100644 tests/Avalonia.Layout.UnitTests/LayoutQueueTests.cs diff --git a/src/Avalonia.Layout/Properties/AssemblyInfo.cs b/src/Avalonia.Layout/Properties/AssemblyInfo.cs index 70fc1e9330..392ad323e5 100644 --- a/src/Avalonia.Layout/Properties/AssemblyInfo.cs +++ b/src/Avalonia.Layout/Properties/AssemblyInfo.cs @@ -1,6 +1,10 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using System.Runtime.CompilerServices; using Avalonia.Metadata; +[assembly: InternalsVisibleTo("Avalonia.Layout.UnitTests")] + [assembly: XmlnsDefinition("https://github.com/avaloniaui", "Avalonia.Layout")] + diff --git a/tests/Avalonia.Layout.UnitTests/LayoutQueueTests.cs b/tests/Avalonia.Layout.UnitTests/LayoutQueueTests.cs new file mode 100644 index 0000000000..f047677a8a --- /dev/null +++ b/tests/Avalonia.Layout.UnitTests/LayoutQueueTests.cs @@ -0,0 +1,196 @@ +using System.Collections.Generic; +using System.Linq; +using Xunit; + +namespace Avalonia.Layout.UnitTests +{ + public class LayoutQueueTests + { + [Fact] + public void Should_Enqueue() + { + var target = new LayoutQueue(_ => true); + var refQueue = new Queue(); + var items = new[] { "1", "2", "3" }; + + foreach (var item in items) + { + target.Enqueue(item); + refQueue.Enqueue(item); + } + + Assert.Equal(refQueue, target); + } + + [Fact] + public void Should_Dequeue() + { + var target = new LayoutQueue(_ => true); + var refQueue = new Queue(); + var items = new[] { "1", "2", "3" }; + + foreach (var item in items) + { + target.Enqueue(item); + refQueue.Enqueue(item); + } + + while (refQueue.Count > 0) + { + Assert.Equal(refQueue.Dequeue(), target.Dequeue()); + } + } + + [Fact] + public void Should_Enqueue_UniqueElements() + { + var target = new LayoutQueue(_ => true); + + var items = new[] { "1", "2", "3", "1" }; + + foreach (var item in items) + { + target.Enqueue(item); + } + + Assert.Equal(3, target.Count); + Assert.Equal(items.Take(3), target); + } + + [Fact] + public void Shouldnt_Enqueue_More_Than_Limit_In_Loop() + { + var target = new LayoutQueue(_ => true); + + //1 + target.Enqueue("Foo"); + + Assert.Equal(1, target.Count); + + target.BeginLoop(3); + + target.Dequeue(); + + //2 + target.Enqueue("Foo"); + target.Dequeue(); + + //3 + target.Enqueue("Foo"); + + Assert.Equal(1, target.Count); + + target.Dequeue(); + + //4 more than limit shouldn't be added + target.Enqueue("Foo"); + + Assert.Equal(0, target.Count); + } + + [Fact] + public void Shouldnt_Count_Unique_Enqueue_For_Limit_In_Loop() + { + var target = new LayoutQueue(_ => true); + + //1 + target.Enqueue("Foo"); + + Assert.Equal(1, target.Count); + + target.BeginLoop(3); + + target.Dequeue(); + + //2 + target.Enqueue("Foo"); + target.Enqueue("Foo"); + target.Dequeue(); + + //3 + target.Enqueue("Foo"); + target.Enqueue("Foo"); + + Assert.Equal(1, target.Count); + + target.Dequeue(); + + //4 more than limit shouldn't be added + target.Enqueue("Foo"); + + Assert.Equal(0, target.Count); + } + + [Fact] + public void Should_Enqueue_When_Condition_True_After_Loop_When_Limit_Met() + { + var target = new LayoutQueue(_ => true); + + //1 + target.Enqueue("Foo"); + + Assert.Equal(1, target.Count); + + target.BeginLoop(3); + + target.Dequeue(); + + //2 + target.Enqueue("Foo"); + target.Dequeue(); + + //3 + target.Enqueue("Foo"); + + Assert.Equal(1, target.Count); + + target.Dequeue(); + + //4 more than limit shouldn't be added to queue + target.Enqueue("Foo"); + + Assert.Equal(0, target.Count); + + target.EndLoop(); + + //after loop should be added once + Assert.Equal(1, target.Count); + Assert.Equal("Foo", target.First()); + } + + [Fact] + public void Shouldnt_Enqueue_When_Condition_False_After_Loop_When_Limit_Met() + { + var target = new LayoutQueue(_ => false); + + //1 + target.Enqueue("Foo"); + + Assert.Equal(1, target.Count); + + target.BeginLoop(3); + + target.Dequeue(); + + //2 + target.Enqueue("Foo"); + target.Dequeue(); + + //3 + target.Enqueue("Foo"); + + Assert.Equal(1, target.Count); + + target.Dequeue(); + + //4 more than limit shouldn't be added + target.Enqueue("Foo"); + + Assert.Equal(0, target.Count); + + target.EndLoop(); + + Assert.Equal(0, target.Count); + } + } +} From 372f0f266ec4c6700ec156bebf30e80ffcc28459 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 20 Feb 2019 15:27:07 +0100 Subject: [PATCH 12/21] Add another failing test relating to #1099. --- .../Avalonia.Visuals.UnitTests/VisualTests.cs | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/Avalonia.Visuals.UnitTests/VisualTests.cs b/tests/Avalonia.Visuals.UnitTests/VisualTests.cs index 0414ac4c74..504f0ada86 100644 --- a/tests/Avalonia.Visuals.UnitTests/VisualTests.cs +++ b/tests/Avalonia.Visuals.UnitTests/VisualTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; using Avalonia.Controls; +using Avalonia.Data; using Avalonia.Media; using Avalonia.Rendering; using Avalonia.UnitTests; @@ -236,5 +237,50 @@ namespace Avalonia.Visuals.UnitTests //child is centered (400 - 100*2 scale)/2 Assert.Equal(new Point(100, 100), point); } + + [Fact] + public void Should_Not_Log_Binding_Error_When_Not_Attached_To_Logical_Tree() + { + var target = new Decorator { DataContext = "foo" }; + var called = false; + + LogCallback checkLogMessage = (level, area, src, mt, pv) => + { + if (level >= Logging.LogEventLevel.Warning) + { + called = true; + } + }; + + using (TestLogSink.Start(checkLogMessage)) + { + target.Bind(Decorator.TagProperty, new Binding("Foo")); + } + + Assert.False(called); + } + + [Fact] + public void Should_Log_Binding_Error_When_Attached_To_Logical_Tree() + { + var target = new Decorator(); + var root = new TestRoot { Child = target, DataContext = "foo" }; + var called = false; + + LogCallback checkLogMessage = (level, area, src, mt, pv) => + { + if (level >= Logging.LogEventLevel.Warning) + { + called = true; + } + }; + + using (TestLogSink.Start(checkLogMessage)) + { + target.Bind(Decorator.TagProperty, new Binding("Foo")); + } + + Assert.True(called); + } } } From 6961d55e7a33883f7c229c6a45f1335bac646fb3 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 20 Feb 2019 15:30:05 +0100 Subject: [PATCH 13/21] Only log binding errors when attached to a tree. --- src/Avalonia.Base/AvaloniaObject.cs | 43 ++++++++++++++- src/Avalonia.Base/IPriorityValueOwner.cs | 8 +++ src/Avalonia.Base/Logging/LoggerExtensions.cs | 53 ------------------- src/Avalonia.Base/PriorityValue.cs | 2 +- src/Avalonia.Base/ValueStore.cs | 5 ++ src/Avalonia.Visuals/Visual.cs | 29 ++++++++++ 6 files changed, 85 insertions(+), 55 deletions(-) delete mode 100644 src/Avalonia.Base/Logging/LoggerExtensions.cs diff --git a/src/Avalonia.Base/AvaloniaObject.cs b/src/Avalonia.Base/AvaloniaObject.cs index 28148f6568..90b6aca505 100644 --- a/src/Avalonia.Base/AvaloniaObject.cs +++ b/src/Avalonia.Base/AvaloniaObject.cs @@ -415,6 +415,7 @@ namespace Avalonia internal void BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification) { + LogIfError(property, notification); UpdateDataValidation(property, notification); } @@ -446,6 +447,23 @@ namespace Avalonia }); } + /// + /// Logs a binding error for a property. + /// + /// The property that the error occurred on. + /// The binding error. + protected internal virtual void LogBindingError(AvaloniaProperty property, Exception e) + { + Logger.Log( + LogEventLevel.Warning, + LogArea.Binding, + this, + "Error in binding to {Target}.{Property}: {Message}", + this, + property, + e.Message); + } + /// /// Called to update the validation state for properties for which data validation is /// enabled. @@ -647,7 +665,7 @@ namespace Avalonia if (notification != null) { - notification.LogIfError(this, property); + LogIfError(property, notification); value = notification.Value; } @@ -779,6 +797,29 @@ namespace Avalonia return description?.Description ?? o.ToString(); } + /// + /// Logs a mesage if the notification represents a binding error. + /// + /// The property being bound. + /// The binding notification. + private void LogIfError(AvaloniaProperty property, BindingNotification notification) + { + if (notification.ErrorType == BindingErrorType.Error) + { + if (notification.Error is AggregateException aggregate) + { + foreach (var inner in aggregate.InnerExceptions) + { + LogBindingError(property, inner); + } + } + else + { + LogBindingError(property, notification.Error); + } + } + } + /// /// Logs a property set message. /// diff --git a/src/Avalonia.Base/IPriorityValueOwner.cs b/src/Avalonia.Base/IPriorityValueOwner.cs index 8cbf212381..540b1bf19b 100644 --- a/src/Avalonia.Base/IPriorityValueOwner.cs +++ b/src/Avalonia.Base/IPriorityValueOwner.cs @@ -1,6 +1,7 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. +using System; using Avalonia.Data; using Avalonia.Utilities; @@ -28,6 +29,13 @@ namespace Avalonia /// The notification. void BindingNotificationReceived(AvaloniaProperty property, BindingNotification notification); + /// + /// Logs a binding error. + /// + /// The property the error occurred on. + /// The binding error. + void LogError(AvaloniaProperty property, Exception e); + /// /// Ensures that the current thread is the UI thread. /// diff --git a/src/Avalonia.Base/Logging/LoggerExtensions.cs b/src/Avalonia.Base/Logging/LoggerExtensions.cs deleted file mode 100644 index 24e44bf9de..0000000000 --- a/src/Avalonia.Base/Logging/LoggerExtensions.cs +++ /dev/null @@ -1,53 +0,0 @@ -using System; -using Avalonia.Data; - -namespace Avalonia.Logging -{ - internal static class LoggerExtensions - { - public static void LogIfError( - this BindingNotification notification, - object source, - AvaloniaProperty property) - { - if (notification.ErrorType == BindingErrorType.Error) - { - if (notification.Error is AggregateException aggregate) - { - foreach (var inner in aggregate.InnerExceptions) - { - LogError(source, property, inner); - } - } - else - { - LogError(source, property, notification.Error); - } - } - } - - private static void LogError(object source, AvaloniaProperty property, Exception e) - { - var level = LogEventLevel.Warning; - - if (e is BindingChainException b && - !string.IsNullOrEmpty(b.Expression) && - string.IsNullOrEmpty(b.ExpressionErrorPoint)) - { - // The error occurred at the root of the binding chain: it's possible that the - // DataContext isn't set up yet, so log at Information level instead of Warning - // to prevent spewing hundreds of errors. - level = LogEventLevel.Information; - } - - Logger.Log( - level, - LogArea.Binding, - source, - "Error in binding to {Target}.{Property}: {Message}", - source, - property, - e.Message); - } - } -} diff --git a/src/Avalonia.Base/PriorityValue.cs b/src/Avalonia.Base/PriorityValue.cs index c8b434c6f9..89a893577f 100644 --- a/src/Avalonia.Base/PriorityValue.cs +++ b/src/Avalonia.Base/PriorityValue.cs @@ -197,7 +197,7 @@ namespace Avalonia /// The binding error. public void LevelError(PriorityLevel level, BindingNotification error) { - error.LogIfError(Owner, Property); + Owner.LogError(Property, error.Error); } /// diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index adbe89aceb..c5bf6ba0a8 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -100,6 +100,11 @@ namespace Avalonia _owner.PriorityValueChanged(property, priority, oldValue, newValue); } + public void LogError(AvaloniaProperty property, Exception e) + { + _owner.LogBindingError(property, e); + } + public IDictionary GetSetValues() => _values; public object GetValue(AvaloniaProperty property) diff --git a/src/Avalonia.Visuals/Visual.cs b/src/Avalonia.Visuals/Visual.cs index f26c21d1b6..bf282db72f 100644 --- a/src/Avalonia.Visuals/Visual.cs +++ b/src/Avalonia.Visuals/Visual.cs @@ -8,6 +8,7 @@ using System.Reactive.Linq; using Avalonia.Collections; using Avalonia.Data; using Avalonia.Logging; +using Avalonia.LogicalTree; using Avalonia.Media; using Avalonia.Rendering; using Avalonia.VisualTree; @@ -448,6 +449,34 @@ namespace Avalonia RaisePropertyChanged(VisualParentProperty, oldParent, newParent, BindingPriority.LocalValue); } + protected override sealed void LogBindingError(AvaloniaProperty property, Exception e) + { + // Don't log a binding error unless the control is attached to a logical or visual tree. + // In theory this should only need to check for logical tree attachment, but in practise + // due to ContentControlMixin only taking effect when the template has finished being + // applied, some controls are attached to the visual tree before the logical tree. + if (((ILogical)this).IsAttachedToLogicalTree || ((IVisual)this).IsAttachedToVisualTree) + { + if (e is BindingChainException b && + string.IsNullOrEmpty(b.ExpressionErrorPoint) && + DataContext == null) + { + // The error occurred at the root of the binding chain and DataContext is null; + // don't log this - the DataContext probably hasn't been set up yet. + return; + } + + Logger.Log( + LogEventLevel.Warning, + LogArea.Binding, + this, + "Error in binding to {Target}.{Property}: {Message}", + this, + property, + e.Message); + } + } + /// /// Gets the visual offset from the specified ancestor. /// From 6dcd634a87a317b772205d8b4257f18bcbce4e59 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 20 Feb 2019 15:42:40 +0100 Subject: [PATCH 14/21] Missed change from merge. --- src/Avalonia.Base/ValueStore.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 22961a7568..24f85ea6b1 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -123,8 +123,6 @@ namespace Avalonia _owner.LogBindingError(property, e); } - public IDictionary GetSetValues() => _values; - public object GetValue(AvaloniaProperty property) { var result = AvaloniaProperty.UnsetValue; From 3d6b4a875bb6c5adb24c49dd3dea159cd0f376e1 Mon Sep 17 00:00:00 2001 From: Dariusz Komosinski Date: Thu, 21 Feb 2019 14:34:24 +0100 Subject: [PATCH 15/21] Do not capture this when creating _drawOperationsRefCounter. --- .../Rendering/SceneGraph/VisualNode.cs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs index 2fb8e84a2e..71e364ffc9 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs @@ -327,31 +327,36 @@ namespace Avalonia.Rendering.SceneGraph if (_drawOperations == null) { _drawOperations = new List>(); - _drawOperationsRefCounter = RefCountable.Create(Disposable.Create(DisposeDrawOperations)); + _drawOperationsRefCounter = RefCountable.Create(CreateDisposeDrawOperations(_drawOperations)); _drawOperationsCloned = false; } else if (_drawOperationsCloned) { _drawOperations = new List>(_drawOperations.Select(op => op.Clone())); _drawOperationsRefCounter.Dispose(); - _drawOperationsRefCounter = RefCountable.Create(Disposable.Create(DisposeDrawOperations)); + _drawOperationsRefCounter = RefCountable.Create(CreateDisposeDrawOperations(_drawOperations)); _drawOperationsCloned = false; } } - public bool Disposed { get; } - - public void Dispose() + private static IDisposable CreateDisposeDrawOperations(List> drawOperations) { - _drawOperationsRefCounter?.Dispose(); + return Disposable.Create(() => + { + foreach (var operation in drawOperations) + { + operation.Dispose(); + } + }); } - private void DisposeDrawOperations() + public bool Disposed { get; private set; } + + public void Dispose() { - foreach (var operation in DrawOperations) - { - operation.Dispose(); - } + _drawOperationsRefCounter?.Dispose(); + + Disposed = true; } } } From 61be769d61ecf869d13f528a78fc9082a8b00697 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 21 Feb 2019 15:50:48 +0200 Subject: [PATCH 16/21] Update tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs Co-Authored-By: donandren --- tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs index 10006609aa..b0e2f9b393 100644 --- a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs +++ b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs @@ -239,7 +239,7 @@ namespace Avalonia.Layout.UnitTests } [Fact] - public void LayoutManager_Should_Prevent_Infinity_Loop_On_Measure() + public void LayoutManager_Should_Prevent_Infinite_Loop_On_Measure() { var control = new LayoutTestControl(); var root = new LayoutTestRoot { Child = control }; From 260789c42506ca90384f658d06c5edcbb842d295 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 21 Feb 2019 15:50:56 +0200 Subject: [PATCH 17/21] Update tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs Co-Authored-By: donandren --- tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs index b0e2f9b393..6ecf5a7b8c 100644 --- a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs +++ b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs @@ -253,7 +253,7 @@ namespace Avalonia.Layout.UnitTests { //emulate a problem in the logic of a control that triggers //invalidate measure during measure - //it can lead to infinity loop in layoutmanager + //it can lead to an infinite loop in layoutmanager if (++cnt < maxcnt) { control.InvalidateMeasure(); From 9b4bd8d62124f552cae798f6ceed79de5048d0f5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 21 Feb 2019 15:51:07 +0200 Subject: [PATCH 18/21] Update tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs Co-Authored-By: donandren --- tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs index 6ecf5a7b8c..4d7e0ee259 100644 --- a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs +++ b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs @@ -270,7 +270,7 @@ namespace Avalonia.Layout.UnitTests } [Fact] - public void LayoutManager_Should_Prevent_Infinity_Loop_On_Arrange() + public void LayoutManager_Should_Prevent_Infinite_Loop_On_Arrange() { var control = new LayoutTestControl(); var root = new LayoutTestRoot { Child = control }; From b32a10ceda9b418cea7ff25d0593cfdae8362dfe Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 21 Feb 2019 15:51:34 +0200 Subject: [PATCH 19/21] Update tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs Co-Authored-By: donandren --- tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs index 4d7e0ee259..4c288b2702 100644 --- a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs +++ b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs @@ -301,7 +301,7 @@ namespace Avalonia.Layout.UnitTests } [Fact] - public void LayoutManager_Should_Properly_Arrange_Visuals_Even_There_Are_Issues_With_Previous_Arranged() + public void LayoutManager_Should_Properly_Arrange_Visuals_Even_When_There_Are_Issues_With_Previous_Arranged() { var nonArrageableTargets = Enumerable.Range(1, 10).Select(_ => new LayoutTestControl()).ToArray(); var targets = Enumerable.Range(1, 10).Select(_ => new LayoutTestControl()).ToArray(); From 823b58b3351a4984b785bf7298b7d34ffbaf71d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20Komosi=C5=84ski?= Date: Thu, 21 Feb 2019 22:16:47 +0100 Subject: [PATCH 20/21] Add doc comment to related methods. --- .../Rendering/SceneGraph/VisualNode.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs index 71e364ffc9..19fb54e125 100644 --- a/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs +++ b/src/Avalonia.Visuals/Rendering/SceneGraph/VisualNode.cs @@ -322,6 +322,9 @@ namespace Avalonia.Rendering.SceneGraph } } + /// + /// Ensures that this node draw operations have been created and are mutable (in case we are using cloned operations). + /// private void EnsureDrawOperationsCreated() { if (_drawOperations == null) @@ -339,6 +342,13 @@ namespace Avalonia.Rendering.SceneGraph } } + /// + /// Creates disposable that will dispose all items in passed draw operations after being disposed. + /// It is crucial that we don't capture current instance + /// as draw operations can be cloned and may persist across subsequent scenes. + /// + /// Draw operations that need to be disposed. + /// Disposable for given draw operations. private static IDisposable CreateDisposeDrawOperations(List> drawOperations) { return Disposable.Create(() => From 2da756aa5304d91fbcfb7469968b2fd6fca20fe3 Mon Sep 17 00:00:00 2001 From: Jumar Macato Date: Fri, 22 Feb 2019 19:17:49 +0800 Subject: [PATCH 21/21] Make parse function in Cue.cs explicitly typed in order for Xaml-IL tocorrectly compile and avoid unnecessary boxing. --- src/Avalonia.Animation/Cue.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Animation/Cue.cs b/src/Avalonia.Animation/Cue.cs index 52d1609cf9..7da7a9382b 100644 --- a/src/Avalonia.Animation/Cue.cs +++ b/src/Avalonia.Animation/Cue.cs @@ -30,7 +30,7 @@ namespace Avalonia.Animation /// /// Parses a string to a object. /// - public static object Parse(string value, CultureInfo culture) + public static Cue Parse(string value, CultureInfo culture) { string v = value; @@ -70,7 +70,7 @@ namespace Avalonia.Animation } } - public class CueTypeConverter : TypeConverter + public class CueTypeConverter : TypeConverter { public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType) {