From 9a9ef657e26ea7f0bf382ce0041eebe575fb211d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 3 Oct 2019 17:46:30 +0200 Subject: [PATCH 1/8] Ported Track layout code from WPF. To fix problem with scrollbar layout when the scrollbar range was large. NOTE: This commit adds `Track.IsDirectionReversed` and modifies the default direction of vertical tracks to be the reverse of before in order to match WPF. --- src/Avalonia.Controls/Primitives/Track.cs | 318 +++++++++++++++------ src/Avalonia.Themes.Default/ScrollBar.xaml | 3 +- src/Avalonia.Themes.Default/Slider.xaml | 2 +- 3 files changed, 236 insertions(+), 87 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/Track.cs b/src/Avalonia.Controls/Primitives/Track.cs index 21a7dd68f8..0af8ac3415 100644 --- a/src/Avalonia.Controls/Primitives/Track.cs +++ b/src/Avalonia.Controls/Primitives/Track.cs @@ -1,10 +1,13 @@ -// 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. +// Portions of this source file are adapted from the Windows Presentation Foundation project. +// (https://github.com/dotnet/wpf/) +// +// Licensed to The Avalonia Project under MIT License, courtesy of The .NET Foundation. using System; using Avalonia.Input; using Avalonia.Layout; using Avalonia.Metadata; +using Avalonia.Utilities; namespace Avalonia.Controls.Primitives { @@ -34,6 +37,9 @@ namespace Avalonia.Controls.Primitives public static readonly StyledProperty + - + + + + + + + diff --git a/samples/ControlCatalog/Pages/ItemsRepeaterPage.xaml.cs b/samples/ControlCatalog/Pages/ItemsRepeaterPage.xaml.cs index 1a607342f3..decd849adc 100644 --- a/samples/ControlCatalog/Pages/ItemsRepeaterPage.xaml.cs +++ b/samples/ControlCatalog/Pages/ItemsRepeaterPage.xaml.cs @@ -74,7 +74,7 @@ namespace ControlCatalog.Pages private void RepeaterClick(object sender, PointerPressedEventArgs e) { - var item = (e.Source as TextBlock)?.DataContext as string; + var item = (e.Source as TextBlock)?.DataContext as ItemsRepeaterPageViewModel.Item; ((ItemsRepeaterPageViewModel)DataContext).SelectedItem = item; } } diff --git a/samples/ControlCatalog/ViewModels/ItemsRepeaterPageViewModel.cs b/samples/ControlCatalog/ViewModels/ItemsRepeaterPageViewModel.cs index 25fd550520..bc2ce80714 100644 --- a/samples/ControlCatalog/ViewModels/ItemsRepeaterPageViewModel.cs +++ b/samples/ControlCatalog/ViewModels/ItemsRepeaterPageViewModel.cs @@ -1,4 +1,5 @@ -using System.Collections.ObjectModel; +using System; +using System.Collections.ObjectModel; using System.Linq; using ReactiveUI; @@ -10,18 +11,44 @@ namespace ControlCatalog.ViewModels public ItemsRepeaterPageViewModel() { - Items = new ObservableCollection( - Enumerable.Range(1, 100000).Select(i => $"Item {i.ToString()}")); + Items = new ObservableCollection( + Enumerable.Range(1, 100000).Select(i => new Item + { + Text = $"Item {i.ToString()}", + })); } - public ObservableCollection Items { get; } + public ObservableCollection Items { get; } - public string SelectedItem { get; set; } + public Item SelectedItem { get; set; } public void AddItem() { var index = SelectedItem != null ? Items.IndexOf(SelectedItem) : -1; - Items.Insert(index + 1, $"New Item {newItemIndex++}"); + Items.Insert(index + 1, new Item { Text = $"New Item {newItemIndex++}" }); + } + + public void RandomizeHeights() + { + var random = new Random(); + + foreach (var i in Items) + { + i.Height = random.Next(240) + 10; + } + } + + public class Item : ReactiveObject + { + private double _height = double.NaN; + + public string Text { get; set; } + + public double Height + { + get => _height; + set => this.RaiseAndSetIfChanged(ref _height, value); + } } } } From b317df99e779e81d3dccee25682742daabb65c6f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 4 Oct 2019 01:20:25 +0200 Subject: [PATCH 5/8] Added failing test for #3041. --- .../LayoutManagerTests.cs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs index 4c288b2702..3f20c9a76a 100644 --- a/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs +++ b/tests/Avalonia.Layout.UnitTests/LayoutManagerTests.cs @@ -341,5 +341,38 @@ namespace Avalonia.Layout.UnitTests //layoutmanager should process properly other visuals Assert.All(targets, c => Assert.True(c.Arranged)); } + + + [Fact] + public void LayoutManager_Should_Recover_From_Infinite_Loop_On_Measure() + { + // Test for issue #3041. + var control = new LayoutTestControl(); + var root = new LayoutTestRoot { Child = control }; + + root.LayoutManager.ExecuteInitialLayoutPass(root); + control.Measured = false; + + control.DoMeasureOverride = (l, s) => + { + control.InvalidateMeasure(); + return new Size(100, 100); + }; + + control.InvalidateMeasure(); + root.LayoutManager.ExecuteLayoutPass(); + + // This is the important part: running a second layout pass in which we exceed the maximum + // retries causes LayoutQueue.Info.Count to exceed _maxEnqueueCountPerLoop. + root.LayoutManager.ExecuteLayoutPass(); + + control.Measured = false; + control.DoMeasureOverride = null; + + root.LayoutManager.ExecuteLayoutPass(); + + Assert.True(control.Measured); + Assert.True(control.IsMeasureValid); + } } } From 9fd8fdb53473372a72c3c705b674596838605176 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 4 Oct 2019 01:22:57 +0200 Subject: [PATCH 6/8] Fix comparison in LayoutQueue. --- src/Avalonia.Layout/LayoutQueue.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Layout/LayoutQueue.cs b/src/Avalonia.Layout/LayoutQueue.cs index ce40fdde49..eb0e4bd9f3 100644 --- a/src/Avalonia.Layout/LayoutQueue.cs +++ b/src/Avalonia.Layout/LayoutQueue.cs @@ -60,7 +60,7 @@ namespace Avalonia.Layout public void EndLoop() { - var notfinalized = _loopQueueInfo.Where(v => v.Value.Count == _maxEnqueueCountPerLoop).ToArray(); + var notfinalized = _loopQueueInfo.Where(v => v.Value.Count >= _maxEnqueueCountPerLoop).ToArray(); _loopQueueInfo.Clear(); From 393bbcf882bb7818937b4b474b421f20e20bb89a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 4 Oct 2019 01:27:00 +0200 Subject: [PATCH 7/8] Disable horizontal scrollbar on ItemsRepeaterPage. --- samples/ControlCatalog/MainView.xaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/samples/ControlCatalog/MainView.xaml b/samples/ControlCatalog/MainView.xaml index c99a6b117b..874560a294 100644 --- a/samples/ControlCatalog/MainView.xaml +++ b/samples/ControlCatalog/MainView.xaml @@ -34,7 +34,8 @@ + ScrollViewer.VerticalScrollBarVisibility="Disabled" + ScrollViewer.HorizontalScrollBarVisibility="Disabled"> From 76840f463e5d4f5da6e3f1bb5ca30414347e7b7c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 4 Oct 2019 02:04:17 +0200 Subject: [PATCH 8/8] Don't hide Track during measure. It causes it to stop being laid out. Instead hide the child controls. --- src/Avalonia.Controls/Primitives/Track.cs | 24 +++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/Track.cs b/src/Avalonia.Controls/Primitives/Track.cs index 0af8ac3415..a569808b35 100644 --- a/src/Avalonia.Controls/Primitives/Track.cs +++ b/src/Avalonia.Controls/Primitives/Track.cs @@ -342,7 +342,7 @@ namespace Avalonia.Controls.Primitives // hide the track and don't arrange the pieces if (notEnoughContentToScroll || thumbLongerThanTrack) { - IsVisible = false; + ShowChildren(false); ThumbCenterOffset = Double.NaN; Density = Double.NaN; decreaseButtonLength = 0.0; @@ -351,7 +351,7 @@ namespace Avalonia.Controls.Primitives } else { - IsVisible = true; + ShowChildren(true); } // Compute lengths of increase and decrease button @@ -413,5 +413,25 @@ namespace Avalonia.Controls.Primitives Minimum, Maximum); } + + private void ShowChildren(bool visible) + { + // WPF sets Visible = Hidden here but we don't have that, and setting IsVisible = false + // will cause us to stop being laid out. Instead show/hide the child controls. + if (Thumb != null) + { + Thumb.IsVisible = visible; + } + + if (IncreaseButton != null) + { + IncreaseButton.IsVisible = visible; + } + + if (DecreaseButton != null) + { + DecreaseButton.IsVisible = visible; + } + } } }