Browse Source

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0169P8BipcMkn44hvhpQ2UNF

---------

Co-authored-by: grokys <grokys@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
pull/22079/head
Yoyo 1 month ago
committed by GitHub
parent
commit
38402fc046
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 69
      src/Avalonia.Controls/VirtualizingStackPanel.cs
  2. 61
      tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs

69
src/Avalonia.Controls/VirtualizingStackPanel.cs

@ -56,7 +56,7 @@ namespace Avalonia.Controls
/// Defines the <see cref="CacheLength"/> property. /// Defines the <see cref="CacheLength"/> property.
/// </summary> /// </summary>
public static readonly StyledProperty<double> CacheLengthProperty = public static readonly StyledProperty<double> CacheLengthProperty =
AvaloniaProperty.Register<VirtualizingStackPanel, double>(nameof(CacheLength), 0.0, AvaloniaProperty.Register<VirtualizingStackPanel, double>(nameof(CacheLength), 0.0,
validate: v => v is >= 0 and <= 2); validate: v => v is >= 0 and <= 2);
private static readonly AttachedProperty<object?> RecycleKeyProperty = private static readonly AttachedProperty<object?> RecycleKeyProperty =
@ -80,8 +80,8 @@ namespace Avalonia.Controls
private int _focusedIndex = -1; private int _focusedIndex = -1;
private Control? _realizingElement; private Control? _realizingElement;
private int _realizingIndex = -1; private int _realizingIndex = -1;
private double _bufferFactor; private double _bufferFactor;
private bool _hasReachedStart = false; private bool _hasReachedStart = false;
private bool _hasReachedEnd = false; private bool _hasReachedEnd = false;
private Rect _lastMeasuredExtendedViewport; private Rect _lastMeasuredExtendedViewport;
@ -270,7 +270,7 @@ namespace Avalonia.Controls
new Rect(0, u, finalSize.Width, sizeU); new Rect(0, u, finalSize.Width, sizeU);
e.Arrange(rect); e.Arrange(rect);
if (e.IsVisible && _viewport.Intersects(rect)) if (e.IsVisible && _viewport.Intersects(rect))
{ {
try try
@ -285,7 +285,7 @@ namespace Avalonia.Controls
e, ex.Message); e, ex.Message);
} }
} }
u += orientation == Orientation.Horizontal ? rect.Width : rect.Height; u += orientation == Orientation.Horizontal ? rect.Width : rect.Height;
} }
} }
@ -543,7 +543,7 @@ namespace Avalonia.Controls
var count = Items.Count; var count = Items.Count;
var fromControl = from as Control; var fromControl = from as Control;
if (count == 0 || if (count == 0 ||
(fromControl is null && direction is not NavigationDirection.First and not NavigationDirection.Last)) (fromControl is null && direction is not NavigationDirection.First and not NavigationDirection.Last))
return null; return null;
@ -644,7 +644,7 @@ namespace Avalonia.Controls
element.BringIntoView(); element.BringIntoView();
return element; 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 // 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. // 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. // 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; anchorIndex > _realizedElements.LastIndex;
return new MeasureViewport 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 // We have an element to scroll to, so we can estimate the desired size based on the
// element's position and the remaining elements. // element's position and the remaining elements.
var remaining = itemCount - _scrollToIndex - 1; var remaining = itemCount - _scrollToIndex - 1;
var u = orientation == Orientation.Horizontal ? var u = orientation == Orientation.Horizontal ?
_scrollToElement.Bounds.Right : _scrollToElement.Bounds.Right :
_scrollToElement.Bounds.Bottom; _scrollToElement.Bounds.Bottom;
var sizeU = u + (remaining * _lastEstimatedElementSizeU); var sizeU = u + (remaining * _lastEstimatedElementSizeU);
return orientation == Orientation.Horizontal ? return orientation == Orientation.Horizontal ?
new(sizeU, DesiredSize.Height) : new(sizeU, DesiredSize.Height) :
new(DesiredSize.Width, sizeU); new(DesiredSize.Width, sizeU);
} }
@ -901,23 +901,26 @@ namespace Avalonia.Controls
{ {
var first = realized.FirstIndex; var first = realized.FirstIndex;
var last = realized.LastIndex; var last = realized.LastIndex;
if (index < first) 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) if (index > last)
{ {
var sizes = realized.SizeU; var sizes = realized.SizeU;
var realizedSpan = 0.0; var realizedSpan = 0.0;
for (var i = 0; i < sizes.Count; ++i) for (var i = 0; i < sizes.Count; ++i)
{ {
var sizeU = sizes[i]; var sizeU = sizes[i];
realizedSpan += double.IsNaN(sizeU) ? estimatedSize : sizeU; realizedSpan += double.IsNaN(sizeU) ? estimatedSize : sizeU;
} }
return realized.StartU + realizedSpan + ((index - last - 1) * estimatedSize); return realized.StartU + realizedSpan + ((index - last - 1) * estimatedSize);
} }
} }
@ -940,7 +943,7 @@ namespace Avalonia.Controls
var viewportEnd = horizontal ? _viewport.Right : _viewport.Bottom; var viewportEnd = horizontal ? _viewport.Right : _viewport.Bottom;
var anchorAtEnd = !_hasReachedEnd && index == items.Count - 1 && var anchorAtEnd = !_hasReachedEnd && index == items.Count - 1 &&
MathUtilities.GreaterThanOrClose(viewportEnd, horizontal ? Bounds.Width : Bounds.Height); MathUtilities.GreaterThanOrClose(viewportEnd, horizontal ? Bounds.Width : Bounds.Height);
// Reset boundary flags // Reset boundary flags
_hasReachedStart = false; _hasReachedStart = false;
_hasReachedEnd = false; _hasReachedEnd = false;
@ -956,9 +959,9 @@ namespace Avalonia.Controls
_realizingIndex = index; _realizingIndex = index;
var e = GetOrCreateElement(items, index); var e = GetOrCreateElement(items, index);
_realizingElement = e; _realizingElement = e;
e.Measure(availableSize); e.Measure(availableSize);
var sizeU = horizontal ? e.DesiredSize.Width : e.DesiredSize.Height; var sizeU = horizontal ? e.DesiredSize.Width : e.DesiredSize.Height;
var sizeV = horizontal ? e.DesiredSize.Height : e.DesiredSize.Width; var sizeV = horizontal ? e.DesiredSize.Height : e.DesiredSize.Width;
@ -976,10 +979,10 @@ namespace Avalonia.Controls
_realizingIndex = -1; _realizingIndex = -1;
_realizingElement = null; _realizingElement = null;
} while (u < viewport.viewportUEnd && index < items.Count); } while (u < viewport.viewportUEnd && index < items.Count);
// Check if we reached the end of the collection // Check if we reached the end of the collection
_hasReachedEnd = index >= items.Count; _hasReachedEnd = index >= items.Count;
// Store the last index and end U position for the desired size calculation. // Store the last index and end U position for the desired size calculation.
viewport.lastIndex = index - 1; viewport.lastIndex = index - 1;
viewport.realizedEndU = u; viewport.realizedEndU = u;
@ -994,7 +997,7 @@ namespace Avalonia.Controls
while (u > viewport.viewportUStart && index >= 0) while (u > viewport.viewportUStart && index >= 0)
{ {
var e = GetOrCreateElement(items, index); var e = GetOrCreateElement(items, index);
e.Measure(availableSize); e.Measure(availableSize);
var sizeU = horizontal ? e.DesiredSize.Width : e.DesiredSize.Height; var sizeU = horizontal ? e.DesiredSize.Width : e.DesiredSize.Height;
var sizeV = horizontal ? e.DesiredSize.Height : e.DesiredSize.Width; var sizeV = horizontal ? e.DesiredSize.Height : e.DesiredSize.Width;
@ -1004,7 +1007,7 @@ namespace Avalonia.Controls
viewport.measuredV = Math.Max(viewport.measuredV, sizeV); viewport.measuredV = Math.Max(viewport.measuredV, sizeV);
--index; --index;
} }
// Check if we reached the start of the collection // Check if we reached the start of the collection
_hasReachedStart = index < 0; _hasReachedStart = index < 0;
@ -1039,7 +1042,7 @@ namespace Avalonia.Controls
{ {
return _realizedElements?.GetElement(index); return _realizedElements?.GetElement(index);
} }
private static Control? GetRealizedElement( private static Control? GetRealizedElement(
int index, int index,
ref int specialIndex, ref int specialIndex,
@ -1118,7 +1121,7 @@ namespace Avalonia.Controls
{ {
Debug.Assert(ItemsControl is not null); Debug.Assert(ItemsControl is not null);
Debug.Assert(ItemContainerGenerator is not null); Debug.Assert(ItemContainerGenerator is not null);
_scrollAnchorProvider?.UnregisterAnchorCandidate(element); _scrollAnchorProvider?.UnregisterAnchorCandidate(element);
var recycleKey = element.GetValue(RecycleKeyProperty); var recycleKey = element.GetValue(RecycleKeyProperty);
@ -1153,7 +1156,7 @@ namespace Avalonia.Controls
_scrollAnchorProvider?.UnregisterAnchorCandidate(element); _scrollAnchorProvider?.UnregisterAnchorCandidate(element);
var recycleKey = element.GetValue(RecycleKeyProperty); var recycleKey = element.GetValue(RecycleKeyProperty);
if (recycleKey is null) if (recycleKey is null)
{ {
ItemContainerGenerator!.ClearItemContainer(element); ItemContainerGenerator!.ClearItemContainer(element);
@ -1181,7 +1184,7 @@ namespace Avalonia.Controls
_focusedElement = null; _focusedElement = null;
_focusedIndex = -1; _focusedIndex = -1;
} }
private void RecycleScrollToElement() private void RecycleScrollToElement()
{ {
if (_scrollToElement != null) if (_scrollToElement != null)
@ -1191,7 +1194,7 @@ namespace Avalonia.Controls
_scrollToElement = null; _scrollToElement = null;
_scrollToIndex = -1; _scrollToIndex = -1;
} }
private void PushToRecyclePool(object recycleKey, Control element) private void PushToRecyclePool(object recycleKey, Control element)
{ {
_recyclePool ??= new(); _recyclePool ??= new();
@ -1211,7 +1214,7 @@ namespace Avalonia.Controls
ItemContainerGenerator.ItemContainerIndexChanged(element, oldIndex, newIndex); ItemContainerGenerator.ItemContainerIndexChanged(element, oldIndex, newIndex);
} }
private Rect CalculateExtendedViewport(bool vertical, double viewportSize, double bufferSize) private Rect CalculateExtendedViewport(bool vertical, double viewportSize, double bufferSize)
{ {
@ -1378,7 +1381,7 @@ namespace Avalonia.Controls
private void OnItemsControlPropertyChanged(object? sender, AvaloniaPropertyChangedEventArgs e) private void OnItemsControlPropertyChanged(object? sender, AvaloniaPropertyChangedEventArgs e)
{ {
if (_focusedElement is not null && if (_focusedElement is not null &&
e.Property == KeyboardNavigation.TabOnceActiveElementProperty && e.Property == KeyboardNavigation.TabOnceActiveElementProperty &&
e.GetOldValue<IInputElement?>() == _focusedElement) e.GetOldValue<IInputElement?>() == _focusedElement)
{ {
// TabOnceActiveElement has moved away from _focusedElement so we can recycle it. // TabOnceActiveElement has moved away from _focusedElement so we can recycle it.
@ -1392,15 +1395,15 @@ namespace Avalonia.Controls
{ {
var newValue = e.GetNewValue<double>(); var newValue = e.GetNewValue<double>();
_bufferFactor = newValue; _bufferFactor = newValue;
// Force a recalculation of the extended viewport on the next layout pass // Force a recalculation of the extended viewport on the next layout pass
InvalidateMeasure(); InvalidateMeasure();
} }
/// <inheritdoc/> /// <inheritdoc/>
public IReadOnlyList<double> GetIrregularSnapPoints(Orientation orientation, SnapPointsAlignment snapPointsAlignment) public IReadOnlyList<double> GetIrregularSnapPoints(Orientation orientation, SnapPointsAlignment snapPointsAlignment)
{ {
if(_realizedElements == null) if (_realizedElements == null)
return new List<double>(); return new List<double>();
return new VirtualizingSnapPointsList(_realizedElements, ItemsControl?.ItemsSource?.Count() ?? 0, orientation, Orientation, snapPointsAlignment, EstimateElementSizeU()); return new VirtualizingSnapPointsList(_realizedElements, ItemsControl?.ItemsSource?.Count() ?? 0, orientation, Orientation, snapPointsAlignment, EstimateElementSizeU());

61
tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs

@ -1,4 +1,4 @@
using System; using System;
using System.Collections; using System.Collections;
using System.Collections.Generic; using System.Collections.Generic;
using System.Collections.ObjectModel; using System.Collections.ObjectModel;
@ -1683,7 +1683,7 @@ namespace Avalonia.Controls.UnitTests
} }
[Fact] [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(); using var app = App();
@ -1691,7 +1691,7 @@ namespace Avalonia.Controls.UnitTests
.Select(x => new ItemWithHeight(x, x < 10 ? 10 : 50)) .Select(x => new ItemWithHeight(x, x < 10 ? 10 : 50))
.ToList(); .ToList();
var (target, _, _) = CreateTarget(items: items, itemTemplate: CanvasWithHeightTemplate); var (target, scroll, _) = CreateTarget(items: items, itemTemplate: CanvasWithHeightTemplate);
var focused = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(5)); var focused = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(5));
focused.Focusable = true; focused.Focusable = true;
@ -1702,18 +1702,19 @@ namespace Avalonia.Controls.UnitTests
Assert.True(target.FirstRealizedIndex > 5); Assert.True(target.FirstRealizedIndex > 5);
var firstIndex = target.FirstRealizedIndex; var firstRealized = Assert.IsType<ContentPresenter>(
var firstRealized = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(firstIndex)); target.ContainerFromIndex(target.FirstRealizedIndex));
var realized = target.GetRealizedElements() focused = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(5));
.Where(x => x is not null)
.Cast<Control>()
.ToList();
var estimatedSize = realized.Average(x => x.DesiredSize.Height); // The focused container's position is estimated, as it's outside the realized range.
var expectedTop = firstRealized.Bounds.Top - ((firstIndex - 5) * estimatedSize); // The estimate must never place it before the panel origin...
Assert.True(focused.Bounds.Top >= 0);
focused = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(5)); // ...must keep it above the realized range rather than overlapping it...
Assert.Equal(expectedTop, focused.Bounds.Top, 3); 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] [Theory]
@ -1765,6 +1766,40 @@ namespace Avalonia.Controls.UnitTests
Assert.Equal(new Rect(0, 140, 100, 20), container.Bounds); 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<object> 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<IDataTemplate?> itemTemplate = horizontal ? CanvasWithWidthTemplate : CanvasWithHeightTemplate;
var (target, scroll, _) = CreateTarget(items: items, itemTemplate: itemTemplate, orientation: orientation);
target.ScrollIntoView(60);
target.ScrollIntoView(targetIndex);
var container = Assert.IsType<ContentPresenter>(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] [Fact]
public void When_Vertical_Calculates_ViewPort_At_Start_Of_List() public void When_Vertical_Calculates_ViewPort_At_Start_Of_List()
{ {

Loading…
Cancel
Save