Skip to content

Commit

Permalink
Merge pull request #6318 from bdach/fix-virtualised-list-dispose-again
Browse files Browse the repository at this point in the history
Fix `VirtualisedListContainer` rows not returning pooled content in a different way
  • Loading branch information
peppy authored Jun 27, 2024
2 parents 0237dc9 + b1d1a6e commit 813a4a6
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,30 @@ public void TestVirtualisedList()
});
}

[Test]
public void TestVirtualisedListDisposal()
{
ExampleVirtualisedList list = null!;
AddStep("create list nested in container", () =>
{
Child = new Container
{
RelativeSizeAxes = Axes.Both,
Child = new Container
{
RelativeSizeAxes = Axes.Both,
Child = list = new ExampleVirtualisedList
{
RelativeSizeAxes = Axes.Both,
}
}
};
list.RowData.AddRange(Enumerable.Range(1, 10000).Select(i => $"Item #{i}"));
});
AddStep("clear", Clear);
AddUntilStep("wait for async disposal", () => list.IsDisposed);
}

[Test]
public void TestCollectionChangeHandling()
{
Expand Down
19 changes: 5 additions & 14 deletions osu.Framework/Graphics/Containers/VirtualisedListContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,11 @@ private void itemsChanged(object? sender, NotifyCollectionChangedEventArgs e)
Debug.Assert(e.OldItems != null);

for (int i = 0; i < e.OldItems.Count; ++i)
Items.Remove(items[e.OldStartingIndex + i], true);
{
var item = items[e.OldStartingIndex + i];
item.Unload();
Items.Remove(item, true);
}

for (int i = e.OldStartingIndex + e.OldItems.Count; i < items.Length; ++i)
Items.SetLayoutPosition(items[i], i - e.OldItems.Count);
Expand Down Expand Up @@ -250,19 +254,6 @@ public void Unload()
Visible = false;
LifetimeEnd = double.NegativeInfinity;
}

protected override void Dispose(bool isDisposing)
{
// CompositeDrawable only disposes the child pooled drawable (if any).
// This is generally not what we want for two reasons:
// - We want to try to return the pooled drawable to the pool so that it can be reused.
// Disposing it obviously makes reuse impossible.
// - Secondly, pooled drawables return to pool when they lose their parent.
// Thus, we proactively clear the pool drawable from ourselves here.
Unload();

base.Dispose(isDisposing);
}
}

protected abstract ScrollContainer<Drawable> CreateScrollContainer();
Expand Down

0 comments on commit 813a4a6

Please sign in to comment.