From a8fc03217a99da2d6af754123b356d030b6216c4 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 3 Oct 2019 23:56:30 +0200 Subject: [PATCH 1/5] Fix ClearElementToPinnedPool. It was previously (incorrectly) implemented the same as `ClearElementToUniqueIdResetPool`. --- src/Avalonia.Controls/Repeater/ViewManager.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Controls/Repeater/ViewManager.cs b/src/Avalonia.Controls/Repeater/ViewManager.cs index 3e09a5b3ee..51c14d47d6 100644 --- a/src/Avalonia.Controls/Repeater/ViewManager.cs +++ b/src/Avalonia.Controls/Repeater/ViewManager.cs @@ -581,13 +581,16 @@ namespace Avalonia.Controls private bool ClearElementToPinnedPool(IControl element, VirtualizationInfo virtInfo, bool isClearedDueToCollectionChange) { - if (_isDataSourceStableResetPending) + bool moveToPinnedPool = + !isClearedDueToCollectionChange && virtInfo.IsPinned; + + if (moveToPinnedPool) { - _resetPool.Add(element); - virtInfo.MoveOwnershipToUniqueIdResetPoolFromLayout(); + _pinnedPool.Add(new PinnedElementInfo(element)); + virtInfo.MoveOwnershipToPinnedPool(); } - return _isDataSourceStableResetPending; + return moveToPinnedPool; } private void UpdateFocusedElement() From 9317aeda6a73c734c9091559a73b28865696b020 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 4 Oct 2019 00:07:21 +0200 Subject: [PATCH 2/5] Added "Randomize Heights" to ItemsRepeaterPage. --- .../Pages/ItemsRepeaterPage.xaml | 9 ++++- .../Pages/ItemsRepeaterPage.xaml.cs | 2 +- .../ViewModels/ItemsRepeaterPageViewModel.cs | 39 ++++++++++++++++--- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/samples/ControlCatalog/Pages/ItemsRepeaterPage.xaml b/samples/ControlCatalog/Pages/ItemsRepeaterPage.xaml index d0631d2cbd..103c016298 100644 --- a/samples/ControlCatalog/Pages/ItemsRepeaterPage.xaml +++ b/samples/ControlCatalog/Pages/ItemsRepeaterPage.xaml @@ -14,12 +14,19 @@ UniformGrid - Horizontal + - + + + + + + + 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 3/5] 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 4/5] 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 5/5] 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">