From ec6520ad9a0ae918e4f966631a0f60975bca79c8 Mon Sep 17 00:00:00 2001 From: jss2a98aj Date: Sun, 12 Jan 2025 15:17:25 -0500 Subject: [PATCH] Revert "Improve Tree performance" This reverts commit d48123e5560a958dc24bf097e1f1d1a95a0f828c. --- scene/gui/tree.cpp | 63 +++++++++-------- scene/gui/tree.h | 1 - tests/scene/test_tree.h | 151 ---------------------------------------- tests/test_main.cpp | 1 - 4 files changed, 34 insertions(+), 182 deletions(-) delete mode 100644 tests/scene/test_tree.h diff --git a/scene/gui/tree.cpp b/scene/gui/tree.cpp index c5ba3f4acf6b..05897f7af011 100644 --- a/scene/gui/tree.cpp +++ b/scene/gui/tree.cpp @@ -802,21 +802,17 @@ TreeItem *TreeItem::create_child(int p_index) { TreeItem *item_prev = nullptr; TreeItem *item_next = first_child; - if (p_index < 0 && last_child) { - item_prev = last_child; - } else { - int idx = 0; - while (item_next) { - if (idx == p_index) { - item_next->prev = ti; - ti->next = item_next; - break; - } - - item_prev = item_next; - item_next = item_next->next; - idx++; + int idx = 0; + while (item_next) { + if (idx == p_index) { + item_next->prev = ti; + ti->next = item_next; + break; } + + item_prev = item_next; + item_next = item_next->next; + idx++; } if (item_prev) { @@ -837,10 +833,6 @@ TreeItem *TreeItem::create_child(int p_index) { } } - if (item_prev == last_child) { - last_child = ti; - } - ti->parent = this; ti->parent_visible_in_tree = is_visible_in_tree(); @@ -857,13 +849,17 @@ void TreeItem::add_child(TreeItem *p_item) { p_item->parent_visible_in_tree = is_visible_in_tree(); p_item->_handle_visibility_changed(p_item->parent_visible_in_tree); - if (last_child) { - last_child->next = p_item; - p_item->prev = last_child; + TreeItem *item_prev = first_child; + while (item_prev && item_prev->next) { + item_prev = item_prev->next; + } + + if (item_prev) { + item_prev->next = p_item; + p_item->prev = item_prev; } else { first_child = p_item; } - last_child = p_item; if (!children_cache.is_empty()) { children_cache.append(p_item); @@ -943,8 +939,13 @@ TreeItem *TreeItem::_get_prev_in_tree(bool p_wrap, bool p_include_invisible) { } } else { current = prev_item; - while ((!current->collapsed || p_include_invisible) && current->last_child) { - current = current->last_child; + while ((!current->collapsed || p_include_invisible) && current->first_child) { + //go to the very end + + current = current->first_child; + while (current->next) { + current = current->next; + } } } @@ -1065,8 +1066,6 @@ void TreeItem::clear_children() { } first_child = nullptr; - last_child = nullptr; - children_cache.clear(); }; int TreeItem::get_index() { @@ -1171,7 +1170,6 @@ void TreeItem::move_after(TreeItem *p_item) { if (next) { parent->children_cache.clear(); } else { - parent->last_child = this; // If the cache is empty, it has not been built but there // are items in the tree (note p_item != nullptr,) so we cannot update it. if (!parent->children_cache.is_empty()) { @@ -4500,8 +4498,15 @@ TreeItem *Tree::get_root() const { TreeItem *Tree::get_last_item() const { TreeItem *last = root; - while (last && last->last_child && !last->collapsed) { - last = last->last_child; + + while (last) { + if (last->next) { + last = last->next; + } else if (last->first_child && !last->collapsed) { + last = last->first_child; + } else { + break; + } } return last; diff --git a/scene/gui/tree.h b/scene/gui/tree.h index dcc793f1e1e7..e021acb0642e 100644 --- a/scene/gui/tree.h +++ b/scene/gui/tree.h @@ -140,7 +140,6 @@ class TreeItem : public Object { TreeItem *prev = nullptr; // previous in list TreeItem *next = nullptr; // next in list TreeItem *first_child = nullptr; - TreeItem *last_child = nullptr; Vector children_cache; bool is_root = false; // for tree root diff --git a/tests/scene/test_tree.h b/tests/scene/test_tree.h deleted file mode 100644 index 41ef39d62178..000000000000 --- a/tests/scene/test_tree.h +++ /dev/null @@ -1,151 +0,0 @@ -/**************************************************************************/ -/* test_tree.h */ -/**************************************************************************/ -/* This file is part of: */ -/* GODOT ENGINE */ -/* https://godotengine.org */ -/**************************************************************************/ -/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ -/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ -/* */ -/* Permission is hereby granted, free of charge, to any person obtaining */ -/* a copy of this software and associated documentation files (the */ -/* "Software"), to deal in the Software without restriction, including */ -/* without limitation the rights to use, copy, modify, merge, publish, */ -/* distribute, sublicense, and/or sell copies of the Software, and to */ -/* permit persons to whom the Software is furnished to do so, subject to */ -/* the following conditions: */ -/* */ -/* The above copyright notice and this permission notice shall be */ -/* included in all copies or substantial portions of the Software. */ -/* */ -/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ -/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ -/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ -/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ -/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ -/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ -/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -/**************************************************************************/ - -#ifndef TEST_TREE_H -#define TEST_TREE_H - -#include "scene/gui/tree.h" - -#include "tests/test_macros.h" - -namespace TestTree { - -TEST_CASE("[SceneTree][Tree]") { - SUBCASE("[Tree] Create and remove items.") { - Tree *tree = memnew(Tree); - TreeItem *root = tree->create_item(); - - TreeItem *child1 = tree->create_item(); - CHECK_EQ(root->get_child_count(), 1); - - TreeItem *child2 = tree->create_item(root); - CHECK_EQ(root->get_child_count(), 2); - - TreeItem *child3 = tree->create_item(root, 0); - CHECK_EQ(root->get_child_count(), 3); - - CHECK_EQ(root->get_child(0), child3); - CHECK_EQ(root->get_child(1), child1); - CHECK_EQ(root->get_child(2), child2); - - root->remove_child(child3); - CHECK_EQ(root->get_child_count(), 2); - - root->add_child(child3); - CHECK_EQ(root->get_child_count(), 3); - - TreeItem *child4 = root->create_child(); - CHECK_EQ(root->get_child_count(), 4); - - CHECK_EQ(root->get_child(0), child1); - CHECK_EQ(root->get_child(1), child2); - CHECK_EQ(root->get_child(2), child3); - CHECK_EQ(root->get_child(3), child4); - - memdelete(tree); - } - - SUBCASE("[Tree] Clear items.") { - Tree *tree = memnew(Tree); - TreeItem *root = tree->create_item(); - - for (int i = 0; i < 10; i++) { - tree->create_item(); - } - CHECK_EQ(root->get_child_count(), 10); - - root->clear_children(); - CHECK_EQ(root->get_child_count(), 0); - - memdelete(tree); - } - - SUBCASE("[Tree] Get last item.") { - Tree *tree = memnew(Tree); - TreeItem *root = tree->create_item(); - - TreeItem *last; - for (int i = 0; i < 10; i++) { - last = tree->create_item(); - } - CHECK_EQ(root->get_child_count(), 10); - CHECK_EQ(tree->get_last_item(), last); - - // Check nested. - TreeItem *old_last = last; - for (int i = 0; i < 10; i++) { - last = tree->create_item(old_last); - } - CHECK_EQ(tree->get_last_item(), last); - - memdelete(tree); - } - - SUBCASE("[Tree] Previous and Next items.") { - Tree *tree = memnew(Tree); - TreeItem *root = tree->create_item(); - - TreeItem *child1 = tree->create_item(); - TreeItem *child2 = tree->create_item(); - TreeItem *child3 = tree->create_item(); - CHECK_EQ(child1->get_next(), child2); - CHECK_EQ(child1->get_next_in_tree(), child2); - CHECK_EQ(child2->get_next(), child3); - CHECK_EQ(child2->get_next_in_tree(), child3); - CHECK_EQ(child3->get_next(), nullptr); - CHECK_EQ(child3->get_next_in_tree(), nullptr); - - CHECK_EQ(child1->get_prev(), nullptr); - CHECK_EQ(child1->get_prev_in_tree(), root); - CHECK_EQ(child2->get_prev(), child1); - CHECK_EQ(child2->get_prev_in_tree(), child1); - CHECK_EQ(child3->get_prev(), child2); - CHECK_EQ(child3->get_prev_in_tree(), child2); - - TreeItem *nested1 = tree->create_item(child2); - TreeItem *nested2 = tree->create_item(child2); - TreeItem *nested3 = tree->create_item(child2); - - CHECK_EQ(child1->get_next(), child2); - CHECK_EQ(child1->get_next_in_tree(), child2); - CHECK_EQ(child2->get_next(), child3); - CHECK_EQ(child2->get_next_in_tree(), nested1); - CHECK_EQ(child3->get_prev(), child2); - CHECK_EQ(child3->get_prev_in_tree(), nested3); - CHECK_EQ(nested1->get_prev_in_tree(), child2); - CHECK_EQ(nested1->get_next_in_tree(), nested2); - - memdelete(tree); - } -} - -} // namespace TestTree - -#endif // TEST_TREE_H diff --git a/tests/test_main.cpp b/tests/test_main.cpp index 4029aa8e36a7..7f72a89eedf6 100644 --- a/tests/test_main.cpp +++ b/tests/test_main.cpp @@ -135,7 +135,6 @@ #include "tests/scene/test_color_picker.h" #include "tests/scene/test_graph_node.h" #include "tests/scene/test_text_edit.h" -#include "tests/scene/test_tree.h" #endif // ADVANCED_GUI_DISABLED #ifndef _3D_DISABLED