Anim: add failure propagation to more lower-level keying functions #121517

Merged
Nathan Vegdahl merged 1 commits from nathanvegdahl/blender:keyframing_failure_propagation into main 2024-05-07 15:07:08 +02:00
10 changed files with 137 additions and 98 deletions

View File

@ -460,11 +460,11 @@ class KeyframeStrip : public ::KeyframeAnimationStrip {
*/
FCurve &fcurve_find_or_create(const Binding &binding, StringRefNull rna_path, int array_index);
FCurve *keyframe_insert(const Binding &binding,
StringRefNull rna_path,
int array_index,
float2 time_value,
const KeyframeSettings &settings);
SingleKeyingResult keyframe_insert(const Binding &binding,
StringRefNull rna_path,
int array_index,
float2 time_value,
const KeyframeSettings &settings);
};
static_assert(sizeof(KeyframeStrip) == sizeof(::KeyframeAnimationStrip),
"DNA struct and its C++ wrapper must have the same size");

View File

@ -14,6 +14,8 @@
#include "DNA_anim_types.h"
#include "ANIM_keyframing.hh"
struct AnimData;
struct FCurve;
@ -72,22 +74,20 @@ int insert_bezt_fcurve(FCurve *fcu, const BezTriple *bezt, eInsertKeyFlags flag)
*
* Use this when validation of necessary animation data isn't necessary as it
* already exists. It will insert a keyframe using the current value being keyframed.
* Returns the index at which a keyframe was added (or -1 if failed).
*
* This function is a wrapper for #insert_bezt_fcurve(), and should be used when
* adding a new keyframe to a curve, when the keyframe doesn't exist anywhere else yet.
* It returns the index at which the keyframe was added.
*
* \returns The index of the keyframe array into which the bezt has been added.
* \returns Either success or an indicator of why keying failed.
*
* \param keyframe_type: The type of keyframe (#eBezTriple_KeyframeType).
* \param flag: Optional flags (#eInsertKeyFlags) for controlling how keys get added
* and/or whether updates get done.
*/
int insert_vert_fcurve(FCurve *fcu,
const float2 position,
const KeyframeSettings &settings,
eInsertKeyFlags flag);
SingleKeyingResult insert_vert_fcurve(FCurve *fcu,
const float2 position,
const KeyframeSettings &settings,
eInsertKeyFlags flag);
/**
* \param sample_rate: indicates how many samples per frame should be generated.

View File

@ -17,7 +17,6 @@
#include "BLI_bit_span.hh"
#include "BLI_vector.hh"
#include "DNA_anim_types.h"
#include "ED_transform.hh"
#include "RNA_types.hh"
struct ID;
@ -33,6 +32,10 @@ namespace blender::animrig {
enum class SingleKeyingResult {
SUCCESS = 0,
/* TODO: remove `UNKNOWN_FAILURE` and replace all usages with proper, specific
* cases. This is needed right now as a stop-gap while progressively moving
* the keyframing code over to propagate errors properly.*/
UNKNOWN_FAILURE,
CANNOT_CREATE_FCURVE,
FCURVE_NOT_KEYFRAMEABLE,
NO_KEY_NEEDED,

View File

@ -862,36 +862,20 @@ FCurve &KeyframeStrip::fcurve_find_or_create(const Binding &binding,
return *new_fcurve;
}
FCurve *KeyframeStrip::keyframe_insert(const Binding &binding,
const StringRefNull rna_path,
const int array_index,
const float2 time_value,
const KeyframeSettings &settings)
SingleKeyingResult KeyframeStrip::keyframe_insert(const Binding &binding,
const StringRefNull rna_path,
const int array_index,
const float2 time_value,
const KeyframeSettings &settings)
{
FCurve &fcurve = this->fcurve_find_or_create(binding, rna_path, array_index);
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",
rna_path.c_str(),
array_index,
binding.name);
return nullptr;
return SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE;
}
/* TODO: Handle the eInsertKeyFlags. */
const int index = insert_vert_fcurve(&fcurve, time_value, settings, eInsertKeyFlags(0));
if (index < 0) {
std::fprintf(stderr,
"Could not insert key into FCurve %s[%d] for binding %s.\n",
rna_path.c_str(),
array_index,
binding.name);
return nullptr;
}
return &fcurve;
return insert_vert_fcurve(&fcurve, time_value, settings, eInsertKeyFlags(0));
}
/* AnimationChannelBag implementation. */

View File

@ -437,10 +437,10 @@ TEST_F(AnimationLayersTest, KeyframeStrip__keyframe_insert)
KeyframeStrip &key_strip = strip.as<KeyframeStrip>();
const KeyframeSettings settings = get_keyframe_settings(false);
FCurve *fcurve_loc_a = key_strip.keyframe_insert(
SingleKeyingResult result_loc_a = key_strip.keyframe_insert(
binding, "location", 0, {1.0f, 47.0f}, settings);
ASSERT_NE(nullptr, fcurve_loc_a)
<< "Expect all the necessary data structures to be created on insertion of a key";
ASSERT_EQ(SingleKeyingResult::SUCCESS, result_loc_a)
<< "Expected keyframe insertion to be successful";
/* Check the strip was created correctly, with the channels for the binding. */
ASSERT_EQ(1, key_strip.channelbags().size());
@ -448,21 +448,25 @@ TEST_F(AnimationLayersTest, KeyframeStrip__keyframe_insert)
EXPECT_EQ(binding.handle, channels->binding_handle);
/* Insert a second key, should insert into the same FCurve as before. */
FCurve *fcurve_loc_b = key_strip.keyframe_insert(
SingleKeyingResult result_loc_b = key_strip.keyframe_insert(
binding, "location", 0, {5.0f, 47.1f}, settings);
ASSERT_EQ(fcurve_loc_a, fcurve_loc_b)
<< "Expect same (binding/rna path/array index) tuple to return the same FCurve.";
ASSERT_EQ(SingleKeyingResult::SUCCESS, result_loc_b);
ASSERT_EQ(1, channels->fcurves().size()) << "Expect insertion with the same (binding/rna "
"path/array index) tuple to go into the same FCurve";
ASSERT_EQ(2, channels->fcurves()[0]->totvert)
<< "Expect insertion with the same (binding/rna "
"path/array index) tuple to go into the same FCurve";
EXPECT_EQ(2, fcurve_loc_b->totvert);
EXPECT_EQ(47.0f, evaluate_fcurve(fcurve_loc_a, 1.0f));
EXPECT_EQ(47.1f, evaluate_fcurve(fcurve_loc_a, 5.0f));
EXPECT_EQ(47.0f, evaluate_fcurve(channels->fcurves()[0], 1.0f));
EXPECT_EQ(47.1f, evaluate_fcurve(channels->fcurves()[0], 5.0f));
/* Insert another key for another property, should create another FCurve. */
FCurve *fcurve_rot = key_strip.keyframe_insert(
SingleKeyingResult result_rot = key_strip.keyframe_insert(
binding, "rotation_quaternion", 0, {1.0f, 0.25f}, settings);
EXPECT_NE(fcurve_loc_b, fcurve_rot)
<< "Expected rotation and location curves to be different FCurves.";
EXPECT_EQ(2, channels->fcurves().size()) << "Expected a second FCurve to be created.";
ASSERT_EQ(SingleKeyingResult::SUCCESS, result_rot);
ASSERT_EQ(2, channels->fcurves().size()) << "Expected a second FCurve to be created.";
ASSERT_EQ(2, channels->fcurves()[0]->totvert);
ASSERT_EQ(1, channels->fcurves()[1]->totvert);
}
} // namespace blender::animrig::tests

View File

@ -271,10 +271,10 @@ void initialize_bezt(BezTriple *beztr,
beztr->period = 4.1f;
}
int insert_vert_fcurve(FCurve *fcu,
const float2 position,
const KeyframeSettings &settings,
eInsertKeyFlags flag)
SingleKeyingResult insert_vert_fcurve(FCurve *fcu,
const float2 position,
const KeyframeSettings &settings,
eInsertKeyFlags flag)
{
BezTriple beztr = {{{0}}};
initialize_bezt(&beztr, position, settings, eFCurve_Flags(fcu->flag));
@ -286,9 +286,11 @@ int insert_vert_fcurve(FCurve *fcu,
a = insert_bezt_fcurve(fcu, &beztr, flag);
BKE_fcurve_active_keyframe_set(fcu, &fcu->bezt[a]);
/* If `a` is negative return to avoid segfaults. */
/* Key insertion failed. */
if (a < 0) {
return -1;
/* TODO: we need more info from `insert_bezt_fcurve()` called above to
* return a more specific failure. */
return SingleKeyingResult::UNKNOWN_FAILURE;
}
/* Set handle-type and interpolation. */
@ -323,7 +325,7 @@ int insert_vert_fcurve(FCurve *fcu,
}
/* Return the index at which the keyframe was added. */
return a;
return SingleKeyingResult::SUCCESS;
}
void sample_fcurve_segment(FCurve *fcu,

View File

@ -92,6 +92,17 @@ void CombinedKeyingResult::generate_reports(ReportList *reports)
}
Vector<std::string> errors;
if (this->get_count(SingleKeyingResult::UNKNOWN_FAILURE) > 0) {
const int error_count = this->get_count(SingleKeyingResult::UNKNOWN_FAILURE);
if (error_count == 1) {
errors.append(RPT_("Could not insert one key for unknown reasons."));
}
else {
errors.append(
fmt::format(RPT_("Could not insert {:d} keys for unknown reasons."), error_count));
}
}
if (this->get_count(SingleKeyingResult::CANNOT_CREATE_FCURVE) > 0) {
const int error_count = this->get_count(SingleKeyingResult::CANNOT_CREATE_FCURVE);
if (error_count == 1) {
@ -485,17 +496,11 @@ static SingleKeyingResult insert_keyframe_value(
KeyframeSettings settings = get_keyframe_settings((flag & INSERTKEY_NO_USERPREF) == 0);
settings.keyframe_type = keytype;
if (flag & INSERTKEY_NEEDED) {
if (!new_key_needed(fcu, cfra, curval)) {
return SingleKeyingResult::NO_KEY_NEEDED;
}
if (insert_vert_fcurve(fcu, {cfra, curval}, settings, flag) < 0) {
return SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE;
}
return SingleKeyingResult::SUCCESS;
if ((flag & INSERTKEY_NEEDED) && !new_key_needed(fcu, cfra, curval)) {
return SingleKeyingResult::NO_KEY_NEEDED;
}
if (insert_vert_fcurve(fcu, {cfra, curval}, settings, flag) < 0) {
if (insert_vert_fcurve(fcu, {cfra, curval}, settings, flag) != SingleKeyingResult::SUCCESS) {
return SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE;
}

View File

@ -57,8 +57,12 @@ TEST(evaluate_fcurve, InterpolationConstant)
FCurve *fcu = BKE_fcurve_create();
const KeyframeSettings settings = get_keyframe_settings(false);
EXPECT_EQ(insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS), 0);
EXPECT_EQ(insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS), 1);
insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS);
insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS);
EXPECT_EQ(fcu->bezt[0].vec[1][0], 1.0f);
EXPECT_EQ(fcu->bezt[0].vec[1][1], 7.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][0], 2.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][1], 13.0f);
fcu->bezt[0].ipo = BEZT_IPO_CONST;
fcu->bezt[1].ipo = BEZT_IPO_CONST;
@ -74,8 +78,12 @@ TEST(evaluate_fcurve, InterpolationLinear)
FCurve *fcu = BKE_fcurve_create();
const KeyframeSettings settings = get_keyframe_settings(false);
EXPECT_EQ(insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS), 0);
EXPECT_EQ(insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS), 1);
insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS);
insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS);
EXPECT_EQ(fcu->bezt[0].vec[1][0], 1.0f);
EXPECT_EQ(fcu->bezt[0].vec[1][1], 7.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][0], 2.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][1], 13.0f);
fcu->bezt[0].ipo = BEZT_IPO_LIN;
fcu->bezt[1].ipo = BEZT_IPO_LIN;
@ -92,8 +100,12 @@ TEST(evaluate_fcurve, InterpolationBezier)
FCurve *fcu = BKE_fcurve_create();
const KeyframeSettings settings = get_keyframe_settings(false);
EXPECT_EQ(insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS), 0);
EXPECT_EQ(insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS), 1);
insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS);
insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS);
EXPECT_EQ(fcu->bezt[0].vec[1][0], 1.0f);
EXPECT_EQ(fcu->bezt[0].vec[1][1], 7.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][0], 2.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][1], 13.0f);
EXPECT_EQ(fcu->bezt[0].ipo, BEZT_IPO_BEZ);
EXPECT_EQ(fcu->bezt[1].ipo, BEZT_IPO_BEZ);
@ -126,8 +138,12 @@ TEST(evaluate_fcurve, InterpolationBounce)
FCurve *fcu = BKE_fcurve_create();
const KeyframeSettings settings = get_keyframe_settings(false);
EXPECT_EQ(insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS), 0);
EXPECT_EQ(insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS), 1);
insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS);
insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS);
EXPECT_EQ(fcu->bezt[0].vec[1][0], 1.0f);
EXPECT_EQ(fcu->bezt[0].vec[1][1], 7.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][0], 2.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][1], 13.0f);
fcu->bezt[0].ipo = BEZT_IPO_BOUNCE;
fcu->bezt[1].ipo = BEZT_IPO_BOUNCE;
@ -147,8 +163,13 @@ TEST(evaluate_fcurve, ExtrapolationLinearKeys)
FCurve *fcu = BKE_fcurve_create();
const KeyframeSettings settings = get_keyframe_settings(false);
EXPECT_EQ(insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS), 0);
EXPECT_EQ(insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS), 1);
insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS);
insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS);
EXPECT_EQ(fcu->bezt[0].vec[1][0], 1.0f);
EXPECT_EQ(fcu->bezt[0].vec[1][1], 7.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][0], 2.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][1], 13.0f);
fcu->bezt[0].ipo = BEZT_IPO_LIN;
fcu->bezt[1].ipo = BEZT_IPO_LIN;
@ -177,8 +198,12 @@ TEST(evaluate_fcurve, ExtrapolationBezierKeys)
FCurve *fcu = BKE_fcurve_create();
const KeyframeSettings settings = get_keyframe_settings(false);
EXPECT_EQ(insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS), 0);
EXPECT_EQ(insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS), 1);
insert_vert_fcurve(fcu, {1.0f, 7.0f}, settings, INSERTKEY_NOFLAGS);
insert_vert_fcurve(fcu, {2.0f, 13.0f}, settings, INSERTKEY_NOFLAGS);
EXPECT_EQ(fcu->bezt[0].vec[1][0], 1.0f);
EXPECT_EQ(fcu->bezt[0].vec[1][1], 7.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][0], 2.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][1], 13.0f);
fcu->bezt[0].vec[0][0] = 0.71855f; /* left handle X */
fcu->bezt[0].vec[0][1] = 6.22482f; /* left handle Y */
@ -215,8 +240,12 @@ TEST(fcurve_subdivide, BKE_fcurve_bezt_subdivide_handles)
const KeyframeSettings settings = get_keyframe_settings(false);
/* Insert two keyframes and set handles to something non-default. */
EXPECT_EQ(insert_vert_fcurve(fcu, {1.0f, 0.0f}, settings, INSERTKEY_NOFLAGS), 0);
EXPECT_EQ(insert_vert_fcurve(fcu, {13.0f, 2.0f}, settings, INSERTKEY_NOFLAGS), 1);
insert_vert_fcurve(fcu, {1.0f, 0.0f}, settings, INSERTKEY_NOFLAGS);
insert_vert_fcurve(fcu, {13.0f, 2.0f}, settings, INSERTKEY_NOFLAGS);
EXPECT_EQ(fcu->bezt[0].vec[1][0], 1.0f);
EXPECT_EQ(fcu->bezt[0].vec[1][1], 0.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][0], 13.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][1], 2.0f);
fcu->bezt[0].h1 = fcu->bezt[0].h2 = HD_FREE;
fcu->bezt[0].vec[0][0] = -5.0f;
@ -282,11 +311,17 @@ TEST(fcurve_active_keyframe, ActiveKeyframe)
const KeyframeSettings settings = get_keyframe_settings(false);
/* Check that adding new points sets the active index. */
EXPECT_EQ(insert_vert_fcurve(fcu, {1.0f, 7.5f}, settings, INSERTKEY_NOFLAGS), 0);
insert_vert_fcurve(fcu, {1.0f, 7.5f}, settings, INSERTKEY_NOFLAGS);
EXPECT_EQ(fcu->bezt[0].vec[1][0], 1.0f);
EXPECT_EQ(fcu->bezt[0].vec[1][1], 7.5f);
EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), 0);
EXPECT_EQ(insert_vert_fcurve(fcu, {8.0f, 15.0f}, settings, INSERTKEY_NOFLAGS), 1);
insert_vert_fcurve(fcu, {8.0f, 15.0f}, settings, INSERTKEY_NOFLAGS);
EXPECT_EQ(fcu->bezt[1].vec[1][0], 8.0f);
EXPECT_EQ(fcu->bezt[1].vec[1][1], 15.0f);
EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), 1);
EXPECT_EQ(insert_vert_fcurve(fcu, {14.0f, 8.2f}, settings, INSERTKEY_NOFLAGS), 2);
insert_vert_fcurve(fcu, {14.0f, 8.2f}, settings, INSERTKEY_NOFLAGS);
EXPECT_EQ(fcu->bezt[2].vec[1][0], 14.0f);
EXPECT_EQ(fcu->bezt[2].vec[1][1], 8.2f);
EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), 2);
/* Check clearing the index. */

View File

@ -382,33 +382,34 @@ static int rna_iterator_keyframestrip_channelbags_length(PointerRNA *ptr)
return key_strip.channelbags().size();
}
static FCurve *rna_KeyframeAnimationStrip_key_insert(ID *id,
KeyframeAnimationStrip *dna_strip,
Main *bmain,
ReportList *reports,
AnimationBinding *dna_binding,
const char *rna_path,
const int array_index,
const float value,
const float time)
static bool rna_KeyframeAnimationStrip_key_insert(ID *id,
KeyframeAnimationStrip *dna_strip,
Main *bmain,
ReportList *reports,
AnimationBinding *dna_binding,
const char *rna_path,
const int array_index,
const float value,
const float time)
{
if (dna_binding == nullptr) {
BKE_report(reports, RPT_ERROR, "Binding cannot be None");
return nullptr;
return false;
}
animrig::KeyframeStrip &key_strip = dna_strip->wrap();
const animrig::Binding &binding = dna_binding->wrap();
const animrig::KeyframeSettings settings = animrig::get_keyframe_settings(true);
FCurve *fcurve = key_strip.keyframe_insert(
animrig::SingleKeyingResult result = key_strip.keyframe_insert(
binding, rna_path, array_index, {time, value}, settings);
if (fcurve) {
if (result == animrig::SingleKeyingResult::SUCCESS) {
DEG_id_tag_update_ex(bmain, id, ID_RECALC_ANIMATION);
}
return fcurve;
return result == animrig::SingleKeyingResult::SUCCESS;
}
static void rna_iterator_ChannelBag_fcurves_begin(CollectionPropertyIterator *iter,
@ -775,7 +776,8 @@ static void rna_def_animation_keyframe_strip(BlenderRNA *brna)
FLT_MAX);
RNA_def_parameter_flags(parm, PropertyFlag(0), PARM_REQUIRED);
parm = RNA_def_pointer(func, "fcurve", "FCurve", "", "The FCurve this key was inserted on");
parm = RNA_def_boolean(
func, "success", true, "Success", "Whether the key was successfully inserted");
RNA_def_function_return(func, parm);
}
}

View File

@ -1065,11 +1065,15 @@ static BezTriple *rna_FKeyframe_points_insert(
using namespace blender::animrig;
KeyframeSettings settings = get_keyframe_settings(false);
settings.keyframe_type = eBezTriple_KeyframeType(keyframe_type);
int index = insert_vert_fcurve(fcu, {frame, value}, settings, eInsertKeyFlags(flag));
const SingleKeyingResult result = insert_vert_fcurve(
fcu, {frame, value}, settings, eInsertKeyFlags(flag));
if ((fcu->bezt) && (index >= 0)) {
if ((fcu->bezt) && (result == SingleKeyingResult::SUCCESS)) {
rna_tag_animation_update(bmain, id);
bool replace;
const int index = BKE_fcurve_bezt_binarysearch_index(fcu->bezt, frame, fcu->totvert, &replace);
return fcu->bezt + index;
}