Skip to content

Commit 040f241

Browse files
aaronp64aaronp64
committed
Improve Tree performance
Added TreeItem::last_child to avoid needing to iterate through all children to get to the end. This mainly helps in cases where one TreeItem has many children (1000s), and new children are added to the end, as each add had to iterate through all previously added children.
1 parent e343dbb commit 040f241

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)