Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix VirtualisedListContainer rows not returning child to pool on disposal #6316

Merged
merged 2 commits into from
Jun 26, 2024

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Jun 26, 2024

CompositeDrawable.Dispose() doesn't do anything to its children except dispose them, which is problematic, since it leads to two things happening in the context of VirtualisedListContainer:

  • The pooled child is never returned to pool (because returning happens automagically on parent change, which doesn't happen here)
  • However, the pool continues to retain the reference to a disposed pooled drawable, which means that it (a) keeps drawables that will blow up if they were ever to be used, which isn't possible because (b) these drawables are never returned, which means that the pool will inevitably start overflowing.

Thus, aggressively remove the pooled child at the start of disposal to counteract this happening.

Can be semi-manually tested using 0b7106b:

  • Open test scene
  • Open performance tool window (ctrl-f2)
  • Scroll to the part that shows active / total pool capacity
  • Spam the added "replace items" test step
  • Watch the pool begin to overflow.

…sposal

`CompositeDrawable.Dispose()` doesn't do anything to its children except
dispose them, which is problematic, since it leads to two things
happening in the context of `VirtualisedListContainer`:

- The pooled child is never returned to pool (because returning happens
  automagically on parent change, which doesn't happen here)

- However, the pool continues to retain the reference to a disposed
  pooled drawable, which means that it (a) keeps drawables that will
  blow up if they were ever to be used, which isn't possible because (b)
  these drawables are never returned, which means that the pool will
  inevitably start overflowing.

Thus, aggressively remove the pooled child at the start of disposal to
counteract this happening.
@peppy peppy merged commit 8882a94 into ppy:master Jun 26, 2024
21 checks passed
@bdach bdach deleted the fix-virtualised-list-dispose branch June 27, 2024 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants