Reverted the IDRemapper code to remap mapping at a time.temp-T94185-id-remapper-ui

1 files changed, 64 insertions, 38 deletions

diff --git a/source/blender/editors/space_node/space_node.cc b/source/blender/editors/space_node/space_node.cc
index 8f465ffbf80..00fd328b2ed 100644
--- a/source/blender/editors/space_node/space_node.cc
+++ b/source/blender/editors/space_node/space_node.cc
@@ -897,66 +897,92 @@ static void node_widgets()
   WM_gizmogrouptype_append_and_link(gzmap_type, NODE_GGT_backdrop_corner_pin);
 }
 
-static void node_id_remap(ScrArea *UNUSED(area),
-                          SpaceLink *slink,
-                          const struct IDRemapper *mappings)
+static void node_id_remap_cb(ID *old_id, ID *new_id, void *user_data)
 {
-  SpaceNode *snode = (SpaceNode *)slink;
+  SpaceNode *snode = static_cast<SpaceNode *>(user_data);
 
-  if (ELEM(BKE_id_remapper_apply(mappings, &snode->id, ID_REMAP_APPLY_DEFAULT),
-           ID_REMAP_RESULT_SOURCE_REMAPPED,
-           ID_REMAP_RESULT_SOURCE_UNASSIGNED)) {
+  if (snode->id == old_id) {
     /* nasty DNA logic for SpaceNode:
      * ideally should be handled by editor code, but would be bad level call
      */
     BLI_freelistN(&snode->treepath);
 
     /* XXX Untested in case new_id != nullptr... */
+    snode->id = new_id;
     snode->from = nullptr;
     snode->nodetree = nullptr;
     snode->edittree = nullptr;
   }
-  if (BKE_id_remapper_apply(mappings, &snode->from, ID_REMAP_APPLY_DEFAULT) ==
-      ID_REMAP_RESULT_SOURCE_UNASSIGNED) {
-    snode->flag &= ~SNODE_PIN;
+  else if (GS(old_id->name) == ID_OB) {
+    if (snode->from == old_id) {
+      if (new_id == nullptr) {
+        snode->flag &= ~SNODE_PIN;
+      }
+      snode->from = new_id;
+    }
   }
-  BKE_id_remapper_apply(mappings, (ID **)&snode->gpd, ID_REMAP_APPLY_UPDATE_REFCOUNT);
-
-  if (!BKE_id_remapper_has_mapping_for(mappings, FILTER_ID_NT)) {
-    return;
+  else if (GS(old_id->name) == ID_GD) {
+    if ((ID *)snode->gpd == old_id) {
+      snode->gpd = (bGPdata *)new_id;
+      id_us_min(old_id);
+      id_us_plus(new_id);
+    }
   }
+  else if (GS(old_id->name) == ID_NT) {
+    bNodeTreePath *path, *path_next;
 
-  bNodeTreePath *path, *path_next;
-  for (path = (bNodeTreePath *)snode->treepath.first; path; path = path->next) {
-    BKE_id_remapper_apply(mappings, (ID **)&path->nodetree, ID_REMAP_APPLY_ENSURE_REAL);
-    if (path == snode->treepath.first) {
-      /* first nodetree in path is same as snode->nodetree */
-      snode->nodetree = path->nodetree;
-    }
-    if (path->nodetree == nullptr) {
-      break;
+    for (path = (bNodeTreePath *)snode->treepath.first; path; path = path->next) {
+      if ((ID *)path->nodetree == old_id) {
+        path->nodetree = (bNodeTree *)new_id;
+        id_us_ensure_real(new_id);
+      }
+      if (path == snode->treepath.first) {
+        /* first nodetree in path is same as snode->nodetree */
+        snode->nodetree = path->nodetree;
+      }
+      if (path->nodetree == nullptr) {
+        break;
+      }
     }
-  }
 
-  /* remaining path entries are invalid, remove */
-  for (; path; path = path_next) {
-    path_next = path->next;
+    /* remaining path entries are invalid, remove */
+    for (; path; path = path_next) {
+      path_next = path->next;
 
-    BLI_remlink(&snode->treepath, path);
-    MEM_freeN(path);
-  }
+      BLI_remlink(&snode->treepath, path);
+      MEM_freeN(path);
+    }
 
-  /* edittree is just the last in the path,
-   * set this directly since the path may have been shortened above */
-  if (snode->treepath.last) {
-    path = (bNodeTreePath *)snode->treepath.last;
-    snode->edittree = path->nodetree;
-  }
-  else {
-    snode->edittree = nullptr;
+    /* edittree is just the last in the path,
+     * set this directly since the path may have been shortened above */
+    if (snode->treepath.last) {
+      path = (bNodeTreePath *)snode->treepath.last;
+      snode->edittree = path->nodetree;
+    }
+    else {
+      snode->edittree = nullptr;
+    }
   }
 }
 
+static void node_id_remap(ScrArea *UNUSED(area),
+                          SpaceLink *slink,
+                          const struct IDRemapper *mappings)
+{
+  /* Although we should be able to perform all the mappings in a single go this lead to issues when
+   * running the python test cases. Somehow the nodetree/edittree weren't updated to the new
+   * pointers that generated a SEGFAULT.
+   *
+   * To move forward we should perhaps remove snode->edittree and snode->nodetree as they are just
+   * copies of pointers. All usages should be calling a function that will receive the appropriate
+   * instance.
+   *
+   * We could also move a remap address at a time to ise the IDRemapper as that should get closer
+   * to cleaner code. See {D13615} for more information about this topic.
+   */
+  BKE_id_remapper_iter(mappings, node_id_remap_cb, slink);
+}
+
 static int node_space_subtype_get(ScrArea *area)
 {
   SpaceNode *snode = (SpaceNode *)area->spacedata.first;