Cleanup: No longer require VSE Strip struct memory layout to never change

Previously code that was reading Strip data assumed that seqbasep
and channels members would stay at fixed offsets within a struct,
forever into the future. Fix this by inferring their offsets from
the file SDNA data where needed.

Actual Strip DNA layout is not changed in this commit yet.

Co-authored-by: Sergey Sharybin <sergey@blender.org>
Pull Request: https://projects.blender.org/blender/blender/pulls/142940

source/blender/blenkernel/intern/scene.cc

@@ -1370,34 +1370,24 @@ static void scene_blend_read_data(BlendDataReader *reader, ID *id)
     /* link metastack, slight abuse of structs here,
      * have to restore pointer to internal part in struct */
     {
-      void *seqbase_poin;
-      void *channels_poin;
-      /* This whole thing with seqbasep offsets is really not good
-       * and prevents changes to the Sequence struct. A more correct approach
-       * would be to calculate offset using sDNA from the file (NOT from the
-       * current Blender). Even better would be having some sort of dedicated
-       * map of seqbase pointers to avoid this offset magic. */
-      constexpr intptr_t seqbase_offset = offsetof(Strip, seqbase);
-      constexpr intptr_t channels_offset = offsetof(Strip, channels);
-#if ARCH_CPU_64_BITS
-      static_assert(seqbase_offset == 264, "Sequence seqbase member offset cannot be changed");
-      static_assert(channels_offset == 280, "Sequence channels member offset cannot be changed");
-#else
-      static_assert(seqbase_offset == 204, "Sequence seqbase member offset cannot be changed");
-      static_assert(channels_offset == 212, "Sequence channels member offset cannot be changed");
-#endif
+      const int seqbase_offset_file = BLO_read_struct_member_offset(
+          reader, "Strip", "ListBase", "seqbase");
+      const int channels_offset_file = BLO_read_struct_member_offset(
+          reader, "Strip", "ListBase", "channels");
+      const size_t seqbase_offset_mem = offsetof(Strip, seqbase);
+      const size_t channels_offset_mem = offsetof(Strip, channels);
 
       /* seqbase root pointer */
-      if (ed->seqbasep == old_seqbasep) {
+      if (ed->seqbasep == old_seqbasep || seqbase_offset_file < 0) {
         ed->seqbasep = &ed->seqbase;
       }
       else {
-        seqbase_poin = POINTER_OFFSET(ed->seqbasep, -seqbase_offset);
+        void *seqbase_poin = POINTER_OFFSET(ed->seqbasep, -seqbase_offset_file);
 
         seqbase_poin = BLO_read_get_new_data_address_no_us(reader, seqbase_poin, sizeof(Strip));
 
         if (seqbase_poin) {
-          ed->seqbasep = (ListBase *)POINTER_OFFSET(seqbase_poin, seqbase_offset);
+          ed->seqbasep = (ListBase *)POINTER_OFFSET(seqbase_poin, seqbase_offset_mem);
         }
         else {
           ed->seqbasep = &ed->seqbase;
@@ -1405,16 +1395,18 @@ static void scene_blend_read_data(BlendDataReader *reader, ID *id)
       }
 
       /* Active channels root pointer. */
-      if (ELEM(ed->displayed_channels, old_displayed_channels, nullptr)) {
+      if (ELEM(ed->displayed_channels, old_displayed_channels, nullptr) ||
+          channels_offset_file < 0)
+      {
         ed->displayed_channels = &ed->channels;
       }
       else {
-        channels_poin = POINTER_OFFSET(ed->displayed_channels, -channels_offset);
+        void *channels_poin = POINTER_OFFSET(ed->displayed_channels, -channels_offset_file);
         channels_poin = BLO_read_get_new_data_address_no_us(
             reader, channels_poin, sizeof(SeqTimelineChannel));
 
         if (channels_poin) {
-          ed->displayed_channels = (ListBase *)POINTER_OFFSET(channels_poin, channels_offset);
+          ed->displayed_channels = (ListBase *)POINTER_OFFSET(channels_poin, channels_offset_mem);
         }
         else {
           ed->displayed_channels = &ed->channels;
@@ -1427,30 +1419,30 @@ static void scene_blend_read_data(BlendDataReader *reader, ID *id)
       LISTBASE_FOREACH (MetaStack *, ms, &ed->metastack) {
         BLO_read_struct(reader, Strip, &ms->parent_strip);
 
-        if (ms->oldbasep == old_seqbasep) {
+        if (ms->oldbasep == old_seqbasep || seqbase_offset_file < 0) {
           ms->oldbasep = &ed->seqbase;
         }
         else {
-          seqbase_poin = POINTER_OFFSET(ms->oldbasep, -seqbase_offset);
+          void *seqbase_poin = POINTER_OFFSET(ms->oldbasep, -seqbase_offset_file);
           seqbase_poin = BLO_read_get_new_data_address_no_us(reader, seqbase_poin, sizeof(Strip));
           if (seqbase_poin) {
-            ms->oldbasep = (ListBase *)POINTER_OFFSET(seqbase_poin, seqbase_offset);
+            ms->oldbasep = (ListBase *)POINTER_OFFSET(seqbase_poin, seqbase_offset_mem);
           }
           else {
             ms->oldbasep = &ed->seqbase;
           }
         }
 
-        if (ELEM(ms->old_channels, old_displayed_channels, nullptr)) {
+        if (ELEM(ms->old_channels, old_displayed_channels, nullptr) || channels_offset_file < 0) {
           ms->old_channels = &ed->channels;
         }
         else {
-          channels_poin = POINTER_OFFSET(ms->old_channels, -channels_offset);
+          void *channels_poin = POINTER_OFFSET(ms->old_channels, -channels_offset_file);
           channels_poin = BLO_read_get_new_data_address_no_us(
               reader, channels_poin, sizeof(SeqTimelineChannel));
 
           if (channels_poin) {
-            ms->old_channels = (ListBase *)POINTER_OFFSET(channels_poin, channels_offset);
+            ms->old_channels = (ListBase *)POINTER_OFFSET(channels_poin, channels_offset_mem);
           }
           else {
             ms->old_channels = &ed->channels;

source/blender/blenloader/BLO_read_write.hh

@@ -377,6 +377,11 @@ void BLO_read_glob_list(BlendDataReader *reader, ListBase *list);
 BlendFileReadReport *BLO_read_data_reports(BlendDataReader *reader);
 struct Library *BLO_read_data_current_library(BlendDataReader *reader);
 
+int BLO_read_struct_member_offset(const BlendDataReader *reader,
+                                  const char *stype,
+                                  const char *vartype,
+                                  const char *name);
+
 /** \} */
 
 /* -------------------------------------------------------------------- */

source/blender/blenloader/intern/readfile.cc

@@ -5224,6 +5224,14 @@ int BLO_read_fileversion_get(BlendDataReader *reader)
   return reader->fd->fileversion;
 }
 
+int BLO_read_struct_member_offset(const BlendDataReader *reader,
+                                  const char *stype,
+                                  const char *vartype,
+                                  const char *name)
+{
+  return DNA_struct_member_offset_by_name_with_alias(reader->fd->filesdna, stype, vartype, name);
+}
+
 void BLO_read_struct_list_with_size(BlendDataReader *reader,
                                     const size_t expected_elem_size,
                                     ListBase *list)

source/blender/makesdna/DNA_sequence_types.h

@@ -237,8 +237,8 @@ typedef struct Strip {
   /** Effect strip inputs (`nullptr` if not an effect strip). */
   struct Strip *input1, *input2;
 
-  /* This strange padding is needed due to how `seqbasep` de-serialization is
-   * done right now in #scene_blend_read_data. */
+  /* This strange padding is needed for compatibility with older versions
+   * that assumed `seqbasep` is at fixed offset. */
   void *_pad7;
   int _pad8[2];
 

tests/python/CMakeLists.txt

@@ -1297,6 +1297,12 @@ if(TEST_SRC_DIR_EXISTS)
     --
     --testdir "${TEST_SRC_DIR}/sequence_editing"
   )
+
+  add_blender_test(
+    sequencer_load_meta_stack
+    ${TEST_SRC_DIR}/sequence_editing/vse_load_meta_stack.blend
+    --python ${TEST_PYTHON_DIR}/sequencer_load_meta_stack.py
+  )
 endif()
 
 # ------------------------------------------------------------------------------

tests/python/sequencer_load_meta_stack.py

@@ -0,0 +1,46 @@
+# SPDX-FileCopyrightText: 2020-2022 Blender Authors
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+# To run all tests, use
+# BLENDER_VERBOSE=1 ./bin/blender --background ../tests/files/sequence_editing/vse_load_meta_stack.blend --python ../blender/tests/python/sequencer_load_meta_stack.py
+# (that assumes the test is run from a build directory in the same directory as the source code)
+import bpy
+
+import argparse
+import sys
+import unittest
+
+
+class SequencerLoadMetastaskTest(unittest.TestCase):
+    def get_sequence_editor(self):
+        return bpy.context.scene.sequence_editor
+
+    def test_meta_stack(self):
+        sequence_editor = self.get_sequence_editor()
+
+        meta_stack = sequence_editor.meta_stack
+        self.assertEqual(len(meta_stack), 1)
+        self.assertEqual(meta_stack[0].name, "MetaStrip")
+
+        self.assertEqual(len(meta_stack[0].sequences), 1)
+        self.assertEqual(meta_stack[0].sequences[0].name, "Color")
+
+        # accesses ed->seqbasep through screen_ctx_selected_editable_sequences
+        bpy.context.copy()
+
+
+def main():
+    argv = [sys.argv[0]]
+    if '--' in sys.argv:
+        argv += sys.argv[sys.argv.index('--') + 1:]
+
+    parser = argparse.ArgumentParser()
+
+    args, remaining = parser.parse_known_args(argv)
+
+    unittest.main(argv=remaining)
+
+
+if __name__ == "__main__":
+    main()