Merge e8de3adc50 into 60a767032e

* 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: https://github.com/limbonaut/limboai/pull/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

editor/task_tree.cpp

@@ -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();

editor/tree_search.cpp

@@ -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;
@@ -63,44 +77,69 @@ 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);
+
+		for (int i = 0; i < cur_item->get_child_count(); i++) {
+			items.push_back(cur_item->get_child(i));
+		}
+	}
+}
+
+void TreeSearch::_highlight_tree() {
+	ERR_FAIL_COND(!tree_reference);
+	// This might not be the prettiest, but it is the most efficient solution probably.
+	for (HashMap<TreeItem *, int>::Iterator it = number_matches.begin(); it != number_matches.end(); ++it) {
+		TreeItem *tree_item = it->key;
+		_highlight_tree_item(tree_item);
+	}
+	tree_reference->queue_redraw();
+}
+
+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;
 
-		int num_m = number_matches.has(entry) ? number_matches.get(entry) : 0;
 	if (num_m == 0) {
-			continue;
+		return;
 	}
 
 	// 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);
+	if (p_tree_item->get_cell_mode(0) == TreeItem::CELL_MODE_CUSTOM) {
+		parent_draw_method = p_tree_item->get_custom_draw_callback(0);
+	}
+
+	// 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 = 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;
+	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
-		entry->set_custom_draw_callback(0, draw_callback);
-		entry->set_cell_mode(0, TreeItem::CELL_MODE_CUSTOM);
+	p_tree_item->set_custom_draw_callback(0, draw_callback);
+	p_tree_item->set_cell_mode(0, TreeItem::CELL_MODE_CUSTOM);
 
 	// restore render properties
-		entry->set_text(0, cached_text);
-		entry->set_icon(0, cached_icon);
-		entry->set_icon_max_width(0, cached_max_width);
-	}
+	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)
-void TreeSearch::_draw_highlight_item(TreeItem *p_tree_item, Rect2 p_rect, Callable p_parent_draw_method) {
+void TreeSearch::_draw_highlight_item(TreeItem *p_tree_item, const Rect2 p_rect, const Callable p_parent_draw_method) {
 	if (!p_tree_item) {
 		return;
 	}
@@ -143,7 +182,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;
@@ -212,12 +251,12 @@ void TreeSearch::_update_number_matches() {
 	}
 }
 
-String TreeSearch::_get_search_mask() {
+String TreeSearch::_get_search_mask() const {
 	ERR_FAIL_COND_V(!search_panel, "");
 	return search_panel->get_text();
 }
 
-void TreeSearch::_find_matching_entries(TreeItem *p_tree_item, const String &p_search_mask, Vector<TreeItem *> &p_accum) {
+void TreeSearch::_find_matching_entries(TreeItem *p_tree_item, const String &p_search_mask, Vector<TreeItem *> &p_accum) const {
 	if (!p_tree_item) {
 		return;
 	}
@@ -317,12 +356,13 @@ void TreeSearch::_select_next_match() {
 	if (matching_entries.size() == 0) {
 		return;
 	}
-	TreeItem *selected = tree_reference->get_selected(); // we care about a single item here.
+	TreeItem *selected = tree_reference->get_selected();
 	if (!selected) {
 		_select_first_match();
 		return;
 	}
 
+	// find [selected_idx] among ordered_tree_items
 	int selected_idx = -1;
 	for (int i = 0; i < ordered_tree_items.size(); i++) {
 		if (ordered_tree_items[i] == selected) {
@@ -331,7 +371,7 @@ void TreeSearch::_select_next_match() {
 		}
 	}
 
-	// find the best fitting entry.
+	// find first entry after [selected_idx].
 	for (int i = MAX(0, selected_idx) + 1; i < ordered_tree_items.size(); i++) {
 		TreeItem *item = ordered_tree_items[i];
 		if (_vector_has_bsearch(matching_entries, item)) {
@@ -339,11 +379,12 @@ void TreeSearch::_select_next_match() {
 			return;
 		}
 	}
-	_select_first_match(); // wrap around.
+	// wrap around.
+	_select_first_match();
 }
 
 template <typename T>
-inline bool TreeSearch::_vector_has_bsearch(Vector<T *> p_vec, T *element) {
+inline bool TreeSearch::_vector_has_bsearch(Vector<T *> &p_vec, T *element) const {
 	int idx = p_vec.bsearch(element, true);
 	bool in_array = idx >= 0 && idx < p_vec.size();
 
@@ -354,24 +395,27 @@ 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();
+			number_matches.clear();
+			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();
@@ -380,11 +424,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) {
@@ -464,14 +513,14 @@ TreeSearchPanel::TreeSearchPanel() {
 	set_visible(false);
 }
 
-TreeSearch::TreeSearchMode TreeSearchPanel::get_search_mode() {
+TreeSearch::TreeSearchMode TreeSearchPanel::get_search_mode() const {
 	if (!check_button_filter_highlight || !check_button_filter_highlight->is_pressed()) {
 		return TreeSearch::TreeSearchMode::HIGHLIGHT;
 	}
 	return TreeSearch::TreeSearchMode::FILTER;
 }
 
-String TreeSearchPanel::get_text() {
+String TreeSearchPanel::get_text() const {
 	if (!line_edit_search) {
 		return String();
 	}

editor/tree_search.h

@@ -53,32 +53,46 @@ private:
 
 	// For TaskTree: These are updated when the tree is updated through TaskTree::_create_tree.
 	Tree *tree_reference;
+	// linearized ordering of tree items.
 	Vector<TreeItem *> ordered_tree_items;
+	// entires that match the search mask.
+	// TODO: Decide if this can be removed. It can be implicitly inferred from number_matches.
 	Vector<TreeItem *> matching_entries;
+	// number of descendant matches for each tree item.
 	HashMap<TreeItem *, int> number_matches;
+	// custom draw-callbacks for each tree item.
 	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);
+	void _draw_highlight_item(TreeItem *p_tree_item, const Rect2 p_rect, const Callable p_parent_draw_method);
 
 	void _update_matching_entries(const String &p_search_mask);
 	void _update_ordered_tree_items(TreeItem *p_tree_item);
 	void _update_number_matches();
 
-	void _find_matching_entries(TreeItem *p_tree_item, const String &p_search_mask, Vector<TreeItem *> &p_accum);
-	String _get_search_mask();
+	void _find_matching_entries(TreeItem *p_tree_item, const String &p_search_mask, Vector<TreeItem *> &p_accum) const;
+	String _get_search_mask() const;
 	StringSearchIndices _substring_bounds(const String &p_searchable, const String &p_search_mask) const;
 
 	void _select_item(TreeItem *p_item);
 	void _select_first_match();
 	void _select_next_match();
 
+	// TODO: make p_vec ref `const` once Vector::bsearch is const. See: https://github.com/godotengine/godot/pull/90341
 	template <typename T>
-	bool _vector_has_bsearch(Vector<T *> p_vec, T *element);
+	bool _vector_has_bsearch(Vector<T *> &p_vec, T *element) const;
 
 protected:
 	static void _bind_methods() {}
@@ -110,15 +124,14 @@ private:
 	void _initialize_controls();
 	void _add_spacer(float width_multiplier = 1.f);
 
-	void _on_draw_highlight(TreeItem *p_item, Rect2 p_rect);
 	void _notification(int p_what);
 
 protected:
 	static void _bind_methods();
 
 public:
-	TreeSearch::TreeSearchMode get_search_mode();
-	String get_text();
+	TreeSearch::TreeSearchMode get_search_mode() const;
+	String get_text() const;
 	void show_and_focus();
 	TreeSearchPanel();
 };