From 942ca6be6d90fa870457889e4e375e3f47723158 Mon Sep 17 00:00:00 2001 From: Iliya Katueshenock Date: Mon, 8 Apr 2024 16:19:40 +0200 Subject: [PATCH] Fix #120071: quadratic complexity for dangling reroute check To know if link is connected to dangling reroute and can be skipped as value-less, we need to know if reroute is dangling. This requires graph traversal. Currently this is done by non-recursive iteration. But this can lead quadratic complexity for some of the cases. Other way is to make this linear while cache building. Pull Request: https://projects.blender.org/blender/blender/pulls/120375 --- source/blender/blenkernel/BKE_node_runtime.hh | 10 ++++++ source/blender/blenkernel/intern/node.cc | 31 ------------------- .../blender/blenkernel/intern/node_runtime.cc | 20 ++++++++++++ .../blenkernel/intern/node_tree_update.cc | 2 +- .../blenkernel/intern/node_tree_zones.cc | 3 +- source/blender/makesdna/DNA_node_types.h | 3 ++ .../intern/geometry_nodes_lazy_function.cc | 11 +++---- 7 files changed, 40 insertions(+), 40 deletions(-) diff --git a/source/blender/blenkernel/BKE_node_runtime.hh b/source/blender/blenkernel/BKE_node_runtime.hh index 9b9f07adf2b..33462f7ad86 100644 --- a/source/blender/blenkernel/BKE_node_runtime.hh +++ b/source/blender/blenkernel/BKE_node_runtime.hh @@ -300,6 +300,10 @@ class bNodeRuntime : NonCopyable, NonMovable { /** Used to avoid running forward compatibility code more often than necessary. */ bool forward_compatible_versioning_done = false; + /** If this node is reroute and this reroute is not logically linked with any source except other + * reroute, this will be true. */ + bool is_dangling_reroute = false; + /** Only valid if #topology_cache_is_dirty is false. */ Vector inputs; Vector outputs; @@ -731,6 +735,12 @@ inline blender::Span bNode::internal_links() const return this->runtime->internal_links; } +inline bool bNode::is_dangling_reroute() const +{ + BLI_assert(blender::bke::node_tree_runtime::topology_cache_is_available(*this)); + return this->runtime->is_dangling_reroute; +} + inline bool bNode::is_socket_drawn(const bNodeSocket &socket) const { return socket.is_visible(); diff --git a/source/blender/blenkernel/intern/node.cc b/source/blender/blenkernel/intern/node.cc index 331d91eea19..75d71175d49 100644 --- a/source/blender/blenkernel/intern/node.cc +++ b/source/blender/blenkernel/intern/node.cc @@ -2528,37 +2528,6 @@ void nodeParentsIter(bNode *node, bool (*callback)(bNode *, void *), void *userd } } -bool nodeIsDanglingReroute(const bNodeTree *ntree, const bNode *node) -{ - ntree->ensure_topology_cache(); - BLI_assert(node_tree_runtime::topology_cache_is_available(*ntree)); - - const bNode *iter_node = node; - Set visited_nodes; - while (true) { - if (!iter_node->is_reroute()) { - return false; - } - if (!visited_nodes.add(iter_node)) { - /* Treat cycle of reroute as dangling reroute branch. */ - return true; - } - const Span links = iter_node->input_socket(0).directly_linked_links(); - BLI_assert(links.size() <= 1); - if (links.is_empty()) { - return true; - } - const bNodeLink &link = *links[0]; - if (!link.is_available()) { - return false; - } - if (link.is_muted()) { - return false; - } - iter_node = link.fromnode; - } -} - } // namespace blender::bke void nodeUniqueName(bNodeTree *ntree, bNode *node) diff --git a/source/blender/blenkernel/intern/node_runtime.cc b/source/blender/blenkernel/intern/node_runtime.cc index 09b0c59a671..8e75e9b8b22 100644 --- a/source/blender/blenkernel/intern/node_runtime.cc +++ b/source/blender/blenkernel/intern/node_runtime.cc @@ -509,6 +509,25 @@ static void update_group_output_node(const bNodeTree &ntree) } } +static void update_dangling_reroute_nodes(const bNodeTree &ntree) +{ + for (const bNode *node : ntree.runtime->toposort_left_to_right) { + bNodeRuntime &node_runtime = *node->runtime; + if (!node->is_reroute()) { + node_runtime.is_dangling_reroute = false; + continue; + } + const Span links = node_runtime.inputs[0]->runtime->directly_linked_links; + if (links.is_empty()) { + node_runtime.is_dangling_reroute = true; + continue; + } + BLI_assert(links.size() == 1); + const bNode &source_node = *links.first()->fromnode; + node_runtime.is_dangling_reroute = source_node.runtime->is_dangling_reroute; + } +} + static void ensure_topology_cache(const bNodeTree &ntree) { bNodeTreeRuntime &tree_runtime = *ntree.runtime; @@ -546,6 +565,7 @@ static void ensure_topology_cache(const bNodeTree &ntree) [&]() { update_root_frames(ntree); }, [&]() { update_direct_frames_childrens(ntree); }); update_group_output_node(ntree); + update_dangling_reroute_nodes(ntree); tree_runtime.topology_cache_exists = true; }); } diff --git a/source/blender/blenkernel/intern/node_tree_update.cc b/source/blender/blenkernel/intern/node_tree_update.cc index fe3cff598d7..d5252925750 100644 --- a/source/blender/blenkernel/intern/node_tree_update.cc +++ b/source/blender/blenkernel/intern/node_tree_update.cc @@ -625,7 +625,7 @@ class NodeTreeMainUpdater { const Span links) const { for (const bNodeLink *link : links) { - if (!bke::nodeIsDanglingReroute(&ntree, link->fromnode)) { + if (!link->fromnode->is_dangling_reroute()) { return link; } } diff --git a/source/blender/blenkernel/intern/node_tree_zones.cc b/source/blender/blenkernel/intern/node_tree_zones.cc index 54288723286..388c74c5c12 100644 --- a/source/blender/blenkernel/intern/node_tree_zones.cc +++ b/source/blender/blenkernel/intern/node_tree_zones.cc @@ -189,6 +189,7 @@ static bool update_zone_per_node(const Span all_nodes, static void update_zone_border_links(const bNodeTree &tree, bNodeTreeZones &tree_zones) { + tree.ensure_topology_cache(); for (const bNodeLink *link : tree.all_links()) { if (!link->is_available()) { continue; @@ -196,7 +197,7 @@ static void update_zone_border_links(const bNodeTree &tree, bNodeTreeZones &tree if (link->is_muted()) { continue; } - if (bke::nodeIsDanglingReroute(&tree, link->fromnode)) { + if (link->fromnode->is_dangling_reroute()) { continue; } bNodeTreeZone *from_zone = const_cast( diff --git a/source/blender/makesdna/DNA_node_types.h b/source/blender/makesdna/DNA_node_types.h index 50ee3f41235..e3d0f6ece6d 100644 --- a/source/blender/makesdna/DNA_node_types.h +++ b/source/blender/makesdna/DNA_node_types.h @@ -439,6 +439,9 @@ typedef struct bNode { /** A span containing all internal links when the node is muted. */ blender::Span internal_links() const; + /* This node is reroute which is not logically connected to any source of value. */ + bool is_dangling_reroute() const; + /* True if the socket is visible and has a valid location. The icon may not be visible. */ bool is_socket_drawn(const bNodeSocket &socket) const; /* True if the socket is drawn and the icon is visible. */ diff --git a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc index fb88a0d01ee..52bbb581030 100644 --- a/source/blender/nodes/intern/geometry_nodes_lazy_function.cc +++ b/source/blender/nodes/intern/geometry_nodes_lazy_function.cc @@ -396,10 +396,9 @@ class LazyFunctionForMultiInput : public LazyFunction { base_type_ = get_socket_cpp_type(socket); BLI_assert(base_type_ != nullptr); BLI_assert(socket.is_multi_input()); - const bNodeTree &btree = socket.owner_tree(); for (const bNodeLink *link : socket.directly_linked_links()) { if (link->is_muted() || !link->fromsock->is_available() || - bke::nodeIsDanglingReroute(&btree, link->fromnode)) + link->fromnode->is_dangling_reroute()) { continue; } @@ -777,9 +776,7 @@ class LazyFunctionForViewerNode : public LazyFunction { continue; } const Span links = bsocket->directly_linked_links(); - if (links.is_empty() || - bke::nodeIsDanglingReroute(&bnode.owner_tree(), links.first()->fromnode)) - { + if (links.is_empty() || links.first()->fromnode->is_dangling_reroute()) { use_field_input_ = false; inputs_.pop_last(); r_lf_index_by_bsocket[bsocket->index_in_tree()] = -1; @@ -3784,7 +3781,7 @@ struct GeometryNodesLazyFunctionBuilder { lf::OutputSocket &from_lf_socket, BuildGraphParams &graph_params) { - if (bke::nodeIsDanglingReroute(&btree_, &from_bsocket.owner_node())) { + if (from_bsocket.owner_node().is_dangling_reroute()) { return; } @@ -3871,7 +3868,7 @@ struct GeometryNodesLazyFunctionBuilder { break; } if (multi_input_link->is_muted() || !multi_input_link->fromsock->is_available() || - bke::nodeIsDanglingReroute(&btree_, multi_input_link->fromnode)) + multi_input_link->fromnode->is_dangling_reroute()) { continue; }