Fix T98196: Crash when moving tweaked NLA strip with empty track below

`BKE_nlastrip_find_active()` and `update_active_strip()` now do a more
thorough search for the active strip, by also recursing into
meta-strips.

There were some assumptions in the NLA code that the active strips is
contained in the active track. This assumption turned out to be false:
when there is a meta-strip, the active strip could be inside that, and
then it's not contained in `active_track.strips` directly.

Apart from the above, there are other situations in which the track
pointed to by `AnimData::act_track` does *not* contain the active strip
(even indirectly).

Both these cases can happen when the transform system is moving a strip
that's in tweak mode. Entering tweak mode doesn't just search for the
active track/strip, but also falls back to the last-selected ones. This
means that the track/strip pointers & flags can be out of sync with
what's being tweaked/transformed. Because of this, the assumption that
there is any relation between "active strip" and "active track" has been
removed from the `update_active_strip()` function.

All this searching could be prevented by passing the `AnimData` to the
code that duplicates tracks & strips. That way, when the active
track/strip is dup'ed, the `AnimData` pointers can be updated as well.
That would require more refactoring and thus could introduce other bugs,
so that's left for later.

1 files changed, 51 insertions, 28 deletions

diff --git a/source/blender/blenkernel/intern/nla.c b/source/blender/blenkernel/intern/nla.c
index 5d804f53779..a5f6c453ed4 100644
--- a/source/blender/blenkernel/intern/nla.c
+++ b/source/blender/blenkernel/intern/nla.c
@@ -244,24 +244,38 @@ void BKE_nla_tracks_copy(Main *bmain, ListBase *dst, const ListBase *src, const
   }
 }
 
-/* Set adt_dest->actstrip to the strip with the same index as adt_source->actstrip. */
-static void update_active_strip(AnimData *adt_dest,
-                                NlaTrack *track_dest,
-                                const AnimData *adt_source,
-                                NlaTrack *track_source)
+static void update_active_strip_from_listbase(AnimData *adt_dest,
+                                              NlaTrack *track_dest,
+                                              const NlaStrip *active_strip,
+                                              const ListBase /* NlaStrip */ *strips_source)
 {
-  BLI_assert(BLI_listbase_count(&track_source->strips) == BLI_listbase_count(&track_dest->strips));
-
   NlaStrip *strip_dest = track_dest->strips.first;
-  LISTBASE_FOREACH (NlaStrip *, strip_source, &track_source->strips) {
-    if (strip_source == adt_source->actstrip) {
+  LISTBASE_FOREACH (const NlaStrip *, strip_source, strips_source) {
+    if (strip_source == active_strip) {
       adt_dest->actstrip = strip_dest;
+      return;
+    }
+
+    if (strip_source->type == NLASTRIP_TYPE_META) {
+      update_active_strip_from_listbase(adt_dest, track_dest, active_strip, &strip_source->strips);
     }
 
     strip_dest = strip_dest->next;
   }
 }
 
+/* Set adt_dest->actstrip to the strip with the same index as adt_source->actstrip. */
+static void update_active_strip(AnimData *adt_dest,
+                                NlaTrack *track_dest,
+                                const AnimData *adt_source,
+                                const NlaTrack *track_source)
+{
+  BLI_assert(BLI_listbase_count(&track_source->strips) == BLI_listbase_count(&track_dest->strips));
+
+  update_active_strip_from_listbase(
+      adt_dest, track_dest, adt_source->actstrip, &track_source->strips);
+}
+
 /* Set adt_dest->act_track to the track with the same index as adt_source->act_track. */
 static void update_active_track(AnimData *adt_dest, const AnimData *adt_source)
 {
@@ -272,20 +286,20 @@ static void update_active_track(AnimData *adt_dest, const AnimData *adt_source)
   LISTBASE_FOREACH (NlaTrack *, track_source, &adt_source->nla_tracks) {
     if (track_source == adt_source->act_track) {
       adt_dest->act_track = track_dest;
-      /* Assumption: the active strip is on the active track. */
-      update_active_strip(adt_dest, track_dest, adt_source, track_source);
     }
+    update_active_strip(adt_dest, track_dest, adt_source, track_source);
 
     track_dest = track_dest->next;
   }
 
-  /* If the above assumption failed to hold, do a more thorough search for the active strip. */
-  if (adt_source->actstrip != NULL && adt_dest->actstrip == NULL) {
-    nla_tweakmode_find_active(&adt_source->nla_tracks, &track_dest, &adt_dest->actstrip);
+#ifndef NDEBUG
+  {
+    const bool source_has_actstrip = adt_source->actstrip != NULL;
+    const bool dest_has_actstrip = adt_dest->actstrip != NULL;
+    BLI_assert_msg(source_has_actstrip == dest_has_actstrip,
+                   "Active strip did not copy correctly");
   }
-
-  BLI_assert_msg((adt_source->actstrip == NULL) == (adt_dest->actstrip == NULL),
-                 "Active strip did not copy correctly");
+#endif
 }
 
 void BKE_nla_tracks_copy_from_adt(Main *bmain,
@@ -1171,26 +1185,35 @@ bool BKE_nlatrack_is_nonlocal_in_liboverride(const ID *id, const NlaTrack *nlt)
 
 /* NLA Strips -------------------------------------- */
 
-NlaStrip *BKE_nlastrip_find_active(NlaTrack *nlt)
+static NlaStrip *nlastrip_find_active(ListBase /* NlaStrip */ *strips)
 {
-  NlaStrip *strip;
-
-  /* sanity check */
-  if (ELEM(NULL, nlt, nlt->strips.first)) {
-    return NULL;
-  }
-
-  /* try to find the first active strip */
-  for (strip = nlt->strips.first; strip; strip = strip->next) {
+  LISTBASE_FOREACH (NlaStrip *, strip, strips) {
     if (strip->flag & NLASTRIP_FLAG_ACTIVE) {
       return strip;
     }
+
+    if (strip->type != NLASTRIP_TYPE_META) {
+      continue;
+    }
+
+    NlaStrip *inner_active = nlastrip_find_active(&strip->strips);
+    if (inner_active != NULL) {
+      return inner_active;
+    }
   }
 
-  /* none found */
   return NULL;
 }
 
+NlaStrip *BKE_nlastrip_find_active(NlaTrack *nlt)
+{
+  if (nlt == NULL) {
+    return NULL;
+  }
+
+  return nlastrip_find_active(&nlt->strips);
+}
+
 void BKE_nlastrip_set_active(AnimData *adt, NlaStrip *strip)
 {
   NlaTrack *nlt;