Optimize tree size computation and the scene tree dock filter - #110759
Conversation
4126f6d to
a64397d
Compare
|
I see more usages of |
a64397d to
505edc7
Compare
505edc7 to
357cace
Compare
| if (n && editor_selection->is_selected(n)) { | ||
| if (p_scroll_to_selected) { | ||
| // Needs to be deferred to account for possible root visibility change. | ||
| callable_mp(tree, &Tree::scroll_to_item).call_deferred(p_parent, false); |
There was a problem hiding this comment.
Looks like this (or the one above) is called for every selected item, so the Tree will needlessly scroll to each item, eventually settling on the last one. This can be queued with something like
void _queue_scroll_to_item(TreeItem *p_item) {
if (!queued_item) {
callable_mp(this, &SceneTreeEditor::_scroll_to_item).call_deferred();
}
queued_item = p_item;
}
void _scroll_to_item() {
tree->scroll_to_item(queued_item );
queued_item = nullptr;
}There was a problem hiding this comment.
It used to be called every time, but this PR actually prevents that via the condition update on line 1041. p_scroll_to_selected will only be true the first time a child is found, and will be false subsequently, so it actually only scrolls to the first element it finds. That's the main performance boost this PR provides, actually.
Calling it a single time on the last item only isn't ideal. Really, you want to call it on the first element once, and then the last element once, because doing so will frame as many of the elements as possible. If you do it only to the first or last element, it might not scroll to show as many of the selected elements. In order to preserve that behavior, I used this somewhat complicated boolean passing instead of a solution like the one you proposed.
I could accomplish the same result as what I'm doing now via a similar method, though, if you'd prefer.
|
Thanks! |
…imizations Optimize tree size computation and the scene tree dock filter
…imizations Optimize tree size computation and the scene tree dock filter
These are fixes for the last issues I've encountered related to large node counts in the editor. You can trigger them in a variety of ways, but they all come back to either:
O(m*n)behavior where n is the current selection size and m is the number of items in the treeFor the filter issue, here are two ways to see the problem:
The core of the issue is that when updating the filter, it attempts to scroll to every selected item in turn. This seems equivalent to scrolling to the first item and them scrolling to the last item, so that's what this PR updates it to do, instead. Since scrolling to an item is
O(m)on the number of items, that cuts anO(m*n)algorithm (which isO(m^2)with a large enough selection) down to justO(m).For the font height calculation:
This commonly triggers any time that you redraw the scene tree and whenever you mouse over the scene tree, since it checks what your mouse is over. This doesn't seem like it should be that expensive, but it turns out it is, and the difference in responsiveness of the Scene Tree dock with it cached is massive. You can very obviously see a visual difference just by opening a scene with 20k+ nodes and trying to scroll around and interact with the Scene Tree dock.
One thing I'm nore sure of: this PR assumes that
fontandfont_sizeintheme_cacheonly change when the theme changes. This is fine as far as I know, but I'm not exactly sure howtheme_cacheworks, so I wanted to call it out. If this isn't safe to cache on the longer term, it still seems worth caching on a per frame basis somehow.It's data time!

Typing "Label" into a filter with 25k items that all match and 1 item selected: 1.5s before, 1.5s after
Typing "Label" into a filter with 25k items that all match and 1k items selected: 74s before, 1.5s after
Here's a flamegraph of the before, stuck in the
scroll_to_selectedswamp, which itself is mostly spent getting font heights:Switching to a scene with 100k items, 1 item selected: 5.5s before, 4.9s after

Switching to a scene with 100k items, 1k items selected: manually killed 10 minutes in before, 4.9s after
Here's a flamegraph that might give you deja vu:
The font height calculation can seen as a problem just scrolling through and clicking around a large Scene Tree dock, here's me doing that for a little bit:

Before
After

I also recorded a little video of the difference. Pretty clear how impactful this is.
Before
https://imgur.com/g7VQimY
After
https://imgur.com/mdmfZWA
All of the data in this PR was collected on builds built with
scons optimize=speed_trace production=yes debug_symbols=yes platform=linuxbsd dev_build=no use_llvm=yes. The before numbers aren't actually frommaster, they're frommasterwith my other editor performance PRs cherry-picked on, as well. The other, existing performance problems make these tests difficult to run without them. The sample project I've been using to test iseditor-optimization-test-scenes.zip.
With this PR and my other optimization PRs applied, I'm able to work reasonably comfortably on a scene with 100,000 nodes in it!