Merge pull request #1021 from AvaloniaUI/fixup-layout-manager

Fix up the Layout Manager

src/Avalonia.Layout/LayoutManager.cs

@@ -14,8 +14,8 @@ namespace Avalonia.Layout
     /// </summary>
     public class LayoutManager : ILayoutManager
     {
-        private readonly HashSet<ILayoutable> _toMeasure = new HashSet<ILayoutable>();
-        private readonly HashSet<ILayoutable> _toArrange = new HashSet<ILayoutable>();
+        private readonly Queue<ILayoutable> _toMeasure = new Queue<ILayoutable>();
+        private readonly Queue<ILayoutable> _toArrange = new Queue<ILayoutable>();
         private bool _queued;
         private bool _running;
 
@@ -30,8 +30,18 @@ namespace Avalonia.Layout
             Contract.Requires<ArgumentNullException>(control != null);
             Dispatcher.UIThread.VerifyAccess();
 
-            _toMeasure.Add(control);
-            _toArrange.Add(control);
+            if (!control.IsAttachedToVisualTree)
+            {
+#if DEBUG
+                throw new AvaloniaInternalException(
+                    "LayoutManager.InvalidateMeasure called on a control that is detached from the visual tree.");
+#else
+                return;
+#endif
+            }
+
+            _toMeasure.Enqueue(control);
+            _toArrange.Enqueue(control);
             QueueLayoutPass();
         }
 
@@ -41,7 +51,17 @@ namespace Avalonia.Layout
             Contract.Requires<ArgumentNullException>(control != null);
             Dispatcher.UIThread.VerifyAccess();
 
-            _toArrange.Add(control);
+            if (!control.IsAttachedToVisualTree)
+            {
+#if DEBUG
+                throw new AvaloniaInternalException(
+                    "LayoutManager.InvalidateArrange called on a control that is detached from the visual tree.");
+#else
+                return;
+#endif
+            }
+
+            _toArrange.Enqueue(control);
             QueueLayoutPass();
         }
 
@@ -108,8 +128,12 @@ namespace Avalonia.Layout
         {
             while (_toMeasure.Count > 0)
             {
-                var next = _toMeasure.First();
-                Measure(next);
+                var control = _toMeasure.Dequeue();
+
+                if (!control.IsMeasureValid && control.IsAttachedToVisualTree)
+                {
+                    Measure(control);
+                }
             }
         }
 
@@ -117,53 +141,60 @@ namespace Avalonia.Layout
         {
             while (_toArrange.Count > 0 && _toMeasure.Count == 0)
             {
-                var next = _toArrange.First();
-                Arrange(next);
+                var control = _toArrange.Dequeue();
+
+                if (!control.IsArrangeValid && control.IsAttachedToVisualTree)
+                {
+                    Arrange(control);
+                }
             }
         }
 
         private void Measure(ILayoutable control)
         {
-            var root = control as ILayoutRoot;
-            var parent = control.VisualParent as ILayoutable;
-
-            if (root != null)
-            {
-                root.Measure(root.MaxClientSize);
-            }
-            else if (parent != null)
+            // Controls closest to the visual root need to be arranged first. We don't try to store
+            // ordered invalidation lists, instead we traverse the tree upwards, measuring the
+            // controls closest to the root first. This has been shown by benchmarks to be the
+            // fastest and most memory-efficent algorithm.
+            if (control.VisualParent is ILayoutable parent)
             {
                 Measure(parent);
             }
 
-            if (!control.IsMeasureValid)
+            // If the control being measured has IsMeasureValid == true here then its measure was
+            // handed by an ancestor and can be ignored. The measure may have also caused the
+            // control to be removed.
+            if (!control.IsMeasureValid && control.IsAttachedToVisualTree)
             {
-                control.Measure(control.PreviousMeasure.Value);
+                if (control is ILayoutRoot root)
+                {
+                    root.Measure(Size.Infinity);
+                }
+                else
+                {
+                    control.Measure(control.PreviousMeasure.Value);
+                }
             }
-
-            _toMeasure.Remove(control);
         }
 
         private void Arrange(ILayoutable control)
         {
-            var root = control as ILayoutRoot;
-            var parent = control.VisualParent as ILayoutable;
-
-            if (root != null)
-            {
-                root.Arrange(new Rect(root.DesiredSize));
-            }
-            else if (parent != null)
+            if (control.VisualParent is ILayoutable parent)
             {
                 Arrange(parent);
             }
 
-            if (control.PreviousArrange.HasValue)
+            if (!control.IsArrangeValid && control.IsAttachedToVisualTree)
             {
-                control.Arrange(control.PreviousArrange.Value);
+                if (control is ILayoutRoot root)
+                {
+                    root.Arrange(new Rect(control.DesiredSize));
+                }
+                else
+                {
+                    control.Arrange(control.PreviousArrange.Value);
+                }
             }
-
-            _toArrange.Remove(control);
         }
 
         private void QueueLayoutPass()

src/Avalonia.Layout/Layoutable.cs

@@ -378,8 +378,12 @@ namespace Avalonia.Layout
 
                 IsMeasureValid = false;
                 IsArrangeValid = false;
-                LayoutManager.Instance?.InvalidateMeasure(this);
-                InvalidateVisual();
+
+                if (((ILayoutable)this).IsAttachedToVisualTree)
+                {
+                    LayoutManager.Instance?.InvalidateMeasure(this);
+                    InvalidateVisual();
+                }
             }
         }
 
@@ -393,8 +397,12 @@ namespace Avalonia.Layout
                 Logger.Verbose(LogArea.Layout, this, "Invalidated arrange");
 
                 IsArrangeValid = false;
-                LayoutManager.Instance?.InvalidateArrange(this);
-                InvalidateVisual();
+
+                if (((ILayoutable)this).IsAttachedToVisualTree)
+                {
+                    LayoutManager.Instance?.InvalidateArrange(this);
+                    InvalidateVisual();
+                }
             }
         }
 

tests/Avalonia.Benchmarks/Avalonia.Benchmarks.csproj

@@ -49,6 +49,7 @@
     <Reference Include="System.Xml" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="Layout\Measure.cs" />
     <Compile Include="Styling\ApplyStyling.cs" />
     <Compile Include="Program.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
@@ -100,7 +101,7 @@
   </ItemGroup>
   <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
   <ItemGroup>
-    <PackageReference Include="BenchmarkDotNet" Version="0.9.2" />
+    <PackageReference Include="BenchmarkDotNet" Version="0.10.8" />
   </ItemGroup>
   <Import Project="$(MSBuildThisFileDirectory)..\..\src\Shared\nuget.workaround.targets" />
 </Project>

tests/Avalonia.Benchmarks/Layout/Measure.cs

@@ -0,0 +1,65 @@
+using System;
+using System.Collections.Generic;
+using Avalonia.Controls;
+using Avalonia.Layout;
+using Avalonia.UnitTests;
+using BenchmarkDotNet.Attributes;
+
+namespace Avalonia.Benchmarks.Layout
+{
+    [MemoryDiagnoser]
+    public class Measure : IDisposable
+    {
+        private IDisposable _app;
+        private TestRoot root;
+        private List<Control> controls = new List<Control>();
+
+        public Measure()
+        {
+            _app = UnitTestApplication.Start(TestServices.RealLayoutManager);
+
+            var panel = new StackPanel();
+            root = new TestRoot { Child = panel };
+            controls.Add(panel);
+            CreateChildren(panel, 3, 5);
+            LayoutManager.Instance.ExecuteInitialLayoutPass(root);
+        }
+
+        public void Dispose()
+        {
+            _app.Dispose();
+        }
+
+        [Benchmark]
+        public void Remeasure_Half()
+        {
+            var random = new Random(1);
+
+            foreach (var control in controls)
+            {
+                if (random.Next(2) == 0)
+                {
+                    control.InvalidateMeasure();
+                }
+            }
+
+            LayoutManager.Instance.ExecuteLayoutPass();
+        }
+
+        private void CreateChildren(IPanel parent, int childCount, int iterations)
+        {
+            for (var i = 0; i < childCount; ++i)
+            {
+                var control = new StackPanel();
+                parent.Children.Add(control);
+
+                if (iterations > 0)
+                {
+                    CreateChildren(control, childCount, iterations - 1);
+                }
+
+                controls.Add(control);
+            }
+        }
+    }
+}

tests/Avalonia.Benchmarks/Styling/ApplyStyling.cs

@@ -11,6 +11,7 @@ using Avalonia.VisualTree;
 
 namespace Avalonia.Benchmarks.Styling
 {
+    [MemoryDiagnoser]
     public class ApplyStyling : IDisposable
     {
         private IDisposable _app;

tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs

@@ -2,25 +2,276 @@
 // Licensed under the MIT license. See licence.md file in the project root for full license information.
 
 using Avalonia.Controls;
+using Avalonia.UnitTests;
+using System;
 using Xunit;
+using System.Collections.Generic;
 
 namespace Avalonia.Layout.UnitTests
 {
     public class LayoutManagerTests
     {
         [Fact]
-        public void Invalidating_Child_Should_Remeasure_Parent()
+        public void Measures_And_Arranges_InvalidateMeasured_Control()
         {
-            var layoutManager = new LayoutManager();
+            var target = new LayoutManager();
 
-            using (AvaloniaLocator.EnterScope())
+            using (Start(target))
             {
-                AvaloniaLocator.CurrentMutable.Bind<ILayoutManager>().ToConstant(layoutManager);
+                var control = new LayoutTestControl();
+                var root = new LayoutTestRoot { Child = control };
+
+                target.ExecuteInitialLayoutPass(root);
+                control.Measured = control.Arranged = false;
+
+                control.InvalidateMeasure();
+                target.ExecuteLayoutPass();
+
+                Assert.True(control.Measured);
+                Assert.True(control.Arranged);
+            }
+        }
+
+        [Fact]
+        public void Arranges_InvalidateArranged_Control()
+        {
+            var target = new LayoutManager();
+
+            using (Start(target))
+            {
+                var control = new LayoutTestControl();
+                var root = new LayoutTestRoot { Child = control };
+
+                target.ExecuteInitialLayoutPass(root);
+                control.Measured = control.Arranged = false;
+
+                control.InvalidateArrange();
+                target.ExecuteLayoutPass();
+
+                Assert.False(control.Measured);
+                Assert.True(control.Arranged);
+            }
+        }
+
+        [Fact]
+        public void Measures_Parent_Of_Newly_Added_Control()
+        {
+            var target = new LayoutManager();
+
+            using (Start(target))
+            {
+                var control = new LayoutTestControl();
+                var root = new LayoutTestRoot();
+
+                target.ExecuteInitialLayoutPass(root);
+                root.Child = control;
+                root.Measured = root.Arranged = false;
+
+                target.ExecuteLayoutPass();
+
+                Assert.True(root.Measured);
+                Assert.True(root.Arranged);
+                Assert.True(control.Measured);
+                Assert.True(control.Arranged);
+            }
+        }
+
+        [Fact]
+        public void Measures_In_Correct_Order()
+        {
+            var target = new LayoutManager();
+
+            using (Start(target))
+            {
+                LayoutTestControl control1;
+                LayoutTestControl control2;
+                var root = new LayoutTestRoot
+                {
+                    Child = control1 = new LayoutTestControl
+                    {
+                        Child = control2 = new LayoutTestControl(),
+                    }
+                };
+
+
+                var order = new List<ILayoutable>();
+                Size MeasureOverride(ILayoutable control, Size size)
+                {
+                    order.Add(control);
+                    return new Size(10, 10);
+                }
+
+                root.DoMeasureOverride = MeasureOverride;
+                control1.DoMeasureOverride = MeasureOverride;
+                control2.DoMeasureOverride = MeasureOverride;
+                target.ExecuteInitialLayoutPass(root);
+
+                control2.InvalidateMeasure();
+                control1.InvalidateMeasure();
+                root.InvalidateMeasure();
+
+                order.Clear();
+                target.ExecuteLayoutPass();
+
+                Assert.Equal(new ILayoutable[] { root, control1, control2 }, order);
+            }
+        }
+
+        [Fact]
+        public void Measures_Root_And_Grandparent_In_Correct_Order()
+        {
+            var target = new LayoutManager();
+
+            using (Start(target))
+            {
+                LayoutTestControl control1;
+                LayoutTestControl control2;
+                var root = new LayoutTestRoot
+                {
+                    Child = control1 = new LayoutTestControl
+                    {
+                        Child = control2 = new LayoutTestControl(),
+                    }
+                };
+
+
+                var order = new List<ILayoutable>();
+                Size MeasureOverride(ILayoutable control, Size size)
+                {
+                    order.Add(control);
+                    return new Size(10, 10);
+                }
+
+                root.DoMeasureOverride = MeasureOverride;
+                control1.DoMeasureOverride = MeasureOverride;
+                control2.DoMeasureOverride = MeasureOverride;
+                target.ExecuteInitialLayoutPass(root);
+
+                control2.InvalidateMeasure();
+                root.InvalidateMeasure();
+
+                order.Clear();
+                target.ExecuteLayoutPass();
+
+                Assert.Equal(new ILayoutable[] { root, control2 }, order);
+            }
+        }
+
+        [Fact]
+        public void Doesnt_Measure_Non_Invalidated_Root()
+        {
+            var target = new LayoutManager();
+
+            using (Start(target))
+            {
+                var control = new LayoutTestControl();
+                var root = new LayoutTestRoot { Child = control };
+
+                target.ExecuteInitialLayoutPass(root);
+                root.Measured = root.Arranged = false;
+                control.Measured = control.Arranged = false;
+
+                control.InvalidateMeasure();
+                target.ExecuteLayoutPass();
+
+                Assert.False(root.Measured);
+                Assert.False(root.Arranged);
+                Assert.True(control.Measured);
+                Assert.True(control.Arranged);
+            }
+        }
+
+        [Fact]
+        public void Doesnt_Measure_Removed_Control()
+        {
+            var target = new LayoutManager();
+
+            using (Start(target))
+            {
+                var control = new LayoutTestControl();
+                var root = new LayoutTestRoot { Child = control };
+
+                target.ExecuteInitialLayoutPass(root);
+                control.Measured = control.Arranged = false;
+
+                control.InvalidateMeasure();
+                root.Child = null;
+                target.ExecuteLayoutPass();
+
+                Assert.False(control.Measured);
+                Assert.False(control.Arranged);
+            }
+        }
+
+        [Fact]
+        public void Measures_Root_With_Infinity()
+        {
+            var target = new LayoutManager();
+
+            using (Start(target))
+            {
+                var root = new LayoutTestRoot();
+                var availableSize = default(Size);
+
+                // Should not measure with this size.
+                root.MaxClientSize = new Size(123, 456);
+
+                root.DoMeasureOverride = (_, s) =>
+                {
+                    availableSize = s;
+                    return new Size(100, 100);
+                };
+
+                target.ExecuteInitialLayoutPass(root);
+
+                Assert.Equal(Size.Infinity, availableSize);
+            }
+        }
+
+        [Fact]
+        public void Arranges_Root_With_DesiredSize()
+        {
+            var target = new LayoutManager();
+ 
+            using (Start(target))
+            {
+                var root = new LayoutTestRoot
+                {
+                    Width = 100,
+                    Height = 100,
+                };
+ 
+                var arrangeSize = default(Size);
+ 
+                root.DoArrangeOverride = (_, s) =>
+                {
+                    arrangeSize = s;
+                    return s;
+                };
+ 
+                target.ExecuteInitialLayoutPass(root);
+                Assert.Equal(new Size(100, 100), arrangeSize);
+ 
+                root.Width = 120;
+ 
+                target.ExecuteLayoutPass();
+                Assert.Equal(new Size(120, 100), arrangeSize);
+            }
+        }
+
+        [Fact]
+        public void Invalidating_Child_Remeasures_Parent()
+        {
+            var target = new LayoutManager();
+
+            using (Start(target))
+            {
+                AvaloniaLocator.CurrentMutable.Bind<ILayoutManager>().ToConstant(target);
 
                 Border border;
                 StackPanel panel;
 
-                var root = new TestLayoutRoot
+                var root = new LayoutTestRoot
                 {
                     Child = panel = new StackPanel
                     {
@@ -31,15 +282,22 @@ namespace Avalonia.Layout.UnitTests
                     }
                 };
 
-                layoutManager.ExecuteInitialLayoutPass(root);
+                target.ExecuteInitialLayoutPass(root);
                 Assert.Equal(new Size(0, 0), root.DesiredSize);
 
                 border.Width = 100;
                 border.Height = 100;
 
-                layoutManager.ExecuteLayoutPass();
+                target.ExecuteLayoutPass();
                 Assert.Equal(new Size(100, 100), panel.DesiredSize);
             }                
         }
+
+        private IDisposable Start(LayoutManager layoutManager)
+        {
+            var result = AvaloniaLocator.EnterScope();
+            AvaloniaLocator.CurrentMutable.Bind<ILayoutManager>().ToConstant(layoutManager);
+            return result;
+        }
     }
 }

tests/Avalonia.Layout.UnitTests/LayoutTestControl.cs

@@ -0,0 +1,29 @@
+using System;
+using Avalonia.Controls;
+
+namespace Avalonia.Layout.UnitTests
+{
+    internal class LayoutTestControl : Decorator
+    {
+        public bool Measured { get; set; }
+        public bool Arranged { get; set; }
+        public Func<ILayoutable, Size, Size> DoMeasureOverride { get; set; }
+        public Func<ILayoutable, Size, Size> DoArrangeOverride { get; set; }
+
+        protected override Size MeasureOverride(Size availableSize)
+        {
+            Measured = true;
+            return DoMeasureOverride != null ?
+                DoMeasureOverride(this, availableSize) :
+                base.MeasureOverride(availableSize);
+        }
+
+        protected override Size ArrangeOverride(Size finalSize)
+        {
+            Arranged = true;
+            return DoArrangeOverride != null ?
+                DoArrangeOverride(this, finalSize) :
+                base.ArrangeOverride(finalSize);
+        }
+    }
+}

tests/Avalonia.Layout.UnitTests/LayoutTestRoot.cs

@@ -0,0 +1,43 @@
+// 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.UnitTests;
+
+namespace Avalonia.Layout.UnitTests
+{
+    internal class LayoutTestRoot : TestRoot, ILayoutable
+    {
+        public bool Measured { get; set; }
+        public bool Arranged { get; set; }
+        public Func<ILayoutable, Size, Size> DoMeasureOverride { get; set; }
+        public Func<ILayoutable, Size, Size> DoArrangeOverride { get; set; }
+
+        void ILayoutable.Measure(Size availableSize)
+        {
+            Measured = true;
+            Measure(availableSize);
+        }
+
+        void ILayoutable.Arrange(Rect rect)
+        {
+            Arranged = true;
+            Arrange(rect);
+        }
+
+        protected override Size MeasureOverride(Size availableSize)
+        {
+            return DoMeasureOverride != null ?
+                DoMeasureOverride(this, availableSize) :
+                base.MeasureOverride(availableSize);
+        }
+
+        protected override Size ArrangeOverride(Size finalSize)
+        {
+            Arranged = true;
+            return DoArrangeOverride != null ?
+                DoArrangeOverride(this, finalSize) :
+                base.ArrangeOverride(finalSize);
+        }
+    }
+}

tests/Avalonia.Layout.UnitTests/LayoutableTests.cs

@@ -0,0 +1,90 @@
+using System;
+using Avalonia.Controls;
+using Moq;
+using Xunit;
+
+namespace Avalonia.Layout.UnitTests
+{
+    public class LayoutableTests
+    {
+        [Fact]
+        public void Only_Calls_LayoutManager_InvalidateMeasure_Once()
+        {
+            var target = new Mock<ILayoutManager>();
+
+            using (Start(target.Object))
+            {
+                var control = new Decorator();
+                var root = new LayoutTestRoot { Child = control };
+
+                root.Measure(Size.Infinity);
+                root.Arrange(new Rect(root.DesiredSize));
+                target.ResetCalls();
+
+                control.InvalidateMeasure();
+                control.InvalidateMeasure();
+
+                target.Verify(x => x.InvalidateMeasure(control), Times.Once());
+            }
+        }
+
+        [Fact]
+        public void Only_Calls_LayoutManager_InvalidateArrange_Once()
+        {
+            var target = new Mock<ILayoutManager>();
+
+            using (Start(target.Object))
+            {
+                var control = new Decorator();
+                var root = new LayoutTestRoot { Child = control };
+
+                root.Measure(Size.Infinity);
+                root.Arrange(new Rect(root.DesiredSize));
+                target.ResetCalls();
+
+                control.InvalidateArrange();
+                control.InvalidateArrange();
+
+                target.Verify(x => x.InvalidateArrange(control), Times.Once());
+            }
+        }
+
+        [Fact]
+        public void Attaching_Control_To_Tree_Invalidates_Parent_Measure()
+        {
+            var target = new Mock<ILayoutManager>();
+
+            using (Start(target.Object))
+            {
+                var control = new Decorator();
+                var root = new LayoutTestRoot { Child = control };
+
+                root.Measure(Size.Infinity);
+                root.Arrange(new Rect(root.DesiredSize));
+                Assert.True(control.IsMeasureValid);
+
+                root.Child = null;
+                root.Measure(Size.Infinity);
+                root.Arrange(new Rect(root.DesiredSize));
+
+                Assert.False(control.IsMeasureValid);
+                Assert.True(root.IsMeasureValid);
+
+                target.ResetCalls();
+
+                root.Child = control;
+
+                Assert.False(root.IsMeasureValid);
+                Assert.False(control.IsMeasureValid);
+                target.Verify(x => x.InvalidateMeasure(root), Times.Once());
+            }
+        }
+
+        private IDisposable Start(ILayoutManager layoutManager)
+        {
+            var result = AvaloniaLocator.EnterScope();
+            AvaloniaLocator.CurrentMutable.Bind<ILayoutManager>().ToConstant(layoutManager);
+            return result;
+        }
+    }
+}

tests/Avalonia.Layout.UnitTests/TestLayoutRoot.cs

@@ -1,24 +0,0 @@
-// 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;
-
-namespace Avalonia.Layout.UnitTests
-{
-    internal class TestLayoutRoot : Decorator, ILayoutRoot
-    {
-        public TestLayoutRoot()
-        {
-            ClientSize = new Size(500, 500);
-        }
-
-        public Size ClientSize
-        {
-            get;
-            set;
-        }
-
-        public Size MaxClientSize => Size.Infinity;
-        public double LayoutScaling => 1;
-    }
-}

tests/Avalonia.UnitTests/TestRoot.cs

@@ -43,7 +43,7 @@ namespace Avalonia.UnitTests
 
         public Size ClientSize => new Size(100, 100);
 
-        public Size MaxClientSize => Size.Infinity;
+        public Size MaxClientSize { get; set; } = Size.Infinity;
 
         public double LayoutScaling => 1;