GUI: Fix Tree performance regression by using cache

[dalexeev]

• Closes The tree in the Scene dock has noticeable lag further down you scroll in 4.1 #79292.
• Closes Too many warnings in the debugger causing editor lag especially in Chinese language and GLES3 (Compatibility) . #79927.

On @YuriSizov's advice, I used cache to optimize the bottleneck. This made Tree responsive again for 10,000 rows, but Tree is still slower than in 4.0.3. My guess is that further optimizations (and possible refactoring) of draw_item() are desirable.

Please test this PR carefully to make sure the cache is invalidated when needed!

Test project: GUITree.zip

Before

After

[dalexeev]

But the scene tree starts to lag like hell the more down you scroll (and gets back to snappy once you scroll back to top).

Note that this behavior is still present, albeit to a lesser extent. Some changes are needed in the algorithm for skipping previous rows, but their height must still be taken into account. Perhaps the problem is that the previous rows are being drawn, although they are invisible. See draw_item().

[YuriSizov]

Perhaps the problem is that the previous rows are being drawn, although they are invisible. See draw_item().

I haven't checked the code, but indeed, we don't need to draw them and I think previously we didn't do that. Can you make the necessary changes so that the previous rows are only used to compute the offset?

[dalexeev]

@YuriSizov Done! Regression caused by this, fixed by this. But I think that caching the minimum width still makes sense.

[YuriSizov]

This seems to be the only place where we care for the returned self height value. So I think this line adding vertical separation can be removed and instead we can assign r_self_height after we add this value to label_h.

[dalexeev]

I'm not sure about whether to include the separator's height in the item's self_height. But I replaced the code with the following:

r_self_height = compute_item_height(p_item);
label_h = r_self_height + theme_cache.v_separation;

[YuriSizov]

It just seems redundant to add it in two places if these are the only two places that use this value. The name may be not the best, so I understand your hesitation. But I'd say this is pretty minor as far as semantics go.

[YuriSizov]

That said, I'm not insisting. We can merge it as is.

[YuriSizov]

I've tested it, and I can confirm that it doesn't fully solve the slowdown when you scroll all the way down on larger trees, but it does significantly improve the experience. So I think we should go with this, at least for now.

One curiosity that I noticed was that I could clearly experience a slowdown in the editor, but not in a test scene. The scene is just a tree and I fill it up as follows:

extends Tree

const ICON = preload("res://icon.svg")

func _init():
	var root := create_item()
	
	for i in range(0, 10000):
		var item := create_item(root)
		item.set_text(0, str(i))
		item.set_icon_max_width(0, 16)
		item.set_icon(0, ICON)

This works much smoother than the scene tree in the editor with 5.8k items in it (which I think I took from the linked issue). I suspect it may be something to do with buttons. I think if I add buttons to each row with item.add_button(0, ICON, -1, false, "Hello"), it does get slightly slower at the bottom.

That's the only difference that I can think of between my test scene and the scene tree view in the editor, as in the editor every node has a visibility toggle in my case.

There are several things to improve about buttons in trees, however, as I outlined in #79792 (comment). So this is something that can be done in a separate PR (I can take a look but I'm giving a chance to the author of the linked PR to work a bit more on their contribution, if they want to).

[dalexeev]

After rebasing, I noticed crashes. They look unrelated to this PR, but they didn't exist before.

handle_crash: Program crashed with signal 4
Engine version: Godot Engine v4.2.dev.custom_build (4bc871ae0f30f435655df4b5e2c214607a2e7e45)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x3c4b0) [0x7f44ee43c4b0] (??:0)
[2] Window::_event_callback(DisplayServer::WindowEvent) (/home/danil/godot-src/scene/main/window.cpp:684 (discriminator 1))
[3] void call_with_variant_args_helper<Window, DisplayServer::WindowEvent, 0ul>(Window*, void (Window::*)(DisplayServer::WindowEvent), Variant const**, Callable::CallError&, IndexSequence<0ul>) (/home/danil/godot-src/./core/variant/binder_common.h:308 (discriminator 4))
[4] void call_with_variant_args<Window, DisplayServer::WindowEvent>(Window*, void (Window::*)(DisplayServer::WindowEvent), Variant const**, int, Callable::CallError&) (/home/danil/godot-src/./core/variant/binder_common.h:418)
[5] CallableCustomMethodPointer<Window, DisplayServer::WindowEvent>::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/danil/godot-src/./core/object/callable_method_pointer.h:105)
[6] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/home/danil/godot-src/core/variant/callable.cpp:64)
[7] DisplayServerX11::_send_window_event(DisplayServerX11::WindowData const&, DisplayServer::WindowEvent) (/home/danil/godot-src/platform/linuxbsd/x11/display_server_x11.cpp:3772)
[8] DisplayServerX11::process_events() (/home/danil/godot-src/platform/linuxbsd/x11/display_server_x11.cpp:4298)
[9] OS_LinuxBSD::run() (/home/danil/godot-src/platform/linuxbsd/os_linuxbsd.cpp:910)
[10] /home/danil/godot-src/bin/godot.linuxbsd.editor.dev.x86_64(main+0x19f) [0x56467c099b48] (/home/danil/godot-src/platform/linuxbsd/godot_linuxbsd.cpp:76)
[11] /lib/x86_64-linux-gnu/libc.so.6(+0x23a90) [0x7f44ee423a90] (??:0)
[12] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x89) [0x7f44ee423b49] (??:0)
[13] /home/danil/godot-src/bin/godot.linuxbsd.editor.dev.x86_64(_start+0x25) [0x56467c0998e5] (??:?)
-- END OF BACKTRACE --

[akien-mga]

Might be related to / fixed by #80187? CC @Sauermann

[Sauermann]

Yes, that crash gets fixed by #80187. I was previously unable to cause this crash on X11, so I will take a look at the Test Project to see, where in the X11-platform-code this problem appears.

[akien-mga]

Thanks!

[YuriSizov]

Cherry-picked for 4.1.2.