Nodes: better integrate node init and versioning in file reading

Node versioning code was added before there was a mechanism to do versioning
after lib linking. Now integrate with that system and make it less of a strange
exception. Node versioning is now skipped on undo, like other versioning code.

4 files changed, 196 insertions, 247 deletions

diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index 16e8c040fda..26a14c12229 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -154,7 +154,6 @@
 
 #include "DEG_depsgraph.h"
 
-#include "NOD_common.h"
 #include "NOD_socket.h"
 
 #include "BLO_blend_defs.h"
@@ -3400,35 +3399,46 @@ static void lib_link_workspace_instance_hook(FileData *fd, WorkSpaceInstanceHook
 /* Single node tree (also used for material/scene trees), ntree is not NULL */
 static void lib_link_ntree(FileData *fd, ID *id, bNodeTree *ntree)
 {
-  bNode *node;
-  bNodeSocket *sock;
-
   IDP_LibLinkProperty(ntree->id.properties, fd);
   lib_link_animdata(fd, &ntree->id, ntree->adt);
 
   ntree->gpd = newlibadr_us(fd, id->lib, ntree->gpd);
 
-  for (node = ntree->nodes.first; node; node = node->next) {
+  for (bNode *node = ntree->nodes.first; node; node = node->next) {
     /* Link ID Properties -- and copy this comment EXACTLY for easy finding
      * of library blocks that implement this.*/
     IDP_LibLinkProperty(node->prop, fd);
 
     node->id = newlibadr_us(fd, id->lib, node->id);
 
-    for (sock = node->inputs.first; sock; sock = sock->next) {
+    for (bNodeSocket *sock = node->inputs.first; sock; sock = sock->next) {
       IDP_LibLinkProperty(sock->prop, fd);
     }
-    for (sock = node->outputs.first; sock; sock = sock->next) {
+    for (bNodeSocket *sock = node->outputs.first; sock; sock = sock->next) {
       IDP_LibLinkProperty(sock->prop, fd);
     }
   }
 
-  for (sock = ntree->inputs.first; sock; sock = sock->next) {
+  for (bNodeSocket *sock = ntree->inputs.first; sock; sock = sock->next) {
     IDP_LibLinkProperty(sock->prop, fd);
   }
-  for (sock = ntree->outputs.first; sock; sock = sock->next) {
+  for (bNodeSocket *sock = ntree->outputs.first; sock; sock = sock->next) {
     IDP_LibLinkProperty(sock->prop, fd);
   }
+
+  /* Set node->typeinfo pointers. This is done in lib linking, after the
+   * first versioning that can change types still without functions that
+   * update the typeinfo pointers. Versioning after lib linking needs
+   * these top be valid. */
+  ntreeSetTypes(NULL, ntree);
+
+  /* For nodes with static socket layout, add/remove sockets as needed
+   * to match the static layout. */
+  if (fd->memfile == NULL) {
+    for (bNode *node = ntree->nodes.first; node; node = node->next) {
+      node_verify_socket_templates(ntree, node);
+    }
+  }
 }
 
 /* library ntree linking after fileread */
@@ -3444,214 +3454,11 @@ static void lib_link_nodetree(FileData *fd, Main *main)
   }
 }
 
-/* updates group node socket identifier so that
- * external links to/from the group node are preserved.
- */
-static void lib_node_do_versions_group_indices(bNode *gnode)
+/* Verify group nodes have sockets matching their node groups. All data has
+ * to be read and versioned at this point, since this accesses data across
+ * files. */
+static void lib_verify_nodetree(Main *main)
 {
-  bNodeTree *ngroup = (bNodeTree *)gnode->id;
-  bNodeSocket *sock;
-  bNodeLink *link;
-
-  for (sock = gnode->outputs.first; sock; sock = sock->next) {
-    int old_index = sock->to_index;
-
-    for (link = ngroup->links.first; link; link = link->next) {
-      if (link->tonode == NULL && link->fromsock->own_index == old_index) {
-        strcpy(sock->identifier, link->fromsock->identifier);
-        /* deprecated */
-        sock->own_index = link->fromsock->own_index;
-        sock->to_index = 0;
-        sock->groupsock = NULL;
-      }
-    }
-  }
-  for (sock = gnode->inputs.first; sock; sock = sock->next) {
-    int old_index = sock->to_index;
-
-    for (link = ngroup->links.first; link; link = link->next) {
-      if (link->fromnode == NULL && link->tosock->own_index == old_index) {
-        strcpy(sock->identifier, link->tosock->identifier);
-        /* deprecated */
-        sock->own_index = link->tosock->own_index;
-        sock->to_index = 0;
-        sock->groupsock = NULL;
-      }
-    }
-  }
-}
-
-/* verify types for nodes and groups, all data has to be read */
-/* open = 0: appending/linking, open = 1: open new file (need to clean out dynamic
- * typedefs */
-static void lib_verify_nodetree(Main *main, int UNUSED(open))
-{
-  /* this crashes blender on undo/redo */
-#if 0
-  if (open == 1) {
-    reinit_nodesystem();
-  }
-#endif
-
-  /* set node->typeinfo pointers */
-  FOREACH_NODETREE_BEGIN (main, ntree, id) {
-    ntreeSetTypes(NULL, ntree);
-  }
-  FOREACH_NODETREE_END;
-
-  /* verify static socket templates */
-  FOREACH_NODETREE_BEGIN (main, ntree, id) {
-    bNode *node;
-    for (node = ntree->nodes.first; node; node = node->next) {
-      node_verify_socket_templates(ntree, node);
-    }
-  }
-  FOREACH_NODETREE_END;
-
-  {
-    bool has_old_groups = false;
-    /* XXX this should actually be part of do_versions, but since we need
-     * finished library linking, it is not possible there. Instead in do_versions
-     * we have set the NTREE_DO_VERSIONS_GROUP_EXPOSE_2_56_2 flag, so at this point we can do the
-     * actual group node updates.
-     */
-    for (bNodeTree *ntree = main->nodetrees.first; ntree; ntree = ntree->id.next) {
-      if (ntree->flag & NTREE_DO_VERSIONS_GROUP_EXPOSE_2_56_2) {
-        has_old_groups = 1;
-      }
-    }
-
-    if (has_old_groups) {
-      FOREACH_NODETREE_BEGIN (main, ntree, id) {
-        /* updates external links for all group nodes in a tree */
-        bNode *node;
-        for (node = ntree->nodes.first; node; node = node->next) {
-          if (node->type == NODE_GROUP) {
-            bNodeTree *ngroup = (bNodeTree *)node->id;
-            if (ngroup && (ngroup->flag & NTREE_DO_VERSIONS_GROUP_EXPOSE_2_56_2)) {
-              lib_node_do_versions_group_indices(node);
-            }
-          }
-        }
-      }
-      FOREACH_NODETREE_END;
-    }
-
-    for (bNodeTree *ntree = main->nodetrees.first; ntree; ntree = ntree->id.next) {
-      ntree->flag &= ~NTREE_DO_VERSIONS_GROUP_EXPOSE_2_56_2;
-    }
-  }
-
-  {
-    /* Convert the previously used ntree->inputs/ntree->outputs lists to interface nodes.
-     * Pre 2.56.2 node trees automatically have all unlinked sockets exposed already
-     * (see NTREE_DO_VERSIONS_GROUP_EXPOSE_2_56_2).
-     *
-     * XXX this should actually be part of do_versions,
-     * but needs valid typeinfo pointers to create interface nodes.
-     *
-     * Note: theoretically only needed in node groups (main->nodetree),
-     * but due to a temporary bug such links could have been added in all trees,
-     * so have to clean up all of them ...
-     */
-
-    FOREACH_NODETREE_BEGIN (main, ntree, id) {
-      if (ntree->flag & NTREE_DO_VERSIONS_CUSTOMNODES_GROUP) {
-        bNode *input_node = NULL, *output_node = NULL;
-        int num_inputs = 0, num_outputs = 0;
-        bNodeLink *link, *next_link;
-        /* Only create new interface nodes for actual older files.
-         * New file versions already have input/output nodes with duplicate links,
-         * in that case just remove the invalid links.
-         */
-        const bool create_io_nodes = (ntree->flag &
-                                      NTREE_DO_VERSIONS_CUSTOMNODES_GROUP_CREATE_INTERFACE) != 0;
-
-        float input_locx = 1000000.0f, input_locy = 0.0f;
-        float output_locx = -1000000.0f, output_locy = 0.0f;
-        /* rough guess, not nice but we don't have access to UI constants here ... */
-        static const float offsetx = 42 + 3 * 20 + 20;
-        /*static const float offsety = 0.0f;*/
-
-        if (create_io_nodes) {
-          if (ntree->inputs.first) {
-            input_node = nodeAddStaticNode(NULL, ntree, NODE_GROUP_INPUT);
-          }
-
-          if (ntree->outputs.first) {
-            output_node = nodeAddStaticNode(NULL, ntree, NODE_GROUP_OUTPUT);
-          }
-        }
-
-        /* Redirect links from/to the node tree interface to input/output node.
-         * If the fromnode/tonode pointers are NULL, this means a link from/to
-         * the ntree interface sockets, which need to be redirected to new interface nodes.
-         */
-        for (link = ntree->links.first; link; link = next_link) {
-          bool free_link = false;
-          next_link = link->next;
-
-          if (link->fromnode == NULL) {
-            if (input_node) {
-              link->fromnode = input_node;
-              link->fromsock = node_group_input_find_socket(input_node,
-                                                            link->fromsock->identifier);
-              ++num_inputs;
-
-              if (link->tonode) {
-                if (input_locx > link->tonode->locx - offsetx) {
-                  input_locx = link->tonode->locx - offsetx;
-                }
-                input_locy += link->tonode->locy;
-              }
-            }
-            else {
-              free_link = true;
-            }
-          }
-
-          if (link->tonode == NULL) {
-            if (output_node) {
-              link->tonode = output_node;
-              link->tosock = node_group_output_find_socket(output_node, link->tosock->identifier);
-              ++num_outputs;
-
-              if (link->fromnode) {
-                if (output_locx < link->fromnode->locx + offsetx) {
-                  output_locx = link->fromnode->locx + offsetx;
-                }
-                output_locy += link->fromnode->locy;
-              }
-            }
-            else {
-              free_link = true;
-            }
-          }
-
-          if (free_link) {
-            nodeRemLink(ntree, link);
-          }
-        }
-
-        if (num_inputs > 0) {
-          input_locy /= num_inputs;
-          input_node->locx = input_locx;
-          input_node->locy = input_locy;
-        }
-        if (num_outputs > 0) {
-          output_locy /= num_outputs;
-          output_node->locx = output_locx;
-          output_node->locy = output_locy;
-        }
-
-        /* clear do_versions flags */
-        ntree->flag &= ~(NTREE_DO_VERSIONS_CUSTOMNODES_GROUP |
-                         NTREE_DO_VERSIONS_CUSTOMNODES_GROUP_CREATE_INTERFACE);
-      }
-    }
-    FOREACH_NODETREE_END;
-  }
-
   /* verify all group user nodes */
   for (bNodeTree *ntree = main->nodetrees.first; ntree; ntree = ntree->id.next) {
     ntreeVerifyNodes(main, &ntree->id);
@@ -9645,6 +9452,8 @@ static void do_versions_after_linking(Main *main)
   //  printf("%s for %s (%s), %d.%d\n", __func__, main->curlib ? main->curlib->name : main->name,
   //         main->curlib ? "LIB" : "MAIN", main->versionfile, main->subversionfile);
 
+  do_versions_after_linking_250(main);
+  do_versions_after_linking_260(main);
   do_versions_after_linking_270(main);
   do_versions_after_linking_280(main);
 }
@@ -9929,8 +9738,8 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
 
   BKE_main_id_tag_all(bfd->main, LIB_TAG_NEW, false);
 
-  /* Before static overrides, which needs typeinfo. */
-  lib_verify_nodetree(bfd->main, true);
+  /* After all data has been read and versioned. */
+  lib_verify_nodetree(bfd->main);
 
   /* Now that all our data-blocks are loaded, we can re-generate overrides from their references. */
   if (fd->memfile == NULL) {
@@ -11701,7 +11510,7 @@ static void library_link_end(Main *mainl,
 
   BKE_main_id_tag_all(mainvar, LIB_TAG_NEW, false);
 
-  lib_verify_nodetree(mainvar, false);
+  lib_verify_nodetree(mainvar);
   fix_relpaths_library(BKE_main_blendfile_path(mainvar),
                        mainvar); /* make all relative paths, relative to the open blend file */
 
diff --git a/source/blender/blenloader/intern/readfile.h b/source/blender/blenloader/intern/readfile.h
index 1122950771c..f4b82e60e31 100644
--- a/source/blender/blenloader/intern/readfile.h
+++ b/source/blender/blenloader/intern/readfile.h
@@ -183,6 +183,8 @@ void blo_do_versions_260(struct FileData *fd, struct Library *lib, struct Main *
 void blo_do_versions_270(struct FileData *fd, struct Library *lib, struct Main *bmain);
 void blo_do_versions_280(struct FileData *fd, struct Library *lib, struct Main *bmain);
 
+void do_versions_after_linking_250(struct Main *bmain);
+void do_versions_after_linking_260(struct Main *bmain);
 void do_versions_after_linking_270(struct Main *bmain);
 void do_versions_after_linking_280(struct Main *bmain);
 
diff --git a/source/blender/blenloader/intern/versioning_250.c b/source/blender/blenloader/intern/versioning_250.c
index d7c26564576..a34b441d511 100644
--- a/source/blender/blenloader/intern/versioning_250.c
+++ b/source/blender/blenloader/intern/versioning_250.c
@@ -2030,12 +2030,9 @@ void blo_do_versions_250(FileData *fd, Library *lib, Main *bmain)
         }
       }
 
-      /* XXX The external group node sockets needs to adjust their own_index to point at
-       * associated ntree inputs/outputs internal sockets. However, this can only happen
-       * after lib-linking (needs access to internal node group tree)!
-       * Setting a temporary flag here, actual do_versions happens in lib_verify_nodetree.
-       */
-      ntree->flag |= NTREE_DO_VERSIONS_GROUP_EXPOSE_2_56_2;
+      /* Externl group node socket need to adjust their own_index to point at
+       * associated ntree inputs/outputs internal sockets. This happens in
+       * do_versions_after_linking_250, after lib linking. */
     }
   }
 
@@ -2296,3 +2293,59 @@ void blo_do_versions_250(FileData *fd, Library *lib, Main *bmain)
     }
   }
 }
+
+/* updates group node socket identifier so that
+ * external links to/from the group node are preserved.
+ */
+static void lib_node_do_versions_group_indices(bNode *gnode)
+{
+  bNodeTree *ngroup = (bNodeTree *)gnode->id;
+  bNodeSocket *sock;
+  bNodeLink *link;
+
+  for (sock = gnode->outputs.first; sock; sock = sock->next) {
+    int old_index = sock->to_index;
+
+    for (link = ngroup->links.first; link; link = link->next) {
+      if (link->tonode == NULL && link->fromsock->own_index == old_index) {
+        strcpy(sock->identifier, link->fromsock->identifier);
+        /* deprecated */
+        sock->own_index = link->fromsock->own_index;
+        sock->to_index = 0;
+        sock->groupsock = NULL;
+      }
+    }
+  }
+  for (sock = gnode->inputs.first; sock; sock = sock->next) {
+    int old_index = sock->to_index;
+
+    for (link = ngroup->links.first; link; link = link->next) {
+      if (link->fromnode == NULL && link->tosock->own_index == old_index) {
+        strcpy(sock->identifier, link->tosock->identifier);
+        /* deprecated */
+        sock->own_index = link->tosock->own_index;
+        sock->to_index = 0;
+        sock->groupsock = NULL;
+      }
+    }
+  }
+}
+
+void do_versions_after_linking_250(Main *bmain)
+{
+  if (bmain->versionfile < 256 || (bmain->versionfile == 256 && bmain->subversionfile < 2)) {
+    FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
+      /* updates external links for all group nodes in a tree */
+      bNode *node;
+      for (node = ntree->nodes.first; node; node = node->next) {
+        if (node->type == NODE_GROUP) {
+          bNodeTree *ngroup = (bNodeTree *)node->id;
+          if (ngroup) {
+            lib_node_do_versions_group_indices(node);
+          }
+        }
+      }
+    }
+    FOREACH_NODETREE_END;
+  }
+}
diff --git a/source/blender/blenloader/intern/versioning_260.c b/source/blender/blenloader/intern/versioning_260.c
index 6c9ec7607a2..8010ca8b1b8 100644
--- a/source/blender/blenloader/intern/versioning_260.c
+++ b/source/blender/blenloader/intern/versioning_260.c
@@ -67,6 +67,7 @@
 
 #include "IMB_imbuf.h"  // for proxy / timecode versioning stuff
 
+#include "NOD_common.h"
 #include "NOD_texture.h"
 
 #include "BLO_readfile.h"
@@ -1949,28 +1950,6 @@ void blo_do_versions_260(FileData *fd, Library *UNUSED(lib), Main *bmain)
     }
   }
 
-  /* Set flag for delayed do_versions in lib_verify_nodetree.
-   * It needs valid typeinfo pointers ... */
-  {
-    FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
-      /* XXX This should be kept without version check for now!
-       * As long as USE_NODE_COMPAT_CUSTOMNODES is active, files will write links
-       * to tree interface sockets for forward compatibility. These links need to be removed again
-       * on file load in new versions.
-       * Once forward compatibility is not required any longer, make a subversion bump
-       * and only execute this for older versions.
-       */
-      ntree->flag |= NTREE_DO_VERSIONS_CUSTOMNODES_GROUP;
-
-      /* Only add interface nodes once.
-       * In old Blender versions they will be removed automatically due to undefined type */
-      if (MAIN_VERSION_OLDER(bmain, 266, 2)) {
-        ntree->flag |= NTREE_DO_VERSIONS_CUSTOMNODES_GROUP_CREATE_INTERFACE;
-      }
-    }
-    FOREACH_NODETREE_END;
-  }
-
   if (MAIN_VERSION_OLDER(bmain, 266, 3)) {
     {
       /* Fix for a very old issue:
@@ -2571,3 +2550,109 @@ void blo_do_versions_260(FileData *fd, Library *UNUSED(lib), Main *bmain)
     }
   }
 }
+
+void do_versions_after_linking_260(Main *bmain)
+{
+  /* Convert the previously used ntree->inputs/ntree->outputs lists to interface nodes.
+   * Pre 2.56.2 node trees automatically have all unlinked sockets exposed already,
+   * see do_versions_after_linking_250.
+   *
+   * This assumes valid typeinfo pointers, as set in lib_link_ntree.
+   *
+   * Note: theoretically only needed in node groups (main->nodetree),
+   * but due to a temporary bug such links could have been added in all trees,
+   * so have to clean up all of them ...
+   *
+   * Note: this always runs, without it links with NULL fromnode and tonode remain
+   * which causes problems.
+   */
+  {
+    FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
+      bNode *input_node = NULL, *output_node = NULL;
+      int num_inputs = 0, num_outputs = 0;
+      bNodeLink *link, *next_link;
+      /* Only create new interface nodes for actual older files.
+       * New file versions already have input/output nodes with duplicate links,
+       * in that case just remove the invalid links.
+       */
+      const bool create_io_nodes = MAIN_VERSION_OLDER(bmain, 266, 2);
+
+      float input_locx = 1000000.0f, input_locy = 0.0f;
+      float output_locx = -1000000.0f, output_locy = 0.0f;
+      /* rough guess, not nice but we don't have access to UI constants here ... */
+      static const float offsetx = 42 + 3 * 20 + 20;
+      /*static const float offsety = 0.0f;*/
+
+      if (create_io_nodes) {
+        if (ntree->inputs.first) {
+          input_node = nodeAddStaticNode(NULL, ntree, NODE_GROUP_INPUT);
+        }
+
+        if (ntree->outputs.first) {
+          output_node = nodeAddStaticNode(NULL, ntree, NODE_GROUP_OUTPUT);
+        }
+      }
+
+      /* Redirect links from/to the node tree interface to input/output node.
+       * If the fromnode/tonode pointers are NULL, this means a link from/to
+       * the ntree interface sockets, which need to be redirected to new interface nodes.
+       */
+      for (link = ntree->links.first; link; link = next_link) {
+        bool free_link = false;
+        next_link = link->next;
+
+        if (link->fromnode == NULL) {
+          if (input_node) {
+            link->fromnode = input_node;
+            link->fromsock = node_group_input_find_socket(input_node, link->fromsock->identifier);
+            ++num_inputs;
+
+            if (link->tonode) {
+              if (input_locx > link->tonode->locx - offsetx) {
+                input_locx = link->tonode->locx - offsetx;
+              }
+              input_locy += link->tonode->locy;
+            }
+          }
+          else {
+            free_link = true;
+          }
+        }
+
+        if (link->tonode == NULL) {
+          if (output_node) {
+            link->tonode = output_node;
+            link->tosock = node_group_output_find_socket(output_node, link->tosock->identifier);
+            ++num_outputs;
+
+            if (link->fromnode) {
+              if (output_locx < link->fromnode->locx + offsetx) {
+                output_locx = link->fromnode->locx + offsetx;
+              }
+              output_locy += link->fromnode->locy;
+            }
+          }
+          else {
+            free_link = true;
+          }
+        }
+
+        if (free_link) {
+          nodeRemLink(ntree, link);
+        }
+      }
+
+      if (num_inputs > 0) {
+        input_locy /= num_inputs;
+        input_node->locx = input_locx;
+        input_node->locy = input_locy;
+      }
+      if (num_outputs > 0) {
+        output_locy /= num_outputs;
+        output_node->locx = output_locx;
+        output_node->locy = output_locy;
+      }
+    }
+    FOREACH_NODETREE_END;
+  }
+}