From 38402fc046e7f56298a7c9cedd51600e1ff6f9e0 Mon Sep 17 00:00:00 2001 From: Yoyo Date: Thu, 27 Aug 2026 09:39:45 +0000 Subject: [PATCH] Fix VirtualizingStackPanel offset when scrolling to variable-sized items (#21975) * Fix VirtualizingStackPanel offset when scrolling to variable-sized items * Remove the `isScrollIntoView` parameter. * Tweak now-failing test. Change the the test to check that the element is outside of the viewport, instead of asserting its exact coordinates - that's the important part. * Retrigger CI. Azure Pipelines never queued a build for 3d81ceb9. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0169P8BipcMkn44hvhpQ2UNF --------- Co-authored-by: grokys Co-authored-by: Claude Opus 5 (1M context) --- .../VirtualizingStackPanel.cs | 69 ++++++++++--------- .../VirtualizingStackPanelTests.cs | 61 ++++++++++++---- 2 files changed, 84 insertions(+), 46 deletions(-) diff --git a/src/Avalonia.Controls/VirtualizingStackPanel.cs b/src/Avalonia.Controls/VirtualizingStackPanel.cs index fbb9c83de2..e78643ef82 100644 --- a/src/Avalonia.Controls/VirtualizingStackPanel.cs +++ b/src/Avalonia.Controls/VirtualizingStackPanel.cs @@ -56,7 +56,7 @@ namespace Avalonia.Controls /// Defines the property. /// public static readonly StyledProperty CacheLengthProperty = - AvaloniaProperty.Register(nameof(CacheLength), 0.0, + AvaloniaProperty.Register(nameof(CacheLength), 0.0, validate: v => v is >= 0 and <= 2); private static readonly AttachedProperty RecycleKeyProperty = @@ -80,8 +80,8 @@ namespace Avalonia.Controls private int _focusedIndex = -1; private Control? _realizingElement; private int _realizingIndex = -1; - private double _bufferFactor; - + private double _bufferFactor; + private bool _hasReachedStart = false; private bool _hasReachedEnd = false; private Rect _lastMeasuredExtendedViewport; @@ -270,7 +270,7 @@ namespace Avalonia.Controls new Rect(0, u, finalSize.Width, sizeU); e.Arrange(rect); - + if (e.IsVisible && _viewport.Intersects(rect)) { try @@ -285,7 +285,7 @@ namespace Avalonia.Controls e, ex.Message); } } - + u += orientation == Orientation.Horizontal ? rect.Width : rect.Height; } } @@ -543,7 +543,7 @@ namespace Avalonia.Controls var count = Items.Count; var fromControl = from as Control; - if (count == 0 || + if (count == 0 || (fromControl is null && direction is not NavigationDirection.First and not NavigationDirection.Last)) return null; @@ -644,7 +644,7 @@ namespace Avalonia.Controls element.BringIntoView(); return element; } - else if (this.GetLayoutRoot() is {} root) + else if (this.GetLayoutRoot() is { } root) { // Create and measure the element to be brought into view. Store it in a field so that // it can be re-used in the layout pass. @@ -746,7 +746,7 @@ namespace Avalonia.Controls } // Check if the anchor element is not within the currently realized elements. - var disjunct = anchorIndex < _realizedElements.FirstIndex || + var disjunct = anchorIndex < _realizedElements.FirstIndex || anchorIndex > _realizedElements.LastIndex; return new MeasureViewport @@ -780,12 +780,12 @@ namespace Avalonia.Controls // We have an element to scroll to, so we can estimate the desired size based on the // element's position and the remaining elements. var remaining = itemCount - _scrollToIndex - 1; - var u = orientation == Orientation.Horizontal ? + var u = orientation == Orientation.Horizontal ? _scrollToElement.Bounds.Right : _scrollToElement.Bounds.Bottom; var sizeU = u + (remaining * _lastEstimatedElementSizeU); - return orientation == Orientation.Horizontal ? - new(sizeU, DesiredSize.Height) : + return orientation == Orientation.Horizontal ? + new(sizeU, DesiredSize.Height) : new(DesiredSize.Width, sizeU); } @@ -901,23 +901,26 @@ namespace Avalonia.Controls { var first = realized.FirstIndex; var last = realized.LastIndex; - + if (index < first) { - return realized.StartU - ((first - index) * estimatedSize); + // Interpolate between the known panel origin and the first realized item. + // Using the realized items' average size to extrapolate backwards can place + // the target before the panel origin. + return realized.StartU * index / first; } - + if (index > last) { var sizes = realized.SizeU; var realizedSpan = 0.0; - + for (var i = 0; i < sizes.Count; ++i) { var sizeU = sizes[i]; realizedSpan += double.IsNaN(sizeU) ? estimatedSize : sizeU; } - + return realized.StartU + realizedSpan + ((index - last - 1) * estimatedSize); } } @@ -940,7 +943,7 @@ namespace Avalonia.Controls var viewportEnd = horizontal ? _viewport.Right : _viewport.Bottom; var anchorAtEnd = !_hasReachedEnd && index == items.Count - 1 && MathUtilities.GreaterThanOrClose(viewportEnd, horizontal ? Bounds.Width : Bounds.Height); - + // Reset boundary flags _hasReachedStart = false; _hasReachedEnd = false; @@ -956,9 +959,9 @@ namespace Avalonia.Controls _realizingIndex = index; var e = GetOrCreateElement(items, index); _realizingElement = e; - + e.Measure(availableSize); - + var sizeU = horizontal ? e.DesiredSize.Width : e.DesiredSize.Height; var sizeV = horizontal ? e.DesiredSize.Height : e.DesiredSize.Width; @@ -976,10 +979,10 @@ namespace Avalonia.Controls _realizingIndex = -1; _realizingElement = null; } while (u < viewport.viewportUEnd && index < items.Count); - + // Check if we reached the end of the collection _hasReachedEnd = index >= items.Count; - + // Store the last index and end U position for the desired size calculation. viewport.lastIndex = index - 1; viewport.realizedEndU = u; @@ -994,7 +997,7 @@ namespace Avalonia.Controls while (u > viewport.viewportUStart && index >= 0) { var e = GetOrCreateElement(items, index); - + e.Measure(availableSize); var sizeU = horizontal ? e.DesiredSize.Width : e.DesiredSize.Height; var sizeV = horizontal ? e.DesiredSize.Height : e.DesiredSize.Width; @@ -1004,7 +1007,7 @@ namespace Avalonia.Controls viewport.measuredV = Math.Max(viewport.measuredV, sizeV); --index; } - + // Check if we reached the start of the collection _hasReachedStart = index < 0; @@ -1039,7 +1042,7 @@ namespace Avalonia.Controls { return _realizedElements?.GetElement(index); } - + private static Control? GetRealizedElement( int index, ref int specialIndex, @@ -1118,7 +1121,7 @@ namespace Avalonia.Controls { Debug.Assert(ItemsControl is not null); Debug.Assert(ItemContainerGenerator is not null); - + _scrollAnchorProvider?.UnregisterAnchorCandidate(element); var recycleKey = element.GetValue(RecycleKeyProperty); @@ -1153,7 +1156,7 @@ namespace Avalonia.Controls _scrollAnchorProvider?.UnregisterAnchorCandidate(element); var recycleKey = element.GetValue(RecycleKeyProperty); - + if (recycleKey is null) { ItemContainerGenerator!.ClearItemContainer(element); @@ -1181,7 +1184,7 @@ namespace Avalonia.Controls _focusedElement = null; _focusedIndex = -1; } - + private void RecycleScrollToElement() { if (_scrollToElement != null) @@ -1191,7 +1194,7 @@ namespace Avalonia.Controls _scrollToElement = null; _scrollToIndex = -1; } - + private void PushToRecyclePool(object recycleKey, Control element) { _recyclePool ??= new(); @@ -1211,7 +1214,7 @@ namespace Avalonia.Controls ItemContainerGenerator.ItemContainerIndexChanged(element, oldIndex, newIndex); } - + private Rect CalculateExtendedViewport(bool vertical, double viewportSize, double bufferSize) { @@ -1378,7 +1381,7 @@ namespace Avalonia.Controls private void OnItemsControlPropertyChanged(object? sender, AvaloniaPropertyChangedEventArgs e) { if (_focusedElement is not null && - e.Property == KeyboardNavigation.TabOnceActiveElementProperty && + e.Property == KeyboardNavigation.TabOnceActiveElementProperty && e.GetOldValue() == _focusedElement) { // TabOnceActiveElement has moved away from _focusedElement so we can recycle it. @@ -1392,15 +1395,15 @@ namespace Avalonia.Controls { var newValue = e.GetNewValue(); _bufferFactor = newValue; - + // Force a recalculation of the extended viewport on the next layout pass InvalidateMeasure(); } - + /// public IReadOnlyList GetIrregularSnapPoints(Orientation orientation, SnapPointsAlignment snapPointsAlignment) { - if(_realizedElements == null) + if (_realizedElements == null) return new List(); return new VirtualizingSnapPointsList(_realizedElements, ItemsControl?.ItemsSource?.Count() ?? 0, orientation, Orientation, snapPointsAlignment, EstimateElementSizeU()); diff --git a/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs b/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs index aefae1fce7..8a2298ff5a 100644 --- a/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; @@ -1683,7 +1683,7 @@ namespace Avalonia.Controls.UnitTests } [Fact] - public void Focused_Container_Is_Positioned_Correctly_When_Scrolled_Past_Items_With_Different_Heights() + public void Focused_Container_Is_Positioned_Outside_Viewport_When_Scrolled_Past_Items_With_Different_Heights() { using var app = App(); @@ -1691,7 +1691,7 @@ namespace Avalonia.Controls.UnitTests .Select(x => new ItemWithHeight(x, x < 10 ? 10 : 50)) .ToList(); - var (target, _, _) = CreateTarget(items: items, itemTemplate: CanvasWithHeightTemplate); + var (target, scroll, _) = CreateTarget(items: items, itemTemplate: CanvasWithHeightTemplate); var focused = Assert.IsType(target.ContainerFromIndex(5)); focused.Focusable = true; @@ -1702,18 +1702,19 @@ namespace Avalonia.Controls.UnitTests Assert.True(target.FirstRealizedIndex > 5); - var firstIndex = target.FirstRealizedIndex; - var firstRealized = Assert.IsType(target.ContainerFromIndex(firstIndex)); - var realized = target.GetRealizedElements() - .Where(x => x is not null) - .Cast() - .ToList(); + var firstRealized = Assert.IsType( + target.ContainerFromIndex(target.FirstRealizedIndex)); + focused = Assert.IsType(target.ContainerFromIndex(5)); - var estimatedSize = realized.Average(x => x.DesiredSize.Height); - var expectedTop = firstRealized.Bounds.Top - ((firstIndex - 5) * estimatedSize); + // The focused container's position is estimated, as it's outside the realized range. + // The estimate must never place it before the panel origin... + Assert.True(focused.Bounds.Top >= 0); - focused = Assert.IsType(target.ContainerFromIndex(5)); - Assert.Equal(expectedTop, focused.Bounds.Top, 3); + // ...must keep it above the realized range rather than overlapping it... + Assert.True(focused.Bounds.Bottom <= firstRealized.Bounds.Top); + + // ...and must keep it out of the viewport, so it can't appear as a ghost item. + Assert.True(focused.Bounds.Bottom <= scroll.Offset.Y); } [Theory] @@ -1765,6 +1766,40 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(new Rect(0, 140, 100, 20), container.Bounds); } + [Theory] + [InlineData(25, Orientation.Vertical)] + [InlineData(99, Orientation.Vertical)] + [InlineData(25, Orientation.Horizontal)] + [InlineData(99, Orientation.Horizontal)] + public void ScrollIntoView_With_Variable_Size_Items_Keeps_Target_In_Viewport(int targetIndex, Orientation orientation) + { + using var app = App(); + + var firstHalfSize = targetIndex < 60 ? 20 : 40; + var secondHalfSize = targetIndex < 60 ? 40 : 20; + var horizontal = orientation == Orientation.Horizontal; + IEnumerable items = horizontal ? + Enumerable.Range(0, 100).Select(x => new ItemWithWidth(x, x < 50 ? firstHalfSize : secondHalfSize)) : + Enumerable.Range(0, 100).Select(x => new ItemWithHeight(x, x < 50 ? firstHalfSize : secondHalfSize)); + Optional itemTemplate = horizontal ? CanvasWithWidthTemplate : CanvasWithHeightTemplate; + var (target, scroll, _) = CreateTarget(items: items, itemTemplate: itemTemplate, orientation: orientation); + + target.ScrollIntoView(60); + target.ScrollIntoView(targetIndex); + + var container = Assert.IsType(target.ContainerFromIndex(targetIndex)); + var message = $"Bounds={container.Bounds}, Offset={scroll.Offset}, Viewport={scroll.Viewport}, Extent={scroll.Extent}"; + + var containerStart = horizontal ? container.Bounds.Left : container.Bounds.Top; + var containerEnd = horizontal ? container.Bounds.Right : container.Bounds.Bottom; + var viewportStart = horizontal ? scroll.Offset.X : scroll.Offset.Y; + var viewportEnd = viewportStart + (horizontal ? scroll.Viewport.Width : scroll.Viewport.Height); + + Assert.True(containerStart > 0, message); + Assert.True(containerStart >= viewportStart, message); + Assert.True(containerEnd <= viewportEnd, message); + } + [Fact] public void When_Vertical_Calculates_ViewPort_At_Start_Of_List() {