Anim: implement 3D viewport keyframing functionality for layered actions #121661

Merged
Nathan Vegdahl merged 76 commits from nathanvegdahl/blender:baklava_3d_viewport_keyframing into main 2024-06-07 13:26:30 +02:00
7 changed files with 963 additions and 105 deletions

View File

@ -116,6 +116,12 @@ class Action : public ::bAction {
*/
bool layer_remove(Layer &layer_to_remove);
/**
* If the Action is empty, create a default layer with a single infinite
* keyframe strip.
*/
void layer_ensure_at_least_one();
nathanvegdahl marked this conversation as resolved Outdated

Given layer_add() and layer_remove(), a better name would be layer_ensure().

Given `layer_add()` and `layer_remove()`, a better name would be `layer_ensure()`.
/* Animation Binding access. */
blender::Span<const Binding *> bindings() const;
blender::MutableSpan<Binding *> bindings();
@ -217,6 +223,15 @@ class Action : public ::bAction {
*/
bool is_binding_animated(binding_handle_t binding_handle) const;
/**
* Get the layer that should be used for user-level keyframe insertion.
*
* \return The layer, or nullptr if no layer exists that can currently be used
* for keyframing (e.g. all layers are locked, once we've implemented
* locking).
*/
Layer *get_layer_for_keyframing();
protected:
/** Return the layer's index, or -1 if not found in this animation. */
int64_t find_layer_index(const Layer &layer) const;
@ -301,6 +316,7 @@ class Strip : public ::ActionStrip {
template<typename T> T &as();
template<typename T> const T &as() const;
bool is_infinite() const;
bool contains_frame(float frame_time) const;
bool is_last_frame(float frame_time) const;
@ -532,7 +548,8 @@ class KeyframeStrip : public ::KeyframeActionStrip {
StringRefNull rna_path,
int array_index,
float2 time_value,
const KeyframeSettings &settings);
const KeyframeSettings &settings,
eInsertKeyFlags insert_key_flags = INSERTKEY_NOFLAGS);
};
static_assert(sizeof(KeyframeStrip) == sizeof(::KeyframeActionStrip),
"DNA struct and its C++ wrapper must have the same size");

View File

@ -9,6 +9,8 @@
*/
#pragma once
#include "ANIM_keyframing.hh"
#include "BLI_math_vector_types.hh"
#include "BLI_string_ref.hh"

View File

@ -32,6 +32,14 @@ struct NlaKeyframingContext;
namespace blender::animrig {
/**
* Represents a single success/failure in the keyframing process.
*
* What is considered "single" depends on the level at which the failure
* happens. For example, it can be at the level of a single key on a single
* fcurve, all the way up to the level of an entire ID not being animatable.
* Both are considered "single" events.
*/
enum class SingleKeyingResult {
SUCCESS = 0,
/* TODO: remove `UNKNOWN_FAILURE` and replace all usages with proper, specific
@ -44,6 +52,9 @@ enum class SingleKeyingResult {
UNABLE_TO_INSERT_TO_NLA_STACK,
ID_NOT_EDITABLE,
ID_NOT_ANIMATABLE,
NO_VALID_LAYER,
NO_VALID_STRIP,
NO_VALID_BINDING,
CANNOT_RESOLVE_PATH,
/* Make sure to always keep this at the end of the enum. */
_KEYING_RESULT_MAX,
@ -62,7 +73,10 @@ class CombinedKeyingResult {
public:
CombinedKeyingResult();
void add(const SingleKeyingResult result);
/**
* Increase the count of the given `SingleKeyingResult` by `count`.
*/
void add(SingleKeyingResult result, int count = 1);
/* Add values of the given result to this result. */
void merge(const CombinedKeyingResult &combined_result);
nathanvegdahl marked this conversation as resolved Outdated

Can't this just be a single function with int count=1 as default value? Or am I mixing up my languages?

Can't this just be a single function with `int count=1` as default value? Or am I mixing up my languages?
@ -84,6 +98,17 @@ class CombinedKeyingResult {
const char *default_channel_group_for_path(const PointerRNA *animated_struct,
const StringRef prop_rna_path);
/* -------------------------------------------------------------------- */
/**
* Return whether key insertion functions are allowed to create new fcurves,
* according to the given flags.
*
* Specifically, both `INSERTKEY_REPLACE` and `INSERTKEY_AVAILABLE` prohibit the
* creation of new F-Curves.
*/
bool key_insertion_may_create_fcurve(eInsertKeyFlags insert_key_flags);
/* -------------------------------------------------------------------- */
/** \name Key-Framing Management
* \{ */

View File

@ -60,6 +60,8 @@ namespace {
constexpr const char *binding_default_name = "Binding";
constexpr const char *binding_unbound_prefix = "XX";
constexpr const char *layer_default_name = "Layer";
} // namespace
static animrig::Layer &ActionLayer_alloc()
@ -193,6 +195,16 @@ bool Action::layer_remove(Layer &layer_to_remove)
return true;
}
void Action::layer_ensure_at_least_one()
{
if (!this->layers().is_empty()) {
return;
}
Layer &layer = this->layer_add(DATA_(layer_default_name));
layer.strip_add(Strip::Type::Keyframe);
}
int64_t Action::find_layer_index(const Layer &layer) const
{
for (const int64_t layer_index : this->layers().index_range()) {
@ -405,6 +417,21 @@ bool Action::is_binding_animated(const binding_handle_t binding_handle) const
return !fcurves.is_empty();
}
Layer *Action::get_layer_for_keyframing()
{
/* TODO: handle multiple layers. */
if (this->layers().is_empty()) {
return nullptr;
}
if (this->layers().size() > 1) {
std::fprintf(stderr,
"Action '%s' has multiple layers, which isn't handled by keyframing code yet.",
this->id.name);
}
return this->layer(0);
}
bool Action::assign_id(Binding *binding, ID &animated_id)
{
AnimData *adt = BKE_animdata_ensure_id(&animated_id);
@ -689,6 +716,12 @@ Strip::~Strip()
BLI_assert_unreachable();
}
bool Strip::is_infinite() const
{
return this->frame_start == -std::numeric_limits<float>::infinity() &&
this->frame_end == std::numeric_limits<float>::infinity();
}
bool Strip::contains_frame(const float frame_time) const
{
return this->frame_start <= frame_time && frame_time <= this->frame_end;
@ -863,11 +896,25 @@ SingleKeyingResult KeyframeStrip::keyframe_insert(const Binding &binding,
const StringRefNull rna_path,
const int array_index,
const float2 time_value,
const KeyframeSettings &settings)
const KeyframeSettings &settings,
const eInsertKeyFlags insert_key_flags)
{
FCurve &fcurve = this->fcurve_find_or_create(binding, rna_path, array_index);
/* Get the fcurve, or create one if it doesn't exist and the keying flags
* allow. */
FCurve *fcurve = key_insertion_may_create_fcurve(insert_key_flags) ?
&this->fcurve_find_or_create(binding, rna_path, array_index) :
this->fcurve_find(binding, rna_path, array_index);
if (!fcurve) {
std::fprintf(stderr,
"FCurve %s[%d] for binding %s was not created due to either the Only Insert "
"Available setting or Replace keyframing mode.\n",
rna_path.c_str(),
array_index,
binding.name);
return SingleKeyingResult::CANNOT_CREATE_FCURVE;
}
if (!BKE_fcurve_is_keyframable(&fcurve)) {
if (!BKE_fcurve_is_keyframable(fcurve)) {
/* TODO: handle this properly, in a way that can be communicated to the user. */
std::fprintf(stderr,
"FCurve %s[%d] for binding %s doesn't allow inserting keys.\n",
@ -877,9 +924,8 @@ SingleKeyingResult KeyframeStrip::keyframe_insert(const Binding &binding,
return SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE;
}
/* TODO: Handle the eInsertKeyFlags. */
const SingleKeyingResult insert_vert_result = insert_vert_fcurve(
&fcurve, time_value, settings, eInsertKeyFlags(0));
fcurve, time_value, settings, insert_key_flags);
if (insert_vert_result != SingleKeyingResult::SUCCESS) {
std::fprintf(stderr,

View File

@ -52,9 +52,9 @@ CombinedKeyingResult::CombinedKeyingResult()
result_counter.fill(0);
}
void CombinedKeyingResult::add(const SingleKeyingResult result)
void CombinedKeyingResult::add(const SingleKeyingResult result, const int count)
{
result_counter[int(result)]++;
result_counter[int(result)] += count;
}
void CombinedKeyingResult::merge(const CombinedKeyingResult &other)
@ -94,7 +94,7 @@ void CombinedKeyingResult::generate_reports(ReportList *reports)
if (this->get_count(SingleKeyingResult::UNKNOWN_FAILURE) > 0) {
const int error_count = this->get_count(SingleKeyingResult::UNKNOWN_FAILURE);
errors.append(
fmt::format(RPT_("Could not insert {:d} key(s) for unknown reasons."), error_count));
fmt::format(RPT_("There were {:d} keying failures for unknown reasons."), error_count));
}
if (this->get_count(SingleKeyingResult::CANNOT_CREATE_FCURVE) > 0) {
@ -127,22 +127,43 @@ void CombinedKeyingResult::generate_reports(ReportList *reports)
if (this->get_count(SingleKeyingResult::ID_NOT_EDITABLE) > 0) {
const int error_count = this->get_count(SingleKeyingResult::ID_NOT_EDITABLE);
errors.append(fmt::format(
RPT_("Inserting keys on {:d} ID(s) has been skipped because they are not editable."),
error_count));
errors.append(fmt::format(RPT_("Inserting keys on {:d} data-block(s) has been skipped because "
"they are not editable."),
error_count));
}
if (this->get_count(SingleKeyingResult::ID_NOT_ANIMATABLE) > 0) {
const int error_count = this->get_count(SingleKeyingResult::ID_NOT_ANIMATABLE);
errors.append(fmt::format(
RPT_("Inserting keys on {:d} ID(s) has been skipped because they cannot be animated."),
error_count));
errors.append(fmt::format(RPT_("Inserting keys on {:d} data-block(s) has been skipped because "
"they cannot be animated."),
error_count));
}
if (this->get_count(SingleKeyingResult::CANNOT_RESOLVE_PATH) > 0) {
const int error_count = this->get_count(SingleKeyingResult::CANNOT_RESOLVE_PATH);
errors.append(fmt::format(RPT_("Inserting keys on {:d} ID(s) has been skipped because the RNA "
"path wasn't valid for them."),
errors.append(fmt::format(RPT_("Inserting keys on {:d} data-block(s) has been skipped because "
"the RNA path wasn't valid for them."),
error_count));
}
if (this->get_count(SingleKeyingResult::NO_VALID_LAYER) > 0) {
const int error_count = this->get_count(SingleKeyingResult::NO_VALID_LAYER);
errors.append(fmt::format(RPT_("Inserting keys on {:d} data-block(s) has been skipped because "
"there were no layers that could accept the keys."),
error_count));
}
if (this->get_count(SingleKeyingResult::NO_VALID_STRIP) > 0) {
nathanvegdahl marked this conversation as resolved Outdated

"ID" is not used in the interface, use "data-block".

"ID" is not used in the interface, use "data-block".
const int error_count = this->get_count(SingleKeyingResult::NO_VALID_STRIP);
errors.append(fmt::format(RPT_("Inserting keys on {:d} data-block(s) has been skipped because "
"there were no strips that could accept the keys."),
error_count));
}
if (this->get_count(SingleKeyingResult::NO_VALID_BINDING) > 0) {
const int error_count = this->get_count(SingleKeyingResult::NO_VALID_BINDING);
errors.append(fmt::format(RPT_("Inserting keys on {:d} data-block(s) has been skipped because "
"of missing animation bindings."),
error_count));
}
@ -237,6 +258,11 @@ eInsertKeyFlags get_keyframing_flags(Scene *scene)
return flag;
}
bool key_insertion_may_create_fcurve(const eInsertKeyFlags insert_key_flags)
{
return (insert_key_flags & (INSERTKEY_REPLACE | INSERTKEY_AVAILABLE)) == 0;
}
/** Used to make curves newly added to a cyclic Action cycle with the correct period. */
static void make_new_fcurve_cyclic(FCurve *fcu, const blender::float2 &action_range)
{
@ -540,8 +566,8 @@ static SingleKeyingResult insert_keyframe_fcurve_value(Main *bmain,
* - if we're replacing keyframes only, DO NOT create new F-Curves if they do not exist yet
* but still try to get the F-Curve if it exists...
*/
const bool can_create_curve = (flag & (INSERTKEY_REPLACE | INSERTKEY_AVAILABLE)) == 0;
FCurve *fcu = can_create_curve ?
FCurve *fcu = key_insertion_may_create_fcurve(flag) ?
action_fcurve_ensure(bmain, act, group, ptr, rna_path, array_index) :
action_fcurve_find(act, rna_path, array_index);
@ -934,19 +960,20 @@ int clear_keyframe(Main *bmain,
return key_count;
}
CombinedKeyingResult insert_key_action(Main *bmain,
bAction *action,
PointerRNA *ptr,
PropertyRNA *prop,
const std::string &rna_path,
const float frame,
const Span<float> values,
eInsertKeyFlags insert_key_flag,
eBezTriple_KeyframeType key_type,
const BitSpan keying_mask)
static CombinedKeyingResult insert_key_legacy_action(Main *bmain,
bAction *action,
PointerRNA *ptr,
PropertyRNA *prop,
const std::string &rna_path,
const float frame,
const Span<float> values,
eInsertKeyFlags insert_key_flag,
eBezTriple_KeyframeType key_type,
const BitSpan keying_mask)
{
BLI_assert(bmain != nullptr);
BLI_assert(action != nullptr);
BLI_assert(action->wrap().is_action_legacy());
const char *group = default_channel_group_for_path(ptr, rna_path);
@ -975,6 +1002,105 @@ CombinedKeyingResult insert_key_action(Main *bmain,
return combined_result;
}
struct KeyInsertData {
float2 position;
int array_index;
};
static SingleKeyingResult insert_key_layer(Layer &layer,
Binding &binding,
const std::string &rna_path,
const KeyInsertData &key_data,
const KeyframeSettings &key_settings,
const eInsertKeyFlags insert_key_flags)
{
/* TODO: we currently assume there will always be precisely one strip, which
* is infinite and has no time offset. This will not hold true in the future
* when we add support for multiple strips. */
BLI_assert(layer.strips().size() == 1);
Strip *strip = layer.strip(0);
nathanvegdahl marked this conversation as resolved Outdated

Either use position.x or position[0], but not both in the same bit of code. I'd prefer .x in both places.

Either use `position.x` or `position[0]`, but not both in the same bit of code. I'd prefer `.x` in both places.

Oops! Thanks for catching that.

After addressing another of your comments those lines disappeared completely, though.

Oops! Thanks for catching that. After addressing another of your comments those lines disappeared completely, though.
BLI_assert(strip->is_infinite());
BLI_assert(strip->frame_offset == 0.0);
return strip->as<KeyframeStrip>().keyframe_insert(
binding, rna_path, key_data.array_index, key_data.position, key_settings, insert_key_flags);
}
nathanvegdahl marked this conversation as resolved Outdated

Add a BLI_assert(strip->is_infinite()) (and the Strip::is_infinite() function ;-) )

Add a `BLI_assert(strip->is_infinite())` (and the `Strip::is_infinite()` function ;-) )

This code doesn't assume that the strip is infinite: if the strip doesn't overlap with the key insertion time, then it wouldn't be returned by strip_at_time() in the first place.

However, it probably does make sense to assert that invariant, so I've added an assertion for that using the existing Strip::contains_frame() method.

The code also currently assumes that strip time aligns with scene time (offset = 0), so I've added an assertion for that as well.

This code doesn't assume that the strip is infinite: if the strip doesn't overlap with the key insertion time, then it wouldn't be returned by `strip_at_time()` in the first place. However, it probably does make sense to assert *that* invariant, so I've added an assertion for that using the existing `Strip::contains_frame()` method. The code also currently assumes that strip time aligns with scene time (offset = 0), so I've added an assertion for that as well.

Hmmm not too thrilled to have three functions in the Layer class now, all to support a feature that's currently not implemented.
I'd rather have all the relevant "assume 1-layer/1-strip" code in insert_key_layer().

Hmmm not too thrilled to have three functions in the `Layer` class now, all to support a feature that's currently not implemented. I'd rather have all the relevant "assume 1-layer/1-strip" code in `insert_key_layer()`.

Yeah, I think you're right. I'm trying to predict the future too much here. I'll collapse it down, and then we can figure out the right APIs once we're actually implementing the layered animation functionality.

Yeah, I think you're right. I'm trying to predict the future too much here. I'll collapse it down, and then we can figure out the right APIs once we're actually implementing the layered animation functionality.
static CombinedKeyingResult insert_key_layered_action(Action &action,
const int32_t binding_handle,
PointerRNA *rna_pointer,
const blender::Span<RNAPath> rna_paths,
const float scene_frame,
const KeyframeSettings &key_settings,
const eInsertKeyFlags insert_key_flags)
{
BLI_assert(action.is_action_layered());
ID *id = rna_pointer->owner_id;
CombinedKeyingResult combined_result;
Binding *binding = action.binding_for_handle(binding_handle);
if (binding == nullptr) {
binding = &action.binding_add_for_id(*id);
const bool success = action.assign_id(binding, *id);
nathanvegdahl marked this conversation as resolved

This creates a binding named 'Binding'. It would be better if the new binding would get the name from id->adt->binding_name, or id->name if not yet set.

This creates a binding named 'Binding'. It would be better if the new binding would get the name from `id->adt->binding_name`, or `id->name` if not yet set.

Check Action::binding_add_for_id()

Check `Action::binding_add_for_id()`
UNUSED_VARS_NDEBUG(success);
nathanvegdahl marked this conversation as resolved Outdated

Despite the comment, I'm not sure it makes sense to leave this null check in. I think there's a lot about the future structure of the code that we just don't know, and won't know until we actually start implementing the layered functionality. So this feels premature / trying to predict the future.

Thoughts?

Despite the comment, I'm not sure it makes sense to leave this null check in. I think there's a lot about the future structure of the code that we just don't know, and won't know until we actually start implementing the layered functionality. So this feels premature / trying to predict the future. Thoughts?

I agree, it's probably better to change this to an assert + comment.

I agree, it's probably better to change this to an assert + comment.
BLI_assert_msg(
success,
nathanvegdahl marked this conversation as resolved Outdated

Add UNUSED_VARS_NDEBUG(success);

Add `UNUSED_VARS_NDEBUG(success);`

Aside: that should probably be part of the definition of BLI_assert() and friends.

Aside: that should probably be part of the definition of `BLI_assert()` and friends.
"With a new Binding, the only reason this could fail is that the ID itself cannot be "
nathanvegdahl marked this conversation as resolved Outdated

I think it might be better to change the meaning of SingleKeyingResult a bit.
It was meant to represent errors that occur at the FCurve level, but NO_VALID_LAYER is above that so we can't give a meaningful count of FCurves that failed.

I we re-interpret SingleKeyingResult as "One thing has failed during keying", NO_VALID_LAYER could create an error message like "Could not add keyframes into n Layer(s) because ... " Which is probably more meaningful than counting the FCurves.

I think it might be better to change the meaning of `SingleKeyingResult` a bit. It was meant to represent errors that occur at the FCurve level, but `NO_VALID_LAYER` is above that so we can't give a meaningful count of FCurves that failed. I we re-interpret `SingleKeyingResult` as "One thing has failed during keying", `NO_VALID_LAYER` could create an error message like "Could not add keyframes into n Layer(s) because ... " Which is probably more meaningful than counting the FCurves.

Yeah, I think that's a great idea. Thanks!

Yeah, I think that's a great idea. Thanks!
"animated, which should have been caught and handled by higher-level functions.");
}
/* Ensure that at least one layer exists. If not, create the default layer
* with the default infinite keyframe strip. */
action.layer_ensure_at_least_one();
nathanvegdahl marked this conversation as resolved Outdated

This assumes that "ensure layer" creates one that will be returned by the "get layer" function, creating tightly coupled code. Why not have an action.ensure_layer_for_keyframing() that returns the appropriate layer?

This assumes that "ensure layer" creates one that will be returned by the "get layer" function, creating tightly coupled code. Why not have an `action.ensure_layer_for_keyframing()` that returns the appropriate layer?

That would be semantically different than the current code (at least if ensure_layer_for_keyframing() would do what its name sounds like to me).

Specifically, ensure_layer_for_keyframing() sounds to me like it ensures there is a layer that can accept the keys, creating one if no such layer exists. Whereas the current code creates a layer only if there are no layers at all, which I think(?) is what we want. For example, if there are already layers but they're all locked, the current code would fail to find a layer and subsequently report a keying error to the user, whereas code based on an ensure_layer_for_keyframing() function would automatically create another layer and insert the keys there.

To me the current code is actually decoupled, because the concern of ensuring at least one layer and the concern of trying to find a layer appropriate for keying are separate.

I think the issue is just that there should be some white space between the two statements and a better name for the ensure method (maybe layer_ensure_at_least_one()) to better establish that these two statements aren't that closely related.

That would be semantically different than the current code (at least if `ensure_layer_for_keyframing()` would do what its name sounds like to me). Specifically, `ensure_layer_for_keyframing()` sounds to me like it ensures there is a layer that can accept the keys, creating one if no such layer exists. Whereas the current code creates a layer only if there are no layers at all, which I think(?) is what we want. For example, if there are already layers but they're all locked, the current code would fail to find a layer and subsequently report a keying error to the user, whereas code based on an `ensure_layer_for_keyframing()` function would automatically create another layer and insert the keys there. To me the current code is actually *decoupled*, because the concern of ensuring at least one layer and the concern of trying to find a layer appropriate for keying are separate. I think the issue is just that there should be some white space between the two statements and a better name for the ensure method (maybe `layer_ensure_at_least_one()`) to better establish that these two statements aren't *that* closely related.

Also, I think it would be good if I changed get_layer_for_keyframing() to return a std::optional<Layer &> to make it explicit that it may not succeed, and harder to forget to handle that in the code that calls it.

Also, I think it would be good if I changed `get_layer_for_keyframing()` to return a `std::optional<Layer &>` to make it explicit that it may not succeed, and harder to forget to handle that in the code that calls it.
Damn it, you [can't (conveniently) have `std::optional`s of references](https://stackoverflow.com/questions/26858034/why-does-stdoptional-not-have-a-specialization-for-reference-types). Thanks C++.

Okay, I made the other changes at least, and also attempted to make the docs of get_layer_for_keyframing() a little clearer.

Okay, I made the other changes at least, and also attempted to make the docs of `get_layer_for_keyframing()` a little clearer.

Creating a new layer when there is none sounds like a good idea.

When there is already a layer, but that cannot be used for keyframing (let's say there's an infinite simulation strip on there), I'm not convinced that Blender shouldn't just create a new layer for those keys.

This is a discussion for when we actually support layered animation, so let's keep the code as-is for now.

Creating a new layer when there is none sounds like a good idea. When there is already a layer, but that cannot be used for keyframing (let's say there's an infinite simulation strip on there), I'm not convinced that Blender shouldn't just create a new layer for those keys. This is a discussion for when we actually support layered animation, so let's keep the code as-is for now.
/* TODO: we currently assume this will always successfully find a layer.
* However, that may not be true in the future when we implement features like
* layer locking: if layers already exist, but they are all locked, then the
* default layer won't be added by the line above, but there also won't be any
* layers we can insert keys into. */
Layer *layer = action.get_layer_for_keyframing();
BLI_assert(layer != nullptr);
const bool use_visual_keyframing = insert_key_flags & INSERTKEY_MATRIX;
for (const RNAPath &rna_path : rna_paths) {
PointerRNA ptr;
PropertyRNA *prop = nullptr;
const bool path_resolved = RNA_path_resolve_property(
rna_pointer, rna_path.path.c_str(), &ptr, &prop);
if (!path_resolved) {
std::fprintf(stderr,
"Failed to insert key on binding %s due to unresolved RNA path: %s\n",
binding->name,
rna_path.path.c_str());
combined_result.add(SingleKeyingResult::CANNOT_RESOLVE_PATH);
continue;
}
const std::optional<std::string> rna_path_id_to_prop = RNA_path_from_ID_to_property(&ptr,
prop);
BLI_assert(rna_path_id_to_prop.has_value());
nathanvegdahl marked this conversation as resolved Outdated

const int -- the property_index doesn't change within the for-body.

`const int` -- the `property_index` doesn't change within the `for`-body.
Vector<float> rna_values = get_keyframe_values(&ptr, prop, use_visual_keyframing);
for (const int property_index : rna_values.index_range()) {
/* If we're only keying one array element, skip all elements other than
* that one. */
if (rna_path.index.has_value() && *rna_path.index != property_index) {
continue;
}
const KeyInsertData key_data = {{scene_frame, rna_values[property_index]}, property_index};
const SingleKeyingResult result = insert_key_layer(
*layer, *binding, *rna_path_id_to_prop, key_data, key_settings, insert_key_flags);
combined_result.add(result);
}
}
DEG_id_tag_update(&action.id, ID_RECALC_ANIMATION_NO_FLUSH);
return combined_result;
}
CombinedKeyingResult insert_key_rna(PointerRNA *rna_pointer,
const blender::Span<RNAPath> rna_paths,
const float scene_frame,
@ -984,14 +1110,32 @@ CombinedKeyingResult insert_key_rna(PointerRNA *rna_pointer,
const AnimationEvalContext &anim_eval_context)
{
ID *id = rna_pointer->owner_id;
bAction *action = id_action_ensure(bmain, id);
CombinedKeyingResult combined_result;
if (action == nullptr) {
/* Init animdata if none available yet. */
AnimData *adt = BKE_animdata_ensure_id(id);
if (adt == nullptr) {
combined_result.add(SingleKeyingResult::ID_NOT_ANIMATABLE);
return combined_result;
}
AnimData *adt = BKE_animdata_from_id(id);
bAction *action = id_action_ensure(bmain, id);
BLI_assert(action != nullptr);
if (USER_EXPERIMENTAL_TEST(&U, use_animation_baklava) && action->wrap().is_action_layered()) {
/* TODO: Don't hardcode key settings. */
KeyframeSettings key_settings;
key_settings.keyframe_type = key_type;
key_settings.handle = HD_AUTO_ANIM;
key_settings.interpolation = BEZT_IPO_BEZ;
return insert_key_layered_action(action->wrap(),
adt->binding_handle,
rna_pointer,
rna_paths,
scene_frame,
key_settings,
insert_key_flags);
}
/* Keyframing functions can deal with the nla_context being a nullptr. */
ListBase nla_cache = {nullptr, nullptr};
@ -1012,6 +1156,7 @@ CombinedKeyingResult insert_key_rna(PointerRNA *rna_pointer,
const bool path_resolved = RNA_path_resolve_property(
rna_pointer, rna_path.path.c_str(), &ptr, &prop);
if (!path_resolved) {
combined_result.add(SingleKeyingResult::CANNOT_RESOLVE_PATH);
continue;
}
const std::optional<std::string> rna_path_id_to_prop = RNA_path_from_ID_to_property(&ptr,
@ -1027,16 +1172,16 @@ CombinedKeyingResult insert_key_rna(PointerRNA *rna_pointer,
&anim_eval_context,
nullptr,
successful_remaps);
const CombinedKeyingResult result = insert_key_action(bmain,
action,
rna_pointer,
prop,
rna_path_id_to_prop->c_str(),
nla_frame,
rna_values.as_span(),
insert_key_flags,
key_type,
successful_remaps);
const CombinedKeyingResult result = insert_key_legacy_action(bmain,
action,
rna_pointer,
prop,
rna_path_id_to_prop->c_str(),
nla_frame,
rna_values.as_span(),
insert_key_flags,
key_type,
successful_remaps);
combined_result.merge(result);
}
BKE_animsys_free_nla_keyframing_context_cache(&nla_cache);

View File

@ -61,11 +61,20 @@ class KeyframingTest : public testing::Test {
static void TearDownTestSuite()
{
/* Ensure experimental baklava flag is turned off after all tests are run. */
U.flag &= ~USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 0;
CLG_exit();
}
void SetUp() override
{
/* Ensure experimental baklava flag is turned off first (to be enabled
* selectively in the layered action tests. */
U.flag &= ~USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 0;
bmain = BKE_main_new();
object = BKE_object_add_only_object(bmain, OB_EMPTY, "OBEmpty");
@ -110,19 +119,633 @@ class KeyframingTest : public testing::Test {
};
/* ------------------------------------------------------------
* Tests for `insert_key_rna()`.
* Tests for `insert_key_rna()` with layered actions.
*/
/* Keying a non-array property. */
TEST_F(KeyframingTest, insert_key_rna__non_array_property)
TEST_F(KeyframingTest, insert_key_rna__layered_action__non_array_property)
{
/* Turn on Baklava experimental flag. */
U.flag |= USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 1;
AnimationEvalContext anim_eval_context = {nullptr, 1.0};
/* First time should create:
* - AnimData
* - Action
* - Binding
* - Layer
* - Infinite KeyframeStrip
* - FCurve with a single key
*/
object->empty_drawsize = 42.0;
nathanvegdahl marked this conversation as resolved Outdated

Use different values for the frame number and the property value. Right now these two could be swapped, and the test wouldn't fail.

For tests I always try and avoid the trivial numbers, and go for, say, frame=47 and value=0.327 (just to have some references to my two favourite agents in there).

Also empty_drawsize = 1.0 is actually setting it to its default value. Just for test resilience, I'd use something different for that reason as well.

Same for the tests in source/blender/animrig/intern/keyframing_test.cc

Use different values for the frame number and the property value. Right now these two could be swapped, and the test wouldn't fail. For tests I always try and avoid the trivial numbers, and go for, say, frame=47 and value=0.327 (just to have some references to my two favourite agents in there). Also `empty_drawsize = 1.0` is actually setting it to its default value. Just for test resilience, I'd use something different for that reason as well. Same for the tests in `source/blender/animrig/intern/keyframing_test.cc`
const CombinedKeyingResult result_1 = insert_key_rna(&object_rna_pointer,
{{"empty_display_size"}},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(1, result_1.get_count(SingleKeyingResult::SUCCESS));
ASSERT_NE(nullptr, object->adt);
ASSERT_NE(nullptr, object->adt->action);
Action &action = object->adt->action->wrap();
/* The action has a binding, it's named properly, and it's correctly assigned
* to the object. */
ASSERT_EQ(1, action.bindings().size());
Binding *binding = action.binding(0);
nathanvegdahl marked this conversation as resolved

This should also check the binding name, as this is super important for re-assigning to other actions with automatic selection of the binding in that new action.

Also it could check that object->adt->binding_name is set correctly to the same name. This is not directly the responsibility of the function under test (the action+binding assignment code is responsible for that), but given that we have so few tests in Blender, I think it's nice to not only check "is assigned" but check "is assigned correctly".

This should also check the binding name, as this is super important for re-assigning to other actions with automatic selection of the binding in that new action. Also it could check that `object->adt->binding_name` is set correctly to the same name. This is not directly the responsibility of the function under test (the action+binding assignment code is responsible for that), but given that we have so few tests in Blender, I think it's nice to not only check "is assigned" but check "is assigned correctly".
EXPECT_STREQ(object->id.name, binding->name);
nathanvegdahl marked this conversation as resolved Outdated

Use EXPECT_STREQ(), or wrap char* in std::string or StringRef and use EXPECT_EQ(A, B). That way, when there is a test failure, the actual+expected names are shown, rather than "0 is not 1".

Use `EXPECT_STREQ()`, or wrap `char*` in `std::string` or `StringRef` and use `EXPECT_EQ(A, B)`. That way, when there is a test failure, the actual+expected names are shown, rather than "0 is not 1".
EXPECT_STREQ(object->adt->binding_name, binding->name);
EXPECT_EQ(object->adt->binding_handle, binding->handle);
nathanvegdahl marked this conversation as resolved

This should check the layer name. The exact name isn't that important here, but at least it shouldn't be empty.

This should check the layer name. The exact name isn't that important here, but at least it shouldn't be empty.
/* We have the default layer and strip. */
ASSERT_TRUE(action.is_action_layered());
ASSERT_EQ(1, action.layers().size());
ASSERT_EQ(1, action.layer(0)->strips().size());
EXPECT_TRUE(strlen(action.layer(0)->name) > 0);
Strip *strip = action.layer(0)->strip(0);
ASSERT_TRUE(strip->is_infinite());
ASSERT_EQ(Strip::Type::Keyframe, strip->type());
KeyframeStrip *keyframe_strip = &strip->as<KeyframeStrip>();
/* We have a channel bag for the binding. */
ChannelBag *channel_bag = keyframe_strip->channelbag_for_binding(*binding);
ASSERT_NE(nullptr, channel_bag);
nathanvegdahl marked this conversation as resolved Outdated

As discussed face-to-face, better to use empty_display_size as the property to animate, as that's actually a float property.

As discussed face-to-face, better to use `empty_display_size` as the property to animate, as that's actually a `float` property.
/* The fcurves in the channel bag are what we expect. */
EXPECT_EQ(1, channel_bag->fcurves().size());
const FCurve *fcurve = channel_bag->fcurve_find("empty_display_size", 0);
ASSERT_NE(nullptr, fcurve);
ASSERT_NE(nullptr, fcurve->bezt);
EXPECT_EQ(1, fcurve->totvert);
EXPECT_EQ(1.0, fcurve->bezt[0].vec[1][0]);
EXPECT_EQ(42.0, fcurve->bezt[0].vec[1][1]);
/* Second time inserting with a different value on the same frame should
* simply replace the key. */
object->empty_drawsize = 86.0;
const CombinedKeyingResult result_2 = insert_key_rna(&object_rna_pointer,
{{"empty_display_size"}},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(1, result_2.get_count(SingleKeyingResult::SUCCESS));
EXPECT_EQ(1, fcurve->totvert);
EXPECT_EQ(1.0, fcurve->bezt[0].vec[1][0]);
EXPECT_EQ(86.0, fcurve->bezt[0].vec[1][1]);
/* Third time inserting on a different time should add a second key. */
object->empty_drawsize = 7.0;
const CombinedKeyingResult result_3 = insert_key_rna(&object_rna_pointer,
{{"empty_display_size"}},
10.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(1, result_3.get_count(SingleKeyingResult::SUCCESS));
EXPECT_EQ(2, fcurve->totvert);
EXPECT_EQ(1.0, fcurve->bezt[0].vec[1][0]);
EXPECT_EQ(86.0, fcurve->bezt[0].vec[1][1]);
EXPECT_EQ(10.0, fcurve->bezt[1].vec[1][0]);
EXPECT_EQ(7.0, fcurve->bezt[1].vec[1][1]);
}
/* Keying a single element of an array property. */
TEST_F(KeyframingTest, insert_key_rna__layered_action__single_element)
{
/* Turn on Baklava experimental flag. */
U.flag |= USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 1;
AnimationEvalContext anim_eval_context = {nullptr, 1.0};
const CombinedKeyingResult result = insert_key_rna(&object_rna_pointer,
{{"rotation_euler", std::nullopt, 0}},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(1, result.get_count(SingleKeyingResult::SUCCESS));
ASSERT_NE(nullptr, object->adt);
ASSERT_NE(nullptr, object->adt->action);
Action &action = object->adt->action->wrap();
ASSERT_EQ(1, action.bindings().size());
ASSERT_EQ(1, action.layers().size());
ASSERT_EQ(1, action.layer(0)->strips().size());
KeyframeStrip *strip = &action.layer(0)->strip(0)->as<KeyframeStrip>();
ASSERT_EQ(1, strip->channelbags().size());
ChannelBag *channel_bag = strip->channelbag(0);
EXPECT_EQ(1, channel_bag->fcurves().size());
EXPECT_NE(nullptr, channel_bag->fcurve_find("rotation_euler", 0));
}
/* Keying all elements of an array property. */
TEST_F(KeyframingTest, insert_key_rna__layered_action__all_elements)
{
/* Turn on Baklava experimental flag. */
U.flag |= USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 1;
AnimationEvalContext anim_eval_context = {nullptr, 1.0};
const CombinedKeyingResult result = insert_key_rna(&object_rna_pointer,
{{"rotation_euler"}},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(3, result.get_count(SingleKeyingResult::SUCCESS));
ASSERT_NE(nullptr, object->adt);
ASSERT_NE(nullptr, object->adt->action);
Action &action = object->adt->action->wrap();
ASSERT_EQ(1, action.bindings().size());
ASSERT_EQ(1, action.layers().size());
ASSERT_EQ(1, action.layer(0)->strips().size());
KeyframeStrip *strip = &action.layer(0)->strip(0)->as<KeyframeStrip>();
ASSERT_EQ(1, strip->channelbags().size());
ChannelBag *channel_bag = strip->channelbag(0);
EXPECT_EQ(3, channel_bag->fcurves().size());
EXPECT_NE(nullptr, channel_bag->fcurve_find("rotation_euler", 0));
EXPECT_NE(nullptr, channel_bag->fcurve_find("rotation_euler", 1));
EXPECT_NE(nullptr, channel_bag->fcurve_find("rotation_euler", 2));
}
/* Keying a pose bone from its own RNA pointer. */
TEST_F(KeyframingTest, insert_key_rna__layered_action__pose_bone_rna_pointer)
{
/* Turn on Baklava experimental flag. */
U.flag |= USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 1;
AnimationEvalContext anim_eval_context = {nullptr, 1.0};
bPoseChannel *pchan = BKE_pose_channel_find_name(armature_object->pose, "Bone");
PointerRNA pose_bone_rna_pointer = RNA_pointer_create(
&armature_object->id, &RNA_PoseBone, pchan);
const CombinedKeyingResult result = insert_key_rna(&pose_bone_rna_pointer,
{{"rotation_euler", std::nullopt, 0}},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(1, result.get_count(SingleKeyingResult::SUCCESS));
ASSERT_NE(nullptr, armature_object->adt);
ASSERT_NE(nullptr, armature_object->adt->action);
Action &action = armature_object->adt->action->wrap();
ASSERT_EQ(1, action.bindings().size());
ASSERT_EQ(1, action.layers().size());
ASSERT_EQ(1, action.layer(0)->strips().size());
KeyframeStrip *strip = &action.layer(0)->strip(0)->as<KeyframeStrip>();
ASSERT_EQ(1, strip->channelbags().size());
ChannelBag *channel_bag = strip->channelbag(0);
EXPECT_EQ(1, channel_bag->fcurves().size());
EXPECT_NE(nullptr, channel_bag->fcurve_find("pose.bones[\"Bone\"].rotation_euler", 0));
}
/* Keying a pose bone from its owning ID's RNA pointer. */
TEST_F(KeyframingTest, insert_key_rna__layered_action__pose_bone_owner_id_pointer)
{
/* Turn on Baklava experimental flag. */
U.flag |= USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 1;
AnimationEvalContext anim_eval_context = {nullptr, 1.0};
const CombinedKeyingResult result = insert_key_rna(
&armature_object_rna_pointer,
{{"pose.bones[\"Bone\"].rotation_euler", std::nullopt, 0}},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(1, result.get_count(SingleKeyingResult::SUCCESS));
ASSERT_NE(nullptr, armature_object->adt);
ASSERT_NE(nullptr, armature_object->adt->action);
Action &action = armature_object->adt->action->wrap();
ASSERT_EQ(1, action.bindings().size());
ASSERT_EQ(1, action.layers().size());
ASSERT_EQ(1, action.layer(0)->strips().size());
KeyframeStrip *strip = &action.layer(0)->strip(0)->as<KeyframeStrip>();
ASSERT_EQ(1, strip->channelbags().size());
ChannelBag *channel_bag = strip->channelbag(0);
EXPECT_EQ(1, channel_bag->fcurves().size());
EXPECT_NE(nullptr, channel_bag->fcurve_find("pose.bones[\"Bone\"].rotation_euler", 0));
}
/* Keying multiple elements of multiple properties at once. */
TEST_F(KeyframingTest, insert_key_rna__layered_action__multiple_properties)
{
/* Turn on Baklava experimental flag. */
U.flag |= USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 1;
AnimationEvalContext anim_eval_context = {nullptr, 1.0};
const CombinedKeyingResult result = insert_key_rna(&object_rna_pointer,
{
{"empty_display_size"},
{"location"},
{"rotation_euler", std::nullopt, 0},
{"rotation_euler", std::nullopt, 2},
},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(6, result.get_count(SingleKeyingResult::SUCCESS));
ASSERT_NE(nullptr, object->adt);
ASSERT_NE(nullptr, object->adt->action);
Action &action = object->adt->action->wrap();
ASSERT_EQ(1, action.bindings().size());
ASSERT_EQ(1, action.layers().size());
ASSERT_EQ(1, action.layer(0)->strips().size());
KeyframeStrip *strip = &action.layer(0)->strip(0)->as<KeyframeStrip>();
ASSERT_EQ(1, strip->channelbags().size());
ChannelBag *channel_bag = strip->channelbag(0);
EXPECT_EQ(6, channel_bag->fcurves().size());
EXPECT_NE(nullptr, channel_bag->fcurve_find("empty_display_size", 0));
EXPECT_NE(nullptr, channel_bag->fcurve_find("location", 0));
EXPECT_NE(nullptr, channel_bag->fcurve_find("location", 1));
EXPECT_NE(nullptr, channel_bag->fcurve_find("location", 2));
EXPECT_NE(nullptr, channel_bag->fcurve_find("rotation_euler", 0));
EXPECT_NE(nullptr, channel_bag->fcurve_find("rotation_euler", 2));
}
/* Keying more than one ID on the same action. */
TEST_F(KeyframingTest, insert_key_rna__layered_action__multiple_ids)
{
/* Turn on Baklava experimental flag. */
U.flag |= USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 1;
AnimationEvalContext anim_eval_context = {nullptr, 1.0};
/* First object should crate the action and get a binding and channel bag. */
const CombinedKeyingResult result_1 = insert_key_rna(&object_rna_pointer,
{{"empty_display_size"}},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(1, result_1.get_count(SingleKeyingResult::SUCCESS));
ASSERT_NE(nullptr, object->adt);
ASSERT_NE(nullptr, object->adt->action);
Action &action = object->adt->action->wrap();
nathanvegdahl marked this conversation as resolved

This should check the binding name, which should be the same as the object name.

This should check the binding name, which should be the same as the object name.
/* The action has a binding and it's assigned to the first object. */
ASSERT_EQ(1, action.bindings().size());
Binding *binding_1 = action.binding_for_handle(object->adt->binding_handle);
ASSERT_NE(nullptr, binding_1);
EXPECT_STREQ(object->id.name, binding_1->name);
EXPECT_STREQ(object->adt->binding_name, binding_1->name);
/* Get the keyframe strip. */
ASSERT_TRUE(action.is_action_layered());
ASSERT_EQ(1, action.layers().size());
ASSERT_EQ(1, action.layer(0)->strips().size());
KeyframeStrip *strip = &action.layer(0)->strip(0)->as<KeyframeStrip>();
/* We have a single channel bag, and it's for the first object's binding. */
nathanvegdahl marked this conversation as resolved Outdated

This should use action->assign_id(nullptr, armature_object->id), to explicitly assign-but-not-bind the Action.

This should use `action->assign_id(nullptr, armature_object->id)`, to explicitly assign-but-not-bind the Action.
ASSERT_EQ(1, strip->channelbags().size());
ChannelBag *channel_bag_1 = strip->channelbag_for_binding(*binding_1);
ASSERT_NE(nullptr, channel_bag_1);
/* Assign the action to the second object, with no binding. */
action.assign_id(nullptr, armature_object->id);
/* Keying the second object should go into the same action, creating a new
* binding and channel bag. */
const CombinedKeyingResult result_2 = insert_key_rna(&armature_object_rna_pointer,
{{"empty_display_size"}},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
nathanvegdahl marked this conversation as resolved

This should check the binding name against the object name.

This should check the binding name against the object name.
anim_eval_context);
EXPECT_EQ(1, result_2.get_count(SingleKeyingResult::SUCCESS));
ASSERT_EQ(2, action.bindings().size());
Binding *binding_2 = action.binding_for_handle(armature_object->adt->binding_handle);
ASSERT_NE(nullptr, binding_2);
EXPECT_STREQ(armature_object->id.name, binding_2->name);
EXPECT_STREQ(armature_object->adt->binding_name, binding_2->name);
ASSERT_EQ(2, strip->channelbags().size());
ChannelBag *channel_bag_2 = strip->channelbag_for_binding(*binding_2);
ASSERT_NE(nullptr, channel_bag_2);
}
/* Keying an object with an already-existing legacy action should do legacy
* keying. */
TEST_F(KeyframingTest, insert_key_rna__baklava_legacy_action)
{
AnimationEvalContext anim_eval_context = {nullptr, 1.0};
/* Insert a key with the experimental flag off to create a legacy action. */
const CombinedKeyingResult result_1 = insert_key_rna(&object_rna_pointer,
{{"empty_display_size"}},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(1, result_1.get_count(SingleKeyingResult::SUCCESS));
bAction *action = object->adt->action;
EXPECT_TRUE(action->wrap().is_action_legacy());
EXPECT_FALSE(action->wrap().is_action_layered());
EXPECT_EQ(1, BLI_listbase_count(&action->curves));
/* Turn on Baklava experimental flag. */
U.flag |= USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 1;
/* Insert more keys, which should also get inserted as part of the same legacy
* action, not a layered action. */
const CombinedKeyingResult result_2 = insert_key_rna(&object_rna_pointer,
{{"location"}},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(3, result_2.get_count(SingleKeyingResult::SUCCESS));
EXPECT_EQ(action, object->adt->action);
EXPECT_TRUE(action->wrap().is_action_legacy());
EXPECT_FALSE(action->wrap().is_action_layered());
EXPECT_EQ(4, BLI_listbase_count(&action->curves));
}
/* Keying with the "Only Insert Available" flag. */
TEST_F(KeyframingTest, insert_key_rna__layered_action__only_available)
{
/* Turn on Baklava experimental flag. */
U.flag |= USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 1;
AnimationEvalContext anim_eval_context = {nullptr, 1.0};
/* First attempt should fail, because there are no fcurves yet. */
const CombinedKeyingResult result_1 = insert_key_rna(&object_rna_pointer,
{{"rotation_euler"}},
1.0,
INSERTKEY_AVAILABLE,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(0, result_1.get_count(SingleKeyingResult::SUCCESS));
/* It's unclear why AnimData and an Action should be created if keying fails
* here. It may even be undesireable. These checks are just here to ensure no
* *unintentional* changes in behavior. */
ASSERT_NE(nullptr, object->adt);
ASSERT_NE(nullptr, object->adt->action);
/* If an action is created at all, it should be the default action with one
* layer and an infinite keyframe strip. */
Action &action = object->adt->action->wrap();
ASSERT_EQ(1, action.bindings().size());
ASSERT_EQ(1, action.layers().size());
ASSERT_EQ(1, action.layer(0)->strips().size());
EXPECT_EQ(object->adt->binding_handle, action.binding(0)->handle);
KeyframeStrip *strip = &action.layer(0)->strip(0)->as<KeyframeStrip>();
ASSERT_EQ(0, strip->channelbags().size());
/* Insert a key on two of the elements without using the flag so that there
* will be two fcurves. */
const CombinedKeyingResult result_2 = insert_key_rna(&object_rna_pointer,
{
{"rotation_euler", std::nullopt, 0},
{"rotation_euler", std::nullopt, 2},
},
1.0,
INSERTKEY_NOFLAGS,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(2, result_2.get_count(SingleKeyingResult::SUCCESS));
ASSERT_EQ(1, strip->channelbags().size());
ChannelBag *channel_bag = strip->channelbag(0);
/* Second attempt should succeed with two keys, because two of the elements
* now have fcurves. */
const CombinedKeyingResult result_3 = insert_key_rna(&object_rna_pointer,
{{"rotation_euler"}},
1.0,
INSERTKEY_AVAILABLE,
BEZT_KEYTYPE_KEYFRAME,
bmain,
anim_eval_context);
EXPECT_EQ(2, result_3.get_count(SingleKeyingResult::SUCCESS));
EXPECT_EQ(2, channel_bag->fcurves().size());
EXPECT_NE(nullptr, channel_bag->fcurve_find("rotation_euler", 0));
EXPECT_NE(nullptr, channel_bag->fcurve_find("rotation_euler", 2));
}
/* Keying with the "Only Replace" flag. */
TEST_F(KeyframingTest, insert_key_rna__layered_action__only_replace)
{
/* Turn on Baklava experimental flag. */
U.flag |= USER_DEVELOPER_UI;
U.experimental.use_animation_baklava = 1;
AnimationEvalContext anim_eval_context = {nullptr, 1.0};
/* First attempt should fail, because there are no fcurves yet. */
object->rot[0] = 42.0;
object->rot[1] = 42.0