# Fix 128078, Part I: Fix missing reverse endian switch of Action's `idroot`.

ID type code stored as ints (or shorts) need their endian switch to be
reverted (in case there is endianess conversion) on file read.
Interestingly, this was done for the deprecated IPO data (among others), but
not for the Action one!

NOTE: There is no versioning fix for this mistake, i.e. old files that
were saved from a BE system, then opened and re-saved from a LE system,
will still have totally invalid ID code values. This is not considered
as necessary currently, given that this `idroot` value is only
'informational' and not relied on by any part of the code.

# Fix 128078, Part II: GPv3 conversion code missing animation of Layers' location.

Also add code to the AnimDataConverter to ensure that actions get the
`idroot` matching their new ID owner type in GP data case.

Pull Request: https://projects.blender.org/blender/blender/pulls/128129
This commit is contained in:
Bastien Montagne
2024-10-14 13:59:06 +02:00
committed by Bastien Montagne
parent ae1409b9ae
commit 39c1127dc0
2 changed files with 95 additions and 0 deletions

View File

@@ -24,6 +24,7 @@
#include "DNA_scene_types.h"
#include "BLI_blenlib.h"
#include "BLI_endian_switch.h"
#include "BLI_ghash.h"
#include "BLI_math_color.h"
#include "BLI_math_matrix.h"
@@ -35,6 +36,8 @@
#include "BLT_translation.hh"
#include "BLO_read_write.hh"
#include "BKE_action.hh"
#include "BKE_anim_data.hh"
#include "BKE_anim_visualization.h"
@@ -675,6 +678,13 @@ static void action_blend_read_data(BlendDataReader *reader, ID *id)
{
animrig::Action &action = reinterpret_cast<bAction *>(id)->wrap();
/* Undo generic endian switching (careful, only the two least significant bytes of the int32 must
* be swapped back here, since this value is actually an int16). */
if (BLO_read_requires_endian_switch(reader)) {
bAction *act = reinterpret_cast<bAction *>(id);
BLI_endian_switch_int16(reinterpret_cast<short *>(&act->idroot));
}
#ifdef WITH_ANIM_BAKLAVA
read_strip_keyframe_data_array(reader, action);
read_layers(reader, action);

View File

@@ -217,6 +217,7 @@ class AnimDataConvertor {
private:
using FCurveCallback = bool(bAction *owner_action, FCurve &fcurve);
using ActionCallback = bool(bAction &action);
/** \return True if this AnimDataConvertor is valid, i.e. can be used to process animation data
* from source ID. */
@@ -324,6 +325,9 @@ class AnimDataConvertor {
is_changed = is_changed || local_is_changed;
}
/* NOTE: New layered actions system can be ignored here, it did not exist together with GPv2.
*/
if (this->skip_nla) {
return is_changed;
}
@@ -337,6 +341,55 @@ class AnimDataConvertor {
return is_changed;
}
bool action_process(bAction &action, blender::FunctionRef<ActionCallback> callback) const
{
if (callback(action)) {
DEG_id_tag_update(&action.id, ID_RECALC_ANIMATION);
return true;
}
return false;
}
bool nla_strip_action_foreach(NlaStrip &nla_strip,
blender::FunctionRef<ActionCallback> callback) const
{
bool is_changed = false;
if (nla_strip.act) {
is_changed = action_process(*nla_strip.act, callback);
}
LISTBASE_FOREACH (NlaStrip *, nla_strip_children, &nla_strip.strips) {
is_changed = is_changed || this->nla_strip_action_foreach(*nla_strip_children, callback);
}
return is_changed;
}
bool animdata_action_foreach(AnimData &anim_data,
blender::FunctionRef<ActionCallback> callback) const
{
bool is_changed = false;
if (anim_data.action) {
is_changed = is_changed || action_process(*anim_data.action, callback);
}
if (anim_data.tmpact) {
is_changed = is_changed || action_process(*anim_data.tmpact, callback);
}
/* NOTE: New layered actions system can be ignored here, it did not exist together with GPv2.
*/
if (this->skip_nla) {
return is_changed;
}
LISTBASE_FOREACH (NlaTrack *, nla_track, &anim_data.nla_tracks) {
LISTBASE_FOREACH (NlaStrip *, nla_strip, &nla_track->strips) {
is_changed = is_changed || this->nla_strip_action_foreach(*nla_strip, callback);
}
}
return is_changed;
}
public:
/**
* Check whether the source animation data contains FCurves that need to be converted/moved to
@@ -348,6 +401,10 @@ class AnimDataConvertor {
return false;
}
if (GS(id_src.name) != GS(id_dst.name)) {
return true;
}
bool has_animation = false;
auto animation_detection_cb = [&](bAction *owner_action, FCurve &fcurve) -> bool {
/* Early out if we already know that the target data is animated. */
@@ -472,6 +529,19 @@ class AnimDataConvertor {
return;
}
/* Ensure existing actions moved to a different ID type keep a 'valid' `idroot` value. Not
* essential, but 'nice to have'. */
if (GS(this->id_src.name) != GS(this->id_dst.name)) {
if (!this->animdata_dst) {
this->animdata_dst = BKE_animdata_ensure_id(&this->id_dst);
}
auto actions_idroot_ensure = [&](bAction &action) -> bool {
action.idroot = GS(this->id_dst.name);
return true;
};
this->animdata_action_foreach(*this->animdata_dst, actions_idroot_ensure);
}
if (&id_src == &id_dst) {
if (this->has_changes) {
DEG_id_tag_update(&this->id_src, ID_RECALC_ANIMATION);
@@ -479,6 +549,7 @@ class AnimDataConvertor {
}
return;
}
if (this->fcurves_from_src_main_action.is_empty() &&
this->fcurves_from_src_tmp_action.is_empty() && this->fcurves_from_src_drivers.is_empty())
{
@@ -1086,6 +1157,20 @@ static void legacy_gpencil_to_grease_pencil(ConversionData &conversion_data,
if (AnimData *gpd_animdata = BKE_animdata_from_id(&gpd.id)) {
grease_pencil.adt = BKE_animdata_copy_in_lib(
&conversion_data.bmain, gpd.id.lib, gpd_animdata, LIB_ID_COPY_DEFAULT);
/* Some property was renamed between legacy GP layers and new GreasePencil ones. */
AnimDataConvertor animdata_gpdata_transfer(
conversion_data, grease_pencil.id, gpd.id, {{".location", ".translation"}});
for (const Layer *layer_iter : grease_pencil.layers()) {
/* Data comes from versioned GPv2 layers, which have a fixed max length. */
char layer_name_esc[sizeof((bGPDlayer{}).info) * 2];
BLI_str_escape(layer_name_esc, layer_iter->name().c_str(), sizeof(layer_name_esc));
std::string layer_root_path = fmt::format("layers[\"{}\"]", layer_name_esc);
animdata_gpdata_transfer.root_path_dst = layer_root_path;
animdata_gpdata_transfer.root_path_src = layer_root_path;
animdata_gpdata_transfer.fcurves_convert();
}
animdata_gpdata_transfer.fcurves_convert_finalize();
}
}