Correctly handle control items being removed.

src/Avalonia.Controls/VirtualizingPanel.cs

@@ -154,6 +154,17 @@ namespace Avalonia.Controls
             Children.Insert(index, control);
         }
 
+        /// <summary>
+        /// Removes a child element from the <see cref="Panel.Children"/> collection.
+        /// </summary>
+        /// <param name="child">The child to remove/</param>
+        protected void RemoveInternalChild(Control child)
+        {
+            var itemsControl = EnsureItemsControl();
+            itemsControl.RemoveLogicalChild(child);
+            Children.Remove(child);
+        }
+
         /// <summary>
         /// Removes child elements from the <see cref="Panel.Children"/> collection.
         /// </summary>

src/Avalonia.Controls/VirtualizingStackPanel.cs

@@ -27,6 +27,7 @@ namespace Avalonia.Controls
 
         private static readonly Rect s_invalidViewport = new(double.PositiveInfinity, double.PositiveInfinity, 0, 0);
         private readonly Action<Control> _recycleElement;
+        private readonly Action<Control> _recycleElementOnItemRemoved;
         private readonly Action<Control, int, int> _updateElementIndex;
         private int _anchorIndex = -1;
         private Control? _anchorElement;
@@ -41,6 +42,7 @@ namespace Avalonia.Controls
         public VirtualizingStackPanel()
         {
             _recycleElement = RecycleElement;
+            _recycleElementOnItemRemoved = RecycleElementOnItemRemoved;
             _updateElementIndex = UpdateElementIndex;
             EffectiveViewportChanged += OnEffectiveViewportChanged;
         }
@@ -68,13 +70,6 @@ namespace Avalonia.Controls
             try
             {
                 var items = Items;
-
-                if (items.Count == 0)
-                {
-                    RemoveInternalChildRange(0, Children.Count);
-                    return default;
-                }
-
                 var orientation = Orientation;
 
                 _realizedElements ??= new();
@@ -153,6 +148,8 @@ namespace Avalonia.Controls
 
         protected override void OnItemsChanged(IReadOnlyList<object?> items, NotifyCollectionChangedEventArgs e)
         {
+            InvalidateMeasure();
+
             if (_realizedElements is null)
                 return;
 
@@ -162,14 +159,12 @@ namespace Avalonia.Controls
                     _realizedElements.ItemsInserted(e.NewStartingIndex, e.NewItems!.Count, _updateElementIndex);
                     break;
                 case NotifyCollectionChangedAction.Remove:
-                    _realizedElements.ItemsRemoved(e.OldStartingIndex, e.OldItems!.Count, _updateElementIndex, _recycleElement);
+                    _realizedElements.ItemsRemoved(e.OldStartingIndex, e.OldItems!.Count, _updateElementIndex, _recycleElementOnItemRemoved);
                     break;
                 case NotifyCollectionChangedAction.Reset:
-                    _realizedElements.RecycleAllElements(_recycleElement);
+                    _realizedElements.RecycleAllElements(_recycleElementOnItemRemoved);
                     break;
             }
-
-            InvalidateMeasure();
         }
 
         protected override IInputElement? GetControl(NavigationDirection direction, IInputElement? from, bool wrap)
@@ -314,6 +309,7 @@ namespace Avalonia.Controls
             var (lastIndex, _) = _realizedElements.GetIndexAt(viewportEnd);
             var estimatedElementSize = -1.0;
             var itemCount = items?.Count ?? 0;
+            var maxIndex = Math.Max(itemCount - 1, 0);
 
             if (firstIndex == -1)
             {
@@ -331,8 +327,8 @@ namespace Avalonia.Controls
 
             return new MeasureViewport
             {
-                firstIndex = MathUtilities.Clamp(firstIndex, 0, itemCount - 1),
-                lastIndex = MathUtilities.Clamp(lastIndex, 0, itemCount - 1),
+                firstIndex = MathUtilities.Clamp(firstIndex, 0, maxIndex),
+                lastIndex = MathUtilities.Clamp(lastIndex, 0, maxIndex),
                 viewportUStart = viewportStart,
                 viewportUEnd = viewportEnd,
                 startU = firstIndexU,
@@ -468,7 +464,6 @@ namespace Avalonia.Controls
             var generator = ItemContainerGenerator!;
             var item = items[index];
 
-
             if (_recyclePool?.Count > 0)
             {
                 var recycled = _recyclePool.Pop();
@@ -519,6 +514,24 @@ namespace Avalonia.Controls
             }
         }
 
+        private void RecycleElementOnItemRemoved(Control element)
+        {
+            Debug.Assert(ItemContainerGenerator is not null);
+
+            if (element.IsSet(ItemIsOwnContainerProperty))
+            {
+                RemoveInternalChild(element);
+            }
+            else
+            {
+                ItemContainerGenerator!.ClearItemContainer(element);
+                _recyclePool ??= new();
+                _recyclePool.Push(element);
+                element.IsVisible = false;
+                element.GetObservable(Visual.VisualParentProperty).Subscribe(x => { });
+            }
+        }
+
         private void UpdateElementIndex(Control element, int oldIndex, int newIndex)
         {
             Debug.Assert(ItemContainerGenerator is not null);

tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs

@@ -12,6 +12,8 @@ using Avalonia.UnitTests;
 using Avalonia.VisualTree;
 using Xunit;
 
+#nullable enable
+
 namespace Avalonia.Controls.UnitTests
 {
     public class VirtualizingStackPanelTests
@@ -20,7 +22,6 @@ namespace Avalonia.Controls.UnitTests
         public void Creates_Initial_Items()
         {
             using var app = App();
-
             var (target, scroll, itemsControl) = CreateTarget();
 
             Assert.Equal(1000, scroll.Extent.Height);
@@ -29,10 +30,35 @@ namespace Avalonia.Controls.UnitTests
         }
 
         [Fact]
-        public void Scrolls_Down_One_Item()
+        public void Initializes_Initial_Control_Items()
+        {
+            using var app = App();
+            var items = Enumerable.Range(0, 100).Select(x => new Button { Width = 25, Height = 10});
+            var (target, scroll, itemsControl) = CreateTarget(items: items, useItemTemplate: false);
+
+            Assert.Equal(1000, scroll.Extent.Height);
+
+            AssertRealizedControlItems<Button>(target, itemsControl, 0, 10);
+        }
+
+        [Fact]
+        public void Creates_Reassigned_Items()
         {
             using var app = App();
+            var (target, scroll, itemsControl) = CreateTarget(items: Array.Empty<object>());
+
+            Assert.Empty(itemsControl.GetRealizedContainers());
 
+            itemsControl.Items = new[] { "foo", "bar" };
+            Layout(target);
+
+            AssertRealizedItems(target, itemsControl, 0, 2);
+        }
+
+        [Fact]
+        public void Scrolls_Down_One_Item()
+        {
+            using var app = App();
             var (target, scroll, itemsControl) = CreateTarget();
 
             scroll.Offset = new Vector(0, 10);
@@ -45,7 +71,6 @@ namespace Avalonia.Controls.UnitTests
         public void Scrolls_Down_More_Than_A_Page()
         {
             using var app = App();
-
             var (target, scroll, itemsControl) = CreateTarget();
 
             scroll.Offset = new Vector(0, 200);
@@ -55,12 +80,11 @@ namespace Avalonia.Controls.UnitTests
         }
 
         [Fact]
-        public void Creates_Elements_For_Inserted_Item()
+        public void Creates_Elements_On_Item_Insert()
         {
             using var app = App();
-
             var (target, _, itemsControl) = CreateTarget();
-            var items = (IList)itemsControl.Items;
+            var items = (IList)itemsControl.Items!;
 
             Assert.Equal(10, target.GetRealizedElements().Count);
 
@@ -88,12 +112,11 @@ namespace Avalonia.Controls.UnitTests
         }
 
         [Fact]
-        public void Updates_Elements_On_Removed_Row()
+        public void Updates_Elements_On_Item_Remove()
         {
             using var app = App();
-
             var (target, _, itemsControl) = CreateTarget();
-            var items = (IList)itemsControl.Items;
+            var items = (IList)itemsControl.Items!;
 
             Assert.Equal(10, target.GetRealizedElements().Count);
 
@@ -118,10 +141,26 @@ namespace Avalonia.Controls.UnitTests
             Assert.Equal(elements, target.GetRealizedElements());
         }
 
+        [Fact]
+        public void Removes_Control_Items_From_Panel_On_Item_Remove()
+        {
+            using var app = App();
+            var items = new ObservableCollection<Button>(Enumerable.Range(0, 100).Select(x => new Button { Width = 25, Height = 10 }));
+            var (target, scroll, itemsControl) = CreateTarget(items: items, useItemTemplate: false);
+
+            Assert.Equal(1000, scroll.Extent.Height);
+
+            var removed = items[1];
+            items.RemoveAt(1);
+
+            Assert.Null(removed.Parent);
+            Assert.Null(removed.VisualParent);
+        }
+
         private static IReadOnlyList<int> GetRealizedIndexes(VirtualizingStackPanel target, ItemsControl itemsControl)
         {
             return target.GetRealizedElements()
-                .Select(x => x is null ? -1 : itemsControl.ItemContainerGenerator.IndexFromContainer((Control)x))
+                .Select(x => x is null ? -1 : itemsControl.IndexFromContainer((Control)x))
                 .ToList();
         }
 
@@ -131,19 +170,43 @@ namespace Avalonia.Controls.UnitTests
             int firstIndex,
             int count)
         {
-            var childIndexes = target.GetLogicalChildren()
-                .Select(x => itemsControl.ItemContainerGenerator.IndexFromContainer((Control)x))
+            Assert.All(target.GetRealizedContainers(), x => Assert.Same(target, x.VisualParent));
+            Assert.All(target.GetRealizedContainers(), x => Assert.Same(itemsControl, x.Parent));
+
+            var childIndexes = target.GetRealizedContainers()?
+                .Select(x => itemsControl.IndexFromContainer(x))
+                .Where(x => x >= 0)
+                .OrderBy(x => x)
+                .ToList();
+            Assert.Equal(Enumerable.Range(firstIndex, count), childIndexes);
+        }
+
+        private static void AssertRealizedControlItems<TContainer>(
+            VirtualizingStackPanel target,
+            ItemsControl itemsControl,
+            int firstIndex,
+            int count)
+        {
+            Assert.All(target.GetRealizedContainers(), x => Assert.IsType<TContainer>(x));
+            Assert.All(target.GetRealizedContainers(), x => Assert.Same(target, x.VisualParent));
+            Assert.All(target.GetRealizedContainers(), x => Assert.Same(itemsControl, x.Parent));
+
+            var childIndexes = target.GetRealizedContainers()?
+                .Select(x => itemsControl.IndexFromContainer(x))
                 .Where(x => x >= 0)
                 .OrderBy(x => x)
                 .ToList();
             Assert.Equal(Enumerable.Range(firstIndex, count), childIndexes);
         }
 
-        private static (VirtualizingStackPanel, ScrollViewer, ItemsControl) CreateTarget(int itemCount = 100)
+        private static (VirtualizingStackPanel, ScrollViewer, ItemsControl) CreateTarget(
+            IEnumerable<object>? items = null,
+            bool useItemTemplate = true)
         {
-            var items = new ObservableCollection<string>(Enumerable.Range(0, itemCount).Select(x => $"Item {x}"));
             var target = new VirtualizingStackPanel();
-            
+
+            items ??= new ObservableCollection<string>(Enumerable.Range(0, 100).Select(x => $"Item {x}"));
+
             var presenter = new ItemsPresenter
             {
                 [~ItemsPresenter.ItemsPanelProperty] = new TemplateBinding(ItemsPresenter.ItemsPanelProperty),
@@ -160,9 +223,11 @@ namespace Avalonia.Controls.UnitTests
                 Items = items,
                 Template = new FuncControlTemplate<ItemsControl>((_, _) => scroll),
                 ItemsPanel = new FuncTemplate<Panel>(() => target),
-                ItemTemplate = new FuncDataTemplate<string>((x, _) => new Canvas { Width = 100, Height = 10 }),
             };
 
+            if (useItemTemplate)
+                itemsControl.ItemTemplate = new FuncDataTemplate<object>((x, _) => new Canvas { Width = 100, Height = 10 });
+
             var root = new TestRoot(true, itemsControl);
             root.ClientSize = new(100, 100);
             root.LayoutManager.ExecuteInitialLayoutPass();