From 90ea1b76434fe175e99d43da8f463f28a108d0ad Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Thu, 1 Dec 2022 14:53:27 -0600 Subject: [PATCH] Nodes: Use persistent integer to identify to nodes This patch adds an integer identifier to nodes that doesn't change when the node name changes. This identifier can be used by different systems to reference a node. This may be important to store caches and simulation states per node, because otherwise those would always be invalidated when a node name changes. Additionally, this kind of identifier could make some things more efficient, because with it an integer is enough to identify a node and one does not have to store the node name. I observed a 10% improvement in evaluation time in a file with an extreme number of simple math nodes, due to reduced logging overhead-- from 0.226s to 0.205s. Differential Revision: https://developer.blender.org/D15775 --- .../blenkernel/BKE_compute_contexts.hh | 16 +++- source/blender/blenkernel/BKE_node.h | 10 ++- source/blender/blenkernel/BKE_node_runtime.hh | 73 ++++++++++++--- .../blenkernel/intern/compute_contexts.cc | 24 +++-- source/blender/blenkernel/intern/node.cc | 64 +++++++++++-- .../blender/blenkernel/intern/node_runtime.cc | 51 ++++++----- .../blenkernel/intern/node_tree_update.cc | 9 ++ .../blender/blenkernel/intern/viewer_path.cc | 3 +- .../blender/editors/include/ED_viewer_path.hh | 4 +- .../blender/editors/space_node/node_draw.cc | 90 ++++++------------- .../node_geometry_attribute_search.cc | 10 +-- .../blender/editors/space_node/node_group.cc | 15 ++-- source/blender/editors/util/ed_viewer_path.cc | 78 ++++++++-------- source/blender/makesdna/DNA_node_types.h | 21 ++++- .../blender/makesdna/DNA_viewer_path_types.h | 8 ++ source/blender/modifiers/intern/MOD_nodes.cc | 20 +---- .../blender/nodes/NOD_geometry_nodes_log.hh | 18 ++-- .../intern/geometry_nodes_lazy_function.cc | 9 +- .../nodes/intern/geometry_nodes_log.cc | 62 +++++++------ .../nodes/intern/node_geometry_exec.cc | 9 +- .../blender/nodes/shader/node_shader_tree.cc | 5 ++ 21 files changed, 351 insertions(+), 248 deletions(-) diff --git a/source/blender/blenkernel/BKE_compute_contexts.hh b/source/blender/blenkernel/BKE_compute_contexts.hh index a8f0022f49b..8e4bbbba524 100644 --- a/source/blender/blenkernel/BKE_compute_contexts.hh +++ b/source/blender/blenkernel/BKE_compute_contexts.hh @@ -8,6 +8,8 @@ #include "BLI_compute_context.hh" +struct bNode; + namespace blender::bke { class ModifierComputeContext : public ComputeContext { @@ -32,12 +34,20 @@ class NodeGroupComputeContext : public ComputeContext { private: static constexpr const char *s_static_type = "NODE_GROUP"; - std::string node_name_; + int32_t node_id_; + +#ifdef DEBUG + std::string debug_node_name_; +#endif public: - NodeGroupComputeContext(const ComputeContext *parent, std::string node_name); + NodeGroupComputeContext(const ComputeContext *parent, int32_t node_id); + NodeGroupComputeContext(const ComputeContext *parent, const bNode &node); - StringRefNull node_name() const; + int32_t node_id() const + { + return node_id_; + } private: void print_current_in_line(std::ostream &stream) const override; diff --git a/source/blender/blenkernel/BKE_node.h b/source/blender/blenkernel/BKE_node.h index 2cd2fa9ac62..74d6f8becd1 100644 --- a/source/blender/blenkernel/BKE_node.h +++ b/source/blender/blenkernel/BKE_node.h @@ -668,6 +668,7 @@ void nodeUnlinkNode(struct bNodeTree *ntree, struct bNode *node); * Find the first available, non-duplicate name for a given node. */ void nodeUniqueName(struct bNodeTree *ntree, struct bNode *node); +void nodeUniqueID(struct bNodeTree *ntree, struct bNode *node); /** * Delete node, associated animation data and ID user count. @@ -687,16 +688,17 @@ namespace blender::bke { /** * \note keeps socket list order identical, for copying links. - * \note `unique_name` should usually be true, unless the \a dst_tree is temporary, - * or the names can already be assumed valid. + * \param use_unique: If true, make sure the node's identifier and name are unique in the new + * tree. Must be *true* if the \a dst_tree had nodes that weren't in the source node's tree. + * Must be *false* when simply copying a node tree, so that identifiers don't change. */ bNode *node_copy_with_mapping(bNodeTree *dst_tree, const bNode &node_src, int flag, - bool unique_name, + bool use_unique, Map &new_socket_map); -bNode *node_copy(bNodeTree *dst_tree, const bNode &src_node, int flag, bool unique_name); +bNode *node_copy(bNodeTree *dst_tree, const bNode &src_node, int flag, bool use_unique); } // namespace blender::bke diff --git a/source/blender/blenkernel/BKE_node_runtime.hh b/source/blender/blenkernel/BKE_node_runtime.hh index 56d51169934..a2241557315 100644 --- a/source/blender/blenkernel/BKE_node_runtime.hh +++ b/source/blender/blenkernel/BKE_node_runtime.hh @@ -8,6 +8,7 @@ #include "BLI_multi_value_map.hh" #include "BLI_utility_mixins.hh" #include "BLI_vector.hh" +#include "BLI_vector_set.hh" #include "DNA_node_types.h" @@ -24,6 +25,36 @@ class NodeDeclaration; struct GeometryNodesLazyFunctionGraphInfo; } // namespace blender::nodes +namespace blender { + +struct NodeIDHash { + uint64_t operator()(const bNode *node) const + { + return node->identifier; + } + uint64_t operator()(const int32_t id) const + { + return id; + } +}; + +struct NodeIDEquality { + bool operator()(const bNode *a, const bNode *b) const + { + return a->identifier == b->identifier; + } + bool operator()(const bNode *a, const int32_t b) const + { + return a->identifier == b; + } + bool operator()(const int32_t a, const bNode *b) const + { + return this->operator()(b, a); + } +}; + +} // namespace blender + namespace blender::bke { class bNodeTreeRuntime : NonCopyable, NonMovable { @@ -46,6 +77,13 @@ class bNodeTreeRuntime : NonCopyable, NonMovable { */ uint8_t runtime_flag = 0; + /** + * Storage of nodes based on their identifier. Also used as a contiguous array of nodes to + * allow simpler and more cache friendly iteration. Supports lookup by integer or by node. + * Unlike other caches, this is maintained eagerly while changing the tree. + */ + VectorSet nodes_by_id; + /** Execution data. * * XXX It would be preferable to completely move this data out of the underlying node tree, @@ -91,7 +129,6 @@ class bNodeTreeRuntime : NonCopyable, NonMovable { mutable std::atomic allow_use_dirty_topology_cache = 0; /** Only valid when #topology_cache_is_dirty is false. */ - Vector nodes; Vector links; Vector sockets; Vector input_sockets; @@ -292,6 +329,28 @@ inline bool topology_cache_is_available(const bNodeSocket &socket) /** \name #bNodeTree Inline Methods * \{ */ +inline blender::Span bNodeTree::all_nodes() const +{ + return this->runtime->nodes_by_id.as_span(); +} + +inline blender::Span bNodeTree::all_nodes() +{ + return this->runtime->nodes_by_id; +} + +inline bNode *bNodeTree::node_by_id(const int32_t identifier) +{ + bNode *const *node = this->runtime->nodes_by_id.lookup_key_ptr_as(identifier); + return node ? *node : nullptr; +} + +inline const bNode *bNodeTree::node_by_id(const int32_t identifier) const +{ + const bNode *const *node = this->runtime->nodes_by_id.lookup_key_ptr_as(identifier); + return node ? *node : nullptr; +} + inline blender::Span bNodeTree::nodes_by_type(const blender::StringRefNull type_idname) { BLI_assert(blender::bke::node_tree_runtime::topology_cache_is_available(*this)); @@ -329,18 +388,6 @@ inline blender::Span bNodeTree::toposort_right_to_left() return this->runtime->toposort_right_to_left; } -inline blender::Span bNodeTree::all_nodes() const -{ - BLI_assert(blender::bke::node_tree_runtime::topology_cache_is_available(*this)); - return this->runtime->nodes; -} - -inline blender::Span bNodeTree::all_nodes() -{ - BLI_assert(blender::bke::node_tree_runtime::topology_cache_is_available(*this)); - return this->runtime->nodes; -} - inline blender::Span bNodeTree::group_nodes() const { BLI_assert(blender::bke::node_tree_runtime::topology_cache_is_available(*this)); diff --git a/source/blender/blenkernel/intern/compute_contexts.cc b/source/blender/blenkernel/intern/compute_contexts.cc index 026706d363e..4d0fba49f85 100644 --- a/source/blender/blenkernel/intern/compute_contexts.cc +++ b/source/blender/blenkernel/intern/compute_contexts.cc @@ -1,5 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#include "DNA_node_types.h" + #include "BKE_compute_contexts.hh" namespace blender::bke { @@ -17,22 +19,30 @@ void ModifierComputeContext::print_current_in_line(std::ostream &stream) const stream << "Modifier: " << modifier_name_; } -NodeGroupComputeContext::NodeGroupComputeContext(const ComputeContext *parent, - std::string node_name) - : ComputeContext(s_static_type, parent), node_name_(std::move(node_name)) +NodeGroupComputeContext::NodeGroupComputeContext(const ComputeContext *parent, const int node_id) + : ComputeContext(s_static_type, parent), node_id_(node_id) { hash_.mix_in(s_static_type, strlen(s_static_type)); - hash_.mix_in(node_name_.data(), node_name_.size()); + hash_.mix_in(&node_id_, sizeof(int32_t)); } -StringRefNull NodeGroupComputeContext::node_name() const +NodeGroupComputeContext::NodeGroupComputeContext(const ComputeContext *parent, const bNode &node) + : NodeGroupComputeContext(parent, node.identifier) { - return node_name_; +#ifdef DEBUG + debug_node_name_ = node.name; +#endif } void NodeGroupComputeContext::print_current_in_line(std::ostream &stream) const { - stream << "Node: " << node_name_; +#ifdef DEBUG + if (!debug_node_name_.empty()) { + stream << "Node: " << debug_node_name_; + return; + } +#endif + stream << "Node ID: " << node_id_; } } // namespace blender::bke diff --git a/source/blender/blenkernel/intern/node.cc b/source/blender/blenkernel/intern/node.cc index 7c437f8fa9f..957150441ea 100644 --- a/source/blender/blenkernel/intern/node.cc +++ b/source/blender/blenkernel/intern/node.cc @@ -36,6 +36,7 @@ #include "BLI_listbase.h" #include "BLI_map.hh" #include "BLI_path_util.h" +#include "BLI_rand.hh" #include "BLI_set.hh" #include "BLI_stack.hh" #include "BLI_string.h" @@ -84,6 +85,8 @@ #include "BLO_read_write.h" +#include "PIL_time.h" + #define NODE_DEFAULT_MAX_WIDTH 700 using blender::Array; @@ -146,12 +149,14 @@ static void ntree_copy_data(Main * /*bmain*/, ID *id_dst, const ID *id_src, cons Map node_map; Map socket_map; + ntree_dst->runtime->nodes_by_id.reserve(ntree_src->all_nodes().size()); BLI_listbase_clear(&ntree_dst->nodes); LISTBASE_FOREACH (const bNode *, src_node, &ntree_src->nodes) { /* Don't find a unique name for every node, since they should have valid names already. */ bNode *new_node = blender::bke::node_copy_with_mapping( ntree_dst, *src_node, flag_subdata, false, socket_map); node_map.add(src_node, new_node); + ntree_dst->runtime->nodes_by_id.add_new(new_node); } /* copy links */ @@ -679,6 +684,15 @@ void ntreeBlendReadData(BlendDataReader *reader, ID *owner_id, bNodeTree *ntree) node->runtime = MEM_new(__func__); node->typeinfo = nullptr; + /* Create the `nodes_by_id` cache eagerly so it can be expected to be valid. Because + * we create it here we also have to check for zero identifiers from previous versions. */ + if (ntree->runtime->nodes_by_id.contains_as(node->identifier)) { + nodeUniqueID(ntree, node); + } + else { + ntree->runtime->nodes_by_id.add_new(node); + } + BLO_read_list(reader, &node->inputs); BLO_read_list(reader, &node->outputs); @@ -764,6 +778,7 @@ void ntreeBlendReadData(BlendDataReader *reader, ID *owner_id, bNodeTree *ntree) } } BLO_read_list(reader, &ntree->links); + BLI_assert(ntree->all_nodes().size() == BLI_listbase_count(&ntree->nodes)); /* and we connect the rest */ LISTBASE_FOREACH (bNode *, node, &ntree->nodes) { @@ -2176,11 +2191,28 @@ void nodeUniqueName(bNodeTree *ntree, bNode *node) &ntree->nodes, node, DATA_("Node"), '.', offsetof(bNode, name), sizeof(node->name)); } +void nodeUniqueID(bNodeTree *ntree, bNode *node) +{ + /* Use a pointer cast to avoid overflow warnings. */ + const double time = PIL_check_seconds_timer() * 1000000.0; + blender::RandomNumberGenerator id_rng{*reinterpret_cast(&time)}; + + /* In the unlikely case that the random ID doesn't match, choose a new one until it does. */ + int32_t new_id = id_rng.get_int32(); + while (ntree->runtime->nodes_by_id.contains_as(new_id)) { + new_id = id_rng.get_int32(); + } + + node->identifier = new_id; + ntree->runtime->nodes_by_id.add_new(node); +} + bNode *nodeAddNode(const bContext *C, bNodeTree *ntree, const char *idname) { bNode *node = MEM_cnew("new node"); node->runtime = MEM_new(__func__); BLI_addtail(&ntree->nodes, node); + nodeUniqueID(ntree, node); BLI_strncpy(node->idname, idname, sizeof(node->idname)); node_set_typeinfo(C, ntree, node, nodeTypeFind(idname)); @@ -2241,7 +2273,7 @@ namespace blender::bke { bNode *node_copy_with_mapping(bNodeTree *dst_tree, const bNode &node_src, const int flag, - const bool unique_name, + const bool use_unique, Map &socket_map) { bNode *node_dst = (bNode *)MEM_mallocN(sizeof(bNode), __func__); @@ -2251,8 +2283,9 @@ bNode *node_copy_with_mapping(bNodeTree *dst_tree, /* Can be called for nodes outside a node tree (e.g. clipboard). */ if (dst_tree) { - if (unique_name) { + if (use_unique) { nodeUniqueName(dst_tree, node_dst); + nodeUniqueID(dst_tree, node_dst); } BLI_addtail(&dst_tree->nodes, node_dst); } @@ -2314,13 +2347,10 @@ bNode *node_copy_with_mapping(bNodeTree *dst_tree, return node_dst; } -bNode *node_copy(bNodeTree *dst_tree, - const bNode &src_node, - const int flag, - const bool unique_name) +bNode *node_copy(bNodeTree *dst_tree, const bNode &src_node, const int flag, const bool use_unique) { Map socket_map; - return node_copy_with_mapping(dst_tree, src_node, flag, unique_name, socket_map); + return node_copy_with_mapping(dst_tree, src_node, flag, use_unique, socket_map); } } // namespace blender::bke @@ -2910,8 +2940,20 @@ static void node_unlink_attached(bNodeTree *ntree, bNode *parent) } } -/* Free the node itself. ID user refcounting is up the caller, - * that does not happen here. */ +static void rebuild_nodes_vector(bNodeTree &node_tree) +{ + /* Rebuild nodes #VectorSet which must have the same order as the list. */ + node_tree.runtime->nodes_by_id.clear(); + LISTBASE_FOREACH (bNode *, node, &node_tree.nodes) { + node_tree.runtime->nodes_by_id.add_new(node); + } +} + +/** + * Free the node itself. + * + * \note: ID user refcounting and changing the `nodes_by_id` vector are up to the caller. + */ static void node_free_node(bNodeTree *ntree, bNode *node) { /* since it is called while free database, node->id is undefined */ @@ -2919,6 +2961,8 @@ static void node_free_node(bNodeTree *ntree, bNode *node) /* can be called for nodes outside a node tree (e.g. clipboard) */ if (ntree) { BLI_remlink(&ntree->nodes, node); + /* Rebuild nodes #VectorSet which must have the same order as the list. */ + rebuild_nodes_vector(*ntree); /* texture node has bad habit of keeping exec data around */ if (ntree->type == NTREE_TEXTURE && ntree->runtime->execdata) { @@ -2976,6 +3020,7 @@ void ntreeFreeLocalNode(bNodeTree *ntree, bNode *node) node_unlink_attached(ntree, node); node_free_node(ntree, node); + rebuild_nodes_vector(*ntree); } void nodeRemoveNode(Main *bmain, bNodeTree *ntree, bNode *node, bool do_id_user) @@ -3035,6 +3080,7 @@ void nodeRemoveNode(Main *bmain, bNodeTree *ntree, bNode *node, bool do_id_user) /* Free node itself. */ node_free_node(ntree, node); + rebuild_nodes_vector(*ntree); } static void node_socket_interface_free(bNodeTree * /*ntree*/, diff --git a/source/blender/blenkernel/intern/node_runtime.cc b/source/blender/blenkernel/intern/node_runtime.cc index d5f1136ca48..271be8c0a55 100644 --- a/source/blender/blenkernel/intern/node_runtime.cc +++ b/source/blender/blenkernel/intern/node_runtime.cc @@ -45,15 +45,16 @@ static void double_checked_lock_with_task_isolation(std::mutex &mutex, static void update_node_vector(const bNodeTree &ntree) { bNodeTreeRuntime &tree_runtime = *ntree.runtime; - tree_runtime.nodes.clear(); + const Span nodes = tree_runtime.nodes_by_id; tree_runtime.group_nodes.clear(); tree_runtime.has_undefined_nodes_or_sockets = false; - LISTBASE_FOREACH (bNode *, node, &ntree.nodes) { - node->runtime->index_in_tree = tree_runtime.nodes.append_and_get_index(node); - node->runtime->owner_tree = const_cast(&ntree); - tree_runtime.has_undefined_nodes_or_sockets |= node->typeinfo == &NodeTypeUndefined; - if (node->is_group()) { - tree_runtime.group_nodes.append(node); + for (const int i : nodes.index_range()) { + bNode &node = *nodes[i]; + node.runtime->index_in_tree = i; + node.runtime->owner_tree = const_cast(&ntree); + tree_runtime.has_undefined_nodes_or_sockets |= node.typeinfo == &NodeTypeUndefined; + if (node.is_group()) { + tree_runtime.group_nodes.append(&node); } } } @@ -73,7 +74,7 @@ static void update_socket_vectors_and_owner_node(const bNodeTree &ntree) tree_runtime.sockets.clear(); tree_runtime.input_sockets.clear(); tree_runtime.output_sockets.clear(); - for (bNode *node : tree_runtime.nodes) { + for (bNode *node : tree_runtime.nodes_by_id) { bNodeRuntime &node_runtime = *node->runtime; node_runtime.inputs.clear(); node_runtime.outputs.clear(); @@ -99,7 +100,7 @@ static void update_socket_vectors_and_owner_node(const bNodeTree &ntree) static void update_internal_link_inputs(const bNodeTree &ntree) { bNodeTreeRuntime &tree_runtime = *ntree.runtime; - for (bNode *node : tree_runtime.nodes) { + for (bNode *node : tree_runtime.nodes_by_id) { for (bNodeSocket *socket : node->runtime->outputs) { socket->runtime->internal_link_input = nullptr; } @@ -112,7 +113,7 @@ static void update_internal_link_inputs(const bNodeTree &ntree) static void update_directly_linked_links_and_sockets(const bNodeTree &ntree) { bNodeTreeRuntime &tree_runtime = *ntree.runtime; - for (bNode *node : tree_runtime.nodes) { + for (bNode *node : tree_runtime.nodes_by_id) { for (bNodeSocket *socket : node->runtime->inputs) { socket->runtime->directly_linked_links.clear(); socket->runtime->directly_linked_sockets.clear(); @@ -207,9 +208,10 @@ static void find_logical_origins_for_socket_recursive( static void update_logical_origins(const bNodeTree &ntree) { bNodeTreeRuntime &tree_runtime = *ntree.runtime; - threading::parallel_for(tree_runtime.nodes.index_range(), 128, [&](const IndexRange range) { + Span nodes = tree_runtime.nodes_by_id; + threading::parallel_for(nodes.index_range(), 128, [&](const IndexRange range) { for (const int i : range) { - bNode &node = *tree_runtime.nodes[i]; + bNode &node = *nodes[i]; for (bNodeSocket *socket : node.runtime->inputs) { Vector sockets_in_current_chain; socket->runtime->logically_linked_sockets.clear(); @@ -229,7 +231,7 @@ static void update_nodes_by_type(const bNodeTree &ntree) { bNodeTreeRuntime &tree_runtime = *ntree.runtime; tree_runtime.nodes_by_type.clear(); - for (bNode *node : tree_runtime.nodes) { + for (bNode *node : tree_runtime.nodes_by_id) { tree_runtime.nodes_by_type.add(node->typeinfo, node); } } @@ -237,8 +239,9 @@ static void update_nodes_by_type(const bNodeTree &ntree) static void update_sockets_by_identifier(const bNodeTree &ntree) { bNodeTreeRuntime &tree_runtime = *ntree.runtime; - threading::parallel_for(tree_runtime.nodes.index_range(), 128, [&](const IndexRange range) { - for (bNode *node : tree_runtime.nodes.as_span().slice(range)) { + Span nodes = tree_runtime.nodes_by_id; + threading::parallel_for(nodes.index_range(), 128, [&](const IndexRange range) { + for (bNode *node : nodes.slice(range)) { node->runtime->inputs_by_identifier.clear(); node->runtime->outputs_by_identifier.clear(); for (bNodeSocket *socket : node->runtime->inputs) { @@ -337,11 +340,11 @@ static void update_toposort(const bNodeTree &ntree, { bNodeTreeRuntime &tree_runtime = *ntree.runtime; r_sorted_nodes.clear(); - r_sorted_nodes.reserve(tree_runtime.nodes.size()); + r_sorted_nodes.reserve(tree_runtime.nodes_by_id.size()); r_cycle_detected = false; - Array node_states(tree_runtime.nodes.size()); - for (bNode *node : tree_runtime.nodes) { + Array node_states(tree_runtime.nodes_by_id.size()); + for (bNode *node : tree_runtime.nodes_by_id) { if (node_states[node->runtime->index_in_tree].is_done) { /* Ignore nodes that are done already. */ continue; @@ -355,9 +358,9 @@ static void update_toposort(const bNodeTree &ntree, toposort_from_start_node(direction, *node, node_states, r_sorted_nodes, r_cycle_detected); } - if (r_sorted_nodes.size() < tree_runtime.nodes.size()) { + if (r_sorted_nodes.size() < tree_runtime.nodes_by_id.size()) { r_cycle_detected = true; - for (bNode *node : tree_runtime.nodes) { + for (bNode *node : tree_runtime.nodes_by_id) { if (node_states[node->runtime->index_in_tree].is_done) { /* Ignore nodes that are done already. */ continue; @@ -367,13 +370,13 @@ static void update_toposort(const bNodeTree &ntree, } } - BLI_assert(tree_runtime.nodes.size() == r_sorted_nodes.size()); + BLI_assert(tree_runtime.nodes_by_id.size() == r_sorted_nodes.size()); } static void update_root_frames(const bNodeTree &ntree) { bNodeTreeRuntime &tree_runtime = *ntree.runtime; - Span nodes = tree_runtime.nodes; + Span nodes = tree_runtime.nodes_by_id; tree_runtime.root_frames.clear(); @@ -387,7 +390,7 @@ static void update_root_frames(const bNodeTree &ntree) static void update_direct_frames_childrens(const bNodeTree &ntree) { bNodeTreeRuntime &tree_runtime = *ntree.runtime; - Span nodes = tree_runtime.nodes; + Span nodes = tree_runtime.nodes_by_id; for (bNode *node : nodes) { node->runtime->direct_children_in_frame.clear(); @@ -432,7 +435,7 @@ static void ensure_topology_cache(const bNodeTree &ntree) update_internal_link_inputs(ntree); update_directly_linked_links_and_sockets(ntree); threading::parallel_invoke( - tree_runtime.nodes.size() > 32, + tree_runtime.nodes_by_id.size() > 32, [&]() { update_logical_origins(ntree); }, [&]() { update_nodes_by_type(ntree); }, [&]() { update_sockets_by_identifier(ntree); }, diff --git a/source/blender/blenkernel/intern/node_tree_update.cc b/source/blender/blenkernel/intern/node_tree_update.cc index 09b8bcd1d93..72ea4da7855 100644 --- a/source/blender/blenkernel/intern/node_tree_update.cc +++ b/source/blender/blenkernel/intern/node_tree_update.cc @@ -1009,6 +1009,15 @@ class NodeTreeMainUpdater { result.interface_changed = true; } +#ifdef DEBUG + /* Check the uniqueness of node identifiers. */ + Set node_identifiers; + LISTBASE_FOREACH (bNode *, node, &ntree.nodes) { + BLI_assert(node->identifier >= 0); + node_identifiers.add_new(node->identifier); + } +#endif + return result; } diff --git a/source/blender/blenkernel/intern/viewer_path.cc b/source/blender/blenkernel/intern/viewer_path.cc index edc71787ac5..468b7990cbd 100644 --- a/source/blender/blenkernel/intern/viewer_path.cc +++ b/source/blender/blenkernel/intern/viewer_path.cc @@ -215,6 +215,7 @@ ViewerPathElem *BKE_viewer_path_elem_copy(const ViewerPathElem *src) case VIEWER_PATH_ELEM_TYPE_NODE: { const auto *old_elem = reinterpret_cast(src); auto *new_elem = reinterpret_cast(dst); + new_elem->node_id = old_elem->node_id; if (old_elem->node_name != nullptr) { new_elem->node_name = BLI_strdup(old_elem->node_name); } @@ -243,7 +244,7 @@ bool BKE_viewer_path_elem_equal(const ViewerPathElem *a, const ViewerPathElem *b case VIEWER_PATH_ELEM_TYPE_NODE: { const auto *a_elem = reinterpret_cast(a); const auto *b_elem = reinterpret_cast(b); - return StringRef(a_elem->node_name) == StringRef(b_elem->node_name); + return a_elem->node_id == b_elem->node_id; } } return false; diff --git a/source/blender/editors/include/ED_viewer_path.hh b/source/blender/editors/include/ED_viewer_path.hh index 957dfddb8af..42bbab0881c 100644 --- a/source/blender/editors/include/ED_viewer_path.hh +++ b/source/blender/editors/include/ED_viewer_path.hh @@ -35,8 +35,8 @@ Object *parse_object_only(const ViewerPath &viewer_path); struct ViewerPathForGeometryNodesViewer { Object *object; blender::StringRefNull modifier_name; - blender::Vector group_node_names; - blender::StringRefNull viewer_node_name; + blender::Vector group_node_ids; + int32_t viewer_node_id; }; /** diff --git a/source/blender/editors/space_node/node_draw.cc b/source/blender/editors/space_node/node_draw.cc index 96550944b9a..be778a75039 100644 --- a/source/blender/editors/space_node/node_draw.cc +++ b/source/blender/editors/space_node/node_draw.cc @@ -245,57 +245,22 @@ static bool compare_nodes(const bNode *a, const bNode *b) void node_sort(bNodeTree &ntree) { - /* Merge sort is the algorithm of choice here. */ - int totnodes = BLI_listbase_count(&ntree.nodes); + Array sort_nodes = ntree.all_nodes(); + std::stable_sort(sort_nodes.begin(), sort_nodes.end(), compare_nodes); - int k = 1; - while (k < totnodes) { - bNode *first_a = (bNode *)ntree.nodes.first; - bNode *first_b = first_a; + /* If nothing was changed, exit early. Otherwise the node tree's runtime + * node vector needs to be rebuilt, since it cannot be reordered in place. */ + if (sort_nodes == ntree.all_nodes()) { + return; + } - do { - /* Set up first_b pointer. */ - for (int b = 0; b < k && first_b; b++) { - first_b = first_b->next; - } - /* All batches merged? */ - if (first_b == nullptr) { - break; - } + BKE_ntree_update_tag_node_reordered(&ntree); - /* Merge batches. */ - bNode *node_a = first_a; - bNode *node_b = first_b; - int a = 0; - int b = 0; - while (a < k && b < k && node_b) { - if (compare_nodes(node_a, node_b) == 0) { - node_a = node_a->next; - a++; - } - else { - bNode *tmp = node_b; - node_b = node_b->next; - b++; - BLI_remlink(&ntree.nodes, tmp); - BLI_insertlinkbefore(&ntree.nodes, node_a, tmp); - BKE_ntree_update_tag_node_reordered(&ntree); - } - } - - /* Set up first pointers for next batch. */ - first_b = node_b; - for (; b < k; b++) { - /* All nodes sorted? */ - if (first_b == nullptr) { - break; - } - first_b = first_b->next; - } - first_a = first_b; - } while (first_b); - - k = k << 1; + ntree.runtime->nodes_by_id.clear(); + BLI_listbase_clear(&ntree.nodes); + for (const int i : sort_nodes.index_range()) { + BLI_addtail(&ntree.nodes, sort_nodes[i]); + ntree.runtime->nodes_by_id.add_new(sort_nodes[i]); } } @@ -1691,7 +1656,7 @@ static void node_add_error_message_button(TreeDrawContext &tree_draw_ctx, Span warnings; if (tree_draw_ctx.geo_tree_log) { - geo_log::GeoNodeLog *node_log = tree_draw_ctx.geo_tree_log->nodes.lookup_ptr(node.name); + geo_log::GeoNodeLog *node_log = tree_draw_ctx.geo_tree_log->nodes.lookup_ptr(node.identifier); if (node_log != nullptr) { warnings = node_log->warnings; } @@ -1751,7 +1716,8 @@ static std::optional node_get_execution_time( } } else { - if (const geo_log::GeoNodeLog *node_log = tree_log->nodes.lookup_ptr_as(tnode->name)) { + if (const geo_log::GeoNodeLog *node_log = tree_log->nodes.lookup_ptr_as( + tnode->identifier)) { found_node = true; run_time += node_log->run_time; } @@ -1762,7 +1728,7 @@ static std::optional node_get_execution_time( } return std::nullopt; } - if (const geo_log::GeoNodeLog *node_log = tree_log->nodes.lookup_ptr(node.name)) { + if (const geo_log::GeoNodeLog *node_log = tree_log->nodes.lookup_ptr(node.identifier)) { return node_log->run_time; } return std::nullopt; @@ -1903,7 +1869,7 @@ static std::optional node_get_accessed_attributes_row( } } tree_draw_ctx.geo_tree_log->ensure_used_named_attributes(); - geo_log::GeoNodeLog *node_log = tree_draw_ctx.geo_tree_log->nodes.lookup_ptr(node.name); + geo_log::GeoNodeLog *node_log = tree_draw_ctx.geo_tree_log->nodes.lookup_ptr(node.identifier); if (node_log == nullptr) { return std::nullopt; } @@ -1944,15 +1910,17 @@ static Vector node_get_extra_info(TreeDrawContext &tree_draw_c } } - if (snode.edittree->type == NTREE_GEOMETRY && tree_draw_ctx.geo_tree_log != nullptr) { - tree_draw_ctx.geo_tree_log->ensure_debug_messages(); - const geo_log::GeoNodeLog *node_log = tree_draw_ctx.geo_tree_log->nodes.lookup_ptr(node.name); - if (node_log != nullptr) { - for (const StringRef message : node_log->debug_messages) { - NodeExtraInfoRow row; - row.text = message; - row.icon = ICON_INFO; - rows.append(std::move(row)); + if (snode.edittree->type == NTREE_GEOMETRY) { + if (geo_log::GeoTreeLog *tree_log = tree_draw_ctx.geo_tree_log) { + tree_log->ensure_debug_messages(); + const geo_log::GeoNodeLog *node_log = tree_log->nodes.lookup_ptr(node.identifier); + if (node_log != nullptr) { + for (const StringRef message : node_log->debug_messages) { + NodeExtraInfoRow row; + row.text = message; + row.icon = ICON_INFO; + rows.append(std::move(row)); + } } } } diff --git a/source/blender/editors/space_node/node_geometry_attribute_search.cc b/source/blender/editors/space_node/node_geometry_attribute_search.cc index 002faffd0fb..982ea27374f 100644 --- a/source/blender/editors/space_node/node_geometry_attribute_search.cc +++ b/source/blender/editors/space_node/node_geometry_attribute_search.cc @@ -40,7 +40,7 @@ using blender::nodes::geo_eval_log::GeometryAttributeInfo; namespace blender::ed::space_node { struct AttributeSearchData { - char node_name[MAX_NAME]; + int32_t node_id; char socket_identifier[MAX_NAME]; }; @@ -62,7 +62,7 @@ static Vector get_attribute_info_from_context( BLI_assert_unreachable(); return {}; } - bNode *node = nodeFindNodebyName(node_tree, data.node_name); + const bNode *node = node_tree->node_by_id(data.node_id); if (node == nullptr) { BLI_assert_unreachable(); return {}; @@ -84,7 +84,7 @@ static Vector get_attribute_info_from_context( } return attributes; } - GeoNodeLog *node_log = tree_log->nodes.lookup_ptr(node->name); + GeoNodeLog *node_log = tree_log->nodes.lookup_ptr(node->identifier); if (node_log == nullptr) { return {}; } @@ -173,7 +173,7 @@ static void attribute_search_exec_fn(bContext *C, void *data_v, void *item_v) return; } AttributeSearchData *data = static_cast(data_v); - bNode *node = nodeFindNodebyName(node_tree, data->node_name); + bNode *node = node_tree->node_by_id(data->node_id); if (node == nullptr) { BLI_assert_unreachable(); return; @@ -243,7 +243,7 @@ void node_geometry_add_attribute_search_button(const bContext & /*C*/, const bNodeSocket &socket = *static_cast(socket_ptr.data); AttributeSearchData *data = MEM_new(__func__); - BLI_strncpy(data->node_name, node.name, sizeof(data->node_name)); + data->node_id = node.identifier; BLI_strncpy(data->socket_identifier, socket.identifier, sizeof(data->socket_identifier)); UI_but_func_search_set_results_are_suggestions(but, true); diff --git a/source/blender/editors/space_node/node_group.cc b/source/blender/editors/space_node/node_group.cc index f95561035a3..dfbd95abb40 100644 --- a/source/blender/editors/space_node/node_group.cc +++ b/source/blender/editors/space_node/node_group.cc @@ -249,11 +249,11 @@ static bool node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode) /* migrate node */ BLI_remlink(&wgroup->nodes, node); BLI_addtail(&ntree->nodes, node); - BKE_ntree_update_tag_node_new(ntree, node); - - /* ensure unique node name in the node tree */ + nodeUniqueID(ntree, node); nodeUniqueName(ntree, node); + BKE_ntree_update_tag_node_new(ntree, node); + if (wgroup->adt) { PointerRNA ptr; RNA_pointer_create(&ntree->id, &RNA_Node, node, &ptr); @@ -494,8 +494,7 @@ static bool node_group_separate_selected( /* migrate node */ BLI_remlink(&ngroup.nodes, newnode); BLI_addtail(&ntree.nodes, newnode); - - /* ensure unique node name in the node tree */ + nodeUniqueID(&ntree, newnode); nodeUniqueName(&ntree, newnode); if (!newnode->parent) { @@ -871,11 +870,11 @@ static void node_group_make_insert_selected(const bContext &C, bNodeTree &ntree, /* change node-collection membership */ BLI_remlink(&ntree.nodes, node); BLI_addtail(&ngroup->nodes, node); + nodeUniqueID(ngroup, node); + nodeUniqueName(ngroup, node); + BKE_ntree_update_tag_node_removed(&ntree); BKE_ntree_update_tag_node_new(ngroup, node); - - /* ensure unique node name in the ngroup */ - nodeUniqueName(ngroup, node); } } diff --git a/source/blender/editors/util/ed_viewer_path.cc b/source/blender/editors/util/ed_viewer_path.cc index 4da1559b726..48b1d08bf96 100644 --- a/source/blender/editors/util/ed_viewer_path.cc +++ b/source/blender/editors/util/ed_viewer_path.cc @@ -56,13 +56,18 @@ static void viewer_path_for_geometry_node(const SpaceNode &snode, BLI_addtail(&r_dst.path, modifier_elem); Vector tree_path = snode.treepath; - for (const bNodeTreePath *tree_path_elem : tree_path.as_span().drop_front(1)) { + for (const int i : tree_path.index_range().drop_back(1)) { + /* The tree path contains the name of the node but not its ID. */ + const bNode *node = nodeFindNodebyName(tree_path[i]->nodetree, tree_path[i + 1]->node_name); + /* The name in the tree path should match a group node in the tree. */ + BLI_assert(node != nullptr); NodeViewerPathElem *node_elem = BKE_viewer_path_elem_new_node(); - node_elem->node_name = BLI_strdup(tree_path_elem->node_name); - BLI_addtail(&r_dst.path, node_elem); + node_elem->node_id = node->identifier; + node_elem->node_name = BLI_strdup(node->name); } NodeViewerPathElem *viewer_node_elem = BKE_viewer_path_elem_new_node(); + viewer_node_elem->node_id = node.identifier; viewer_node_elem->node_name = BLI_strdup(node.name); BLI_addtail(&r_dst.path, viewer_node_elem); } @@ -171,19 +176,16 @@ std::optional parse_geometry_nodes_viewer( return std::nullopt; } remaining_elems = remaining_elems.drop_front(1); - Vector node_names; + Vector node_ids; for (const ViewerPathElem *elem : remaining_elems) { if (elem->type != VIEWER_PATH_ELEM_TYPE_NODE) { return std::nullopt; } - const char *node_name = reinterpret_cast(elem)->node_name; - if (node_name == nullptr) { - return std::nullopt; - } - node_names.append(node_name); + const int32_t node_id = reinterpret_cast(elem)->node_id; + node_ids.append(node_id); } - const StringRefNull viewer_node_name = node_names.pop_last(); - return ViewerPathForGeometryNodesViewer{root_ob, modifier_name, node_names, viewer_node_name}; + const int32_t viewer_node_id = node_ids.pop_last(); + return ViewerPathForGeometryNodesViewer{root_ob, modifier_name, node_ids, viewer_node_id}; } bool exists_geometry_nodes_viewer(const ViewerPathForGeometryNodesViewer &parsed_viewer_path) @@ -207,10 +209,10 @@ bool exists_geometry_nodes_viewer(const ViewerPathForGeometryNodesViewer &parsed } const bNodeTree *ngroup = modifier->node_group; ngroup->ensure_topology_cache(); - for (const StringRefNull group_node_name : parsed_viewer_path.group_node_names) { + for (const int32_t group_node_id : parsed_viewer_path.group_node_ids) { const bNode *group_node = nullptr; for (const bNode *node : ngroup->group_nodes()) { - if (node->name != group_node_name) { + if (node->identifier != group_node_id) { continue; } group_node = node; @@ -226,7 +228,7 @@ bool exists_geometry_nodes_viewer(const ViewerPathForGeometryNodesViewer &parsed } const bNode *viewer_node = nullptr; for (const bNode *node : ngroup->nodes_by_type("GeometryNodeViewer")) { - if (node->name != parsed_viewer_path.viewer_node_name) { + if (node->identifier != parsed_viewer_path.viewer_node_id) { continue; } viewer_node = node; @@ -238,6 +240,25 @@ bool exists_geometry_nodes_viewer(const ViewerPathForGeometryNodesViewer &parsed return true; } +static bool viewer_path_matches_node_editor_path( + const SpaceNode &snode, const ViewerPathForGeometryNodesViewer &parsed_viewer_path) +{ + Vector tree_path = snode.treepath; + if (tree_path.size() != parsed_viewer_path.group_node_ids.size() + 1) { + return false; + } + for (const int i : parsed_viewer_path.group_node_ids.index_range()) { + const bNode *node = tree_path[i]->nodetree->node_by_id(parsed_viewer_path.group_node_ids[i]); + if (!node) { + return false; + } + if (!STREQ(node->name, tree_path[i + 1]->node_name)) { + return false; + } + } + return true; +} + bool is_active_geometry_nodes_viewer(const bContext &C, const ViewerPathForGeometryNodesViewer &parsed_viewer_path) { @@ -297,29 +318,12 @@ bool is_active_geometry_nodes_viewer(const bContext &C, if (snode.nodetree != modifier->node_group) { continue; } - Vector tree_path = snode.treepath; - if (tree_path.size() != parsed_viewer_path.group_node_names.size() + 1) { - continue; - } - bool valid_path = true; - for (const int i : parsed_viewer_path.group_node_names.index_range()) { - if (parsed_viewer_path.group_node_names[i] != tree_path[i + 1]->node_name) { - valid_path = false; - break; - } - } - if (!valid_path) { + if (!viewer_path_matches_node_editor_path(snode, parsed_viewer_path)) { continue; } const bNodeTree *ngroup = snode.edittree; ngroup->ensure_topology_cache(); - const bNode *viewer_node = nullptr; - for (const bNode *node : ngroup->nodes_by_type("GeometryNodeViewer")) { - if (node->name != parsed_viewer_path.viewer_node_name) { - continue; - } - viewer_node = node; - } + const bNode *viewer_node = ngroup->node_by_id(parsed_viewer_path.viewer_node_id); if (viewer_node == nullptr) { continue; } @@ -342,13 +346,7 @@ bNode *find_geometry_nodes_viewer(const ViewerPath &viewer_path, SpaceNode &snod } snode.edittree->ensure_topology_cache(); - bNode *possible_viewer = nullptr; - for (bNode *node : snode.edittree->nodes_by_type("GeometryNodeViewer")) { - if (node->name == parsed_viewer_path->viewer_node_name) { - possible_viewer = node; - break; - } - } + bNode *possible_viewer = snode.edittree->node_by_id(parsed_viewer_path->viewer_node_id); if (possible_viewer == nullptr) { return nullptr; } diff --git a/source/blender/makesdna/DNA_node_types.h b/source/blender/makesdna/DNA_node_types.h index fce5f4d5be6..e87adacbc9a 100644 --- a/source/blender/makesdna/DNA_node_types.h +++ b/source/blender/makesdna/DNA_node_types.h @@ -317,7 +317,14 @@ typedef struct bNode { /** Additional offset from loc. */ float offsetx, offsety; - char _pad0[4]; + /** + * A value that uniquely identifies a node in a node tree even when the name changes. + * This also allows referencing nodes more efficiently than with strings. + * + * Must be set whenever a node is added to a tree, besides a simple tree copy. + * Must always be positive. + */ + int32_t identifier; /** Custom user-defined label, MAX_NAME. */ char label[64]; @@ -548,6 +555,15 @@ typedef struct bNodeTree { bNodeTreeRuntimeHandle *runtime; #ifdef __cplusplus + + /** A span containing all nodes in the node tree. */ + blender::Span all_nodes(); + blender::Span all_nodes() const; + + /** Retrieve a node based on its persistent integer identifier. */ + struct bNode *node_by_id(int32_t identifier); + const struct bNode *node_by_id(int32_t identifier) const; + /** * Update a run-time cache for the node tree based on it's current state. This makes many methods * available which allow efficient lookup for topology information (like neighboring sockets). @@ -557,9 +573,6 @@ typedef struct bNodeTree { /* The following methods are only available when #bNodeTree.ensure_topology_cache has been * called. */ - /** A span containing all nodes in the node tree. */ - blender::Span all_nodes(); - blender::Span all_nodes() const; /** A span containing all group nodes in the node tree. */ blender::Span group_nodes(); blender::Span group_nodes() const; diff --git a/source/blender/makesdna/DNA_viewer_path_types.h b/source/blender/makesdna/DNA_viewer_path_types.h index 8f470b66ca0..350b9519a04 100644 --- a/source/blender/makesdna/DNA_viewer_path_types.h +++ b/source/blender/makesdna/DNA_viewer_path_types.h @@ -31,6 +31,14 @@ typedef struct ModifierViewerPathElem { typedef struct NodeViewerPathElem { ViewerPathElem base; + + int32_t node_id; + char _pad1[4]; + + /** + * The name of the node with the identifier. Not used to lookup nodes, only for display + * in the UI. Still stored here to avoid looking up the name for every redraw. + */ char *node_name; } NodeViewerPathElem; diff --git a/source/blender/modifiers/intern/MOD_nodes.cc b/source/blender/modifiers/intern/MOD_nodes.cc index c2b14262b4f..a8e5e2ab0a2 100644 --- a/source/blender/modifiers/intern/MOD_nodes.cc +++ b/source/blender/modifiers/intern/MOD_nodes.cc @@ -861,14 +861,8 @@ static void find_side_effect_nodes_for_viewer_path( const bNodeTree *group = nmd.node_group; Stack group_node_stack; - for (const StringRefNull group_node_name : parsed_path->group_node_names) { - const bNode *found_node = nullptr; - for (const bNode *node : group->group_nodes()) { - if (node->name == group_node_name) { - found_node = node; - break; - } - } + for (const int32_t group_node_id : parsed_path->group_node_ids) { + const bNode *found_node = group->node_by_id(group_node_id); if (found_node == nullptr) { return; } @@ -880,16 +874,10 @@ static void find_side_effect_nodes_for_viewer_path( } group_node_stack.push(found_node); group = reinterpret_cast(found_node->id); - compute_context_builder.push(group_node_name); + compute_context_builder.push(*found_node); } - const bNode *found_viewer_node = nullptr; - for (const bNode *viewer_node : group->nodes_by_type("GeometryNodeViewer")) { - if (viewer_node->name == parsed_path->viewer_node_name) { - found_viewer_node = viewer_node; - break; - } - } + const bNode *found_viewer_node = group->node_by_id(parsed_path->viewer_node_id); if (found_viewer_node == nullptr) { return; } diff --git a/source/blender/nodes/NOD_geometry_nodes_log.hh b/source/blender/nodes/NOD_geometry_nodes_log.hh index 5a2203a76b7..e2207338823 100644 --- a/source/blender/nodes/NOD_geometry_nodes_log.hh +++ b/source/blender/nodes/NOD_geometry_nodes_log.hh @@ -169,36 +169,36 @@ using TimePoint = Clock::time_point; class GeoTreeLogger { public: std::optional parent_hash; - std::optional group_node_name; + std::optional group_node_id; Vector children_hashes; LinearAllocator<> *allocator = nullptr; struct WarningWithNode { - StringRefNull node_name; + int32_t node_id; NodeWarning warning; }; struct SocketValueLog { - StringRefNull node_name; + int32_t node_id; StringRefNull socket_identifier; destruct_ptr value; }; struct NodeExecutionTime { - StringRefNull node_name; + int32_t node_id; TimePoint start; TimePoint end; }; struct ViewerNodeLogWithNode { - StringRefNull node_name; + int32_t node_id; destruct_ptr viewer_log; }; struct AttributeUsageWithNode { - StringRefNull node_name; + int32_t node_id; StringRefNull attribute_name; NamedAttributeUsage usage; }; struct DebugMessage { - StringRefNull node_name; + int32_t node_id; StringRefNull message; }; @@ -269,8 +269,8 @@ class GeoTreeLog { bool reduced_debug_messages_ = false; public: - Map nodes; - Map viewer_node_logs; + Map nodes; + Map viewer_node_logs; Vector all_warnings; std::chrono::nanoseconds run_time_sum{0}; Vector existing_attributes; diff --git a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc index da994553a29..def3fae4ce7 100644 --- a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc +++ b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc @@ -135,8 +135,7 @@ class LazyFunctionForGeometryNode : public LazyFunction { if (geo_eval_log::GeoModifierLog *modifier_log = user_data->modifier_data->eval_log) { geo_eval_log::GeoTreeLogger &tree_logger = modifier_log->get_local_tree_logger( *user_data->compute_context); - tree_logger.node_execution_times.append( - {tree_logger.allocator->copy_string(node_.name), start_time, end_time}); + tree_logger.node_execution_times.append({node_.identifier, start_time, end_time}); } } }; @@ -663,7 +662,8 @@ class LazyFunctionForGroupNode : public LazyFunction { } /* The compute context changes when entering a node group. */ - bke::NodeGroupComputeContext compute_context{user_data->compute_context, group_node_.name}; + bke::NodeGroupComputeContext compute_context{user_data->compute_context, + group_node_.identifier}; GeoNodesLFUserData group_user_data = *user_data; group_user_data.compute_context = &compute_context; @@ -1399,8 +1399,7 @@ GeometryNodesLazyFunctionGraphInfo::~GeometryNodesLazyFunctionGraphInfo() if (!bsockets.is_empty()) { const bNodeSocket &bsocket = *bsockets[0]; const bNode &bnode = bsocket.owner_node(); - tree_logger.debug_messages.append( - {tree_logger.allocator->copy_string(bnode.name), thread_id_str}); + tree_logger.debug_messages.append({bnode.identifier, thread_id_str}); return true; } } diff --git a/source/blender/nodes/intern/geometry_nodes_log.cc b/source/blender/nodes/intern/geometry_nodes_log.cc index 640296ead66..e8b65a3d319 100644 --- a/source/blender/nodes/intern/geometry_nodes_log.cc +++ b/source/blender/nodes/intern/geometry_nodes_log.cc @@ -151,9 +151,8 @@ void GeoTreeLogger::log_value(const bNode &node, const bNodeSocket &socket, cons auto store_logged_value = [&](destruct_ptr value_log) { auto &socket_values = socket.in_out == SOCK_IN ? this->input_socket_values : this->output_socket_values; - socket_values.append({this->allocator->copy_string(node.name), - this->allocator->copy_string(socket.identifier), - std::move(value_log)}); + socket_values.append( + {node.identifier, this->allocator->copy_string(socket.identifier), std::move(value_log)}); }; auto log_generic_value = [&](const CPPType &type, const void *value) { @@ -195,7 +194,7 @@ void GeoTreeLogger::log_viewer_node(const bNode &viewer_node, GeometrySet geomet destruct_ptr log = this->allocator->construct(); log->geometry = std::move(geometry); log->geometry.ensure_owns_direct_data(); - this->viewer_node_logs.append({this->allocator->copy_string(viewer_node.name), std::move(log)}); + this->viewer_node_logs.append({viewer_node.identifier, std::move(log)}); } void GeoTreeLog::ensure_node_warnings() @@ -205,17 +204,16 @@ void GeoTreeLog::ensure_node_warnings() } for (GeoTreeLogger *tree_logger : tree_loggers_) { for (const GeoTreeLogger::WarningWithNode &warnings : tree_logger->node_warnings) { - this->nodes.lookup_or_add_default(warnings.node_name).warnings.append(warnings.warning); + this->nodes.lookup_or_add_default(warnings.node_id).warnings.append(warnings.warning); this->all_warnings.append(warnings.warning); } } for (const ComputeContextHash &child_hash : children_hashes_) { GeoTreeLog &child_log = modifier_log_->get_tree_log(child_hash); child_log.ensure_node_warnings(); - const std::optional &group_node_name = - child_log.tree_loggers_[0]->group_node_name; - if (group_node_name.has_value()) { - this->nodes.lookup_or_add_default(*group_node_name).warnings.extend(child_log.all_warnings); + const std::optional &group_node_id = child_log.tree_loggers_[0]->group_node_id; + if (group_node_id.has_value()) { + this->nodes.lookup_or_add_default(*group_node_id).warnings.extend(child_log.all_warnings); } this->all_warnings.extend(child_log.all_warnings); } @@ -230,17 +228,16 @@ void GeoTreeLog::ensure_node_run_time() for (GeoTreeLogger *tree_logger : tree_loggers_) { for (const GeoTreeLogger::NodeExecutionTime &timings : tree_logger->node_execution_times) { const std::chrono::nanoseconds duration = timings.end - timings.start; - this->nodes.lookup_or_add_default_as(timings.node_name).run_time += duration; + this->nodes.lookup_or_add_default_as(timings.node_id).run_time += duration; this->run_time_sum += duration; } } for (const ComputeContextHash &child_hash : children_hashes_) { GeoTreeLog &child_log = modifier_log_->get_tree_log(child_hash); child_log.ensure_node_run_time(); - const std::optional &group_node_name = - child_log.tree_loggers_[0]->group_node_name; - if (group_node_name.has_value()) { - this->nodes.lookup_or_add_default(*group_node_name).run_time += child_log.run_time_sum; + const std::optional &group_node_id = child_log.tree_loggers_[0]->group_node_id; + if (group_node_id.has_value()) { + this->nodes.lookup_or_add_default(*group_node_id).run_time += child_log.run_time_sum; } this->run_time_sum += child_log.run_time_sum; } @@ -254,11 +251,11 @@ void GeoTreeLog::ensure_socket_values() } for (GeoTreeLogger *tree_logger : tree_loggers_) { for (const GeoTreeLogger::SocketValueLog &value_log_data : tree_logger->input_socket_values) { - this->nodes.lookup_or_add_as(value_log_data.node_name) + this->nodes.lookup_or_add_as(value_log_data.node_id) .input_values_.add(value_log_data.socket_identifier, value_log_data.value.get()); } for (const GeoTreeLogger::SocketValueLog &value_log_data : tree_logger->output_socket_values) { - this->nodes.lookup_or_add_as(value_log_data.node_name) + this->nodes.lookup_or_add_as(value_log_data.node_id) .output_values_.add(value_log_data.socket_identifier, value_log_data.value.get()); } } @@ -272,7 +269,7 @@ void GeoTreeLog::ensure_viewer_node_logs() } for (GeoTreeLogger *tree_logger : tree_loggers_) { for (const GeoTreeLogger::ViewerNodeLogWithNode &viewer_log : tree_logger->viewer_node_logs) { - this->viewer_node_logs.add(viewer_log.node_name, viewer_log.viewer_log.get()); + this->viewer_node_logs.add(viewer_log.node_id, viewer_log.viewer_log.get()); } } reduced_viewer_node_logs_ = true; @@ -316,26 +313,25 @@ void GeoTreeLog::ensure_used_named_attributes() return; } - auto add_attribute = [&](const StringRefNull node_name, + auto add_attribute = [&](const int32_t node_id, const StringRefNull attribute_name, const NamedAttributeUsage &usage) { - this->nodes.lookup_or_add_default(node_name).used_named_attributes.lookup_or_add( - attribute_name, usage) |= usage; + this->nodes.lookup_or_add_default(node_id).used_named_attributes.lookup_or_add(attribute_name, + usage) |= usage; this->used_named_attributes.lookup_or_add_as(attribute_name, usage) |= usage; }; for (GeoTreeLogger *tree_logger : tree_loggers_) { for (const GeoTreeLogger::AttributeUsageWithNode &item : tree_logger->used_named_attributes) { - add_attribute(item.node_name, item.attribute_name, item.usage); + add_attribute(item.node_id, item.attribute_name, item.usage); } } for (const ComputeContextHash &child_hash : children_hashes_) { GeoTreeLog &child_log = modifier_log_->get_tree_log(child_hash); child_log.ensure_used_named_attributes(); - if (const std::optional &group_node_name = - child_log.tree_loggers_[0]->group_node_name) { + if (const std::optional &group_node_id = child_log.tree_loggers_[0]->group_node_id) { for (const auto &item : child_log.used_named_attributes.items()) { - add_attribute(*group_node_name, item.key, item.value); + add_attribute(*group_node_id, item.key, item.value); } } } @@ -349,7 +345,7 @@ void GeoTreeLog::ensure_debug_messages() } for (GeoTreeLogger *tree_logger : tree_loggers_) { for (const GeoTreeLogger::DebugMessage &debug_message : tree_logger->debug_messages) { - this->nodes.lookup_or_add_as(debug_message.node_name) + this->nodes.lookup_or_add_as(debug_message.node_id) .debug_messages.append(debug_message.message); } } @@ -378,7 +374,7 @@ ValueLog *GeoTreeLog::find_socket_value_log(const bNodeSocket &query_socket) while (!sockets_to_check.is_empty()) { const bNodeSocket &socket = *sockets_to_check.pop(); const bNode &node = socket.owner_node(); - if (GeoNodeLog *node_log = this->nodes.lookup_ptr(node.name)) { + if (GeoNodeLog *node_log = this->nodes.lookup_ptr(node.identifier)) { ValueLog *value_log = socket.is_input() ? node_log->input_values_.lookup_default(socket.identifier, nullptr) : @@ -453,7 +449,7 @@ GeoTreeLogger &GeoModifierLog::get_local_tree_logger(const ComputeContext &compu } if (const bke::NodeGroupComputeContext *node_group_compute_context = dynamic_cast(&compute_context)) { - tree_logger.group_node_name.emplace(node_group_compute_context->node_name()); + tree_logger.group_node_id.emplace(node_group_compute_context->node_id()); } return tree_logger; } @@ -538,8 +534,10 @@ GeoTreeLog *GeoModifierLog::get_tree_log_for_node_editor(const SpaceNode &snode) ComputeContextBuilder compute_context_builder; compute_context_builder.push( object_and_modifier->nmd->modifier.name); - for (const bNodeTreePath *path_item : tree_path.as_span().drop_front(1)) { - compute_context_builder.push(path_item->node_name); + for (const int i : tree_path.index_range().drop_back(1)) { + /* The tree path contains the name of the node but not its ID. */ + const bNode *node = nodeFindNodebyName(tree_path[i]->nodetree, tree_path[i + 1]->node_name); + compute_context_builder.push(*node); } return &modifier_log->get_tree_log(compute_context_builder.hash()); } @@ -571,15 +569,15 @@ const ViewerNodeLog *GeoModifierLog::find_viewer_node_log_for_path(const ViewerP ComputeContextBuilder compute_context_builder; compute_context_builder.push(parsed_path->modifier_name); - for (const StringRef group_node_name : parsed_path->group_node_names) { - compute_context_builder.push(group_node_name); + for (const int32_t group_node_id : parsed_path->group_node_ids) { + compute_context_builder.push(group_node_id); } const ComputeContextHash context_hash = compute_context_builder.hash(); nodes::geo_eval_log::GeoTreeLog &tree_log = modifier_log->get_tree_log(context_hash); tree_log.ensure_viewer_node_logs(); const ViewerNodeLog *viewer_log = tree_log.viewer_node_logs.lookup_default( - parsed_path->viewer_node_name, nullptr); + parsed_path->viewer_node_id, nullptr); return viewer_log; } diff --git a/source/blender/nodes/intern/node_geometry_exec.cc b/source/blender/nodes/intern/node_geometry_exec.cc index 1de92fa8409..868b3b2c539 100644 --- a/source/blender/nodes/intern/node_geometry_exec.cc +++ b/source/blender/nodes/intern/node_geometry_exec.cc @@ -17,8 +17,8 @@ void GeoNodeExecParams::error_message_add(const NodeWarningType type, const StringRef message) const { if (geo_eval_log::GeoTreeLogger *tree_logger = this->get_local_tree_logger()) { - tree_logger->node_warnings.append({tree_logger->allocator->copy_string(node_.name), - {type, tree_logger->allocator->copy_string(message)}}); + tree_logger->node_warnings.append( + {node_.identifier, {type, tree_logger->allocator->copy_string(message)}}); } } @@ -26,9 +26,8 @@ void GeoNodeExecParams::used_named_attribute(const StringRef attribute_name, const NamedAttributeUsage usage) { if (geo_eval_log::GeoTreeLogger *tree_logger = this->get_local_tree_logger()) { - tree_logger->used_named_attributes.append({tree_logger->allocator->copy_string(node_.name), - tree_logger->allocator->copy_string(attribute_name), - usage}); + tree_logger->used_named_attributes.append( + {node_.identifier, tree_logger->allocator->copy_string(attribute_name), usage}); } } diff --git a/source/blender/nodes/shader/node_shader_tree.cc b/source/blender/nodes/shader/node_shader_tree.cc index 4324457d3fb..1a82dc9c2b9 100644 --- a/source/blender/nodes/shader/node_shader_tree.cc +++ b/source/blender/nodes/shader/node_shader_tree.cc @@ -578,8 +578,13 @@ static bNode *ntree_shader_copy_branch(bNodeTree *ntree, LISTBASE_FOREACH (bNode *, node, &ntree->nodes) { if (node->runtime->tmp_flag >= 0) { int id = node->runtime->tmp_flag; + /* Avoid creating unique names in the new tree, since it is very slow. The names on the new + * nodes will be invalid. But identifiers must be created for the `bNodeTree::all_nodes()` + * vector, though they won't match the original. */ nodes_copy[id] = blender::bke::node_copy( ntree, *node, LIB_ID_CREATE_NO_USER_REFCOUNT | LIB_ID_CREATE_NO_MAIN, false); + nodeUniqueID(ntree, nodes_copy[id]); + nodes_copy[id]->runtime->tmp_flag = -2; /* Copy */ /* Make sure to clear all sockets links as they are invalid. */ LISTBASE_FOREACH (bNodeSocket *, sock, &nodes_copy[id]->inputs) {