Fix T64247: Crash on playback with special shader node tree

The root of the problem goes to the fact that node tree copying
uses source tree and nodes for a temporary storage.

This makes it so multiple dependency graphs can not be reliably
evaluated from different threads if they are using same original
node tree.

Solved by doing the following:

- Commonly used tree copying function (which is used by library
  manager) keeps source tree, nodes and sockets untouched.

- All the related areas (like node tree's callback) now have
  const qualifier on the input.

- Areas which needs to have those temporary pointers assigned are
  now using explicit function.

  Would be really cool to get rid of those temporary pointers
  completely, but this is a bit tricky due to hairy nature of the
  code. Can happen any time now though: is easy enough to generalize
  the new pointers mapping.

Note that this change is only intended to solve the crash.
The fact that icons shouldn't be updated on playback will be fixed
as a separate change.

Reviewers: brecht, fclem

Reviewed By: brecht, fclem

Subscribers: brecht, fclem

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

1 files changed, 71 insertions, 17 deletions

diff --git a/source/blender/blenkernel/intern/node.c b/source/blender/blenkernel/intern/node.c
index b7db434e234..d8b63dac99d 100644
--- a/source/blender/blenkernel/intern/node.c
+++ b/source/blender/blenkernel/intern/node.c
@@ -41,6 +41,7 @@
 #include "DNA_linestyle_types.h"
 
 #include "BLI_listbase.h"
+#include "BLI_ghash.h"
 #include "BLI_math.h"
 #include "BLI_path_util.h"
 #include "BLI_string.h"
@@ -1008,10 +1009,8 @@ bNode *nodeAddStaticNode(const struct bContext *C, bNodeTree *ntree, int type)
   return nodeAddNode(C, ntree, idname);
 }
 
-static void node_socket_copy(bNodeSocket *sock_dst, bNodeSocket *sock_src, const int flag)
+static void node_socket_copy(bNodeSocket *sock_dst, const bNodeSocket *sock_src, const int flag)
 {
-  sock_src->new_sock = sock_dst;
-
   if (sock_src->prop) {
     sock_dst->prop = IDP_CopyProperty_ex(sock_src->prop, flag);
   }
@@ -1029,7 +1028,7 @@ static void node_socket_copy(bNodeSocket *sock_dst, bNodeSocket *sock_src, const
 
 /* keep socket listorder identical, for copying links */
 /* ntree is the target tree */
-bNode *BKE_node_copy_ex(bNodeTree *ntree, bNode *node_src, const int flag)
+bNode *BKE_node_copy_ex(bNodeTree *ntree, const bNode *node_src, const int flag)
 {
   bNode *node_dst = MEM_callocN(sizeof(bNode), "dupli node");
   bNodeSocket *sock_dst, *sock_src;
@@ -1063,10 +1062,17 @@ bNode *BKE_node_copy_ex(bNodeTree *ntree, bNode *node_src, const int flag)
   for (link_dst = node_dst->internal_links.first, link_src = node_src->internal_links.first;
        link_dst != NULL;
        link_dst = link_dst->next, link_src = link_src->next) {
+    /* This is a bit annoying to do index lookups in a list, but is likely to be faster than
+     * trying to create a hash-map. At least for usual nodes, which only have so much sockets
+     * and internal links. */
+    const int from_sock_index = BLI_findindex(&node_src->inputs, link_src->fromsock);
+    const int to_sock_index = BLI_findindex(&node_src->outputs, link_src->tosock);
+    BLI_assert(from_sock_index != -1);
+    BLI_assert(to_sock_index != -1);
     link_dst->fromnode = node_dst;
     link_dst->tonode = node_dst;
-    link_dst->fromsock = link_dst->fromsock->new_sock;
-    link_dst->tosock = link_dst->tosock->new_sock;
+    link_dst->fromsock = BLI_findlink(&node_dst->inputs, from_sock_index);
+    link_dst->tosock = BLI_findlink(&node_dst->outputs, to_sock_index);
   }
 
   if ((flag & LIB_ID_CREATE_NO_USER_REFCOUNT) == 0) {
@@ -1077,7 +1083,6 @@ bNode *BKE_node_copy_ex(bNodeTree *ntree, bNode *node_src, const int flag)
     node_src->typeinfo->copyfunc(ntree, node_dst, node_src);
   }
 
-  node_src->new_node = node_dst;
   node_dst->new_node = NULL;
 
   bool do_copy_api = !((flag & LIB_ID_CREATE_NO_MAIN) || (flag & LIB_ID_COPY_LOCALIZE));
@@ -1095,6 +1100,30 @@ bNode *BKE_node_copy_ex(bNodeTree *ntree, bNode *node_src, const int flag)
   return node_dst;
 }
 
+bNode *BKE_node_copy_store_new_pointers(bNodeTree *ntree, bNode *node_src, const int flag)
+{
+  bNode *new_node = BKE_node_copy_ex(ntree, node_src, flag);
+  /* Store mapping to the node itself. */
+  node_src->new_node = new_node;
+  /* Store mapping to inputs. */
+  bNodeSocket *new_input_sock = new_node->inputs.first;
+  bNodeSocket *input_sock_src = node_src->inputs.first;
+  while (new_input_sock != NULL) {
+    input_sock_src->new_sock = 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 = new_node->outputs.first;
+  bNodeSocket *output_sock_src = node_src->outputs.first;
+  while (new_output_sock != NULL) {
+    output_sock_src->new_sock = new_output_sock;
+    new_output_sock = new_output_sock->next;
+    output_sock_src = output_sock_src->next;
+  }
+  return new_node;
+}
+
 /* also used via rna api, so we check for proper input output direction */
 bNodeLink *nodeAddLink(
     bNodeTree *ntree, bNode *fromnode, bNodeSocket *fromsock, bNode *tonode, bNodeSocket *tosock)
@@ -1402,23 +1431,45 @@ void BKE_node_tree_copy_data(Main *UNUSED(bmain),
   BLI_listbase_clear(&ntree_dst->nodes);
   BLI_listbase_clear(&ntree_dst->links);
 
-  for (bNode *node_src = ntree_src->nodes.first; node_src; node_src = node_src->next) {
-    BKE_node_copy_ex(ntree_dst, node_src, flag_subdata);
+  /* Since source nodes and sockets are unique pointers we can put everything in a single map. */
+  GHash *new_pointers = BLI_ghash_ptr_new("BKE_node_tree_copy_data");
+
+  for (const bNode *node_src = ntree_src->nodes.first; node_src; node_src = node_src->next) {
+    bNode *new_node = BKE_node_copy_ex(ntree_dst, node_src, flag_subdata);
+    BLI_ghash_insert(new_pointers, (void *)node_src, new_node);
+    /* Store mapping to inputs. */
+    bNodeSocket *new_input_sock = new_node->inputs.first;
+    const bNodeSocket *input_sock_src = node_src->inputs.first;
+    while (new_input_sock != NULL) {
+      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 = new_node->outputs.first;
+    const bNodeSocket *output_sock_src = node_src->outputs.first;
+    while (new_output_sock != NULL) {
+      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;
+    }
   }
 
   /* copy links */
   BLI_duplicatelist(&ntree_dst->links, &ntree_src->links);
   for (link_dst = ntree_dst->links.first; link_dst; link_dst = link_dst->next) {
-    link_dst->fromnode = (link_dst->fromnode ? link_dst->fromnode->new_node : NULL);
-    link_dst->fromsock = (link_dst->fromsock ? link_dst->fromsock->new_sock : NULL);
-    link_dst->tonode = (link_dst->tonode ? link_dst->tonode->new_node : NULL);
-    link_dst->tosock = (link_dst->tosock ? link_dst->tosock->new_sock : NULL);
+    link_dst->fromnode = BLI_ghash_lookup_default(new_pointers, link_dst->fromnode, NULL);
+    link_dst->fromsock = BLI_ghash_lookup_default(new_pointers, link_dst->fromsock, NULL);
+    link_dst->tonode = BLI_ghash_lookup_default(new_pointers, link_dst->tonode, NULL);
+    link_dst->tosock = BLI_ghash_lookup_default(new_pointers, link_dst->tosock, NULL);
     /* update the link socket's pointer */
     if (link_dst->tosock) {
       link_dst->tosock->link = link_dst;
     }
   }
 
+  BLI_ghash_free(new_pointers, NULL, NULL);
+
   /* copy interface sockets */
   BLI_duplicatelist(&ntree_dst->inputs, &ntree_src->inputs);
   for (sock_dst = ntree_dst->inputs.first, sock_src = ntree_src->inputs.first; sock_dst != NULL;
@@ -2201,9 +2252,12 @@ bNodeTree *ntreeLocalize(bNodeTree *ntree)
     /* ensures only a single output node is enabled */
     ntreeSetOutput(ntree);
 
-    for (node = ntree->nodes.first; node; node = node->next) {
-      /* store new_node pointer to original */
-      node->new_node->original = node;
+    bNode *node_src = ntree->nodes.first;
+    bNode *node_local = ltree->nodes.first;
+    while (node_src != NULL) {
+      node_local->original = node_src;
+      node_src = node_src->next;
+      node_local = node_local->next;
     }
 
     if (ntree->typeinfo->localize) {
@@ -3595,7 +3649,7 @@ void node_type_storage(bNodeType *ntype,
                        void (*freefunc)(struct bNode *node),
                        void (*copyfunc)(struct bNodeTree *dest_ntree,
                                         struct bNode *dest_node,
-                                        struct bNode *src_node))
+                                        const struct bNode *src_node))
 {
   if (storagename) {
     BLI_strncpy(ntype->storagename, storagename, sizeof(ntype->storagename));