Cleanup: Refactor BKE_nlatrack_add to multiple methods to handle adding a new NLA track to the track list. Insert before, after, head, and tail

This PR removes the BKE_nlatrack_add, and adds new:

* BKE_nlatrack_insert_before
* BKE_nlatrack_insert_after
* BKE_nlatrack_new_before_and_set_active
* BKE_nlatrack_new_after_and_set_active
* BKE_nlatrack_new_head_and_set_active
* BKE_nlatrack_new_tail_and_set_active

methods to easily add new NLA tracks in relation to the existing track list.

Pull Request #104929

source/blender/blenkernel/BKE_nla.h

@@ -35,6 +35,12 @@ struct PropertyRNA;
 /* ----------------------------- */
 /* Data Management */
 
+/**
+ * Create new NLA Track.
+ * The returned pointer is owned by the caller.
+ */
+struct NlaTrack *BKE_nlatrack_new(void);
+
 /**
  * Frees the given NLA strip, and calls #BKE_nlastrip_remove_and_free to
  * remove and free all children strips.
@@ -88,12 +94,50 @@ void BKE_nla_tracks_copy_from_adt(struct Main *bmain,
                                   int flag);
 
 /**
- * Add a NLA Track to the given AnimData.
- * \param prev: NLA-Track to add the new one after.
+ * Inserts a given NLA track before a specified NLA track within the
+ * passed NLA track list.
  */
-struct NlaTrack *BKE_nlatrack_add(struct AnimData *adt,
-                                  struct NlaTrack *prev,
-                                  bool is_liboverride);
+void BKE_nlatrack_insert_before(ListBase *nla_tracks,
+                                struct NlaTrack *next,
+                                struct NlaTrack *new_track,
+                                bool is_liboverride);
+
+/**
+ * Inserts a given NLA track after a specified NLA track within the
+ * passed NLA track list.
+ */
+void BKE_nlatrack_insert_after(ListBase *nla_tracks,
+                               struct NlaTrack *prev,
+                               struct NlaTrack *new_track,
+                               bool is_liboverride);
+
+/**
+ * Calls #BKE_nlatrack_new to create a new NLA track, inserts it before the
+ * given NLA track with #BKE_nlatrack_insert_before.
+ */
+struct NlaTrack *BKE_nlatrack_new_before(ListBase *nla_tracks,
+                                         struct NlaTrack *next,
+                                         bool is_liboverride);
+
+/**
+ * Calls #BKE_nlatrack_new to create a new NLA track, inserts it after the
+ * given NLA track with #BKE_nlatrack_insert_after.
+ */
+struct NlaTrack *BKE_nlatrack_new_after(ListBase *nla_tracks,
+                                        struct NlaTrack *prev,
+                                        bool is_liboverride);
+
+/**
+ * Calls #BKE_nlatrack_new to create a new NLA track, inserts it as the head of the
+ * NLA track list with #BKE_nlatrack_new_before.
+ */
+struct NlaTrack *BKE_nlatrack_new_head(ListBase *nla_tracks, bool is_liboverride);
+
+/**
+ * Calls #BKE_nlatrack_new to create a new NLA track, inserts it as the tail of the
+ * NLA track list with #BKE_nlatrack_new_after.
+ */
+struct NlaTrack *BKE_nlatrack_new_tail(ListBase *nla_tracks, const bool is_liboverride);
 
 /**
  * Removes the given NLA track from the list of tracks provided.

source/blender/blenkernel/intern/ipo.c

@@ -2005,7 +2005,8 @@ static void nlastrips_to_animdata(ID *id, ListBase *strips)
         /* trying to add to the current failed (no space),
          * so add a new track to the stack, and add to that...
          */
-        nlt = BKE_nlatrack_add(adt, NULL, false);
+        nlt = BKE_nlatrack_new_tail(&adt->nla_tracks, false);
+        BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
         BKE_nlatrack_add_strip(nlt, strip, false);
       }
 

source/blender/blenkernel/intern/nla.c

@@ -336,21 +336,62 @@ void BKE_nla_tracks_copy_from_adt(Main *bmain,
 
 /* Adding ------------------------------------------- */
 
-NlaTrack *BKE_nlatrack_add(AnimData *adt, NlaTrack *prev, const bool is_liboverride)
+NlaTrack *BKE_nlatrack_new()
 {
-  NlaTrack *nlt;
-
-  /* sanity checks */
-  if (adt == NULL) {
-    return NULL;
-  }
-
   /* allocate new track */
-  nlt = MEM_callocN(sizeof(NlaTrack), "NlaTrack");
+  NlaTrack *nlt = MEM_callocN(sizeof(NlaTrack), "NlaTrack");
 
   /* set settings requiring the track to not be part of the stack yet */
   nlt->flag = NLATRACK_SELECTED | NLATRACK_OVERRIDELIBRARY_LOCAL;
-  nlt->index = BLI_listbase_count(&adt->nla_tracks);
+
+  return nlt;
+}
+
+void BKE_nlatrack_insert_before(ListBase *nla_tracks,
+                                struct NlaTrack *next,
+                                struct NlaTrack *new_track,
+                                bool is_liboverride)
+{
+
+  if (is_liboverride) {
+    /* Currently, all library override tracks are assumed to be grouped together at the start of
+     * the list. Non overridden must be placed after last library track. */
+    if (next != NULL && (next->flag & NLATRACK_OVERRIDELIBRARY_LOCAL) == 0) {
+      BKE_nlatrack_insert_after(nla_tracks, next, new_track, is_liboverride);
+      return;
+    }
+  }
+
+  BLI_insertlinkbefore(nla_tracks, next, new_track);
+  new_track->index = BLI_findindex(nla_tracks, new_track);
+
+  /* Must have unique name, but we need to seed this. */
+  strcpy(new_track->name, "NlaTrack");
+
+  BLI_uniquename(nla_tracks,
+                 new_track,
+                 DATA_("NlaTrack"),
+                 '.',
+                 offsetof(NlaTrack, name),
+                 sizeof(new_track->name));
+}
+
+void BKE_nlatrack_insert_after(ListBase *nla_tracks,
+                               struct NlaTrack *prev,
+                               struct NlaTrack *new_track,
+                               const bool is_liboverride)
+{
+  BLI_assert(nla_tracks);
+  BLI_assert(new_track);
+
+  /** If NULL, then caller intends to insert a new head. But, tracks are not allowed to be placed
+   * before library overrides. So it must inserted after the last override. */
+  if (prev == NULL) {
+    NlaTrack *first_track = (NlaTrack *)nla_tracks->first;
+    if (first_track != NULL && (first_track->flag & NLATRACK_OVERRIDELIBRARY_LOCAL) == 0) {
+      prev = first_track;
+    }
+  }
 
   /* In liboverride case, we only add local tracks after all those coming from the linked data,
    * so we need to find the first local track. */
@@ -361,22 +402,46 @@ NlaTrack *BKE_nlatrack_add(AnimData *adt, NlaTrack *prev, const bool is_liboverr
     }
     prev = first_local != NULL ? first_local->prev : NULL;
   }
+
   /* Add track to stack, and make it the active one. */
-  if (prev != NULL) {
-    BLI_insertlinkafter(&adt->nla_tracks, prev, nlt);
-  }
-  else {
-    BLI_addtail(&adt->nla_tracks, nlt);
-  }
-  BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
+  BLI_insertlinkafter(nla_tracks, prev, new_track);
+  new_track->index = BLI_findindex(nla_tracks, new_track);
 
   /* must have unique name, but we need to seed this */
-  strcpy(nlt->name, "NlaTrack");
-  BLI_uniquename(
-      &adt->nla_tracks, nlt, DATA_("NlaTrack"), '.', offsetof(NlaTrack, name), sizeof(nlt->name));
+  BLI_uniquename(nla_tracks,
+                 new_track,
+                 DATA_("NlaTrack"),
+                 '.',
+                 offsetof(NlaTrack, name),
+                 sizeof(new_track->name));
+}
 
-  /* return the new track */
-  return nlt;
+NlaTrack *BKE_nlatrack_new_before(ListBase *nla_tracks, struct NlaTrack *next, bool is_liboverride)
+{
+  NlaTrack *new_track = BKE_nlatrack_new();
+
+  BKE_nlatrack_insert_before(nla_tracks, next, new_track, is_liboverride);
+
+  return new_track;
+}
+
+NlaTrack *BKE_nlatrack_new_after(ListBase *nla_tracks, struct NlaTrack *prev, bool is_liboverride)
+{
+  NlaTrack *new_track = BKE_nlatrack_new();
+
+  BKE_nlatrack_insert_after(nla_tracks, prev, new_track, is_liboverride);
+
+  return new_track;
+}
+
+NlaTrack *BKE_nlatrack_new_head(ListBase *nla_tracks, bool is_liboverride)
+{
+  return BKE_nlatrack_new_before(nla_tracks, (NlaTrack *)nla_tracks->first, is_liboverride);
+}
+
+NlaTrack *BKE_nlatrack_new_tail(ListBase *nla_tracks, const bool is_liboverride)
+{
+  return BKE_nlatrack_new_after(nla_tracks, (NlaTrack *)nla_tracks->last, is_liboverride);
 }
 
 NlaStrip *BKE_nlastrip_new(bAction *act)
@@ -448,7 +513,8 @@ NlaStrip *BKE_nlastack_add_strip(AnimData *adt, bAction *act, const bool is_libo
     /* trying to add to the last track failed (no track or no space),
      * so add a new track to the stack, and add to that...
      */
-    nlt = BKE_nlatrack_add(adt, NULL, is_liboverride);
+    nlt = BKE_nlatrack_new_tail(&adt->nla_tracks, is_liboverride);
+    BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
     BKE_nlatrack_add_strip(nlt, strip, is_liboverride);
     BLI_strncpy(nlt->name, act->id.name + 2, sizeof(nlt->name));
   }
@@ -848,8 +914,8 @@ void BKE_nlastrips_make_metas(ListBase *strips, bool is_temp)
       mstrip->end = strip->end;
     }
     else {
-      /* current strip wasn't selected, so the end of 'island' of selected strips has been reached,
-       * so stop adding strips to the current meta
+      /* current strip wasn't selected, so the end of 'island' of selected strips has been
+       * reached, so stop adding strips to the current meta.
        */
       mstrip = NULL;
     }
@@ -1180,7 +1246,9 @@ bool BKE_nlatrack_add_strip(NlaTrack *nlt, NlaStrip *strip, const bool is_libove
     return false;
   }
 
-  /* Do not allow adding strips if this track is locked, or not a local one in liboverride case. */
+  /*
+   * Do not allow adding strips if this track is locked, or not a local one in liboverride case.
+   */
   if (nlt->flag & NLATRACK_PROTECTED ||
       (is_liboverride && (nlt->flag & NLATRACK_OVERRIDELIBRARY_LOCAL) == 0)) {
     return false;
@@ -1952,7 +2020,8 @@ bool BKE_nla_action_stash(AnimData *adt, const bool is_liboverride)
     }
   }
 
-  nlt = BKE_nlatrack_add(adt, prev_track, is_liboverride);
+  nlt = BKE_nlatrack_new_after(&adt->nla_tracks, prev_track, is_liboverride);
+  BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
   BLI_assert(nlt != NULL);
 
   /* We need to ensure that if there wasn't any previous instance,
@@ -2075,7 +2144,8 @@ static void nla_tweakmode_find_active(const ListBase /* NlaTrack */ *nla_tracks,
   /* There are situations where we may have multiple strips selected and we want to enter
    * tweak-mode on all of those at once. Usually in those cases,
    * it will usually just be a single strip per AnimData.
-   * In such cases, compromise and take the last selected track and/or last selected strip, #28468.
+   * In such cases, compromise and take the last selected track and/or last selected strip,
+   * #28468.
    */
   if (activeTrack == NULL) {
     /* try last selected track for active strip */
@@ -2150,7 +2220,8 @@ bool BKE_nla_tweakmode_enter(AnimData *adt)
   activeStrip->flag &= ~NLASTRIP_FLAG_TWEAKUSER;
 
   /* go over all the tracks after AND INCLUDING the active one, tagging them as being disabled
-   * - the active track needs to also be tagged, otherwise, it'll overlap with the tweaks going on
+   * - the active track needs to also be tagged, otherwise, it'll overlap with the tweaks going
+   * on.
    */
   activeTrack->flag |= NLATRACK_DISABLED;
   if ((adt->flag & ADT_NLA_EVAL_UPPER_TRACKS) == 0) {

source/blender/blenkernel/intern/nla_test.cc

@@ -94,12 +94,10 @@ TEST(nla_track, BKE_nlatrack_remove_strip)
 TEST(nla_track, BKE_nlatrack_remove_and_free)
 {
   AnimData adt{};
-  NlaTrack *track1;
-  NlaTrack *track2;
 
   /* Add NLA tracks to the Animation Data. */
-  track1 = BKE_nlatrack_add(&adt, NULL, false);
-  track2 = BKE_nlatrack_add(&adt, track1, false);
+  NlaTrack *track1 = BKE_nlatrack_new_tail(&adt.nla_tracks, false);
+  NlaTrack *track2 = BKE_nlatrack_new_tail(&adt.nla_tracks, false);
 
   /* Ensure we have 2 tracks in the track. */
   EXPECT_EQ(2, BLI_listbase_count(&adt.nla_tracks));
@@ -116,4 +114,35 @@ TEST(nla_track, BKE_nlatrack_remove_and_free)
   EXPECT_EQ(-1, BLI_findindex(&adt.nla_tracks, track1));
 }
 
+TEST(nla_track, BKE_nlatrack_new_tail)
+{
+  AnimData adt{};
+  NlaTrack *trackB = BKE_nlatrack_new_tail(&adt.nla_tracks, false);
+  NlaTrack *trackA = BKE_nlatrack_new_tail(&adt.nla_tracks, false);
+
+  // Expect that Track B was added before track A
+  EXPECT_EQ(1, BLI_findindex(&adt.nla_tracks, trackA));
+  EXPECT_EQ(0, BLI_findindex(&adt.nla_tracks, trackB));
+ 
+  // Free the tracks
+  BKE_nlatrack_remove_and_free(&adt.nla_tracks, trackA, false);
+  BKE_nlatrack_remove_and_free(&adt.nla_tracks, trackB, false);
+}
+
+TEST(nla_track, BKE_nlatrack_new_head)
+{
+  AnimData adt{};
+  NlaTrack *trackB = BKE_nlatrack_new_head(&adt.nla_tracks, false);
+  NlaTrack *trackA = BKE_nlatrack_new_head(&adt.nla_tracks, false);
+
+  // Expect that Track A was added before track B
+  EXPECT_EQ(0, BLI_findindex(&adt.nla_tracks, trackA));
+  EXPECT_EQ(1, BLI_findindex(&adt.nla_tracks, trackB));
+
+  // Free the tracks
+  BKE_nlatrack_remove_and_free(&adt.nla_tracks, trackA, false);
+  BKE_nlatrack_remove_and_free(&adt.nla_tracks, trackB, false);
+
+}
+
 }  // namespace blender::bke::tests

source/blender/editors/object/object_add.cc

@@ -1993,7 +1993,8 @@ static int object_speaker_add_exec(bContext *C, wmOperator *op)
   {
     /* create new data for NLA hierarchy */
     AnimData *adt = BKE_animdata_ensure_id(&ob->id);
-    NlaTrack *nlt = BKE_nlatrack_add(adt, nullptr, is_liboverride);
+    NlaTrack *nlt = BKE_nlatrack_new_tail(&adt->nla_tracks, is_liboverride);
+    BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
     NlaStrip *strip = BKE_nla_add_soundstrip(bmain, scene, static_cast<Speaker *>(ob->data));
     strip->start = scene->r.cfra;
     strip->end += strip->start;

source/blender/editors/space_nla/nla_channels.c

@@ -573,6 +573,7 @@ bool nlaedit_add_tracks_existing(bAnimContext *ac, bool above_sel)
     if (ale->type == ANIMTYPE_NLATRACK) {
       NlaTrack *nlt = (NlaTrack *)ale->data;
       AnimData *adt = ale->adt;
+      NlaTrack *new_track = NULL;
 
       const bool is_liboverride = ID_IS_OVERRIDE_LIBRARY(ale->id);
 
@@ -581,14 +582,16 @@ bool nlaedit_add_tracks_existing(bAnimContext *ac, bool above_sel)
        */
       if (above_sel) {
         /* just add a new one above this one */
-        BKE_nlatrack_add(adt, nlt, is_liboverride);
+        new_track = BKE_nlatrack_new_after(&adt->nla_tracks, nlt, is_liboverride);
+        BKE_nlatrack_set_active(&adt->nla_tracks, new_track);
         ale->update = ANIM_UPDATE_DEPS;
         added = true;
       }
       else if ((lastAdt == NULL) || (adt != lastAdt)) {
         /* add one track to the top of the owning AnimData's stack,
          * then don't add anymore to this stack */
-        BKE_nlatrack_add(adt, NULL, is_liboverride);
+        new_track = BKE_nlatrack_new_tail(&adt->nla_tracks, is_liboverride);
+        BKE_nlatrack_set_active(&adt->nla_tracks, new_track);
         lastAdt = adt;
         ale->update = ANIM_UPDATE_DEPS;
         added = true;
@@ -618,6 +621,7 @@ bool nlaedit_add_tracks_empty(bAnimContext *ac)
   /* check if selected AnimData blocks are empty, and add tracks if so... */
   for (ale = anim_data.first; ale; ale = ale->next) {
     AnimData *adt = ale->adt;
+    NlaTrack *new_track;
 
     /* sanity check */
     BLI_assert(adt->flag & ADT_UI_SELECTED);
@@ -625,7 +629,8 @@ bool nlaedit_add_tracks_empty(bAnimContext *ac)
     /* ensure it is empty */
     if (BLI_listbase_is_empty(&adt->nla_tracks)) {
       /* add new track to this AnimData block then */
-      BKE_nlatrack_add(adt, NULL, ID_IS_OVERRIDE_LIBRARY(ale->id));
+      new_track = BKE_nlatrack_new_tail(&adt->nla_tracks, ID_IS_OVERRIDE_LIBRARY(ale->id));
+      BKE_nlatrack_set_active(&adt->nla_tracks, new_track);
       ale->update = ANIM_UPDATE_DEPS;
       added = true;
     }

source/blender/editors/space_nla/nla_edit.c

@@ -717,7 +717,8 @@ static int nlaedit_add_actionclip_exec(bContext *C, wmOperator *op)
       /* trying to add to the current failed (no space),
        * so add a new track to the stack, and add to that...
        */
-      nlt = BKE_nlatrack_add(adt, NULL, is_liboverride);
+      nlt = BKE_nlatrack_new_tail(&adt->nla_tracks, is_liboverride);
+      BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
       BKE_nlatrack_add_strip(nlt, strip, is_liboverride);
     }
 
@@ -955,7 +956,8 @@ static int nlaedit_add_sound_exec(bContext *C, wmOperator *UNUSED(op))
       /* trying to add to the current failed (no space),
        * so add a new track to the stack, and add to that...
        */
-      nlt = BKE_nlatrack_add(adt, NULL, is_liboverride);
+      nlt = BKE_nlatrack_new_tail(&adt->nla_tracks, is_liboverride);
+      BKE_nlatrack_set_active(&adt->nla_tracks, nlt);
       BKE_nlatrack_add_strip(nlt, strip, is_liboverride);
     }
 
@@ -1191,11 +1193,8 @@ static int nlaedit_duplicate_exec(bContext *C, wmOperator *op)
         /* in case there's no space in the track above,
          * or we haven't got a reference to it yet, try adding */
         if (BKE_nlatrack_add_strip(nlt->next, nstrip, is_liboverride) == 0) {
-          /* need to add a new track above the one above the current one
-           * - if the current one is the last one, nlt->next will be NULL, which defaults to adding
-           *   at the top of the stack anyway...
-           */
-          track = BKE_nlatrack_add(adt, nlt->next, is_liboverride);
+          track = BKE_nlatrack_new_after(&adt->nla_tracks, nlt->next, is_liboverride);
+          BKE_nlatrack_set_active(&adt->nla_tracks, track);
           BKE_nlatrack_add_strip(track, nstrip, is_liboverride);
         }
 
@@ -2459,7 +2458,8 @@ static int nlaedit_snap_exec(bContext *C, wmOperator *op)
       /* in case there's no space in the current track, try adding */
       if (BKE_nlatrack_add_strip(nlt, strip, is_liboverride) == 0) {
         /* need to add a new track above the current one */
-        track = BKE_nlatrack_add(adt, nlt, is_liboverride);
+        track = BKE_nlatrack_new_after(&adt->nla_tracks, nlt, is_liboverride);
+        BKE_nlatrack_set_active(&adt->nla_tracks, track);
         BKE_nlatrack_add_strip(track, strip, is_liboverride);
 
         /* clear temp meta-strips on this new track,

source/blender/makesrna/intern/rna_animation.c

@@ -586,7 +586,16 @@ static void rna_KeyingSet_paths_clear(KeyingSet *keyingset, ReportList *reports)
 /* needs wrapper function to push notifier */
 static NlaTrack *rna_NlaTrack_new(ID *id, AnimData *adt, Main *bmain, bContext *C, NlaTrack *track)
 {
-  NlaTrack *new_track = BKE_nlatrack_add(adt, track, ID_IS_OVERRIDE_LIBRARY(id));
+  NlaTrack *new_track;
+
+  if (track == NULL) {
+    new_track = BKE_nlatrack_new_tail(&adt->nla_tracks, ID_IS_OVERRIDE_LIBRARY(id));
+  }
+  else {
+    new_track = BKE_nlatrack_new_after(&adt->nla_tracks, track, ID_IS_OVERRIDE_LIBRARY(id));
+  }
+
+  BKE_nlatrack_set_active(&adt->nla_tracks, new_track);
 
   WM_event_add_notifier(C, NC_ANIMATION | ND_NLA | NA_ADDED, NULL);