Fix T79408: ungroup operation update animation data incorrectly

Reviewers: sybren, sergey

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

1 files changed, 41 insertions, 32 deletions

diff --git a/source/blender/editors/space_node/node_group.c b/source/blender/editors/space_node/node_group.c
index 8dc67ad48f5..7894fade517 100644
--- a/source/blender/editors/space_node/node_group.c
+++ b/source/blender/editors/space_node/node_group.c
@@ -180,6 +180,26 @@ void NODE_OT_group_edit(wmOperatorType *ot)
 
 /* ******************** Ungroup operator ********************** */
 
+/* The given paths will be owned by the returned instance. Both pointers are allowed to point to
+ * the same string. */
+static AnimationBasePathChange *animation_basepath_change_new(const char *src_basepath,
+                                                              const char *dst_basepath)
+{
+  AnimationBasePathChange *basepath_change = MEM_callocN(sizeof(*basepath_change), AT);
+  basepath_change->src_basepath = src_basepath;
+  basepath_change->dst_basepath = dst_basepath;
+  return basepath_change;
+}
+
+static void animation_basepath_change_free(AnimationBasePathChange *basepath_change)
+{
+  if (basepath_change->src_basepath != basepath_change->dst_basepath) {
+    MEM_freeN((void *)basepath_change->src_basepath);
+  }
+  MEM_freeN((void *)basepath_change->dst_basepath);
+  MEM_freeN(basepath_change);
+}
+
 /* returns 1 if its OK */
 static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode)
 {
@@ -218,16 +238,11 @@ static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode)
     /* keep track of this node's RNA "base" path (the part of the path identifying the node)
      * if the old nodetree has animation data which potentially covers this node
      */
+    const char *old_animation_basepath = NULL;
     if (wgroup->adt) {
       PointerRNA ptr;
-      char *path;
-
       RNA_pointer_create(&wgroup->id, &RNA_Node, node, &ptr);
-      path = RNA_path_from_ID_to_struct(&ptr);
-
-      if (path) {
-        BLI_addtail(&anim_basepaths, BLI_genericNodeN(path));
-      }
+      old_animation_basepath = RNA_path_from_ID_to_struct(&ptr);
     }
 
     /* migrate node */
@@ -237,6 +252,14 @@ static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode)
     /* ensure unique node name in the node tree */
     nodeUniqueName(ntree, node);
 
+    if (wgroup->adt) {
+      PointerRNA ptr;
+      RNA_pointer_create(&ntree->id, &RNA_Node, node, &ptr);
+      const char *new_animation_basepath = RNA_path_from_ID_to_struct(&ptr);
+      BLI_addtail(&anim_basepaths,
+                  animation_basepath_change_new(old_animation_basepath, new_animation_basepath));
+    }
+
     if (!node->parent) {
       node->locx += gnode->locx;
       node->locy += gnode->locy;
@@ -259,7 +282,6 @@ static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode)
   /* and copy across the animation,
    * note that the animation data's action can be NULL here */
   if (wgroup->adt) {
-    LinkData *ld, *ldn = NULL;
     bAction *waction;
 
     /* firstly, wgroup needs to temporary dummy action
@@ -267,14 +289,11 @@ static int node_group_ungroup(Main *bmain, bNodeTree *ntree, bNode *gnode)
     waction = wgroup->adt->action = BKE_action_copy(bmain, wgroup->adt->action);
 
     /* now perform the moving */
-    BKE_animdata_separate_by_basepath(bmain, &wgroup->id, &ntree->id, &anim_basepaths);
+    BKE_animdata_transfer_by_basepath(bmain, &wgroup->id, &ntree->id, &anim_basepaths);
 
     /* paths + their wrappers need to be freed */
-    for (ld = anim_basepaths.first; ld; ld = ldn) {
-      ldn = ld->next;
-
-      MEM_freeN(ld->data);
-      BLI_freelinkN(&anim_basepaths, ld);
+    LISTBASE_FOREACH_MUTABLE (AnimationBasePathChange *, basepath_change, &anim_basepaths) {
+      animation_basepath_change_free(basepath_change);
     }
 
     /* free temp action too */
@@ -459,7 +478,7 @@ static int node_group_separate_selected(
       path = RNA_path_from_ID_to_struct(&ptr);
 
       if (path) {
-        BLI_addtail(&anim_basepaths, BLI_genericNodeN(path));
+        BLI_addtail(&anim_basepaths, animation_basepath_change_new(path, path));
       }
     }
 
@@ -512,17 +531,12 @@ static int node_group_separate_selected(
   /* and copy across the animation,
    * note that the animation data's action can be NULL here */
   if (ngroup->adt) {
-    LinkData *ld, *ldn = NULL;
-
     /* now perform the moving */
-    BKE_animdata_separate_by_basepath(bmain, &ngroup->id, &ntree->id, &anim_basepaths);
+    BKE_animdata_transfer_by_basepath(bmain, &ngroup->id, &ntree->id, &anim_basepaths);
 
     /* paths + their wrappers need to be freed */
-    for (ld = anim_basepaths.first; ld; ld = ldn) {
-      ldn = ld->next;
-
-      MEM_freeN(ld->data);
-      BLI_freelinkN(&anim_basepaths, ld);
+    LISTBASE_FOREACH_MUTABLE (AnimationBasePathChange *, basepath_change, &anim_basepaths) {
+      animation_basepath_change_free(basepath_change);
     }
   }
 
@@ -763,7 +777,7 @@ static void node_group_make_insert_selected(const bContext *C, bNodeTree *ntree,
         path = RNA_path_from_ID_to_struct(&ptr);
 
         if (path) {
-          BLI_addtail(&anim_basepaths, BLI_genericNodeN(path));
+          BLI_addtail(&anim_basepaths, animation_basepath_change_new(path, path));
         }
       }
 
@@ -783,16 +797,11 @@ static void node_group_make_insert_selected(const bContext *C, bNodeTree *ntree,
 
   /* move animation data over */
   if (ntree->adt) {
-    LinkData *ld, *ldn = NULL;
-
-    BKE_animdata_separate_by_basepath(bmain, &ntree->id, &ngroup->id, &anim_basepaths);
+    BKE_animdata_transfer_by_basepath(bmain, &ntree->id, &ngroup->id, &anim_basepaths);
 
     /* paths + their wrappers need to be freed */
-    for (ld = anim_basepaths.first; ld; ld = ldn) {
-      ldn = ld->next;
-
-      MEM_freeN(ld->data);
-      BLI_freelinkN(&anim_basepaths, ld);
+    LISTBASE_FOREACH_MUTABLE (AnimationBasePathChange *, basepath_change, &anim_basepaths) {
+      animation_basepath_change_free(basepath_change);
     }
   }