Skip to content

Commit 9e1c63a

Browse files
akien-mgaakien-mga
committed
Merge pull request #94748 from aaronp64/tree_perf
Improve `Tree` performance
2 parents d35bee9 + 040f241 commit 9e1c63a

File tree

4 files changed

+182
-34
lines changed

4 files changed

+182
-34
lines changed

‎scene/gui/tree.cpp

+29-34
Original file line numberDiff line numberDiff line change
@@ -773,17 +773,21 @@ TreeItem *TreeItem::create_child(int p_index) {
773773
TreeItem *item_prev = nullptr;
774774
TreeItem *item_next = first_child;
775775

776-
int idx = 0;
777-
while (item_next) {
778-
if (idx == p_index) {
779-
item_next->prev = ti;
780-
ti->next = item_next;
781-
break;
782-
}
776+
if (p_index < 0 && last_child) {
777+
item_prev = last_child;
778+
} else {
779+
int idx = 0;
780+
while (item_next) {
781+
if (idx == p_index) {
782+
item_next->prev = ti;
783+
ti->next = item_next;
784+
break;
785+
}
783786

784-
item_prev = item_next;
785-
item_next = item_next->next;
786-
idx++;
787+
item_prev = item_next;
788+
item_next = item_next->next;
789+
idx++;
790+
}
787791
}
788792

789793
if (item_prev) {
@@ -804,6 +808,10 @@ TreeItem *TreeItem::create_child(int p_index) {
804808
}
805809
}
806810

811+
if (item_prev == last_child) {
812+
last_child = ti;
813+
}
814+
807815
ti->parent = this;
808816
ti->parent_visible_in_tree = is_visible_in_tree();
809817

@@ -820,17 +828,13 @@ void TreeItem::add_child(TreeItem *p_item) {
820828
p_item->parent_visible_in_tree = is_visible_in_tree();
821829
p_item->_handle_visibility_changed(p_item->parent_visible_in_tree);
822830

823-
TreeItem *item_prev = first_child;
824-
while (item_prev && item_prev->next) {
825-
item_prev = item_prev->next;
826-
}
827-
828-
if (item_prev) {
829-
item_prev->next = p_item;
830-
p_item->prev = item_prev;
831+
if (last_child) {
832+
last_child->next = p_item;
833+
p_item->prev = last_child;
831834
} else {
832835
first_child = p_item;
833836
}
837+
last_child = p_item;
834838

835839
if (!children_cache.is_empty()) {
836840
children_cache.append(p_item);
@@ -910,13 +914,8 @@ TreeItem *TreeItem::_get_prev_in_tree(bool p_wrap, bool p_include_invisible) {
910914
}
911915
} else {
912916
current = prev_item;
913-
while ((!current->collapsed || p_include_invisible) && current->first_child) {
914-
//go to the very end
915-
916-
current = current->first_child;
917-
while (current->next) {
918-
current = current->next;
919-
}
917+
while ((!current->collapsed || p_include_invisible) && current->last_child) {
918+
current = current->last_child;
920919
}
921920
}
922921

@@ -1037,6 +1036,8 @@ void TreeItem::clear_children() {
10371036
}
10381037

10391038
first_child = nullptr;
1039+
last_child = nullptr;
1040+
children_cache.clear();
10401041
};
10411042

10421043
int TreeItem::get_index() {
@@ -1141,6 +1142,7 @@ void TreeItem::move_after(TreeItem *p_item) {
11411142
if (next) {
11421143
parent->children_cache.clear();
11431144
} else {
1145+
parent->last_child = this;
11441146
// If the cache is empty, it has not been built but there
11451147
// are items in the tree (note p_item != nullptr,) so we cannot update it.
11461148
if (!parent->children_cache.is_empty()) {
@@ -4468,15 +4470,8 @@ TreeItem *Tree::get_root() const {
44684470

44694471
TreeItem *Tree::get_last_item() const {
44704472
TreeItem *last = root;
4471-
4472-
while (last) {
4473-
if (last->next) {
4474-
last = last->next;
4475-
} else if (last->first_child && !last->collapsed) {
4476-
last = last->first_child;
4477-
} else {
4478-
break;
4479-
}
4473+
while (last && last->last_child && !last->collapsed) {
4474+
last = last->last_child;
44804475
}
44814476

44824477
return last;

‎scene/gui/tree.h

+1
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ class TreeItem : public Object {
136136
TreeItem *prev = nullptr; // previous in list
137137
TreeItem *next = nullptr; // next in list
138138
TreeItem *first_child = nullptr;
139+
TreeItem *last_child = nullptr;
139140

140141
Vector<TreeItem *> children_cache;
141142
bool is_root = false; // for tree root

‎tests/scene/test_tree.h

+151
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/**************************************************************************/
2+
/* test_tree.h */
3+
/**************************************************************************/
4+
/* This file is part of: */
5+
/* GODOT ENGINE */
6+
/* https://godotengine.org */
7+
/**************************************************************************/
8+
/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */
9+
/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */
10+
/* */
11+
/* Permission is hereby granted, free of charge, to any person obtaining */
12+
/* a copy of this software and associated documentation files (the */
13+
/* "Software"), to deal in the Software without restriction, including */
14+
/* without limitation the rights to use, copy, modify, merge, publish, */
15+
/* distribute, sublicense, and/or sell copies of the Software, and to */
16+
/* permit persons to whom the Software is furnished to do so, subject to */
17+
/* the following conditions: */
18+
/* */
19+
/* The above copyright notice and this permission notice shall be */
20+
/* included in all copies or substantial portions of the Software. */
21+
/* */
22+
/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */
23+
/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */
24+
/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */
25+
/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */
26+
/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */
27+
/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */
28+
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
29+
/**************************************************************************/
30+
31+
#ifndef TEST_TREE_H
32+
#define TEST_TREE_H
33+
34+
#include "scene/gui/tree.h"
35+
36+
#include "tests/test_macros.h"
37+
38+
namespace TestTree {
39+
40+
TEST_CASE("[SceneTree][Tree]") {
41+
SUBCASE("[Tree] Create and remove items.") {
42+
Tree *tree = memnew(Tree);
43+
TreeItem *root = tree->create_item();
44+
45+
TreeItem *child1 = tree->create_item();
46+
CHECK_EQ(root->get_child_count(), 1);
47+
48+
TreeItem *child2 = tree->create_item(root);
49+
CHECK_EQ(root->get_child_count(), 2);
50+
51+
TreeItem *child3 = tree->create_item(root, 0);
52+
CHECK_EQ(root->get_child_count(), 3);
53+
54+
CHECK_EQ(root->get_child(0), child3);
55+
CHECK_EQ(root->get_child(1), child1);
56+
CHECK_EQ(root->get_child(2), child2);
57+
58+
root->remove_child(child3);
59+
CHECK_EQ(root->get_child_count(), 2);
60+
61+
root->add_child(child3);
62+
CHECK_EQ(root->get_child_count(), 3);
63+
64+
TreeItem *child4 = root->create_child();
65+
CHECK_EQ(root->get_child_count(), 4);
66+
67+
CHECK_EQ(root->get_child(0), child1);
68+
CHECK_EQ(root->get_child(1), child2);
69+
CHECK_EQ(root->get_child(2), child3);
70+
CHECK_EQ(root->get_child(3), child4);
71+
72+
memdelete(tree);
73+
}
74+
75+
SUBCASE("[Tree] Clear items.") {
76+
Tree *tree = memnew(Tree);
77+
TreeItem *root = tree->create_item();
78+
79+
for (int i = 0; i < 10; i++) {
80+
tree->create_item();
81+
}
82+
CHECK_EQ(root->get_child_count(), 10);
83+
84+
root->clear_children();
85+
CHECK_EQ(root->get_child_count(), 0);
86+
87+
memdelete(tree);
88+
}
89+
90+
SUBCASE("[Tree] Get last item.") {
91+
Tree *tree = memnew(Tree);
92+
TreeItem *root = tree->create_item();
93+
94+
TreeItem *last;
95+
for (int i = 0; i < 10; i++) {
96+
last = tree->create_item();
97+
}
98+
CHECK_EQ(root->get_child_count(), 10);
99+
CHECK_EQ(tree->get_last_item(), last);
100+
101+
// Check nested.
102+
TreeItem *old_last = last;
103+
for (int i = 0; i < 10; i++) {
104+
last = tree->create_item(old_last);
105+
}
106+
CHECK_EQ(tree->get_last_item(), last);
107+
108+
memdelete(tree);
109+
}
110+
111+
SUBCASE("[Tree] Previous and Next items.") {
112+
Tree *tree = memnew(Tree);
113+
TreeItem *root = tree->create_item();
114+
115+
TreeItem *child1 = tree->create_item();
116+
TreeItem *child2 = tree->create_item();
117+
TreeItem *child3 = tree->create_item();
118+
CHECK_EQ(child1->get_next(), child2);
119+
CHECK_EQ(child1->get_next_in_tree(), child2);
120+
CHECK_EQ(child2->get_next(), child3);
121+
CHECK_EQ(child2->get_next_in_tree(), child3);
122+
CHECK_EQ(child3->get_next(), nullptr);
123+
CHECK_EQ(child3->get_next_in_tree(), nullptr);
124+
125+
CHECK_EQ(child1->get_prev(), nullptr);
126+
CHECK_EQ(child1->get_prev_in_tree(), root);
127+
CHECK_EQ(child2->get_prev(), child1);
128+
CHECK_EQ(child2->get_prev_in_tree(), child1);
129+
CHECK_EQ(child3->get_prev(), child2);
130+
CHECK_EQ(child3->get_prev_in_tree(), child2);
131+
132+
TreeItem *nested1 = tree->create_item(child2);
133+
TreeItem *nested2 = tree->create_item(child2);
134+
TreeItem *nested3 = tree->create_item(child2);
135+
136+
CHECK_EQ(child1->get_next(), child2);
137+
CHECK_EQ(child1->get_next_in_tree(), child2);
138+
CHECK_EQ(child2->get_next(), child3);
139+
CHECK_EQ(child2->get_next_in_tree(), nested1);
140+
CHECK_EQ(child3->get_prev(), child2);
141+
CHECK_EQ(child3->get_prev_in_tree(), nested3);
142+
CHECK_EQ(nested1->get_prev_in_tree(), child2);
143+
CHECK_EQ(nested1->get_next_in_tree(), nested2);
144+
145+
memdelete(tree);
146+
}
147+
}
148+
149+
} // namespace TestTree
150+
151+
#endif // TEST_TREE_H

‎tests/test_main.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@
133133
#include "tests/scene/test_color_picker.h"
134134
#include "tests/scene/test_graph_node.h"
135135
#include "tests/scene/test_text_edit.h"
136+
#include "tests/scene/test_tree.h"
136137
#endif // ADVANCED_GUI_DISABLED
137138

138139
#ifndef _3D_DISABLED

0 commit comments

Comments
 (0)