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) {