Fix #119615: Anim, Crash with NLA tweak mode and linked Armatures

Fix a crash when inserting a key with tweak mode enabled, but where
`AnimData::actstrip` was NULL.

The root cause of this is that two pointers in the `AnimData` struct
(`act_track` and `actstrip`) are expected to be set when NLA tweak mode
is enabled, BUT these are not exposed to RNA and thus invisible to the
library overrides system. As such, they are NULL when loading from disk,
while the `ADT_NLA_EDIT_ON` flag still indicates they are to be used.

Rather than adding a NULL pointer check (and having to add that in many
more places), I used this two-pronged approach:

- Extend the 'NLA tweakmode' override apply code, to set the `act_track`
  and `actstrip` pointers when they are incorrectly NULL. This is done
  by lookup of the track and strip by name.
- Add versioning code to exit out of tweak mode whenever the
  `ADT_NLA_EDIT_ON` flag is set, but those two pointers are still NULL.

The last step was necessary with the example file attached to the bug
report, as that was saved with a buggy blender version. New saves work
just fine.

Pull Request: https://projects.blender.org/blender/blender/pulls/119632

source/blender/blenkernel/BKE_blender_version.h

@@ -29,7 +29,7 @@ extern "C" {
 
 /* Blender file format version. */
 #define BLENDER_FILE_VERSION BLENDER_VERSION
-#define BLENDER_FILE_SUBVERSION 22
+#define BLENDER_FILE_SUBVERSION 23
 
 /* Minimum Blender version that supports reading file written with the current
  * version. Older Blender versions will test this and cancel loading the file, showing a warning to

source/blender/blenkernel/BKE_nla.h

@@ -395,6 +395,12 @@ struct NlaStrip *BKE_nlastrip_find_active(struct NlaTrack *nlt);
  * Make the given NLA-Strip the active one within the given block.
  */
 void BKE_nlastrip_set_active(struct AnimData *adt, struct NlaStrip *strip);
+/**
+ * Find the NLA-strip with the given name within the given track.
+ *
+ * \return pointer to the strip, or nullptr when not found.
+ */
+struct NlaStrip *BKE_nlastrip_find_by_name(struct NlaTrack *nlt, const char *name);
 
 /**
  * Does the given NLA-strip fall within the given bounds (times)?.

source/blender/blenkernel/intern/nla.cc

@@ -1457,6 +1457,31 @@ void BKE_nlastrip_set_active(AnimData *adt, NlaStrip *strip)
   }
 }
 
+static NlaStrip *nlastrip_find_by_name(ListBase /* NlaStrip */ *strips, const char *name)
+{
+  LISTBASE_FOREACH (NlaStrip *, strip, strips) {
+    if (STREQ(strip->name, name)) {
+      return strip;
+    }
+
+    if (strip->type != NLASTRIP_TYPE_META) {
+      continue;
+    }
+
+    NlaStrip *inner_strip = nlastrip_find_by_name(&strip->strips, name);
+    if (inner_strip != nullptr) {
+      return inner_strip;
+    }
+  }
+
+  return nullptr;
+}
+
+NlaStrip *BKE_nlastrip_find_by_name(NlaTrack *nlt, const char *name)
+{
+  return nlastrip_find_by_name(&nlt->strips, name);
+}
+
 bool BKE_nlastrip_within_bounds(NlaStrip *strip, float min, float max)
 {
   const float stripLen = (strip) ? strip->end - strip->start : 0.0f;

source/blender/blenloader/intern/versioning_400.cc

@@ -16,6 +16,7 @@
 /* Define macros in `DNA_genfile.h`. */
 #define DNA_GENFILE_VERSIONING_MACROS
 
+#include "DNA_anim_types.h"
 #include "DNA_brush_types.h"
 #include "DNA_camera_types.h"
 #include "DNA_curve_types.h"
@@ -57,6 +58,7 @@
 #include "BKE_main.hh"
 #include "BKE_material.h"
 #include "BKE_mesh_legacy_convert.hh"
+#include "BKE_nla.h"
 #include "BKE_node.hh"
 #include "BKE_node_runtime.hh"
 #include "BKE_scene.h"
@@ -356,6 +358,51 @@ static void versioning_replace_splitviewer(bNodeTree *ntree)
   }
 }
 
+/**
+ * Exit NLA tweakmode when the AnimData struct has insufficient information.
+ *
+ * When NLA tweakmode is enabled, Blender expects certain pointers to be set up
+ * correctly, and if that fails, can crash. This function ensures that
+ * everything is consistent, by exiting tweakmode everywhere there's missing
+ * pointers.
+ *
+ * This shouldn't happen, but the example blend file attached to #119615 needs
+ * this.
+ */
+static void version_nla_tweakmode_incomplete(Main *bmain)
+{
+  bool any_valid_tweakmode_left = false;
+
+  ID *id;
+  FOREACH_MAIN_ID_BEGIN (bmain, id) {
+    AnimData *adt = BKE_animdata_from_id(id);
+    if (!adt || !(adt->flag & ADT_NLA_EDIT_ON)) {
+      continue;
+    }
+
+    if (adt->act_track && adt->actstrip) {
+      /* Expected case. */
+      any_valid_tweakmode_left = true;
+      continue;
+    }
+
+    /* Not enough info in the blend file to reliably stay in tweak mode. This is the most important
+     * part of this versioning code, as it prevents future nullptr access. */
+    BKE_nla_tweakmode_exit(adt);
+  }
+  FOREACH_MAIN_ID_END;
+
+  if (any_valid_tweakmode_left) {
+    /* There are still NLA strips correctly in tweak mode. */
+    return;
+  }
+
+  /* Nothing is in a valid tweakmode, so just disable the corresponding flags on all scenes. */
+  LISTBASE_FOREACH (Scene *, scene, &bmain->scenes) {
+    scene->flag &= ~SCE_NLA_EDIT_ON;
+  }
+}
+
 void do_versions_after_linking_400(FileData *fd, Main *bmain)
 {
   if (!MAIN_VERSION_FILE_ATLEAST(bmain, 400, 9)) {
@@ -450,6 +497,10 @@ void do_versions_after_linking_400(FileData *fd, Main *bmain)
     }
   }
 
+  if (!MAIN_VERSION_FILE_ATLEAST(bmain, 401, 23)) {
+    version_nla_tweakmode_incomplete(bmain);
+  }
+
   /**
    * Always bump subversion in BKE_blender_version.h when adding versioning
    * code here, and wrap it inside a MAIN_VERSION_FILE_ATLEAST check.

source/blender/makesrna/intern/rna_animation.cc

@@ -12,6 +12,7 @@
 #include "DNA_anim_types.h"
 #include "DNA_scene_types.h"
 
+#include "BLI_listbase_wrapper.hh"
 #include "BLI_utildefines.h"
 
 #include "BLT_translation.h"
@@ -189,6 +190,34 @@ bool rna_AnimData_tweakmode_override_apply(Main * /*bmain*/,
 
   anim_data_dst->flag = (anim_data_dst->flag & ~ADT_NLA_EDIT_ON) |
                         (anim_data_src->flag & ADT_NLA_EDIT_ON);
+
+  if (!(anim_data_dst->flag & ADT_NLA_EDIT_ON)) {
+    /* If tweak mode is not enabled, there's nothing left to do. */
+    return true;
+  }
+
+  if (!anim_data_src->act_track || !anim_data_src->actstrip) {
+    /* If there is not enough information to find the active track/strip, don't bother. */
+    return true;
+  }
+
+  /* AnimData::act_track and AnimData::actstrip are not directly exposed to RNA as editable &
+   * overridable, so the override doesn't contain this info. Reconstruct the pointers by name. */
+  for (NlaTrack *track : blender::ListBaseWrapper<NlaTrack>(anim_data_dst->nla_tracks)) {
+    if (!STREQ(track->name, anim_data_src->act_track->name)) {
+      continue;
+    }
+
+    NlaStrip *strip = BKE_nlastrip_find_by_name(track, anim_data_src->actstrip->name);
+    if (!strip) {
+      continue;
+    }
+
+    anim_data_dst->act_track = track;
+    anim_data_dst->actstrip = strip;
+    break;
+  }
+
   return true;
 }