Cleanup: VSE animation handling

- Move functions that handle animation to own file - `animation.c`
- Refactor `SEQ_offset_animdata` and `SEQ_free_animdata` functions
- Add function `SEQ_fcurves_by_strip_get` to provide more granular
  and explicit way for operators to handle animation
- Remove function `SEQ_dupe_animdata`, do curve duplication explicitly
  in operator code, which makes more sense to do. Further this function
  was also used for renaming strips which makes no sense.
- Refactor usage of function `SEQ_free_animdata` and remove XXX comment.
  Now this functiuon is no longer called when `Sequence` data is freed
  implicitly, it is done explicitly in high level function
  `SEQ_edit_remove_flagged_sequences`

Reviewed By: sergey

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

11 files changed, 184 insertions, 146 deletions

diff --git a/source/blender/sequencer/CMakeLists.txt b/source/blender/sequencer/CMakeLists.txt
index eccc336141a..54524e453d4 100644
--- a/source/blender/sequencer/CMakeLists.txt
+++ b/source/blender/sequencer/CMakeLists.txt
@@ -46,6 +46,7 @@ set(INC_SYS
 
 set(SRC
   SEQ_add.h
+  SEQ_animation.h
   SEQ_clipboard.h
   SEQ_edit.h
   SEQ_effects.h
@@ -62,6 +63,7 @@ set(SRC
   SEQ_transform.h
   SEQ_utils.h
 
+  intern/animation.c
   intern/clipboard.c
   intern/disk_cache.c
   intern/disk_cache.h
diff --git a/source/blender/sequencer/SEQ_animation.h b/source/blender/sequencer/SEQ_animation.h
new file mode 100644
index 00000000000..028f932344f
--- /dev/null
+++ b/source/blender/sequencer/SEQ_animation.h
@@ -0,0 +1,41 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * The Original Code is Copyright (C) 2022 Blender Foundation.
+ * All rights reserved.
+ */
+
+#pragma once
+
+/** \file
+ * \ingroup sequencer
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct GSet;
+struct ListBase;
+struct Scene;
+struct Sequence;
+
+void SEQ_free_animdata(struct Scene *scene, struct Sequence *seq);
+void SEQ_offset_animdata(struct Scene *scene, struct Sequence *seq, int ofs);
+struct GSet *SEQ_fcurves_by_strip_get(const struct Sequence *seq, struct ListBase *fcurve_base);
+
+#ifdef __cplusplus
+}
+#endif
diff --git a/source/blender/sequencer/SEQ_sequencer.h b/source/blender/sequencer/SEQ_sequencer.h
index 1229f6f7bea..2e340049dbd 100644
--- a/source/blender/sequencer/SEQ_sequencer.h
+++ b/source/blender/sequencer/SEQ_sequencer.h
@@ -84,7 +84,7 @@ struct ListBase *SEQ_active_seqbase_get(const struct Editing *ed);
  */
 void SEQ_seqbase_active_set(struct Editing *ed, struct ListBase *seqbase);
 struct Sequence *SEQ_sequence_alloc(ListBase *lb, int timeline_frame, int machine, int type);
-void SEQ_sequence_free(struct Scene *scene, struct Sequence *seq, bool do_clean_animdata);
+void SEQ_sequence_free(struct Scene *scene, struct Sequence *seq);
 /**
  * Create and initialize #MetaStack, append it to `ed->metastack` ListBase
  *
@@ -107,8 +107,6 @@ struct MetaStack *SEQ_meta_stack_active_get(const struct Editing *ed);
  * \param ms: meta stack
  */
 void SEQ_meta_stack_free(struct Editing *ed, struct MetaStack *ms);
-void SEQ_offset_animdata(struct Scene *scene, struct Sequence *seq, int ofs);
-void SEQ_dupe_animdata(struct Scene *scene, const char *name_src, const char *name_dst);
 struct Sequence *SEQ_sequence_dupli_recursive(const struct Scene *scene_src,
                                               struct Scene *scene_dst,
                                               struct ListBase *new_seq_list,
diff --git a/source/blender/sequencer/intern/animation.c b/source/blender/sequencer/intern/animation.c
new file mode 100644
index 00000000000..492b757a4b1
--- /dev/null
+++ b/source/blender/sequencer/intern/animation.c
@@ -0,0 +1,122 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * The Original Code is Copyright (C) 2022 Blender Foundation.
+ * All rights reserved.
+ */
+
+/** \file
+ * \ingroup sequencer
+ */
+
+#include <string.h>
+
+#include "DNA_anim_types.h"
+#include "DNA_scene_types.h"
+#include "DNA_sequence_types.h"
+
+#include "BKE_animsys.h"
+#include "BKE_fcurve.h"
+
+#include "BLI_ghash.h"
+#include "BLI_listbase.h"
+#include "BLI_string.h"
+
+#include "DEG_depsgraph.h"
+
+#include "SEQ_animation.h"
+
+static bool seq_animation_curves_exist(Scene *scene)
+{
+  if (scene->adt == NULL || scene->adt->action == NULL ||
+      BLI_listbase_is_empty(&scene->adt->action->curves)) {
+    return false;
+  }
+  return true;
+}
+
+/* r_prefix + [" + escaped_name + "] + \0 */
+#define SEQ_RNAPATH_MAXSTR ((30 + 2 + (SEQ_NAME_MAXSTR * 2) + 2) + 1)
+
+static size_t sequencer_rna_path_prefix(char str[SEQ_RNAPATH_MAXSTR], const char *name)
+{
+  char name_esc[SEQ_NAME_MAXSTR * 2];
+
+  BLI_str_escape(name_esc, name, sizeof(name_esc));
+  return BLI_snprintf_rlen(
+      str, SEQ_RNAPATH_MAXSTR, "sequence_editor.sequences_all[\"%s\"]", name_esc);
+}
+
+GSet *SEQ_fcurves_by_strip_get(const Sequence *seq, ListBase *fcurve_base)
+{
+  char rna_path[SEQ_RNAPATH_MAXSTR];
+  size_t rna_path_len = sequencer_rna_path_prefix(rna_path, seq->name + 2);
+
+  GSet *fcurves = BLI_gset_ptr_new(__func__);
+  LISTBASE_FOREACH (FCurve *, fcurve, fcurve_base) {
+    if (STREQLEN(fcurve->rna_path, rna_path, rna_path_len)) {
+      BLI_gset_add(fcurves, fcurve);
+    }
+  }
+
+  return fcurves;
+}
+
+#undef SEQ_RNAPATH_MAXSTR
+
+void SEQ_offset_animdata(Scene *scene, Sequence *seq, int ofs)
+{
+  if (!seq_animation_curves_exist(scene) || ofs == 0) {
+    return;
+  }
+
+  GSet *fcurves = SEQ_fcurves_by_strip_get(seq, &scene->adt->action->curves);
+  GSET_FOREACH_BEGIN (FCurve *, fcu, fcurves) {
+    unsigned int i;
+    if (fcu->bezt) {
+      for (i = 0; i < fcu->totvert; i++) {
+        BezTriple *bezt = &fcu->bezt[i];
+        bezt->vec[0][0] += ofs;
+        bezt->vec[1][0] += ofs;
+        bezt->vec[2][0] += ofs;
+      }
+    }
+    if (fcu->fpt) {
+      for (i = 0; i < fcu->totvert; i++) {
+        FPoint *fpt = &fcu->fpt[i];
+        fpt->vec[0] += ofs;
+      }
+    }
+  }
+  GSET_FOREACH_END();
+  BLI_gset_free(fcurves, NULL);
+
+  DEG_id_tag_update(&scene->adt->action->id, ID_RECALC_ANIMATION);
+}
+
+void SEQ_free_animdata(Scene *scene, Sequence *seq)
+{
+  if (!seq_animation_curves_exist(scene)) {
+    return;
+  }
+
+  GSet *fcurves = SEQ_fcurves_by_strip_get(seq, &scene->adt->action->curves);
+  GSET_FOREACH_BEGIN (FCurve *, fcu, fcurves) {
+    BLI_remlink(&scene->adt->action->curves, fcu);
+    BKE_fcurve_free(fcu);
+  }
+  GSET_FOREACH_END();
+  BLI_gset_free(fcurves, NULL);
+}
diff --git a/source/blender/sequencer/intern/clipboard.c b/source/blender/sequencer/intern/clipboard.c
index 73e0f616da4..f4c7077d4c6 100644
--- a/source/blender/sequencer/intern/clipboard.c
+++ b/source/blender/sequencer/intern/clipboard.c
@@ -71,7 +71,7 @@ void SEQ_clipboard_free(void)
 
   for (seq = seqbase_clipboard.first; seq; seq = nseq) {
     nseq = seq->next;
-    seq_free_sequence_recurse(NULL, seq, false, true);
+    seq_free_sequence_recurse(NULL, seq, false);
   }
   BLI_listbase_clear(&seqbase_clipboard);
 }
diff --git a/source/blender/sequencer/intern/proxy.c b/source/blender/sequencer/intern/proxy.c
index 30afa3f9c59..2cb2ba13fb5 100644
--- a/source/blender/sequencer/intern/proxy.c
+++ b/source/blender/sequencer/intern/proxy.c
@@ -591,7 +591,7 @@ void SEQ_proxy_rebuild_finish(SeqIndexBuildContext *context, bool stop)
     IMB_anim_index_rebuild_finish(context->index_context, stop);
   }
 
-  seq_free_sequence_recurse(NULL, context->seq, true, true);
+  seq_free_sequence_recurse(NULL, context->seq, true);
 
   MEM_freeN(context);
 }
diff --git a/source/blender/sequencer/intern/sequencer.c b/source/blender/sequencer/intern/sequencer.c
index e4120a3cf95..f0a45355143 100644
--- a/source/blender/sequencer/intern/sequencer.c
+++ b/source/blender/sequencer/intern/sequencer.c
@@ -34,10 +34,7 @@
 #include "DNA_sound_types.h"
 
 #include "BLI_listbase.h"
-#include "BLI_string.h"
 
-#include "BKE_animsys.h"
-#include "BKE_fcurve.h"
 #include "BKE_idprop.h"
 #include "BKE_lib_id.h"
 #include "BKE_sound.h"
@@ -65,8 +62,6 @@
 #include "sequencer.h"
 #include "utils.h"
 
-static void seq_free_animdata(Scene *scene, Sequence *seq);
-
 /* -------------------------------------------------------------------- */
 /** \name Allocate / Free Functions
  * \{ */
@@ -156,8 +151,7 @@ Sequence *SEQ_sequence_alloc(ListBase *lb, int timeline_frame, int machine, int
 static void seq_sequence_free_ex(Scene *scene,
                                  Sequence *seq,
                                  const bool do_cache,
-                                 const bool do_id_user,
-                                 const bool do_clean_animdata)
+                                 const bool do_id_user)
 {
   if (seq->strip) {
     seq_free_strip(seq->strip);
@@ -191,11 +185,6 @@ static void seq_sequence_free_ex(Scene *scene,
     if (seq->scene_sound && ELEM(seq->type, SEQ_TYPE_SOUND_RAM, SEQ_TYPE_SCENE)) {
       BKE_sound_remove_scene_sound(scene, seq->scene_sound);
     }
-
-    /* XXX This must not be done in BKE code. */
-    if (do_clean_animdata) {
-      seq_free_animdata(scene, seq);
-    }
   }
 
   if (seq->prop) {
@@ -223,24 +212,21 @@ static void seq_sequence_free_ex(Scene *scene,
   MEM_freeN(seq);
 }
 
-void SEQ_sequence_free(Scene *scene, Sequence *seq, const bool do_clean_animdata)
+void SEQ_sequence_free(Scene *scene, Sequence *seq)
 {
-  seq_sequence_free_ex(scene, seq, true, true, do_clean_animdata);
+  seq_sequence_free_ex(scene, seq, true, true);
 }
 
-void seq_free_sequence_recurse(Scene *scene,
-                               Sequence *seq,
-                               const bool do_id_user,
-                               const bool do_clean_animdata)
+void seq_free_sequence_recurse(Scene *scene, Sequence *seq, const bool do_id_user)
 {
   Sequence *iseq, *iseq_next;
 
   for (iseq = seq->seqbase.first; iseq; iseq = iseq_next) {
     iseq_next = iseq->next;
-    seq_free_sequence_recurse(scene, iseq, do_id_user, do_clean_animdata);
+    seq_free_sequence_recurse(scene, iseq, do_id_user);
   }
 
-  seq_sequence_free_ex(scene, seq, false, do_id_user, do_clean_animdata);
+  seq_sequence_free_ex(scene, seq, false, do_id_user);
 }
 
 Editing *SEQ_editing_get(const Scene *scene)
@@ -276,7 +262,7 @@ void SEQ_editing_free(Scene *scene, const bool do_id_user)
 
   /* handle cache freeing above */
   LISTBASE_FOREACH_MUTABLE (Sequence *, seq, &ed->seqbase) {
-    seq_free_sequence_recurse(scene, seq, do_id_user, false);
+    seq_free_sequence_recurse(scene, seq, do_id_user);
   }
 
   BLI_freelistN(&ed->metastack);
@@ -623,120 +609,6 @@ bool SEQ_valid_strip_channel(Sequence *seq)
   return true;
 }
 
-/* r_prefix + [" + escaped_name + "] + \0 */
-#define SEQ_RNAPATH_MAXSTR ((30 + 2 + (SEQ_NAME_MAXSTR * 2) + 2) + 1)
-
-static size_t sequencer_rna_path_prefix(char str[SEQ_RNAPATH_MAXSTR], const char *name)
-{
-  char name_esc[SEQ_NAME_MAXSTR * 2];
-
-  BLI_str_escape(name_esc, name, sizeof(name_esc));
-  return BLI_snprintf_rlen(
-      str, SEQ_RNAPATH_MAXSTR, "sequence_editor.sequences_all[\"%s\"]", name_esc);
-}
-
-void SEQ_offset_animdata(Scene *scene, Sequence *seq, int ofs)
-{
-  /* XXX: hackish function needed for transforming strips!
-   * TODO: have some better solution. */
-
-  char str[SEQ_RNAPATH_MAXSTR];
-  size_t str_len;
-  FCurve *fcu;
-
-  if (scene->adt == NULL || ofs == 0 || scene->adt->action == NULL) {
-    return;
-  }
-
-  str_len = sequencer_rna_path_prefix(str, seq->name + 2);
-
-  for (fcu = scene->adt->action->curves.first; fcu; fcu = fcu->next) {
-    if (STREQLEN(fcu->rna_path, str, str_len)) {
-      unsigned int i;
-      if (fcu->bezt) {
-        for (i = 0; i < fcu->totvert; i++) {
-          BezTriple *bezt = &fcu->bezt[i];
-          bezt->vec[0][0] += ofs;
-          bezt->vec[1][0] += ofs;
-          bezt->vec[2][0] += ofs;
-        }
-      }
-      if (fcu->fpt) {
-        for (i = 0; i < fcu->totvert; i++) {
-          FPoint *fpt = &fcu->fpt[i];
-          fpt->vec[0] += ofs;
-        }
-      }
-    }
-  }
-
-  DEG_id_tag_update(&scene->adt->action->id, ID_RECALC_ANIMATION);
-}
-
-void SEQ_dupe_animdata(Scene *scene, const char *name_src, const char *name_dst)
-{
-  char str_from[SEQ_RNAPATH_MAXSTR];
-  size_t str_from_len;
-  FCurve *fcu;
-  FCurve *fcu_last;
-  FCurve *fcu_cpy;
-  ListBase lb = {NULL, NULL};
-
-  if (scene->adt == NULL || scene->adt->action == NULL) {
-    return;
-  }
-
-  str_from_len = sequencer_rna_path_prefix(str_from, name_src);
-
-  fcu_last = scene->adt->action->curves.last;
-
-  for (fcu = scene->adt->action->curves.first; fcu && fcu->prev != fcu_last; fcu = fcu->next) {
-    if (STREQLEN(fcu->rna_path, str_from, str_from_len)) {
-      fcu_cpy = BKE_fcurve_copy(fcu);
-      BLI_addtail(&lb, fcu_cpy);
-    }
-  }
-
-  /* notice validate is 0, keep this because the seq may not be added to the scene yet */
-  BKE_animdata_fix_paths_rename(
-      &scene->id, scene->adt, NULL, "sequence_editor.sequences_all", name_src, name_dst, 0, 0, 0);
-
-  /* add the original fcurves back */
-  BLI_movelisttolist(&scene->adt->action->curves, &lb);
-}
-
-/* XXX: hackish function needed to remove all fcurves belonging to a sequencer strip. */
-static void seq_free_animdata(Scene *scene, Sequence *seq)
-{
-  char str[SEQ_RNAPATH_MAXSTR];
-  size_t str_len;
-  FCurve *fcu;
-
-  if (scene->adt == NULL || scene->adt->action == NULL) {
-    return;
-  }
-
-  str_len = sequencer_rna_path_prefix(str, seq->name + 2);
-
-  fcu = scene->adt->action->curves.first;
-
-  while (fcu) {
-    if (STREQLEN(fcu->rna_path, str, str_len)) {
-      FCurve *next_fcu = fcu->next;
-
-      BLI_remlink(&scene->adt->action->curves, fcu);
-      BKE_fcurve_free(fcu);
-
-      fcu = next_fcu;
-    }
-    else {
-      fcu = fcu->next;
-    }
-  }
-}
-
-#undef SEQ_RNAPATH_MAXSTR
-
 SequencerToolSettings *SEQ_tool_settings_copy(SequencerToolSettings *tool_settings)
 {
   SequencerToolSettings *tool_settings_copy = MEM_dupallocN(tool_settings);
diff --git a/source/blender/sequencer/intern/sequencer.h b/source/blender/sequencer/intern/sequencer.h
index aff81255d7d..7d7ecbc8178 100644
--- a/source/blender/sequencer/intern/sequencer.h
+++ b/source/blender/sequencer/intern/sequencer.h
@@ -34,10 +34,7 @@ struct Sequence;
  * Cache must be freed before calling this function
  * since it leaves the seqbase in an invalid state.
  */
-void seq_free_sequence_recurse(struct Scene *scene,
-                               struct Sequence *seq,
-                               bool do_id_user,
-                               bool do_clean_animdata);
+void seq_free_sequence_recurse(struct Scene *scene, struct Sequence *seq, bool do_id_user);
 
 #ifdef __cplusplus
 }
diff --git a/source/blender/sequencer/intern/strip_edit.c b/source/blender/sequencer/intern/strip_edit.c
index cf303e5be4e..0479d3012fa 100644
--- a/source/blender/sequencer/intern/strip_edit.c
+++ b/source/blender/sequencer/intern/strip_edit.c
@@ -43,6 +43,7 @@
 #include "utils.h"
 
 #include "SEQ_add.h"
+#include "SEQ_animation.h"
 #include "SEQ_edit.h"
 #include "SEQ_effects.h"
 #include "SEQ_iterator.h"
@@ -199,8 +200,9 @@ void SEQ_edit_remove_flagged_sequences(Scene *scene, ListBase *seqbase)
       if (seq->type == SEQ_TYPE_META) {
         SEQ_edit_remove_flagged_sequences(scene, &seq->seqbase);
       }
+      SEQ_free_animdata(scene, seq);
       BLI_remlink(seqbase, seq);
-      SEQ_sequence_free(scene, seq, true);
+      SEQ_sequence_free(scene, seq);
       SEQ_sequence_lookup_tag(scene, SEQ_LOOKUP_TAG_INVALID);
     }
   }
diff --git a/source/blender/sequencer/intern/strip_transform.c b/source/blender/sequencer/intern/strip_transform.c
index ce5917b999f..432fc1c166f 100644
--- a/source/blender/sequencer/intern/strip_transform.c
+++ b/source/blender/sequencer/intern/strip_transform.c
@@ -33,6 +33,7 @@
 #include "BKE_scene.h"
 #include "BKE_sound.h"
 
+#include "SEQ_animation.h"
 #include "SEQ_effects.h"
 #include "SEQ_iterator.h"
 #include "SEQ_relations.h"
diff --git a/source/blender/sequencer/intern/utils.c b/source/blender/sequencer/intern/utils.c
index 156c6ac4cb9..140aa2d67c5 100644
--- a/source/blender/sequencer/intern/utils.c
+++ b/source/blender/sequencer/intern/utils.c
@@ -35,10 +35,12 @@
 
 #include "BLI_blenlib.h"
 
+#include "BKE_animsys.h"
 #include "BKE_image.h"
 #include "BKE_main.h"
 #include "BKE_scene.h"
 
+#include "SEQ_animation.h"
 #include "SEQ_edit.h"
 #include "SEQ_iterator.h"
 #include "SEQ_relations.h"
@@ -583,7 +585,8 @@ void SEQ_ensure_unique_name(Sequence *seq, Scene *scene)
 
   BLI_strncpy_utf8(name, seq->name + 2, sizeof(name));
   SEQ_sequence_base_unique_name_recursive(scene, &scene->ed->seqbase, seq);
-  SEQ_dupe_animdata(scene, name, seq->name + 2);
+  BKE_animdata_fix_paths_rename(
+      &scene->id, scene->adt, NULL, "sequence_editor.sequences_all", name, seq->name + 2, 0, 0, 0);
 
   if (seq->type == SEQ_TYPE_META) {
     LISTBASE_FOREACH (Sequence *, seq_child, &seq->seqbase) {