diff --git a/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs b/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs index 5f32f5d10c..4d0cf4ac3b 100644 --- a/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs +++ b/src/Avalonia.Controls/Presenters/ItemVirtualizerSimple.cs @@ -47,18 +47,14 @@ namespace Avalonia.Controls.Presenters if (delta != 0) { - RecycleMoveContainers(delta); - FirstIndex += delta; - NextIndex += delta; + RecycleContainersForMove(delta); } } else { // We're moving to a partially obscured item at the end of the list. var firstIndex = ItemCount - panel.Children.Count; - RecycleMoveContainers(firstIndex - FirstIndex); - NextIndex = ItemCount; - FirstIndex = NextIndex - panel.Children.Count; + RecycleContainersForMove(firstIndex - FirstIndex); panel.PixelOffset = VirtualizingPanel.PixelOverflow; } } @@ -77,7 +73,7 @@ namespace Avalonia.Controls.Presenters public override void Arranging(Size finalSize) { - CreateRemoveContainers(); + CreateAndRemoveContainers(); ((ILogicalScrollable)Owner).InvalidateScroll(); } @@ -85,73 +81,44 @@ namespace Avalonia.Controls.Presenters { base.ItemsChanged(items, e); - switch (e.Action) + if (items != null) { - case NotifyCollectionChangedAction.Remove: - if(e.OldStartingIndex >= FirstIndex && - e.OldStartingIndex + e.OldItems.Count <= NextIndex) - { - if (e.OldStartingIndex == FirstIndex) + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + if (e.NewStartingIndex >= FirstIndex && + e.NewStartingIndex + e.NewItems.Count <= NextIndex) { - // We are removing the first in the list. - VirtualizingPanel.Children.RemoveAt(e.OldStartingIndex - FirstIndex); - Owner.ItemContainerGenerator.Dematerialize(e.OldStartingIndex - FirstIndex, 1); - FirstIndex++; // This may not be necessary, but cant get to work without this. - - // If all items are visible we need to reduce the NextIndex too. - if(NextIndex > ItemCount) - { - NextIndex = ItemCount; - } - - CreateRemoveContainers(); + CreateAndRemoveContainers(); RecycleContainers(); } - else if (e.OldStartingIndex + e.OldItems.Count == NextIndex) - { - // We are removing the last one in the list. - VirtualizingPanel.Children.RemoveAt(e.OldStartingIndex - FirstIndex); - Owner.ItemContainerGenerator.Dematerialize(e.OldStartingIndex - FirstIndex, 1); - NextIndex--; - } - else - { - // If all items are visible we need to reduce the NextIndex too. - if (NextIndex > ItemCount) - { - NextIndex = ItemCount; - } - CreateRemoveContainers(); - RecycleContainers(); - } - } - break; + break; - case NotifyCollectionChangedAction.Add: - if (e.NewStartingIndex >= FirstIndex && - e.NewStartingIndex + e.NewItems.Count < NextIndex) - { - CreateRemoveContainers(); - RecycleContainers(); - } + case NotifyCollectionChangedAction.Remove: + if (e.OldStartingIndex >= FirstIndex && + e.OldStartingIndex + e.OldItems.Count <= NextIndex) + { + RecycleContainersOnRemove(); + } - break; + break; - case NotifyCollectionChangedAction.Reset: - // We could recycle items here if this proves to be inefficient, but - // Reset indicates a large change and should (?) be quite rare. - VirtualizingPanel.Children.Clear(); - Owner.ItemContainerGenerator.Clear(); - FirstIndex = NextIndex = 0; - CreateRemoveContainers(); - break; + case NotifyCollectionChangedAction.Reset: + RecycleContainersOnRemove(); + break; + } + } + else + { + Owner.ItemContainerGenerator.Clear(); + VirtualizingPanel.Children.Clear(); } ((ILogicalScrollable)Owner).InvalidateScroll(); } - private void CreateRemoveContainers() + private void CreateAndRemoveContainers() { var generator = Owner.ItemContainerGenerator; var panel = VirtualizingPanel; @@ -164,8 +131,12 @@ namespace Avalonia.Controls.Presenters while (!panel.IsFull) { - if (index == ItemCount) + if (index >= ItemCount) { + // We can fit more containers in the panel, but we're at the end of the + // items. If we're scrolled to the top (FirstIndex == 0), then there are + // no more items to create. Otherwise, go backwards adding containers to + // the beginning of the panel. if (FirstIndex == 0) { break; @@ -204,13 +175,7 @@ namespace Avalonia.Controls.Presenters if (panel.OverflowCount > 0) { - var count = panel.OverflowCount; - var index = panel.Children.Count - count; - - panel.Children.RemoveRange(index, count); - generator.Dematerialize(FirstIndex + index, count); - - NextIndex -= count; + RemoveContainers(panel.OverflowCount); } } @@ -224,29 +189,21 @@ namespace Avalonia.Controls.Presenters foreach (var container in containers) { - if (itemIndex < ItemCount) - { - var item = Items.ElementAt(itemIndex); + var item = Items.ElementAt(itemIndex); - if (!object.Equals(container.Item, item)) + if (!object.Equals(container.Item, item)) + { + if (!generator.TryRecycle(itemIndex, itemIndex, item, selector)) { - if (!generator.TryRecycle(itemIndex, itemIndex, item, selector)) - { - throw new NotImplementedException(); - } + throw new NotImplementedException(); } } - else - { - panel.Children.RemoveAt(panel.Children.Count - 1); - } ++itemIndex; - } } - private void RecycleMoveContainers(int delta) + private void RecycleContainersForMove(int delta) { var panel = VirtualizingPanel; var generator = Owner.ItemContainerGenerator; @@ -283,6 +240,52 @@ namespace Avalonia.Controls.Presenters panel.Children.InsertRange(0, containers); } } + + FirstIndex += delta; + NextIndex += delta; + } + + private void RecycleContainersOnRemove() + { + var panel = VirtualizingPanel; + + if (NextIndex <= ItemCount) + { + // Items have been removed but FirstIndex..NextIndex is still a valid range in the + // items, so just recycle the containers to adapt to the new state. + RecycleContainers(); + } + else + { + // Items have been removed and now the range FirstIndex..NextIndex goes out of + // the item bounds. Try to scroll up and then remove any excess containers. + var newFirstIndex = Math.Max(0, FirstIndex - (NextIndex - ItemCount)); + var delta = newFirstIndex - FirstIndex; + var newNextIndex = NextIndex + delta; + + if (newNextIndex > ItemCount) + { + RemoveContainers(newNextIndex - ItemCount); + } + + if (delta != 0) + { + RecycleContainersForMove(delta); + } + else + { + RecycleContainers(); + } + } + } + + private void RemoveContainers(int count) + { + var index = VirtualizingPanel.Children.Count - count; + + VirtualizingPanel.Children.RemoveRange(index, count); + Owner.ItemContainerGenerator.Dematerialize(FirstIndex + index, count); + NextIndex -= count; } } } diff --git a/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs b/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs index 36d43cd6fe..f409865ada 100644 --- a/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs +++ b/tests/Avalonia.Controls.UnitTests/Presenters/ItemsPresenterTests_Virtualization_Simple.cs @@ -1,9 +1,11 @@ // 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.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; +using Avalonia.Collections; using Avalonia.Controls.Generators; using Avalonia.Controls.Presenters; using Avalonia.Controls.Primitives; @@ -76,7 +78,7 @@ namespace Avalonia.Controls.UnitTests.Presenters } [Fact] - public void Should_Add_New_Items_At_Top_When_Control_Is_Scrolled_To_Bottom_And_Enlarged() + public void Should_Add_New_Containers_At_Top_When_Control_Is_Scrolled_To_Bottom_And_Enlarged() { var target = CreateTarget(); var items = (IList)target.Items; @@ -161,107 +163,140 @@ namespace Avalonia.Controls.UnitTests.Presenters var target = CreateTarget(itemCount: 20); target.ApplyTemplate(); - target.Measure(new Size(100, 95)); - target.Arrange(new Rect(0, 0, 100, 95)); + target.Measure(new Size(100, 100)); + target.Arrange(new Rect(0, 0, 100, 100)); ((ILogicalScrollable)target).Offset = new Vector(0, 5); var expected = Enumerable.Range(5, 10).Select(x => $"Item {x}").ToList(); var items = (ObservableCollection)target.Items; + var actual = target.Panel.Children.Select(x => x.DataContext).ToList(); - Assert.Equal( - expected, - target.Panel.Children.Select(x => x.DataContext)); + Assert.Equal(expected, actual); items.Insert(6, "Inserted"); expected.Insert(1, "Inserted"); expected.RemoveAt(expected.Count - 1); - var actual = target.Panel.Children.Select(x => x.DataContext).ToList(); + actual = target.Panel.Children.Select(x => x.DataContext).ToList(); Assert.Equal(expected, actual); } [Fact] - public void Removing_First_Item_When_Visible_Should_UpdateContainers() + public void Removing_First_Materialized_Item_Should_Update_Containers() { var target = CreateTarget(itemCount: 20); target.ApplyTemplate(); - target.Measure(new Size(100, 195)); - target.Arrange(new Rect(0, 0, 100, 195)); - - ((ILogicalScrollable)target).Offset = new Vector(0, 5); + target.Measure(new Size(100, 100)); + target.Arrange(new Rect(0, 0, 100, 100)); - var expected = Enumerable.Range(0, 20).Select(x => $"Item {x}").ToList(); + var expected = Enumerable.Range(0, 10).Select(x => $"Item {x}").ToList(); var items = (ObservableCollection)target.Items; + var actual = target.Panel.Children.Select(x => x.DataContext).ToList(); - Assert.Equal( - expected, - target.Panel.Children.Select(x => x.DataContext)); + Assert.Equal(expected, actual); - items.Remove(items.First()); - expected.Remove(expected.First()); + items.RemoveAt(0); + expected = Enumerable.Range(1, 10).Select(x => $"Item {x}").ToList(); - var actual = target.Panel.Children.Select(x => x.DataContext).ToList(); + actual = target.Panel.Children.Select(x => x.DataContext).ToList(); Assert.Equal(expected, actual); } [Fact] - public void Removing_Items_From_Middle_Should_Update_Containers() + public void Removing_Items_From_Middle_Should_Update_Containers_When_All_Items_Visible() { var target = CreateTarget(itemCount: 20); target.ApplyTemplate(); - target.Measure(new Size(100, 195)); - target.Arrange(new Rect(0, 0, 100, 195)); - - ((ILogicalScrollable)target).Offset = new Vector(0, 5); + target.Measure(new Size(100, 200)); + target.Arrange(new Rect(0, 0, 100, 200)); - var expected = Enumerable.Range(0, 20).Select(x => $"Item {x}").ToList(); var items = (ObservableCollection)target.Items; + var actual = target.Panel.Children.Select(x => x.DataContext).ToList(); - Assert.Equal( - expected, - target.Panel.Children.Select(x => x.DataContext)); + Assert.Equal(items, actual); items.RemoveAt(2); - expected.RemoveAt(2); - var actual = target.Panel.Children.Select(x => x.DataContext).ToList(); - Assert.Equal(expected, actual); + actual = target.Panel.Children.Select(x => x.DataContext).ToList(); + Assert.Equal(items, actual); items.RemoveAt(items.Count - 2); - expected.RemoveAt(expected.Count -2); actual = target.Panel.Children.Select(x => x.DataContext).ToList(); - Assert.Equal(expected, actual); + Assert.Equal(items, actual); } [Fact] - public void Removing_Last_Item_When_Visible_Should_UpdateContainers() + public void Removing_Last_Item_Should_Update_Containers_When_All_Items_Visible() { var target = CreateTarget(itemCount: 20); target.ApplyTemplate(); - target.Measure(new Size(100, 195)); - target.Arrange(new Rect(0, 0, 100, 195)); + target.Measure(new Size(100, 200)); + target.Arrange(new Rect(0, 0, 100, 200)); ((ILogicalScrollable)target).Offset = new Vector(0, 5); var expected = Enumerable.Range(0, 20).Select(x => $"Item {x}").ToList(); var items = (ObservableCollection)target.Items; + var actual = target.Panel.Children.Select(x => x.DataContext).ToList(); - Assert.Equal( - expected, - target.Panel.Children.Select(x => x.DataContext)); + Assert.Equal(expected, actual); items.Remove(items.Last()); expected.Remove(expected.Last()); + actual = target.Panel.Children.Select(x => x.DataContext).ToList(); + Assert.Equal(expected, actual); + } + + [Fact] + public void Removing_Items_When_Scrolled_To_End_Should_Add_Containers_At_Top() + { + var target = CreateTarget(itemCount: 20, useAvaloniaList: true); + + target.ApplyTemplate(); + target.Measure(new Size(100, 100)); + target.Arrange(new Rect(0, 0, 100, 100)); + + ((ILogicalScrollable)target).Offset = new Vector(0, 10); + + var expected = Enumerable.Range(10, 10).Select(x => $"Item {x}").ToList(); + var items = (AvaloniaList)target.Items; var actual = target.Panel.Children.Select(x => x.DataContext).ToList(); + + Assert.Equal(expected, actual); + + items.RemoveRange(18, 2); + expected = Enumerable.Range(8, 10).Select(x => $"Item {x}").ToList(); + + actual = target.Panel.Children.Select(x => x.DataContext).ToList(); Assert.Equal(expected, actual); } + [Fact] + public void Setting_Items_To_Null_Should_Remove_Containers() + { + var target = CreateTarget(itemCount: 20); + + target.ApplyTemplate(); + target.Measure(new Size(100, 100)); + target.Arrange(new Rect(0, 0, 100, 100)); + + var expected = Enumerable.Range(0, 10).Select(x => $"Item {x}").ToList(); + var items = (ObservableCollection)target.Items; + var actual = target.Panel.Children.Select(x => x.DataContext).ToList(); + + Assert.Equal(expected, actual); + + target.Items = null; + + Assert.Empty(target.Panel.Children); + } + public class WithContainers { [Fact] @@ -342,12 +377,14 @@ namespace Avalonia.Controls.UnitTests.Presenters private static ItemsPresenter CreateTarget( Orientation orientation = Orientation.Vertical, bool useContainers = true, - int itemCount = 20) + int itemCount = 20, + bool useAvaloniaList = false) { ItemsPresenter result; - - var items = new ObservableCollection( - Enumerable.Range(0, itemCount).Select(x => $"Item {x}")); + var itemsSource = Enumerable.Range(0, itemCount).Select(x => $"Item {x}"); + var items = useAvaloniaList ? + (IEnumerable)new AvaloniaList(itemsSource) : + (IEnumerable)new ObservableCollection(itemsSource); var scroller = new ScrollContentPresenter {