Fix T68665: FCurve group disappear on Curve/Surface object data

When going from EDIT to OBJECT mode, Blender updates the object data from
the edit-mode data. This took care of renaming FCurves that animate Curve
control points when control points are added/removed, but this didn't keep
the FCurve groups intact. Since the FCurve groups are tightly connected to
the Action channels, it's hard to keep the group pointers intact during
this process. Instead of making the code even more complex in an attempt to
do that, I implemented a function (`BKE_action_groups_reconstruct()`) that
rebuilds the group channel pointers.

The call to `action_groups_add_channel()` had to be removed because it
updates the the next/prev pointers of the FCurve while we're looping over
them, causing infinite loops.

3 files changed, 48 insertions, 1 deletions

diff --git a/source/blender/blenkernel/BKE_action.h b/source/blender/blenkernel/BKE_action.h
index fdfea95937b..60d988ab21e 100644
--- a/source/blender/blenkernel/BKE_action.h
+++ b/source/blender/blenkernel/BKE_action.h
@@ -122,6 +122,9 @@ void action_groups_add_channel(struct bAction *act,
 /* Remove the given channel from all groups */
 void action_groups_remove_channel(struct bAction *act, struct FCurve *fcu);
 
+/* Reconstruct group channel pointers. */
+void BKE_action_groups_reconstruct(struct bAction *act);
+
 /* Find a group with the given name */
 struct bActionGroup *BKE_action_group_find_name(struct bAction *act, const char name[]);
 
diff --git a/source/blender/blenkernel/intern/action.c b/source/blender/blenkernel/intern/action.c
index ad6c696b655..b474e3f5ec5 100644
--- a/source/blender/blenkernel/intern/action.c
+++ b/source/blender/blenkernel/intern/action.c
@@ -343,6 +343,43 @@ void action_groups_add_channel(bAction *act, bActionGroup *agrp, FCurve *fcurve)
   fcurve->grp = agrp;
 }
 
+/* Reconstruct group channel pointers.
+ * Assumes that the channels are still in the proper order, i.e. that channels of the same group
+ * are adjacent in the act->channels list. It also assumes that the groups
+ * referred to by the FCurves are already in act->groups.
+ */
+void BKE_action_groups_reconstruct(bAction *act)
+{
+  /* Sanity check. */
+  if (ELEM(NULL, act, act->groups.first)) {
+    return;
+  }
+
+  /* Clear out all group channels. Channels that are actually in use are
+   * reconstructed below; this step is necessary to clear out unused groups. */
+  LISTBASE_FOREACH (bActionGroup *, group, &act->groups) {
+    BLI_listbase_clear(&group->channels);
+  }
+
+  bActionGroup *grp;
+  bActionGroup *last_grp = NULL;
+  LISTBASE_FOREACH (FCurve *, fcurve, &act->curves) {
+    if (fcurve->grp == NULL) {
+      continue;
+    }
+
+    grp = fcurve->grp;
+    if (last_grp != grp) {
+      /* If this is the first time we see this group, this must be the first channel. */
+      grp->channels.first = fcurve;
+    }
+
+    /* This is the last channel, until it's overwritten by a later iteration. */
+    grp->channels.last = fcurve;
+    last_grp = grp;
+  }
+}
+
 /* Remove the given channel from all groups */
 void action_groups_remove_channel(bAction *act, FCurve *fcu)
 {
diff --git a/source/blender/editors/curve/editcurve.c b/source/blender/editors/curve/editcurve.c
index f18b6e91d0f..e7803fdaafb 100644
--- a/source/blender/editors/curve/editcurve.c
+++ b/source/blender/editors/curve/editcurve.c
@@ -922,11 +922,15 @@ static void fcurve_path_rename(AnimData *adt,
       nfcu = copy_fcurve(fcu);
       spath = nfcu->rna_path;
       nfcu->rna_path = BLI_sprintfN("%s%s", rna_path, suffix);
+
+      /* copy_fcurve() sets nfcu->grp to NULL. To maintain the groups, we need to keep the pointer.
+       * As a result, the group's 'channels' pointers will be wrong, which is fixed by calling
+       * `action_groups_reconstruct(action)` later, after all fcurves have been renamed. */
+      nfcu->grp = fcu->grp;
       BLI_addtail(curves, nfcu);
 
       if (fcu->grp) {
         action_groups_remove_channel(adt->action, fcu);
-        action_groups_add_channel(adt->action, fcu->grp, nfcu);
       }
       else if ((adt->action) && (&adt->action->curves == orig_curves)) {
         BLI_remlink(&adt->action->curves, fcu);
@@ -1077,6 +1081,9 @@ static void curve_rename_fcurves(Curve *cu, ListBase *orig_curves)
   }
 
   *orig_curves = curves;
+  if (adt != NULL) {
+    BKE_action_groups_reconstruct(adt->action);
+  }
 }
 
 /* return 0 if animation data wasn't changed, 1 otherwise */