Make VirtualizingStackPanel better handle container size changes (#16168)

* Add a failing test for #15712.

* Validate StartU at the start of a measure pass.

If any container U size has changed since the last layout pass then `StartU` must be considered unstable as the average container height will have changed.

* Correctly position focused element.

If the focused element has been moved outside the visible viewport due to a realized container size change, then we need to ensure it's positioned correctly.

* We can skip check if StartU is already unstable.

* Don't invalidate virt. panels more than necessary.

* Add another virt panel test.

And revert the expected results for another test to the way they were at the beginning of this PR.

* Tweak container size estimation.

Use the desired size of _measured_ containers instead of the bounds: a layout pass may not had completed on the containers yet, so the bounds may not be up-to-date. Was easier to move the estimation methods out of `RealizedStackElements` and into `VirtualizingStackPanel` itself in order to do this, and arguably makes more sense.

src/Avalonia.Controls/Utils/RealizedStackElements.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using Avalonia.Layout;
 using Avalonia.Utilities;
 
 namespace Avalonia.Controls.Utils
@@ -42,16 +43,17 @@ namespace Avalonia.Controls.Utils
         public IReadOnlyList<double> SizeU => _sizes ??= new List<double>();
 
         /// <summary>
-        /// Gets the position of the first element on the primary axis.
+        /// Gets the position of the first element on the primary axis, or NaN if the position is
+        /// unstable.
         /// </summary>
-        public double StartU => _startU;
+        public double StartU => _startUUnstable ? double.NaN : _startU;
 
         /// <summary>
         /// Adds a newly realized element to the collection.
         /// </summary>
         /// <param name="index">The index of the element.</param>
         /// <param name="element">The element.</param>
-        /// <param name="u">The position of the elemnt on the primary axis.</param>
+        /// <param name="u">The position of the element on the primary axis.</param>
         /// <param name="sizeU">The size of the element on the primary axis.</param>
         public void Add(int index, Control element, double u, double sizeU)
         {
@@ -99,76 +101,6 @@ namespace Avalonia.Controls.Utils
             return null;
         }
 
-        /// <summary>
-        /// Gets or estimates the index and start U position of the anchor element for the
-        /// specified viewport.
-        /// </summary>
-        /// <param name="viewportStartU">The U position of the start of the viewport.</param>
-        /// <param name="viewportEndU">The U position of the end of the viewport.</param>
-        /// <param name="itemCount">The number of items in the list.</param>
-        /// <param name="estimatedElementSizeU">The current estimated element size.</param>
-        /// <returns>
-        /// A tuple containing:
-        /// - The index of the anchor element, or -1 if an anchor could not be determined
-        /// - The U position of the start of the anchor element, if determined
-        /// </returns>
-        /// <remarks>
-        /// This method tries to find an existing element in the specified viewport from which
-        /// element realization can start. Failing that it estimates the first element in the
-        /// viewport.
-        /// </remarks>
-        public (int index, double position) GetOrEstimateAnchorElementForViewport(
-            double viewportStartU,
-            double viewportEndU,
-            int itemCount,
-            ref double estimatedElementSizeU)
-        {
-            // We have no elements, nothing to do here.
-            if (itemCount <= 0)
-                return (-1, 0);
-
-            // If we're at 0 then display the first item.
-            if (MathUtilities.IsZero(viewportStartU))
-                return (0, 0);
-
-            if (_sizes is not null && !_startUUnstable)
-            {
-                var u = _startU;
-
-                for (var i = 0; i < _sizes.Count; ++i)
-                {
-                    var size = _sizes[i];
-
-                    if (double.IsNaN(size))
-                        break;
-
-                    var endU = u + size;
-
-                    if (endU > viewportStartU && u < viewportEndU)
-                        return (FirstIndex + i, u);
-
-                    u = endU;
-                }
-            }
-
-            // We don't have any realized elements in the requested viewport, or can't rely on
-            // StartU being valid. Estimate the index using only the estimated size. First,
-            // estimate the element size, using defaultElementSizeU if we don't have any realized
-            // elements.
-            var estimatedSize = EstimateElementSizeU() switch
-            {
-                -1 => estimatedElementSizeU,
-                double v => v,
-            };
-
-            // Store the estimated size for the next layout pass.
-            estimatedElementSizeU = estimatedSize;
-
-            // Estimate the element at the start of the viewport.
-            var index = Math.Min((int)(viewportStartU / estimatedSize), itemCount - 1);
-            return (index, index * estimatedSize);
-        }
-
         /// <summary>
         /// Gets the position of the element with the requested index on the primary axis, if realized.
         /// </summary>
@@ -193,61 +125,6 @@ namespace Avalonia.Controls.Utils
             return u;
         }
 
-        public double GetOrEstimateElementU(int index, ref double estimatedElementSizeU)
-        {
-            // Return the position of the existing element if realized.
-            var u = GetElementU(index);
-
-            if (!double.IsNaN(u))
-                return u;
-
-            // Estimate the element size, using defaultElementSizeU if we don't have any realized
-            // elements.
-            var estimatedSize = EstimateElementSizeU() switch
-            {
-                -1 => estimatedElementSizeU,
-                double v => v,
-            };
-
-            // Store the estimated size for the next layout pass.
-            estimatedElementSizeU = estimatedSize;
-
-            // TODO: Use _startU to work this out.
-            return index * estimatedSize;
-        }
-
-        /// <summary>
-        /// Estimates the average U size of all elements in the source collection based on the
-        /// realized elements.
-        /// </summary>
-        /// <returns>
-        /// The estimated U size of an element, or -1 if not enough information is present to make
-        /// an estimate.
-        /// </returns>
-        public double EstimateElementSizeU()
-        {
-            var total = 0.0;
-            var divisor = 0.0;
-
-            // Average the size of the realized elements.
-            if (_sizes is not null)
-            {
-                foreach (var size in _sizes)
-                {
-                    if (double.IsNaN(size))
-                        continue;
-                    total += size;
-                    ++divisor;
-                }
-            }
-
-            // We don't have any elements on which to base our estimate.
-            if (divisor == 0 || total == 0)
-                return -1;
-
-            return total / divisor;
-        }
-
         /// <summary>
         /// Gets the index of the specified element.
         /// </summary>
@@ -538,6 +415,34 @@ namespace Avalonia.Controls.Utils
             _elements?.Clear();
             _sizes?.Clear();
         }
-    }
 
+        /// <summary>
+        /// Validates that <see cref="StartU"/> is still valid.
+        /// </summary>
+        /// <param name="orientation">The panel orientation.</param>
+        /// <remarks>
+        /// If the U size of any element in the realized elements has changed, then the value of
+        /// <see cref="StartU"/> should be considered unstable.
+        /// </remarks>
+        public void ValidateStartU(Orientation orientation)
+        {
+            if (_elements is null || _sizes is null || _startUUnstable)
+                return;
+
+            for (var i = 0; i < _elements.Count; ++i)
+            {
+                if (_elements[i] is not { } element)
+                    continue;
+
+                var sizeU = orientation == Orientation.Horizontal ?
+                    element.DesiredSize.Width : element.DesiredSize.Height;
+
+                if (sizeU != _sizes[i])
+                {
+                    _startUUnstable = true;
+                    break;
+                }
+            }
+        }
+    }
 }

src/Avalonia.Controls/VirtualizingPanel.cs

@@ -192,6 +192,12 @@ namespace Avalonia.Controls
             Children.RemoveRange(index, count);
         }
 
+        private protected override void InvalidateMeasureOnChildrenChanged()
+        {
+            // Don't invalidate measure when children are added or removed: the panel is responsible
+            // for managing its children.
+        }
+
         internal void Attach(ItemsControl itemsControl)
         {
             if (ItemsControl is not null)

src/Avalonia.Controls/VirtualizingStackPanel.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Diagnostics;
+using System.Drawing;
 using System.Linq;
 using Avalonia.Controls.Primitives;
 using Avalonia.Controls.Utils;
@@ -159,6 +160,7 @@ namespace Avalonia.Controls
 
             try
             {
+                _realizedElements?.ValidateStartU(Orientation);
                 _realizedElements ??= new();
                 _measureElements ??= new();
 
@@ -179,6 +181,10 @@ namespace Avalonia.Controls
                 (_measureElements, _realizedElements) = (_realizedElements, _measureElements);
                 _measureElements.ResetForReuse();
 
+                // If there is a focused element is outside the visible viewport (i.e.
+                // _focusedElement is non-null), ensure it's measured.
+                _focusedElement?.Measure(availableSize);
+
                 return CalculateDesiredSize(orientation, items.Count, viewport);
             }
             finally
@@ -215,6 +221,16 @@ namespace Avalonia.Controls
                     }
                 }
 
+                // Ensure that the focused element is in the correct position.
+                if (_focusedElement is not null && _focusedIndex >= 0)
+                {
+                    u = GetOrEstimateElementU(_focusedIndex);
+                    var rect = orientation == Orientation.Horizontal ?
+                        new Rect(u, 0, _focusedElement.DesiredSize.Width, finalSize.Height) :
+                        new Rect(0, u, finalSize.Width, _focusedElement.DesiredSize.Height);
+                    _focusedElement.Arrange(rect);
+                }
+
                 return finalSize;
             }
             finally
@@ -389,7 +405,7 @@ namespace Avalonia.Controls
                 scrollToElement.Measure(Size.Infinity);
 
                 // Get the expected position of the element and put it in place.
-                var anchorU = _realizedElements.GetOrEstimateElementU(index, ref _lastEstimatedElementSizeU);
+                var anchorU = GetOrEstimateElementU(index);
                 var rect = Orientation == Orientation.Horizontal ?
                     new Rect(anchorU, 0, scrollToElement.DesiredSize.Width, scrollToElement.DesiredSize.Height) :
                     new Rect(0, anchorU, scrollToElement.DesiredSize.Width, scrollToElement.DesiredSize.Height);
@@ -472,11 +488,12 @@ namespace Avalonia.Controls
             }
             else
             {
-                (anchorIndex, anchorU) = _realizedElements.GetOrEstimateAnchorElementForViewport(
+                GetOrEstimateAnchorElementForViewport(
                     viewportStart,
                     viewportEnd,
                     items.Count,
-                    ref _lastEstimatedElementSizeU);
+                    out anchorIndex,
+                    out anchorU);
             }
 
             // Check if the anchor element is not within the currently realized elements.
@@ -531,12 +548,98 @@ namespace Avalonia.Controls
             if (_realizedElements is null)
                 return _lastEstimatedElementSizeU;
 
-            var result = _realizedElements.EstimateElementSizeU();
+            var orientation = Orientation;
-            if (result >= 0)
+            var total = 0.0;
-                _lastEstimatedElementSizeU = result;
+            var divisor = 0.0;
-            return _lastEstimatedElementSizeU;
+
+            // Average the desired size of the realized, measured elements.
+            foreach (var element in _realizedElements.Elements)
+            {
+                if (element is null || !element.IsMeasureValid)
+                    continue;
+                var sizeU = orientation == Orientation.Horizontal ?
+                    element.DesiredSize.Width :
+                    element.DesiredSize.Height;
+                total += sizeU;
+                ++divisor;
+            }
+
+            // Check we have enough information on which to base our estimate.
+            if (divisor == 0 || total == 0)
+                return _lastEstimatedElementSizeU;
+
+            // Store and return the estimate.
+            return _lastEstimatedElementSizeU = total / divisor;
+        }
+
+        private void GetOrEstimateAnchorElementForViewport(
+            double viewportStartU,
+            double viewportEndU,
+            int itemCount,
+            out int index,
+            out double position)
+        {
+            // We have no elements, or we're at the start of the viewport.
+            if (itemCount <= 0 || MathUtilities.IsZero(viewportStartU))
+            {
+                index = 0;
+                position = 0;
+                return;
+            }
+
+            // If we have realised elements and a valid StartU then try to use this information to
+            // get the anchor element.
+            if (_realizedElements?.StartU is { } u && !double.IsNaN(u))
+            {
+                var orientation = Orientation;
+
+                for (var i = 0; i < _realizedElements.Elements.Count; ++i)
+                {
+                    if (_realizedElements.Elements[i] is not { } element)
+                        continue;
+
+                    var sizeU = orientation == Orientation.Horizontal ?
+                        element.DesiredSize.Width :
+                        element.DesiredSize.Height;
+                    var endU = u + sizeU;
+
+                    if (endU > viewportStartU && u < viewportEndU)
+                    {
+                        index = _realizedElements.FirstIndex + i;
+                        position = u;
+                        return;
+                    }
+
+                    u = endU;
+                }
+            }
+
+            // We don't have any realized elements in the requested viewport, or can't rely on
+            // StartU being valid. Estimate the index using only the estimated element size.
+            var estimatedSize = EstimateElementSizeU();
+
+            // Estimate the element at the start of the viewport.
+            var startIndex = Math.Min((int)(viewportStartU / estimatedSize), itemCount - 1);
+            index = startIndex;
+            position = startIndex * estimatedSize;
         }
 
+        private double GetOrEstimateElementU(int index)
+        {
+            // Return the position of the existing element if realized.
+            var u = _realizedElements?.GetElementU(index) ?? double.NaN;
+
+            if (!double.IsNaN(u))
+                return u;
+
+            // Estimate the element size.
+            var estimatedSize = EstimateElementSizeU();
+
+            // TODO: Use _startU to work this out.
+            return index * estimatedSize;
+        }
+
+
         private void RealizeElements(
             IReadOnlyList<object?> items,
             Size availableSize,

tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs

@@ -1192,6 +1192,112 @@ namespace Avalonia.Controls.UnitTests
             AssertRealizedItems(target, itemsControl, 15, 5);
         }
 
+        [Fact]
+        public void Extent_And_Offset_Should_Be_Updated_When_Containers_Resize()
+        {
+            using var app = App();
+
+            // All containers start off with a height of 50 (2 containers fit in viewport).
+            var items = Enumerable.Range(0, 20).Select(x => new ItemWithHeight(x, 50)).ToList();
+            var (target, scroll, itemsControl) = CreateTarget(items: items, itemTemplate: CanvasWithHeightTemplate);
+
+            // Scroll to the 5th item (containers 4 and 5 should be visible).
+            target.ScrollIntoView(5);
+            Assert.Equal(4, target.FirstRealizedIndex);
+            Assert.Equal(5, target.LastRealizedIndex);
+
+            // The extent should be 500 (10 * 50) and the offset should be 200 (4 * 50).
+            var container = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(5));
+            Assert.Equal(new Rect(0, 250, 100, 50), container.Bounds);
+            Assert.Equal(new Size(100, 100), scroll.Viewport);
+            Assert.Equal(new Size(100, 1000), scroll.Extent);
+            Assert.Equal(new Vector(0, 200), scroll.Offset);
+
+            // Update the height of all items to 25 and run a layout pass.
+            foreach (var item in items)
+                item.Height = 25;
+            target.UpdateLayout();
+
+            // The extent should be updated to reflect the new heights. The offset should be
+            // unchanged but the first realized index should be updated to 8 (200 / 25).
+            Assert.Equal(new Size(100, 100), scroll.Viewport);
+            Assert.Equal(new Size(100, 500), scroll.Extent);
+            Assert.Equal(new Vector(0, 200), scroll.Offset);
+            Assert.Equal(8, target.FirstRealizedIndex);
+            Assert.Equal(11, target.LastRealizedIndex);
+        }
+
+        [Fact]
+        public void Focused_Container_Is_Positioned_Correctly_when_Container_Size_Change_Causes_It_To_Be_Moved_Out_Of_Visible_Viewport()
+        {
+            using var app = App();
+
+            // All containers start off with a height of 50 (2 containers fit in viewport).
+            var items = Enumerable.Range(0, 20).Select(x => new ItemWithHeight(x, 50)).ToList();
+            var (target, scroll, itemsControl) = CreateTarget(items: items, itemTemplate: CanvasWithHeightTemplate);
+
+            // Scroll to the 5th item (containers 4 and 5 should be visible).
+            target.ScrollIntoView(5);
+            Assert.Equal(4, target.FirstRealizedIndex);
+            Assert.Equal(5, target.LastRealizedIndex);
+
+            // Focus the 5th item.
+            var container = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(5));
+            container.Focusable = true;
+            container.Focus();
+
+            // Update the height of all items to 25 and run a layout pass.
+            foreach (var item in items)
+                item.Height = 25;
+            target.UpdateLayout();
+
+            // The focused container should now be outside the realized range.
+            Assert.Equal(8, target.FirstRealizedIndex);
+            Assert.Equal(11, target.LastRealizedIndex);
+
+            // The container should still exist and be positioned outside the visible viewport.
+            container = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(5));
+            Assert.Equal(new Rect(0, 125, 100, 25), container.Bounds);
+        }
+
+        [Fact]
+        public void Focused_Container_Is_Positioned_Correctly_when_Container_Size_Change_Causes_It_To_Be_Moved_Into_Visible_Viewport()
+        {
+            using var app = App();
+
+            // All containers start off with a height of 25 (4 containers fit in viewport).
+            var items = Enumerable.Range(0, 20).Select(x => new ItemWithHeight(x, 25)).ToList();
+            var (target, scroll, itemsControl) = CreateTarget(items: items, itemTemplate: CanvasWithHeightTemplate);
+
+            // Scroll to the 5th item (containers 4-7 should be visible).
+            target.ScrollIntoView(7);
+            Assert.Equal(4, target.FirstRealizedIndex);
+            Assert.Equal(7, target.LastRealizedIndex);
+
+            // Focus the 7th item.
+            var container = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(7));
+            container.Focusable = true;
+            container.Focus();
+
+            // Scroll up to the 3rd item (containers 3-6 should still be visible).
+            target.ScrollIntoView(3);
+            Assert.Equal(3, target.FirstRealizedIndex);
+            Assert.Equal(6, target.LastRealizedIndex);
+
+            // Update the height of all items to 20 and run a layout pass.
+            foreach (var item in items)
+                item.Height = 20;
+            target.UpdateLayout();
+
+            // The focused container should now be inside the realized range.
+            Assert.Equal(3, target.FirstRealizedIndex);
+            Assert.Equal(7, target.LastRealizedIndex);
+
+            // The container should be positioned correctly.
+            container = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(7));
+            Assert.Equal(new Rect(0, 140, 100, 20), container.Bounds);
+        }
+
         private static IReadOnlyList<int> GetRealizedIndexes(VirtualizingStackPanel target, ItemsControl itemsControl)
         {
             return target.GetRealizedElements()
@@ -1354,8 +1460,10 @@ namespace Avalonia.Controls.UnitTests
 
         private static IDisposable App() => UnitTestApplication.Start(TestServices.RealFocus);
 
-        private class ItemWithHeight
+        private class ItemWithHeight : NotifyingBase
         {
+            private double _height;
+
             public ItemWithHeight(int index, double height = 10)
             {
                 Caption = $"Item {index}";
@@ -1363,7 +1471,12 @@ namespace Avalonia.Controls.UnitTests
             }
             
             public string Caption { get; set; }
-            public double Height { get; set; }
+            
+            public double Height 
+            {
+                get => _height;
+                set => SetField(ref _height, value);
+            }
         }
 
         private class ItemWithIsVisible : NotifyingBase