From 86f7918d4da888765020df56547c26d51b44ff53 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 2 Apr 2019 13:26:31 +0200 Subject: [PATCH 01/11] Added failing tests for #2350. --- .../StackPanelTests.cs | 160 +++++++++++++++++- 1 file changed, 159 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Controls.UnitTests/StackPanelTests.cs b/tests/Avalonia.Controls.UnitTests/StackPanelTests.cs index ba2716775a..722ad1c8ee 100644 --- a/tests/Avalonia.Controls.UnitTests/StackPanelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/StackPanelTests.cs @@ -1,7 +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; +using System.Linq; +using Avalonia.Layout; using Xunit; namespace Avalonia.Controls.UnitTests @@ -147,6 +148,151 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(new Rect(50, 0, 50, 120), target.Children[2].Bounds); } + [Fact] + public void Arranges_Vertical_Children_With_Correct_Bounds() + { + var target = new StackPanel + { + Orientation = Orientation.Vertical, + Children = + { + new TestControl + { + HorizontalAlignment = HorizontalAlignment.Left, + MeasureSize = new Size(50, 10), + }, + new TestControl + { + HorizontalAlignment = HorizontalAlignment.Left, + MeasureSize = new Size(150, 10), + }, + new TestControl + { + HorizontalAlignment = HorizontalAlignment.Center, + MeasureSize = new Size(50, 10), + }, + new TestControl + { + HorizontalAlignment = HorizontalAlignment.Center, + MeasureSize = new Size(150, 10), + }, + new TestControl + { + HorizontalAlignment = HorizontalAlignment.Right, + MeasureSize = new Size(50, 10), + }, + new TestControl + { + HorizontalAlignment = HorizontalAlignment.Right, + MeasureSize = new Size(150, 10), + }, + new TestControl + { + HorizontalAlignment = HorizontalAlignment.Stretch, + MeasureSize = new Size(50, 10), + }, + new TestControl + { + HorizontalAlignment = HorizontalAlignment.Stretch, + MeasureSize = new Size(150, 10), + }, + } + }; + + target.Measure(new Size(100, 150)); + Assert.Equal(new Size(100, 80), target.DesiredSize); + + target.Arrange(new Rect(target.DesiredSize)); + + var bounds = target.Children.Select(x => x.Bounds).ToArray(); + + Assert.Equal( + new[] + { + new Rect(0, 0, 50, 10), + new Rect(0, 10, 150, 10), + new Rect(25, 20, 50, 10), + new Rect(-25, 30, 150, 10), + new Rect(50, 40, 50, 10), + new Rect(-50, 50, 150, 10), + new Rect(0, 60, 100, 10), + new Rect(0, 70, 150, 10), + + }, bounds); + } + + [Fact] + public void Arranges_Horizontal_Children_With_Correct_Bounds() + { + var target = new StackPanel + { + Orientation = Orientation.Horizontal, + Children = + { + new TestControl + { + VerticalAlignment = VerticalAlignment.Top, + MeasureSize = new Size(10, 50), + }, + new TestControl + { + VerticalAlignment = VerticalAlignment.Top, + MeasureSize = new Size(10, 150), + }, + new TestControl + { + VerticalAlignment = VerticalAlignment.Center, + MeasureSize = new Size(10, 50), + }, + new TestControl + { + VerticalAlignment = VerticalAlignment.Center, + MeasureSize = new Size(10, 150), + }, + new TestControl + { + VerticalAlignment = VerticalAlignment.Bottom, + MeasureSize = new Size(10, 50), + }, + new TestControl + { + VerticalAlignment = VerticalAlignment.Bottom, + MeasureSize = new Size(10, 150), + }, + new TestControl + { + VerticalAlignment = VerticalAlignment.Stretch, + MeasureSize = new Size(10, 50), + }, + new TestControl + { + VerticalAlignment = VerticalAlignment.Stretch, + MeasureSize = new Size(10, 150), + }, + } + }; + + target.Measure(new Size(150, 100)); + Assert.Equal(new Size(80, 100), target.DesiredSize); + + target.Arrange(new Rect(target.DesiredSize)); + + var bounds = target.Children.Select(x => x.Bounds).ToArray(); + + Assert.Equal( + new[] + { + new Rect(0, 0, 10, 50), + new Rect(10, 0, 10, 150), + new Rect(20, 25, 10, 50), + new Rect(30, -25, 10, 150), + new Rect(40, 50, 10, 50), + new Rect(50, -50, 10, 150), + new Rect(60, 0, 10, 100), + new Rect(70, 0, 10, 150), + }, bounds); + } + [Theory] [InlineData(Orientation.Horizontal)] [InlineData(Orientation.Vertical)] @@ -185,5 +331,17 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(sizeWithTwoChildren, sizeWithThreeChildren); } + + private class TestControl : Control + { + public Size MeasureConstraint { get; private set; } + public Size MeasureSize { get; set; } + + protected override Size MeasureOverride(Size availableSize) + { + MeasureConstraint = availableSize; + return MeasureSize; + } + } } } From 7ac3556983050b29c8543321d51d272eecefb9b5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 2 Apr 2019 14:24:06 +0200 Subject: [PATCH 02/11] Fixed StackPanel arrange pass. Now follow's WPF's behavior better. In order to get the desired outcome, the `.Constrain(availableSize)` call in `Layoutable.Measure` had to be removed. Controls that relied on that (`Image` and `ViewBox`) have to now call `Constrain` themselves. Fixes #2350 --- src/Avalonia.Controls/Image.cs | 11 ++--- src/Avalonia.Controls/StackPanel.cs | 54 ++++++--------------- src/Avalonia.Controls/Viewbox.cs | 2 +- src/Avalonia.Layout/LayoutExtensions.cs | 62 +++++++++++++++++++++++++ src/Avalonia.Layout/Layoutable.cs | 2 +- 5 files changed, 84 insertions(+), 47 deletions(-) create mode 100644 src/Avalonia.Layout/LayoutExtensions.cs diff --git a/src/Avalonia.Controls/Image.cs b/src/Avalonia.Controls/Image.cs index c696fe7975..fa6f5787be 100644 --- a/src/Avalonia.Controls/Image.cs +++ b/src/Avalonia.Controls/Image.cs @@ -81,23 +81,22 @@ namespace Avalonia.Controls protected override Size MeasureOverride(Size availableSize) { var source = Source; + var result = new Size(); if (source != null) { Size sourceSize = new Size(source.PixelSize.Width, source.PixelSize.Height); if (double.IsInfinity(availableSize.Width) || double.IsInfinity(availableSize.Height)) { - return sourceSize; + result = sourceSize; } else { - return Stretch.CalculateSize(availableSize, sourceSize); + result = Stretch.CalculateSize(availableSize, sourceSize); } } - else - { - return new Size(); - } + + return result.Constrain(availableSize); } /// diff --git a/src/Avalonia.Controls/StackPanel.cs b/src/Avalonia.Controls/StackPanel.cs index df0c113cc0..d89df8556b 100644 --- a/src/Avalonia.Controls/StackPanel.cs +++ b/src/Avalonia.Controls/StackPanel.cs @@ -4,6 +4,7 @@ using System; using System.Linq; using Avalonia.Input; +using Avalonia.Layout; namespace Avalonia.Controls { @@ -219,30 +220,16 @@ namespace Avalonia.Controls measuredWidth -= (hasVisibleChild ? spacing : 0); } - return new Size(measuredWidth, measuredHeight); + return new Size(measuredWidth, measuredHeight).Constrain(availableSize); } - /// - /// Arranges the control's children. - /// - /// The size allocated to the control. - /// The space taken. + /// protected override Size ArrangeOverride(Size finalSize) { var orientation = Orientation; - double arrangedWidth = finalSize.Width; - double arrangedHeight = finalSize.Height; - double spacing = Spacing; - bool hasVisibleChild = Children.Any(c => c.IsVisible); - - if (Orientation == Orientation.Vertical) - { - arrangedHeight = 0; - } - else - { - arrangedWidth = 0; - } + var spacing = Spacing; + var finalRect = new Rect(finalSize); + var pos = 0.0; foreach (Control child in Children) { @@ -251,32 +238,21 @@ namespace Avalonia.Controls if (orientation == Orientation.Vertical) { - double width = Math.Max(childWidth, arrangedWidth); - Rect childFinal = new Rect(0, arrangedHeight, width, childHeight); - ArrangeChild(child, childFinal, finalSize, orientation); - arrangedWidth = Math.Max(arrangedWidth, childWidth); - arrangedHeight += childHeight + (child.IsVisible ? spacing : 0); + var rect = new Rect(0, pos, childWidth, childHeight) + .Align(finalRect, child.HorizontalAlignment, VerticalAlignment.Top); + ArrangeChild(child, rect, finalSize, orientation); + pos += childHeight + spacing; } else { - double height = Math.Max(childHeight, arrangedHeight); - Rect childFinal = new Rect(arrangedWidth, 0, childWidth, height); - ArrangeChild(child, childFinal, finalSize, orientation); - arrangedWidth += childWidth + (child.IsVisible ? spacing : 0); - arrangedHeight = Math.Max(arrangedHeight, childHeight); + var rect = new Rect(pos, 0, childWidth, childHeight) + .Align(finalRect, HorizontalAlignment.Left, child.VerticalAlignment); + ArrangeChild(child, rect, finalSize, orientation); + pos += childWidth + spacing; } } - if (orientation == Orientation.Vertical) - { - arrangedHeight = Math.Max(arrangedHeight - (hasVisibleChild ? spacing : 0), finalSize.Height); - } - else - { - arrangedWidth = Math.Max(arrangedWidth - (hasVisibleChild ? spacing : 0), finalSize.Width); - } - - return new Size(arrangedWidth, arrangedHeight); + return finalSize; } internal virtual void ArrangeChild( diff --git a/src/Avalonia.Controls/Viewbox.cs b/src/Avalonia.Controls/Viewbox.cs index 3a070d07b2..781c93bcbe 100644 --- a/src/Avalonia.Controls/Viewbox.cs +++ b/src/Avalonia.Controls/Viewbox.cs @@ -49,7 +49,7 @@ namespace Avalonia.Controls var scale = GetScale(availableSize, childSize, Stretch); - return childSize * scale; + return (childSize * scale).Constrain(availableSize); } return new Size(); diff --git a/src/Avalonia.Layout/LayoutExtensions.cs b/src/Avalonia.Layout/LayoutExtensions.cs new file mode 100644 index 0000000000..737bbb083c --- /dev/null +++ b/src/Avalonia.Layout/LayoutExtensions.cs @@ -0,0 +1,62 @@ +using System; + +namespace Avalonia.Layout +{ + /// + /// Extension methods for layout types. + /// + public static class LayoutExtensions + { + /// + /// Aligns a rect in a constraining rect according to horizontal and vertical alignment + /// settings. + /// + /// The rect to align. + /// The constraining rect. + /// The horizontal alignment. + /// The vertical alignment. + /// + public static Rect Align( + this Rect rect, + Rect constraint, + HorizontalAlignment horizontalAlignment, + VerticalAlignment verticalAlignment) + { + switch (horizontalAlignment) + { + case HorizontalAlignment.Center: + rect = rect.WithX((constraint.Width - rect.Width) / 2); + break; + case HorizontalAlignment.Right: + rect = rect.WithX(constraint.Width - rect.Width); + break; + case HorizontalAlignment.Stretch: + rect = new Rect( + 0, + rect.Y, + Math.Max(constraint.Width, rect.Width), + rect.Height); + break; + } + + switch (verticalAlignment) + { + case VerticalAlignment.Center: + rect = rect.WithY((constraint.Height - rect.Height) / 2); + break; + case VerticalAlignment.Bottom: + rect = rect.WithY(constraint.Height - rect.Height); + break; + case VerticalAlignment.Stretch: + rect = new Rect( + rect.X, + 0, + rect.Width, + Math.Max(constraint.Height, rect.Height)); + break; + } + + return rect; + } + } +} diff --git a/src/Avalonia.Layout/Layoutable.cs b/src/Avalonia.Layout/Layoutable.cs index d5e3a1631e..662f48ec44 100644 --- a/src/Avalonia.Layout/Layoutable.cs +++ b/src/Avalonia.Layout/Layoutable.cs @@ -314,7 +314,7 @@ namespace Avalonia.Layout try { _measuring = true; - desiredSize = MeasureCore(availableSize).Constrain(availableSize); + desiredSize = MeasureCore(availableSize); } finally { From 3992e0e85514398623abbb9caf844325bfa695e9 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 8 Apr 2019 00:18:20 +0200 Subject: [PATCH 03/11] Added failing test for #2420. --- tests/Avalonia.LeakTests/ControlTests.cs | 33 ++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs index b7aa452a18..cd25b79f1f 100644 --- a/tests/Avalonia.LeakTests/ControlTests.cs +++ b/tests/Avalonia.LeakTests/ControlTests.cs @@ -308,6 +308,39 @@ namespace Avalonia.LeakTests } + [Fact] + public void Slider_Is_Freed() + { + using (Start()) + { + Func run = () => + { + var window = new Window + { + Content = new Slider() + }; + + window.Show(); + + // Do a layout and make sure that Slider gets added to visual tree. + window.LayoutManager.ExecuteInitialLayoutPass(window); + Assert.IsType(window.Presenter.Child); + + // Clear the content and ensure the Slider is removed. + window.Content = null; + window.LayoutManager.ExecuteLayoutPass(); + Assert.Null(window.Presenter.Child); + + return window; + }; + + var result = run(); + + dotMemory.Check(memory => + Assert.Equal(0, memory.GetObjects(where => where.Type.Is()).ObjectsCount)); + } + } + [Fact] public void RendererIsDisposed() { From 1cd900c856a80b370d3dad7e890a2a1654617599 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 8 Apr 2019 18:39:39 +0200 Subject: [PATCH 04/11] Prevent leak when using TemplateBinding from a Setter. When a `TemplateBinding` is used as a `Setter.Value`, then we need to make sure we don't use the `TemplateBinding` itself as the binding because this can cause a memory leak. Replace `IRequiresTemplateInSetter` with `ISetterValue` which can be used to require a template in the setter for `Control` and can also be used to notify `TemplateBinding` that it's in a setter and so should always clone itself. Fixes #2420 --- src/Avalonia.Controls/Control.cs | 10 +++++++++- .../Styling/IRequiresTemplateInSetter.cs | 10 ---------- src/Avalonia.Styling/Styling/ISetterValue.cs | 13 +++++++++++++ src/Avalonia.Styling/Styling/Setter.cs | 8 +------- .../Avalonia.Markup/Data/TemplateBinding.cs | 17 ++++++++++++----- tests/Avalonia.Styling.UnitTests/SetterTests.cs | 14 ++++++-------- 6 files changed, 41 insertions(+), 31 deletions(-) delete mode 100644 src/Avalonia.Styling/Styling/IRequiresTemplateInSetter.cs create mode 100644 src/Avalonia.Styling/Styling/ISetterValue.cs diff --git a/src/Avalonia.Controls/Control.cs b/src/Avalonia.Controls/Control.cs index a7ee027e70..ca5edae4a9 100644 --- a/src/Avalonia.Controls/Control.cs +++ b/src/Avalonia.Controls/Control.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 System.ComponentModel; using Avalonia.Controls.Primitives; using Avalonia.Controls.Templates; @@ -20,7 +21,7 @@ namespace Avalonia.Controls /// /// - A property to allow user-defined data to be attached to the control. /// - public class Control : InputElement, IControl, INamed, ISupportInitialize, IVisualBrushInitialize, IRequiresTemplateInSetter + public class Control : InputElement, IControl, INamed, ISupportInitialize, IVisualBrushInitialize, ISetterValue { /// /// Defines the property. @@ -90,6 +91,13 @@ namespace Avalonia.Controls /// bool IDataTemplateHost.IsDataTemplatesInitialized => _dataTemplates != null; + /// + void ISetterValue.Initialize(ISetter setter) + { + throw new InvalidOperationException( + "Cannot use a control as a Setter value. Wrap the control in a