From be8d4516b9af7e20e9587413f245b2a070d4536d Mon Sep 17 00:00:00 2001 From: Andrey Kunchev Date: Thu, 11 Oct 2018 10:11:42 +0300 Subject: [PATCH 1/8] 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/8] 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/8] 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); From 9be4c05e31718e72294b7bde9c6077cb01d35a64 Mon Sep 17 00:00:00 2001 From: wieslawsoltes Date: Wed, 24 Oct 2018 07:03:45 +0000 Subject: [PATCH 4/8] Use theme resources --- src/Avalonia.Themes.Default/ContextMenu.xaml | 2 +- src/Avalonia.Themes.Default/Expander.xaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Themes.Default/ContextMenu.xaml b/src/Avalonia.Themes.Default/ContextMenu.xaml index 41d53aa6b3..61392a869b 100644 --- a/src/Avalonia.Themes.Default/ContextMenu.xaml +++ b/src/Avalonia.Themes.Default/ContextMenu.xaml @@ -1,5 +1,5 @@ diff --git a/src/Avalonia.Themes.Default/Button.xaml b/src/Avalonia.Themes.Default/Button.xaml index 94fcce5e0c..698ddec2a8 100644 --- a/src/Avalonia.Themes.Default/Button.xaml +++ b/src/Avalonia.Themes.Default/Button.xaml @@ -1,7 +1,7 @@ diff --git a/src/Avalonia.Themes.Default/CalendarDayButton.xaml b/src/Avalonia.Themes.Default/CalendarDayButton.xaml index 4a971ef0cb..2d79e62a75 100644 --- a/src/Avalonia.Themes.Default/CalendarDayButton.xaml +++ b/src/Avalonia.Themes.Default/CalendarDayButton.xaml @@ -100,10 +100,10 @@ diff --git a/src/Avalonia.Themes.Default/DatePicker.xaml b/src/Avalonia.Themes.Default/DatePicker.xaml index 93bafdb56f..3fb760d35b 100644 --- a/src/Avalonia.Themes.Default/DatePicker.xaml +++ b/src/Avalonia.Themes.Default/DatePicker.xaml @@ -29,7 +29,7 @@ HorizontalAlignment="Center" VerticalAlignment="Center" Margin="0" - Background="{DynamicResource ThemeControlLightBrush}" + Background="{DynamicResource ThemeControlLowBrush}" ColumnDefinitions="*,*,*,*" RowDefinitions="23*,19*,19*,19*" ClipToBounds="False"> @@ -47,12 +47,12 @@ Grid.Row="1" Grid.RowSpan="3" BorderThickness="1" - BorderBrush="{DynamicResource ThemeBorderDarkBrush}" + BorderBrush="{DynamicResource ThemeBorderHighBrush}" CornerRadius=".5" /> + Foreground="{DynamicResource ThemeBorderHighBrush}"> - + @@ -123,7 +123,7 @@ diff --git a/src/Avalonia.Themes.Default/DropDown.xaml b/src/Avalonia.Themes.Default/DropDown.xaml index c57d961f4b..451f1c2f23 100644 --- a/src/Avalonia.Themes.Default/DropDown.xaml +++ b/src/Avalonia.Themes.Default/DropDown.xaml @@ -52,6 +52,6 @@ \ No newline at end of file diff --git a/src/Avalonia.Themes.Default/Expander.xaml b/src/Avalonia.Themes.Default/Expander.xaml index b34189bac0..d63f785e77 100644 --- a/src/Avalonia.Themes.Default/Expander.xaml +++ b/src/Avalonia.Themes.Default/Expander.xaml @@ -108,7 +108,7 @@