More work on virtualization with INCC.

Got removes working a bit better.

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;
         }
     }
 }

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<string>)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<string>)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<string>)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<string>)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<string>)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<string>)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<string>)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<string>(
-                Enumerable.Range(0, itemCount).Select(x => $"Item {x}"));
+            var itemsSource = Enumerable.Range(0, itemCount).Select(x => $"Item {x}");
+            var items = useAvaloniaList ?
+                (IEnumerable)new AvaloniaList<string>(itemsSource) :
+                (IEnumerable)new ObservableCollection<string>(itemsSource);
 
             var scroller = new ScrollContentPresenter
             {