From be8d4516b9af7e20e9587413f245b2a070d4536d Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Thu, 11 Oct 2018 10:11:42 +0300 Subject: [PATCH 1/3] unit test for Listbox OutOfRangeException issue #1395 --- .../ListBoxTests.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index 1debccd3c5..bc563c25e2 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -196,6 +196,41 @@ namespace Avalonia.Controls.UnitTests target.Presenter.Panel.Children.Cast().Select(x => (string)x.Content)); } + + [Fact] + public void ListBox_After_Scroll_IndexOutOfRangeException_Shouldnt_Be_Thrown() + { + var items = Enumerable.Range(0, 11).Select(x => $"{x}").ToArray(); + + var target = new ListBox + { + Template = ListBoxTemplate(), + Items = items, + ItemTemplate = new FuncDataTemplate(x => new TextBlock { Height = 11 }) + }; + + Prepare(target); + + var panel = target.Presenter.Panel as IVirtualizingPanel; + + var listBoxItems = panel.Children.OfType(); + + //virtualization should have created exactly 10 items + Assert.Equal(10, listBoxItems.Count()); + Assert.Equal("0", listBoxItems.First().DataContext); + Assert.Equal("9", listBoxItems.Last().DataContext); + + //instead pixeloffset > 0 there could be pretty complex sequence for repro + //it involves add/remove/scroll to end multiple actions + //which i can't find so far :(, but this is the simplest way to add it to unit test + panel.PixelOffset = 1; + + //here scroll to end -> IndexOutOfRangeException is thrown + target.Scroll.Offset = new Vector(0, 2); + + Assert.True(true); + } + private FuncControlTemplate ListBoxTemplate() { return new FuncControlTemplate(parent => From 092a7c965aade6d849212f7a946d01c0e2f5ad3f Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Thu, 11 Oct 2018 11:44:31 +0300 Subject: [PATCH 2/3] fix for ListBox OutOfRangeException issue #1395 --- .../Presenters/ItemVirtualizerSimple.cs | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs b/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs index f31a48b2a0..374066dfad 100644 --- a/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs +++ b/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs @@ -76,9 +76,23 @@ namespace Avalonia.Controls.Presenters var firstIndex = ItemCount - panel.Children.Count; RecycleContainersForMove(firstIndex - FirstIndex); - panel.PixelOffset = VirtualizingPanel.ScrollDirection == Orientation.Vertical ? - panel.Children[0].Bounds.Height : - panel.Children[0].Bounds.Width; + double pixelOffset; + var child = panel.Children[0]; + + if (child.IsArrangeValid) + { + pixelOffset = VirtualizingPanel.ScrollDirection == Orientation.Vertical ? + child.Bounds.Height : + child.Bounds.Width; + } + else + { + pixelOffset = VirtualizingPanel.ScrollDirection == Orientation.Vertical ? + child.DesiredSize.Height : + child.DesiredSize.Width; + } + + panel.PixelOffset = pixelOffset; } } } @@ -402,6 +416,20 @@ namespace Avalonia.Controls.Presenters var panel = VirtualizingPanel; var generator = Owner.ItemContainerGenerator; var selector = Owner.MemberSelector; + + //validate delta it should never overflow last index or generate index < 0 + if (delta > 0) + { + if ((FirstIndex + delta + panel.Children.Count) > ItemCount) + { + delta = ItemCount - FirstIndex - panel.Children.Count; + } + } + else if ((FirstIndex + delta) < 0) + { + delta = -FirstIndex; + } + var sign = delta < 0 ? -1 : 1; var count = Math.Min(Math.Abs(delta), panel.Children.Count); var move = count < panel.Children.Count; From 5c74c49a0a6dc781044f75eda02ede64bcd61d35 Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Mon, 15 Oct 2018 22:37:10 +0300 Subject: [PATCH 3/3] pr notes --- src/Avalonia.Base/Utilities/MathUtilities.cs | 24 ++++++++++++++++++- .../Presenters/ItemVirtualizerSimple.cs | 12 +--------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/Avalonia.Base/Utilities/MathUtilities.cs b/src/Avalonia.Base/Utilities/MathUtilities.cs index ab21e5fda0..dcb3ef4a2b 100644 --- a/src/Avalonia.Base/Utilities/MathUtilities.cs +++ b/src/Avalonia.Base/Utilities/MathUtilities.cs @@ -1,7 +1,6 @@ // 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. - namespace Avalonia.Utilities { /// @@ -31,5 +30,28 @@ namespace Avalonia.Utilities return val; } } + + /// + /// Clamps a value between a minimum and maximum value. + /// + /// The value. + /// The minimum value. + /// The maximum value. + /// The clamped value. + public static int Clamp(int val, int min, int max) + { + if (val < min) + { + return min; + } + else if (val > max) + { + return max; + } + else + { + return val; + } + } } } diff --git a/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs b/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs index 374066dfad..b177dc50cf 100644 --- a/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs +++ b/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs @@ -418,17 +418,7 @@ namespace Avalonia.Controls.Presenters var selector = Owner.MemberSelector; //validate delta it should never overflow last index or generate index < 0 - if (delta > 0) - { - if ((FirstIndex + delta + panel.Children.Count) > ItemCount) - { - delta = ItemCount - FirstIndex - panel.Children.Count; - } - } - else if ((FirstIndex + delta) < 0) - { - delta = -FirstIndex; - } + delta = MathUtilities.Clamp(delta, -FirstIndex, ItemCount - FirstIndex - panel.Children.Count); var sign = delta < 0 ? -1 : 1; var count = Math.Min(Math.Abs(delta), panel.Children.Count);