diff --git a/source/blender/animrig/intern/versioning.cc b/source/blender/animrig/intern/versioning.cc index ea6ff1be26b..c40ee0f507f 100644 --- a/source/blender/animrig/intern/versioning.cc +++ b/source/blender/animrig/intern/versioning.cc @@ -35,6 +35,14 @@ constexpr const char *DEFAULT_VERSIONED_LAYER_NAME = "Legacy Layer"; bool action_is_layered(const bAction &dna_action) { + /* NOTE: due to how forward-compatibility is handled when writing Actions to + * blend files, it is important that this function does NOT check + * `Action.idroot` as part of its determination of whether this is a layered + * action or not. + * + * See: `action_blend_write()` and `action_blend_read_data()` + */ + const animrig::Action &action = dna_action.wrap(); const bool has_layered_data = action.layer_array_num > 0 || action.slot_array_num > 0; diff --git a/source/blender/blenkernel/intern/action.cc b/source/blender/blenkernel/intern/action.cc index 97dab7d7c06..2c2489e7fa9 100644 --- a/source/blender/blenkernel/intern/action.cc +++ b/source/blender/blenkernel/intern/action.cc @@ -511,6 +511,20 @@ static void action_blend_write(BlendWriter *writer, ID *id, const void *id_addre const animrig::Slot &first_slot = *action.slot(0); + /* The forward-compat animation data we write is for IDs of the type that + * the first slot is intended for. Therefore, the Action should have that + * `idroot` when loaded in old versions of Blender. + * + * Note that if there is no slot, this code will never run and therefore the + * action will be written with `idroot = 0`. Despite that, old + * pre-slotted-action files are still guaranteed to round-trip losslessly, + * because old actions (even when empty) are versioned to have one slot with + * `idtype` set to whatever the old action's `idroot` was. In other words, + * zero-slot actions can only be created via non-legacy features, and + * therefore represent animation data that wasn't purely from old files + * anyway. */ + action.idroot = first_slot.idtype; + /* Note: channel group forward-compat data requires that fcurve * forward-compat legacy data is also written, and vice-versa. Both have * pointers to each other that won't resolve properly when loaded in older @@ -531,6 +545,10 @@ static void action_blend_write(BlendWriter *writer, ID *id, const void *id_addre write_slots(writer, action.slots()); if (do_write_forward_compat) { + /* Set the idroot back to 'unspecified', as it always should be for layered + * Actions. */ + action.idroot = 0; + /* The pointers to the first/last FCurve in the `action.curves` have already * been written as part of the Action struct data, so they can be cleared * here, such that the code writing legacy fcurves below does nothing (as @@ -687,6 +705,12 @@ static void action_blend_read_data(BlendDataReader *reader, ID *id) /* Should never be stored as part of the forward-compatible data in a * layered action, and thus should always be empty here. */ BLI_assert(BLI_listbase_is_empty(&action.chanbase)); + + /* Layered actions should always have `idroot == 0`, but when writing an + * action to a blend file `idroot` is typically set otherwise for forward + * compatibility reasons (see `action_blend_write()`). So we set it to zero + * here to put it back as it should be. */ + action.idroot = 0; } else { /* Read legacy data. */