Browse Source

Fix Some Virtualizing List Update Bugs (#13795)

* Added failing unit tests for #11666.

* Handle items replaced with its own logic.

Don't rely on a remove/insert - it creates more work than needed.

* Fix some off-by-N errors in RealizedStackElements.

Fixes #11666
pull/13780/head
Steven Kirk 3 years ago
committed by GitHub
parent
commit
3725f0dc80
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
  1. 41
      src/Avalonia.Controls/Utils/RealizedStackElements.cs
  2. 2
      src/Avalonia.Controls/VirtualizingStackPanel.cs
  3. 119
      tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs

41
src/Avalonia.Controls/Utils/RealizedStackElements.cs

@ -281,13 +281,13 @@ namespace Avalonia.Controls.Utils
// elements after the insertion point.
var elementCount = _elements.Count;
var start = Math.Max(realizedIndex, 0);
var newIndex = realizedIndex + count;
for (var i = start; i < elementCount; ++i)
{
if (_elements[i] is Control element)
updateElementIndex(element, newIndex - count, newIndex);
++newIndex;
if (_elements[i] is not Control element)
continue;
var oldIndex = i + first;
updateElementIndex(element, oldIndex, oldIndex + count);
}
if (realizedIndex < 0)
@ -341,7 +341,7 @@ namespace Avalonia.Controls.Utils
for (var i = 0; i < _elements.Count; ++i)
{
if (_elements[i] is Control element)
updateElementIndex(element, newIndex - count, newIndex);
updateElementIndex(element, newIndex + count, newIndex);
++newIndex;
}
}
@ -384,6 +384,37 @@ namespace Avalonia.Controls.Utils
}
}
/// <summary>
/// Updates the elements in response to items being replaced in the source collection.
/// </summary>
/// <param name="index">The index in the source collection of the remove.</param>
/// <param name="count">The number of items removed.</param>
/// <param name="recycleElement">A method used to recycle elements.</param>
public void ItemsReplaced(int index, int count, Action<Control> recycleElement)
{
if (index < 0)
throw new ArgumentOutOfRangeException(nameof(index));
if (_elements is null || _elements.Count == 0)
return;
// Get the index within the realized _elements collection.
var startIndex = index - FirstIndex;
var endIndex = Math.Min(startIndex + count, Count);
if (startIndex >= 0 && endIndex > startIndex)
{
for (var i = startIndex; i < endIndex; ++i)
{
if (_elements[i] is { } element)
{
recycleElement(element);
_elements[i] = null;
_sizes![i] = double.NaN;
}
}
}
}
/// <summary>
/// Recycles all elements in response to the source collection being reset.
/// </summary>

2
src/Avalonia.Controls/VirtualizingStackPanel.cs

@ -253,6 +253,8 @@ namespace Avalonia.Controls
_realizedElements.ItemsRemoved(e.OldStartingIndex, e.OldItems!.Count, _updateElementIndex, _recycleElementOnItemRemoved);
break;
case NotifyCollectionChangedAction.Replace:
_realizedElements.ItemsReplaced(e.OldStartingIndex, e.OldItems!.Count, _recycleElementOnItemRemoved);
break;
case NotifyCollectionChangedAction.Move:
_realizedElements.ItemsRemoved(e.OldStartingIndex, e.OldItems!.Count, _updateElementIndex, _recycleElementOnItemRemoved);
_realizedElements.ItemsInserted(e.NewStartingIndex, e.NewItems!.Count, _updateElementIndex);

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

@ -611,6 +611,125 @@ namespace Avalonia.Controls.UnitTests
Assert.Equal(10, raised);
}
[Fact]
public void ContainerIndexChanged_Is_Raised_On_Insert()
{
using var app = App();
var (target, scroll, itemsControl) = CreateTarget();
var items = (IList)itemsControl.ItemsSource!;
var raised = 0;
var index = 1;
itemsControl.ContainerIndexChanged += (s, e) =>
{
++raised;
Assert.Equal(index, e.OldIndex);
Assert.Equal(++index, e.NewIndex);
};
items.Insert(index, "new");
Assert.Equal(9, raised);
}
[Fact]
public void ContainerIndexChanged_Is_Raised_When_Item_Inserted_Before_Realized_Elements()
{
using var app = App();
var (target, scroll, itemsControl) = CreateTarget();
var items = (IList)itemsControl.ItemsSource!;
var raised = 0;
var index = 20;
itemsControl.ContainerIndexChanged += (s, e) =>
{
++raised;
Assert.Equal(index, e.OldIndex);
Assert.Equal(++index, e.NewIndex);
};
scroll.Offset = new Vector(0, 200);
Layout(target);
items.Insert(10, "new");
Assert.Equal(10, raised);
}
[Fact]
public void ContainerIndexChanged_Is_Raised_On_Remove()
{
using var app = App();
var (target, scroll, itemsControl) = CreateTarget();
var items = (IList)itemsControl.ItemsSource!;
var raised = 0;
var index = 1;
itemsControl.ContainerIndexChanged += (s, e) =>
{
++raised;
Assert.Equal(index + 1, e.OldIndex);
Assert.Equal(index++, e.NewIndex);
};
items.RemoveAt(index);
Assert.Equal(8, raised);
}
[Fact]
public void ContainerIndexChanged_Is_Raised_When_Item_Removed_Before_Realized_Elements()
{
using var app = App();
var (target, scroll, itemsControl) = CreateTarget();
var items = (IList)itemsControl.ItemsSource!;
var raised = 0;
var index = 20;
itemsControl.ContainerIndexChanged += (s, e) =>
{
Assert.Equal(index, e.OldIndex);
Assert.Equal(index - 1, e.NewIndex);
++index;
++raised;
};
scroll.Offset = new Vector(0, 200);
Layout(target);
items.RemoveAt(10);
Assert.Equal(10, raised);
}
[Fact]
public void Fires_Correct_Container_Lifecycle_Events_On_Replace()
{
using var app = App();
var (target, scroll, itemsControl) = CreateTarget();
var items = (IList)itemsControl.ItemsSource!;
var events = new List<string>();
itemsControl.ContainerPrepared += (s, e) => events.Add($"Prepared #{e.Container.GetHashCode()} = {e.Index}");
itemsControl.ContainerClearing += (s, e) => events.Add($"Clearing #{e.Container.GetHashCode()}");
itemsControl.ContainerIndexChanged += (s, e) => events.Add($"IndexChanged #{e.Container.GetHashCode()} {e.OldIndex} -> {e.NewIndex}");
var toReplace = target.GetRealizedElements().ElementAt(2)!;
items[2] = "New Item";
Assert.Equal(
new[] { $"Clearing #{toReplace.GetHashCode()}" },
events);
events.Clear();
itemsControl.UpdateLayout();
Assert.Equal(
new[] { $"Prepared #{toReplace.GetHashCode()} = 2" },
events);
events.Clear();
}
[Fact]
public void Scrolling_Down_With_Larger_Element_Does_Not_Cause_Jump_And_Arrives_At_End()
{

Loading…
Cancel
Save