From 08d171734c1c8c2de9d2608b2141b28467e2cb44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Fri, 7 Apr 2023 14:54:15 +0200 Subject: [PATCH 1/4] Support for basic copying inside the same node tree. --- .../blender/editors/space_node/clipboard.cc | 31 +++++++++++++++++++ .../blender/editors/space_node/node_edit.cc | 30 ++++++++++++++++++ .../blender/editors/space_node/node_group.cc | 26 ++++++++++++++++ 3 files changed, 87 insertions(+) diff --git a/source/blender/editors/space_node/clipboard.cc b/source/blender/editors/space_node/clipboard.cc index b232dbecf7a..3d8675742c0 100644 --- a/source/blender/editors/space_node/clipboard.cc +++ b/source/blender/editors/space_node/clipboard.cc @@ -16,6 +16,8 @@ #include "ED_render.h" #include "ED_screen.h" +#include "NOD_socket.h" + #include "RNA_access.h" #include "RNA_define.h" @@ -182,6 +184,33 @@ void NODE_OT_clipboard_copy(wmOperatorType *ot) /** \name Paste * \{ */ +static void remap_pairing(bNodeTree &dst_tree, const Map &node_map) +{ + /* We don't have the old tree for looking up output nodes by ID, + * so have to build a map first to find copied output nodes in the new tree. */ + Map dst_output_node_map; + for (const auto &item : node_map.items()) { + if (item.key->type == GEO_NODE_SIMULATION_OUTPUT) { + dst_output_node_map.add_new(item.key->identifier, item.value); + } + } + + for (bNode *dst_node : node_map.values()) { + if (dst_node->type == GEO_NODE_SIMULATION_INPUT) { + NodeGeometrySimulationInput *data = static_cast( + dst_node->storage); + const bNode *dst_output_node = dst_output_node_map.lookup_default(data->output_node_id, nullptr); + if (dst_output_node != nullptr) { + data->output_node_id = dst_output_node->identifier; + } + else { + data->output_node_id = 0; + blender::nodes::update_node_declaration_and_sockets(dst_tree, *dst_node); + } + } + } +} + static int node_clipboard_paste_exec(bContext *C, wmOperator *op) { SpaceNode &snode = *CTX_wm_space_node(C); @@ -289,6 +318,8 @@ static int node_clipboard_paste_exec(bContext *C, wmOperator *op) } } + remap_pairing(tree, node_map); + tree.ensure_topology_cache(); for (bNode *new_node : node_map.values()) { /* Update multi input socket indices in case all connected nodes weren't copied. */ diff --git a/source/blender/editors/space_node/node_edit.cc b/source/blender/editors/space_node/node_edit.cc index 9e556dcc6b0..47d48400ef3 100644 --- a/source/blender/editors/space_node/node_edit.cc +++ b/source/blender/editors/space_node/node_edit.cc @@ -64,6 +64,7 @@ #include "NOD_composite.h" #include "NOD_geometry.h" #include "NOD_shader.h" +#include "NOD_socket.h" #include "NOD_texture.h" #include "node_intern.hh" /* own include */ @@ -1247,6 +1248,33 @@ static void node_duplicate_reparent_recursive(bNodeTree *ntree, } } +static void remap_pairing(bNodeTree &dst_tree, const Map &node_map) +{ + /* We don't have the old tree for looking up output nodes by ID, + * so have to build a map first to find copied output nodes in the new tree. */ + Map dst_output_node_map; + for (const auto &item : node_map.items()) { + if (item.key->type == GEO_NODE_SIMULATION_OUTPUT) { + dst_output_node_map.add_new(item.key->identifier, item.value); + } + } + + for (bNode *dst_node : node_map.values()) { + if (dst_node->type == GEO_NODE_SIMULATION_INPUT) { + NodeGeometrySimulationInput *data = static_cast( + dst_node->storage); + const bNode *dst_output_node = dst_output_node_map.lookup_default(data->output_node_id, nullptr); + if (dst_output_node != nullptr) { + data->output_node_id = dst_output_node->identifier; + } + else { + data->output_node_id = 0; + blender::nodes::update_node_declaration_and_sockets(dst_tree, *dst_node); + } + } + } +} + static int node_duplicate_exec(bContext *C, wmOperator *op) { Main *bmain = CTX_data_main(C); @@ -1334,6 +1362,8 @@ static int node_duplicate_exec(bContext *C, wmOperator *op) } } + remap_pairing(*ntree, node_map); + /* Deselect old nodes, select the copies instead. */ for (const auto item : node_map.items()) { bNode *src_node = item.key; diff --git a/source/blender/editors/space_node/node_group.cc b/source/blender/editors/space_node/node_group.cc index b5edc294760..74f0b3796a0 100644 --- a/source/blender/editors/space_node/node_group.cc +++ b/source/blender/editors/space_node/node_group.cc @@ -436,6 +436,30 @@ void NODE_OT_group_ungroup(wmOperatorType *ot) /** \name Separate Operator * \{ */ +static void remap_pairing(bNodeTree &dst_tree, const Set &nodes) +{ + for (bNode *dst_node : nodes) { + if (dst_node->type == GEO_NODE_SIMULATION_INPUT) { + NodeGeometrySimulationInput *data = static_cast( + dst_node->storage); + /* XXX Technically this is not correct because the output_node_id is only valid + * in the original node group tree and we'd have map old IDs to new nodes first. + * The ungroup operator does not build a node map, it just expects node IDs to + * remain unchanged, which is probably true most of the time because they are + * mostly random numbers out of the uint32_t range. + */ + const bNode *dst_output_node = dst_tree.node_by_id(data->output_node_id); + if (dst_output_node != nullptr) { + data->output_node_id = dst_output_node->identifier; + } + else { + data->output_node_id = 0; + blender::nodes::update_node_declaration_and_sockets(dst_tree, *dst_node); + } + } + } +} + /** * \return True if successful. */ @@ -540,6 +564,8 @@ static bool node_group_separate_selected( } } + remap_pairing(ntree, new_nodes); + BKE_ntree_update_tag_all(&ntree); if (!make_copy) { BKE_ntree_update_tag_all(&ngroup); -- 2.30.2 From b43a85ce5fbaf6399deadaf25a90fe4f5eb8554d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Tue, 11 Apr 2023 14:47:00 +0200 Subject: [PATCH 2/4] Move calls to `nodeDeclarationEnsure` past link remapping. This is necessary because link remapping uses the `socket_map`. If the declaration removes sockets this `socket_map` becomes invalid. So updating the declaration may only happen _after_ any references to previous sockets are no longer needed. --- source/blender/editors/space_node/clipboard.cc | 8 ++++---- source/blender/editors/space_node/node_edit.cc | 8 ++++---- source/blender/editors/space_node/node_group.cc | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/source/blender/editors/space_node/clipboard.cc b/source/blender/editors/space_node/clipboard.cc index 3d8675742c0..33c63178be0 100644 --- a/source/blender/editors/space_node/clipboard.cc +++ b/source/blender/editors/space_node/clipboard.cc @@ -266,10 +266,6 @@ static int node_clipboard_paste_exec(bContext *C, wmOperator *op) } } - for (bNode *new_node : node_map.values()) { - nodeDeclarationEnsure(&tree, new_node); - } - for (bNode *new_node : node_map.values()) { nodeSetSelected(new_node, true); @@ -318,6 +314,10 @@ static int node_clipboard_paste_exec(bContext *C, wmOperator *op) } } + for (bNode *new_node : node_map.values()) { + nodeDeclarationEnsure(&tree, new_node); + } + remap_pairing(tree, node_map); tree.ensure_topology_cache(); diff --git a/source/blender/editors/space_node/node_edit.cc b/source/blender/editors/space_node/node_edit.cc index c99a583480b..e5d087e38ec 100644 --- a/source/blender/editors/space_node/node_edit.cc +++ b/source/blender/editors/space_node/node_edit.cc @@ -1313,10 +1313,6 @@ static int node_duplicate_exec(bContext *C, wmOperator *op) return OPERATOR_CANCELLED; } - for (bNode *node : node_map.values()) { - nodeDeclarationEnsure(ntree, node); - } - /* Copy links between selected nodes. */ bNodeLink *lastlink = (bNodeLink *)ntree->links.last; LISTBASE_FOREACH (bNodeLink *, link, &ntree->links) { @@ -1352,6 +1348,10 @@ static int node_duplicate_exec(bContext *C, wmOperator *op) } } + for (bNode *node : node_map.values()) { + nodeDeclarationEnsure(ntree, node); + } + /* Clear flags for recursive depth-first iteration. */ for (bNode *node : ntree->all_nodes()) { node->flag &= ~NODE_TEST; diff --git a/source/blender/editors/space_node/node_group.cc b/source/blender/editors/space_node/node_group.cc index 7436ba4f688..1ae3adb381b 100644 --- a/source/blender/editors/space_node/node_group.cc +++ b/source/blender/editors/space_node/node_group.cc @@ -521,10 +521,6 @@ static bool node_group_separate_selected( nodeRebuildIDVector(&ngroup); } - for (bNode *node : new_nodes) { - nodeDeclarationEnsure(&ntree, node); - } - /* add internal links to the ntree */ LISTBASE_FOREACH_MUTABLE (bNodeLink *, link, &ngroup.links) { const bool fromselect = (link->fromnode && (link->fromnode->flag & NODE_SELECT)); @@ -552,6 +548,10 @@ static bool node_group_separate_selected( } } + for (bNode *node : new_nodes) { + nodeDeclarationEnsure(&ntree, node); + } + /* and copy across the animation, * note that the animation data's action can be nullptr here */ if (ngroup.adt) { -- 2.30.2 From e6c71cb9890b5a4e63cef5350e6f59bbd57f52db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Tue, 11 Apr 2023 14:48:49 +0200 Subject: [PATCH 3/4] Perform a full socket update when updating dynamic declarations. This is to ensure that dynamic declarations can actually change the socket layout outside of operators that explicitly call `update_node_declaration_and_sockets`. --- source/blender/blenkernel/intern/node.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/blender/blenkernel/intern/node.cc b/source/blender/blenkernel/intern/node.cc index aeac1f0c6fe..13b0a0a6894 100644 --- a/source/blender/blenkernel/intern/node.cc +++ b/source/blender/blenkernel/intern/node.cc @@ -3817,8 +3817,7 @@ bool nodeDeclarationEnsureOnOutdatedNode(bNodeTree *ntree, bNode *node) if (node->typeinfo->declare_dynamic) { BLI_assert(ntree != nullptr); BLI_assert(node != nullptr); - node->runtime->declaration = new blender::nodes::NodeDeclaration(); - blender::nodes::build_node_declaration_dynamic(*ntree, *node, *node->runtime->declaration); + blender::nodes::update_node_declaration_and_sockets(*ntree, *node); return true; } if (node->typeinfo->declare) { -- 2.30.2 From ca69bd28780df870cee81b3e1015e00293b48e83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20T=C3=B6nne?= Date: Wed, 12 Apr 2023 11:51:47 +0200 Subject: [PATCH 4/4] Use int32_t in the clipboard remap_pairing function. --- source/blender/editors/space_node/clipboard.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/blender/editors/space_node/clipboard.cc b/source/blender/editors/space_node/clipboard.cc index 33c63178be0..9e508215cc4 100644 --- a/source/blender/editors/space_node/clipboard.cc +++ b/source/blender/editors/space_node/clipboard.cc @@ -188,7 +188,7 @@ static void remap_pairing(bNodeTree &dst_tree, const Map { /* We don't have the old tree for looking up output nodes by ID, * so have to build a map first to find copied output nodes in the new tree. */ - Map dst_output_node_map; + Map dst_output_node_map; for (const auto &item : node_map.items()) { if (item.key->type == GEO_NODE_SIMULATION_OUTPUT) { dst_output_node_map.add_new(item.key->identifier, item.value); -- 2.30.2