[Bf-blender-cvs] [fdc4a1a590d] master: Nodes: Refactor to remove node and socket "new" pointers

Hans Goudey noreply at git.blender.org
Wed Dec 22 15:47:57 CET 2021


Commit: fdc4a1a590d8befb1ff9ab1de3f02d82aa46d539
Author: Hans Goudey
Date:   Wed Dec 22 08:47:46 2021 -0600
Branches: master
https://developer.blender.org/rBfdc4a1a590d8befb1ff9ab1de3f02d82aa46d539

Nodes: Refactor to remove node and socket "new" pointers

These pointers point to the new nodes when duplicating,
and their even used to point to "original" nodes for
"localized" trees. They're just a bad design decision
that make code confusing and buggy.

Instead, node copy functions now optionally add to a map
of old to new socket pointers. The case where the compositor
abused these pointers as "original" pointers are handled
by looking up the string node names.

Differential Revision: https://developer.blender.org/D13518

===================================================================

M	source/blender/blenkernel/BKE_node.h
M	source/blender/blenkernel/intern/node.cc
M	source/blender/blenloader/intern/versioning_250.c
M	source/blender/editors/space_node/node_edit.cc
M	source/blender/editors/space_node/node_group.cc
M	source/blender/makesdna/DNA_node_types.h
M	source/blender/nodes/composite/node_composite_tree.cc
M	source/blender/nodes/shader/node_shader_tree.c

===================================================================

diff --git a/source/blender/blenkernel/BKE_node.h b/source/blender/blenkernel/BKE_node.h
index 500d8e2e0ac..1e0a75bfc57 100644
--- a/source/blender/blenkernel/BKE_node.h
+++ b/source/blender/blenkernel/BKE_node.h
@@ -34,6 +34,7 @@
 #include "RNA_types.h"
 
 #ifdef __cplusplus
+#  include "BLI_map.hh"
 #  include "BLI_string_ref.hh"
 #endif
 
@@ -518,8 +519,6 @@ void ntreeSetOutput(struct bNodeTree *ntree);
 
 void ntreeFreeCache(struct bNodeTree *ntree);
 
-bool ntreeNodeExists(const struct bNodeTree *ntree, const struct bNode *testnode);
-bool ntreeOutputExists(const struct bNode *node, const struct bNodeSocket *testsock);
 void ntreeNodeFlagSet(const bNodeTree *ntree, const int flag, const bool enable);
 /**
  * Returns localized tree for execution in threads.
@@ -697,31 +696,27 @@ void nodeRemoveNode(struct Main *bmain,
                     struct bNode *node,
                     bool do_id_user);
 
+#ifdef __cplusplus
+
+namespace blender::bke {
+
 /**
- * \param ntree: is the target tree.
- *
- * \note keep socket list order identical, for copying links.
+ * \note keeps socket list order identical, for copying links.
  * \note `unique_name` needs to be true. It's only disabled for speed when doing GPUnodetrees.
  */
-struct bNode *BKE_node_copy_ex(struct bNodeTree *ntree,
-                               const struct bNode *node_src,
-                               const int flag,
-                               const bool unique_name);
+bNode *node_copy_with_mapping(bNodeTree *dst_tree,
+                              const bNode &node_src,
+                              int flag,
+                              bool unique_name,
+                              Map<const bNodeSocket *, bNodeSocket *> &new_socket_map);
 
-/**
- * Same as #BKE_node_copy_ex but stores pointers to a new node and its sockets in the source node.
- *
- * NOTE: DANGER ZONE!
- *
- * TODO(sergey): Maybe it's better to make BKE_node_copy_ex() return a mapping from old node and
- * sockets to new one.
- */
-struct bNode *BKE_node_copy_store_new_pointers(struct bNodeTree *ntree,
-                                               struct bNode *node_src,
-                                               const int flag);
-struct bNodeTree *ntreeCopyTree_ex_new_pointers(const struct bNodeTree *ntree,
-                                                struct Main *bmain,
-                                                const bool do_id_user);
+bNode *node_copy(bNodeTree *dst_tree, const bNode &src_node, int flag, bool unique_name);
+
+}  // namespace blender::bke
+
+#endif
+
+bNode *BKE_node_copy(bNodeTree *dst_tree, const bNode *src_node, int flag, bool unique_name);
 
 /**
  * Also used via RNA API, so we check for proper input output direction.
diff --git a/source/blender/blenkernel/intern/node.cc b/source/blender/blenkernel/intern/node.cc
index 7121ef20207..05096ae63c1 100644
--- a/source/blender/blenkernel/intern/node.cc
+++ b/source/blender/blenkernel/intern/node.cc
@@ -99,6 +99,7 @@
 #define NODE_DEFAULT_MAX_WIDTH 700
 
 using blender::Array;
+using blender::Map;
 using blender::MutableSpan;
 using blender::Set;
 using blender::Span;
@@ -152,62 +153,41 @@ static void ntree_copy_data(Main *UNUSED(bmain), ID *id_dst, const ID *id_src, c
   BLI_listbase_clear(&ntree_dst->nodes);
   BLI_listbase_clear(&ntree_dst->links);
 
-  /* Since source nodes and sockets are unique pointers we can put everything in a single map. */
-  GHash *new_pointers = BLI_ghash_ptr_new(__func__);
+  Map<const bNode *, bNode *> node_map;
+  Map<const bNodeSocket *, bNodeSocket *> socket_map;
 
-  LISTBASE_FOREACH (const bNode *, node_src, &ntree_src->nodes) {
-    bNode *new_node = BKE_node_copy_ex(ntree_dst, node_src, flag_subdata, true);
-    BLI_ghash_insert(new_pointers, (void *)node_src, new_node);
-    /* Store mapping to inputs. */
-    bNodeSocket *new_input_sock = (bNodeSocket *)new_node->inputs.first;
-    const bNodeSocket *input_sock_src = (const bNodeSocket *)node_src->inputs.first;
-    while (new_input_sock != nullptr) {
-      BLI_ghash_insert(new_pointers, (void *)input_sock_src, new_input_sock);
-      new_input_sock = new_input_sock->next;
-      input_sock_src = input_sock_src->next;
-    }
-    /* Store mapping to outputs. */
-    bNodeSocket *new_output_sock = (bNodeSocket *)new_node->outputs.first;
-    const bNodeSocket *output_sock_src = (const bNodeSocket *)node_src->outputs.first;
-    while (new_output_sock != nullptr) {
-      BLI_ghash_insert(new_pointers, (void *)output_sock_src, new_output_sock);
-      new_output_sock = new_output_sock->next;
-      output_sock_src = output_sock_src->next;
-    }
+  BLI_listbase_clear(&ntree_dst->nodes);
+  LISTBASE_FOREACH (const bNode *, src_node, &ntree_src->nodes) {
+    bNode *new_node = blender::bke::node_copy_with_mapping(
+        ntree_dst, *src_node, flag_subdata, true, socket_map);
+    node_map.add(src_node, new_node);
   }
 
   /* copy links */
-  BLI_duplicatelist(&ntree_dst->links, &ntree_src->links);
-  LISTBASE_FOREACH (bNodeLink *, link_dst, &ntree_dst->links) {
-    link_dst->fromnode = (bNode *)BLI_ghash_lookup_default(
-        new_pointers, link_dst->fromnode, nullptr);
-    link_dst->fromsock = (bNodeSocket *)BLI_ghash_lookup_default(
-        new_pointers, link_dst->fromsock, nullptr);
-    link_dst->tonode = (bNode *)BLI_ghash_lookup_default(new_pointers, link_dst->tonode, nullptr);
-    link_dst->tosock = (bNodeSocket *)BLI_ghash_lookup_default(
-        new_pointers, link_dst->tosock, nullptr);
-    /* update the link socket's pointer */
-    if (link_dst->tosock) {
-      link_dst->tosock->link = link_dst;
-    }
+  BLI_listbase_clear(&ntree_dst->links);
+  LISTBASE_FOREACH (const bNodeLink *, src_link, &ntree_src->links) {
+    bNodeLink *dst_link = (bNodeLink *)MEM_dupallocN(src_link);
+    dst_link->fromnode = node_map.lookup(src_link->fromnode);
+    dst_link->fromsock = socket_map.lookup(src_link->fromsock);
+    dst_link->tonode = node_map.lookup(src_link->tonode);
+    dst_link->tosock = socket_map.lookup(src_link->tosock);
+    BLI_assert(dst_link->tosock);
+    dst_link->tosock->link = dst_link;
+    BLI_addtail(&ntree_dst->links, dst_link);
   }
 
   /* copy interface sockets */
-  BLI_duplicatelist(&ntree_dst->inputs, &ntree_src->inputs);
-  bNodeSocket *sock_dst, *sock_src;
-  for (sock_dst = (bNodeSocket *)ntree_dst->inputs.first,
-      sock_src = (bNodeSocket *)ntree_src->inputs.first;
-       sock_dst != nullptr;
-       sock_dst = (bNodeSocket *)sock_dst->next, sock_src = (bNodeSocket *)sock_src->next) {
-    node_socket_copy(sock_dst, sock_src, flag_subdata);
+  BLI_listbase_clear(&ntree_dst->inputs);
+  LISTBASE_FOREACH (const bNodeSocket *, src_socket, &ntree_src->inputs) {
+    bNodeSocket *dst_socket = (bNodeSocket *)MEM_dupallocN(src_socket);
+    node_socket_copy(dst_socket, src_socket, flag_subdata);
+    BLI_addtail(&ntree_dst->inputs, dst_socket);
   }
-
-  BLI_duplicatelist(&ntree_dst->outputs, &ntree_src->outputs);
-  for (sock_dst = (bNodeSocket *)ntree_dst->outputs.first,
-      sock_src = (bNodeSocket *)ntree_src->outputs.first;
-       sock_dst != nullptr;
-       sock_dst = (bNodeSocket *)sock_dst->next, sock_src = (bNodeSocket *)sock_src->next) {
-    node_socket_copy(sock_dst, sock_src, flag_subdata);
+  BLI_listbase_clear(&ntree_dst->outputs);
+  LISTBASE_FOREACH (const bNodeSocket *, src_socket, &ntree_src->outputs) {
+    bNodeSocket *dst_socket = (bNodeSocket *)MEM_dupallocN(src_socket);
+    node_socket_copy(dst_socket, src_socket, flag_subdata);
+    BLI_addtail(&ntree_dst->outputs, dst_socket);
   }
 
   /* copy preview hash */
@@ -227,18 +207,11 @@ static void ntree_copy_data(Main *UNUSED(bmain), ID *id_dst, const ID *id_src, c
   }
 
   /* update node->parent pointers */
-  for (bNode *node_dst = (bNode *)ntree_dst->nodes.first,
-             *node_src = (bNode *)ntree_src->nodes.first;
-       node_dst;
-       node_dst = (bNode *)node_dst->next, node_src = (bNode *)node_src->next) {
-    if (node_dst->parent) {
-      node_dst->parent = (bNode *)BLI_ghash_lookup_default(
-          new_pointers, node_dst->parent, nullptr);
+  LISTBASE_FOREACH (bNode *, new_node, &ntree_dst->nodes) {
+    if (new_node->parent) {
+      new_node->parent = node_map.lookup(new_node->parent);
     }
   }
-
-  BLI_ghash_free(new_pointers, nullptr, nullptr);
-
   /* node tree will generate its own interface type */
   ntree_dst->interface_type = nullptr;
 
@@ -2238,136 +2211,100 @@ static void node_socket_copy(bNodeSocket *sock_dst, const bNodeSocket *sock_src,
   sock_dst->cache = nullptr;
 }
 
-bNode *BKE_node_copy_ex(bNodeTree *ntree,
-                        const bNode *node_src,
-                        const int flag,
-                        const bool unique_name)
-{
-  bNode *node_dst = (bNode *)MEM_callocN(sizeof(bNode), "dupli node");
-  bNodeSocket *sock_dst, *sock_src;
-  bNodeLink *link_dst, *link_src;
+namespace blender::bke {
 
-  *node_dst = *node_src;
+bNode *node_copy_with_mapping(bNodeTree *dst_tree,
+                              const bNode &node_src,
+                              const int flag,
+                              const bool unique_name,
+                              Map<const bNodeSocket *, bNodeSocket *> &socket_map)
+{
+  bNode *node_dst = (bNode *)MEM_mallocN(sizeof(bNode), __func__);
+  *node_dst = node_src;
 
   /* Can be called for nodes outside a node tree (e.g. clipboard). */
-  if (ntree) {
+  if (dst_tree) {
     if (unique_name) {
-      nodeUniqueName(ntree, node_dst);
+      nodeUniqueName(dst_tree, node_dst);
     }
-
-    BLI_addtail(&ntree->nodes, node_dst);
+    BLI_addtail(&dst_tree->nodes, node_dst);
   }
 
-  BLI_duplicatelist(&node_dst->inputs, &node_src->inputs);
-  for (sock_dst = (bNodeSocket *)node_dst->inputs.first,
-      sock_src = (bNodeSocket *)node_src->inputs.first;
-       sock_dst != nullptr;
-       sock_dst = (bNodeSocket *)sock_dst->next, sock_src = (bNodeSocket *)sock_src->next) {
-    node_socket_copy(sock_dst, sock_src, flag);
+  BLI_listbase_clear(&node_dst->inputs);
+  LISTBASE_FOREACH (const bNodeSocket *, src_socket, &node_src.inputs) {
+    bNodeSocket *dst_socket = (bNodeSocket *)MEM_dupallocN(src_socket)

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list