[Tree] Improve Tree Performance by replacing computed height with TreeItem's cached minimum size

[maidopi-usagi]

Originally, TreeItem's content height was computed in every draw while traversing the tree, even if the item content never changes. And there's already minimum_size cached in each item, which is computed in a similar way.

Tested on a massive plain-tree with 15K and 150K items(project file attached below), and got significant performance improvement (About 5~10x).
Here're results on my desktop when scrolling to near the bottom of tree.

Amount	This PR	Master
15K Items	1543FPS	152FPS
150K Items	36FPS	3FPS, and with a lot much longer start time

I also checked the behaviors of more complex Trees in the editor, and didn't notice any obvious artifacts, but I'm not very sure if there were any side-effect after changing these.

Might partially fix these issues?

• Closes: Poor performance of 2D Tree View with many items #70869

EDIT:
After some "extreme" test with huge amount of auto-wrapping text TreeItems and scrolling fast will cause some item's height cache being outdated.
I'm still looking for some other way to make this work. So convert this to draft now and will try reopen when I figured out.
I believe that's not related to this PR's modification, as I can reproduce the above issue in both 4.3 and master branch. I'll open another issue about that.

treetest.zip

[scgm0]

After testing, it works great in my personal project, now I can directly use Tree to build my tools 👍
master:

this pr:

[KoBeWi]

I see some discrepancy between the code you deleted in compute_item_height() and TreeItem's get_minimum_size(), e.g. the latter does not account for theme_cache.checked height.

[maidopi-usagi]

I see some discrepancy between the code you deleted in compute_item_height() and TreeItem's get_minimum_size(), e.g. the latter does not account for theme_cache.checked height.

I'm wondering the reason that get_minimum_size() didn't take theme_cache.checked's height into account. If there's nothing affected I'll re-add that into the method.

Also I noticed that update_item_cell() seems to be called twice every time (another in get_minimum_size()).
Maybe we can remove the following one?

godot/scene/gui/tree.cpp

Lines 1824 to 1827 in f5f813e

	for (int i = 0; i < columns.size(); i++) {
	if (p_item->cells[i].dirty) {
	const_cast<Tree *>(this)->update_item_cell(p_item, i);
	}

godot/scene/gui/tree.cpp

Lines 1523 to 1525 in f5f813e

	if (cell.dirty) {
	parent_tree->update_item_cell(this, p_column);
	}

[scgm0]

This PR is working fine in my project, what's causing it not to be merged yet?

[KoBeWi]

Also I noticed that update_item_cell() seems to be called twice every time (another in get_minimum_size()).

Only once call will be effective, unless the cell becomes dirty again for some reason.

actually I just noticed a bug

[KoBeWi]

The scene tree is noticeably bigger vertically. The left is with this PR, the right is without:

[maidopi-usagi]

The scene tree is noticeably bigger vertically. The left is with this PR, the right is without:

Thanks for pointing out!
I've found that's because of the cached minimum size computes buttons height using the combination of button texture's size and button_pressed's minimum_size, compared with the old calculation method.

godot/scene/gui/tree.cpp

Lines 1542 to 1552 in f5f813e

	// Buttons.
	for (int i = 0; i < cell.buttons.size(); i++) {
	Ref<Texture2D> texture = cell.buttons[i].texture;
	if (texture.is_valid()) {
	Size2 button_size = texture->get_size() + parent_tree->theme_cache.button_pressed->get_minimum_size();
	size.width += button_size.width + parent_tree->theme_cache.button_margin;
	size.height = MAX(size.height, button_size.height);
	}
	}

As cached minimum size was used only for width calculation before this PR, I'm keeping the width combination here.
To match the original Tree::compute_item_height's implementation, I just discard the height of button button_pressed's minimum size. Or should it use the maximum height instead?

[KoBeWi]

Or should it use the maximum height instead?

There is some weird behavior when button_pressed is bigger than other styles, but it's the same as the one on master, so I think it's fine.

[KoBeWi]

Needs rebase.

[Repiteo]

Thanks!