Skip to content

Commit

Permalink
Improve performance for TreeSearch (#1)
Browse files Browse the repository at this point in the history
* Improve TreeSearch performance.

Experimental, hence this is on a different branch.

This commit vastly improves performance by not updating the tree
for search mask changes.

Relates to: limbonaut#229

* Fix SearchTree overdraw after performance optimization

* Manage Performance optimizations: TreeSearch no. 2
- Carefully manage callable_cache
- Only clear filter when previously filtered
- Reintroduce sorting for ordered_tree_items

This commit addresses performance issues in TreeSearch and fixes
a critical bug where ordered_tree_items was not being sorted.
The bug was introduced during a merge with the main feature branch.

* Use queue_redraw as much as possible for Tree updates.

* Fix TreeSearch after performance considerations
  • Loading branch information
monxa authored Sep 29, 2024
1 parent cb163eb commit e68e023
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 41 deletions.
4 changes: 2 additions & 2 deletions editor/task_tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,8 @@ void TaskTree::_notification(int p_what) {
tree->connect("multi_selected", callable_mp(this, &TaskTree::_on_item_selected).unbind(3), CONNECT_DEFERRED);
tree->connect("item_activated", callable_mp(this, &TaskTree::_on_item_activated));
tree->connect("item_collapsed", callable_mp(this, &TaskTree::_on_item_collapsed));
tree_search_panel->connect("update_requested", callable_mp(this, &TaskTree::_update_tree));
tree_search_panel->connect("visibility_changed", callable_mp(this, &TaskTree::_update_tree));
tree_search_panel->connect("update_requested", callable_mp(tree_search.ptr(), &TreeSearch::update_search).bind(tree));
tree_search_panel->connect("visibility_changed", callable_mp(tree_search.ptr(), &TreeSearch::update_search).bind(tree));
} break;
case NOTIFICATION_THEME_CHANGED: {
_do_update_theme_item_cache();
Expand Down
121 changes: 83 additions & 38 deletions editor/tree_search.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,20 @@

/* ------- TreeSearch ------- */

void TreeSearch::_clean_callable_cache() {
ERR_FAIL_COND(!tree_reference);
HashMap<TreeItem *, Callable> new_callable_cache;
new_callable_cache.reserve(callable_cache.size()); // Efficiency

for (int i = 0; i < ordered_tree_items.size(); i++) {
TreeItem *cur_item = ordered_tree_items[i];
if (callable_cache.has(cur_item)) {
new_callable_cache[cur_item] = callable_cache[cur_item];
}
}
callable_cache = new_callable_cache;
}

void TreeSearch::_filter_tree(const String &p_search_mask) {
if (matching_entries.size() == 0) {
return;
Expand All @@ -63,40 +77,64 @@ void TreeSearch::_filter_tree(const String &p_search_mask) {
}
}

void TreeSearch::_highlight_tree(const String &p_search_mask) {
callable_cache.clear();
for (int i = 0; i < ordered_tree_items.size(); i++) {
TreeItem *entry = ordered_tree_items[i];
// makes all tree items visible.
void TreeSearch::_clear_filter() {
ERR_FAIL_COND(!tree_reference);
Vector<TreeItem *> items = { tree_reference->get_root() };
for (int idx = 0; idx < items.size(); idx++) {
TreeItem *cur_item = items[idx];
cur_item->set_visible(true);

int num_m = number_matches.has(entry) ? number_matches.get(entry) : 0;
if (num_m == 0) {
continue;
for (int i = 0; i < cur_item->get_child_count(); i++) {
items.push_back(cur_item->get_child(i));
}
}
}

// make sure to also call any draw method already defined.
Callable parent_draw_method;
if (entry->get_cell_mode(0) == TreeItem::CELL_MODE_CUSTOM) {
parent_draw_method = entry->get_custom_draw_callback(0);
}
void TreeSearch::_highlight_tree() {
ERR_FAIL_COND(!tree_reference);
for (int i = 0; i < matching_entries.size(); i++) {
TreeItem *tree_item = matching_entries[i];
_highlight_tree_item(tree_item);
}
tree_reference->queue_redraw();
}

Callable draw_callback = callable_mp(this, &TreeSearch::_draw_highlight_item).bind(parent_draw_method);
void TreeSearch::_highlight_tree_item(TreeItem *p_tree_item) {
int num_m = number_matches.has(p_tree_item) ? number_matches.get(p_tree_item) : 0;

// -- this is necessary because of the modularity of this implementation
// cache render properties of entry
String cached_text = entry->get_text(0);
Ref<Texture2D> cached_icon = entry->get_icon(0);
int cached_max_width = entry->get_icon_max_width(0);
callable_cache[entry] = draw_callback;
if (num_m == 0) {
return;
}

// this removes render properties in entry
entry->set_custom_draw_callback(0, draw_callback);
entry->set_cell_mode(0, TreeItem::CELL_MODE_CUSTOM);
// make sure to also call any draw method already defined.
Callable parent_draw_method;
if (p_tree_item->get_cell_mode(0) == TreeItem::CELL_MODE_CUSTOM) {
parent_draw_method = p_tree_item->get_custom_draw_callback(0);
}

// restore render properties
entry->set_text(0, cached_text);
entry->set_icon(0, cached_icon);
entry->set_icon_max_width(0, cached_max_width);
// if the cached draw method is already applied, do nothing.
if (callable_cache.has(p_tree_item) && parent_draw_method == callable_cache.get(p_tree_item)){
return;
}

Callable draw_callback = callable_mp(this, &TreeSearch::_draw_highlight_item).bind(parent_draw_method);

// -- this is necessary because of the modularity of this implementation
// cache render properties of entry
String cached_text = p_tree_item->get_text(0);
Ref<Texture2D> cached_icon = p_tree_item->get_icon(0);
int cached_max_width = p_tree_item->get_icon_max_width(0);
callable_cache[p_tree_item] = draw_callback;

// this removes render properties in entry
p_tree_item->set_custom_draw_callback(0, draw_callback);
p_tree_item->set_cell_mode(0, TreeItem::CELL_MODE_CUSTOM);

// restore render properties
p_tree_item->set_text(0, cached_text);
p_tree_item->set_icon(0, cached_icon);
p_tree_item->set_icon_max_width(0, cached_max_width);
}

// custom draw callback for highlighting (bind the parent_drw_method to this)
Expand Down Expand Up @@ -143,7 +181,7 @@ void TreeSearch::_draw_highlight_item(TreeItem *p_tree_item, Rect2 p_rect, Calla

Vector2 rect_offset = Vector2(substring_before_size.x, 0);
rect_offset.x += p_tree_item->get_icon_max_width(0);
rect_offset.x += (h_sep + 4. * EDSCALE); // TODO: Find better way to determine texts x-offset
rect_offset.x += (h_sep + 4. * EDSCALE);
rect_offset.y = (p_rect.size.y - substring_match_size.y) / 2; // center box vertically

draw_rect.position += rect_offset - PADDING / 2;
Expand Down Expand Up @@ -354,24 +392,26 @@ void TreeSearch::notify_item_edited(TreeItem *item) {
if (item->get_cell_mode(0) != TreeItem::CELL_MODE_CUSTOM) {
return;
}

if (!callable_cache.has(item) || item->get_custom_draw_callback(0) == callable_cache.get(item)) {
return;
}

item->set_custom_draw_callback(0, callable_cache.get(item));
_highlight_tree_item(item);
}

// Call this as a post-processing step for the already constructed tree.
void TreeSearch::update_search(Tree *p_tree) {
ERR_FAIL_COND(!search_panel || !p_tree);

// ignore if panel not visible or no search string is given.
tree_reference = p_tree;

if (!search_panel->is_visible() || search_panel->get_text().length() == 0) {
// clear and redraw if search was active recently.
if (was_searched_recently) {
_clear_filter();
matching_entries.clear();
was_searched_recently = false;
p_tree->queue_redraw();
}
return;
}

tree_reference = p_tree;
was_searched_recently = true;

String search_mask = search_panel->get_text();
TreeSearchMode search_mode = search_panel->get_search_mode();
Expand All @@ -380,11 +420,16 @@ void TreeSearch::update_search(Tree *p_tree) {
_update_matching_entries(search_mask);
_update_number_matches();

_highlight_tree(search_mask);

_clear_filter();
_highlight_tree();
if (search_mode == TreeSearchMode::FILTER) {
_filter_tree(search_mask);
was_filtered_recently = true;
} else if (was_filtered_recently) {
_clear_filter();
was_filtered_recently = false;
}
_clean_callable_cache();
}

TreeSearch::TreeSearch(TreeSearchPanel *p_search_panel) {
Expand Down
11 changes: 10 additions & 1 deletion editor/tree_search.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,23 @@ class TreeSearch : public RefCounted {

// For TaskTree: These are updated when the tree is updated through TaskTree::_create_tree.
Tree *tree_reference;

Vector<TreeItem *> ordered_tree_items;
Vector<TreeItem *> matching_entries;
HashMap<TreeItem *, int> number_matches;
HashMap<TreeItem *, Callable> callable_cache;

bool was_searched_recently = false; // Performance
bool was_filtered_recently = false; // Performance

void _clean_callable_cache();

// Update_search() calls these
void _filter_tree(const String &p_search_mask);
void _highlight_tree(const String &p_search_mask);
void _clear_filter();

void _highlight_tree();
void _highlight_tree_item(TreeItem *p_tree_item);

// Custom draw-Callback (bind inherited Callable).
void _draw_highlight_item(TreeItem *p_tree_item, Rect2 p_rect, Callable p_parent_draw_method);
Expand Down

0 comments on commit e68e023

Please sign in to comment.