diff --git a/source/blender/animrig/ANIM_animation.hh b/source/blender/animrig/ANIM_animation.hh index 0d349421b4c..479b91efd18 100644 --- a/source/blender/animrig/ANIM_animation.hh +++ b/source/blender/animrig/ANIM_animation.hh @@ -10,6 +10,7 @@ #pragma once #include "ANIM_fcurve.hh" +#include "ANIM_keyframing.hh" #include "DNA_anim_types.h" @@ -460,11 +461,12 @@ 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, + eInsertKeyFlags insert_key_flags, + const KeyframeSettings &settings); }; static_assert(sizeof(KeyframeStrip) == sizeof(::KeyframeAnimationStrip), "DNA struct and its C++ wrapper must have the same size"); diff --git a/source/blender/animrig/ANIM_animdata.hh b/source/blender/animrig/ANIM_animdata.hh index 2b019f6b647..c8ae35fb9cc 100644 --- a/source/blender/animrig/ANIM_animdata.hh +++ b/source/blender/animrig/ANIM_animdata.hh @@ -30,6 +30,15 @@ class Animation; */ bAction *id_action_ensure(Main *bmain, ID *id); +/** + * Get the animation datablock for the given ID, creating one if it doesn't + * already exist. + * + * Note: this does NOT touch bindings (e.g. it doesn't ensure a binding for the + * given ID). + */ +Animation *id_animation_ensure(Main *bmain, ID *id); + /** * Delete the F-Curve from the given AnimData block (if possible), * as appropriate according to animation context. diff --git a/source/blender/animrig/ANIM_fcurve.hh b/source/blender/animrig/ANIM_fcurve.hh index 654fdbb2ecd..8ca2fa68102 100644 --- a/source/blender/animrig/ANIM_fcurve.hh +++ b/source/blender/animrig/ANIM_fcurve.hh @@ -9,6 +9,8 @@ */ #pragma once +#include "ANIM_keyframing.hh" + #include "BLI_math_vector_types.hh" #include "BLI_string_ref.hh" @@ -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. diff --git a/source/blender/animrig/ANIM_keyframing.hh b/source/blender/animrig/ANIM_keyframing.hh index d8940005f2d..57a924d94a7 100644 --- a/source/blender/animrig/ANIM_keyframing.hh +++ b/source/blender/animrig/ANIM_keyframing.hh @@ -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,12 +32,19 @@ 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, 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, @@ -57,8 +63,16 @@ class CombinedKeyingResult { public: CombinedKeyingResult(); + /** + * Increments the given keying result by one. + */ void add(const SingleKeyingResult result); + /** + * Adds `count` to the given keying result. + */ + void add(const SingleKeyingResult result, int count); + /* Add values of the given result to this result. */ void merge(const CombinedKeyingResult &combined_result); @@ -69,6 +83,14 @@ class CombinedKeyingResult { void generate_reports(ReportList *reports); }; +/* -------------------------------------------------------------------- */ + +/** + * Return whether a new fcurve can be created according to the given keyframing + * flags. + */ +bool can_create_fcurve(eInsertKeyFlags insert_key_flags); + /* -------------------------------------------------------------------- */ /** \name Key-Framing Management * \{ */ diff --git a/source/blender/animrig/intern/animation.cc b/source/blender/animrig/intern/animation.cc index df85fc85749..9544f604005 100644 --- a/source/blender/animrig/intern/animation.cc +++ b/source/blender/animrig/intern/animation.cc @@ -645,7 +645,7 @@ StringRefNull Binding::name_without_prefix() const { BLI_assert(StringRef(this->name).size() >= name_length_min); - /* Avoid accessing an uninitialized part of the string accidentally. */ + /* Avoid accessing an uninitialised part of the string accidentally. */ if (this->name[0] == '\0' || this->name[1] == '\0') { return ""; } @@ -862,36 +862,27 @@ 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 eInsertKeyFlags insert_key_flags, + 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; + /* Get the fcurve, or if one doesn't exist and the keying flags allow then + * create one. */ + FCurve *fcu = can_create_fcurve(insert_key_flags) ? + &this->fcurve_find_or_create(binding, rna_path, array_index) : + this->fcurve_find(binding, rna_path, array_index); + if (!fcu) { + return SingleKeyingResult::CANNOT_CREATE_FCURVE; } - /* 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; + if (!BKE_fcurve_is_keyframable(fcu)) { + return SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE; } - return &fcurve; + return insert_vert_fcurve(fcu, time_value, settings, insert_key_flags); } /* AnimationChannelBag implementation. */ diff --git a/source/blender/animrig/intern/animation_test.cc b/source/blender/animrig/intern/animation_test.cc index d5958705efa..323bebf10ff 100644 --- a/source/blender/animrig/intern/animation_test.cc +++ b/source/blender/animrig/intern/animation_test.cc @@ -128,9 +128,10 @@ TEST_F(AnimationLayersTest, add_strip) /* Add some keys to check that also the strip data is freed correctly. */ const KeyframeSettings settings = get_keyframe_settings(false); Binding &binding = anim->binding_add(); - strip.as().keyframe_insert(binding, "location", 0, {1.0f, 47.0f}, settings); + strip.as().keyframe_insert( + binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); another_strip.as().keyframe_insert( - binding, "location", 0, {1.0f, 47.0f}, settings); + binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); } TEST_F(AnimationLayersTest, remove_strip) @@ -143,9 +144,12 @@ TEST_F(AnimationLayersTest, remove_strip) /* Add some keys to check that also the strip data is freed correctly. */ const KeyframeSettings settings = get_keyframe_settings(false); Binding &binding = anim->binding_add(); - strip0.as().keyframe_insert(binding, "location", 0, {1.0f, 47.0f}, settings); - strip1.as().keyframe_insert(binding, "location", 0, {1.0f, 47.0f}, settings); - strip2.as().keyframe_insert(binding, "location", 0, {1.0f, 47.0f}, settings); + strip0.as().keyframe_insert( + binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); + strip1.as().keyframe_insert( + binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); + strip2.as().keyframe_insert( + binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); EXPECT_TRUE(layer.strip_remove(strip1)); EXPECT_EQ(2, layer.strips().size()); @@ -437,10 +441,10 @@ TEST_F(AnimationLayersTest, KeyframeStrip__keyframe_insert) KeyframeStrip &key_strip = strip.as(); const KeyframeSettings settings = get_keyframe_settings(false); - FCurve *fcurve_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"; + SingleKeyingResult result_loc_a = key_strip.keyframe_insert( + binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); + 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 +452,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( - 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."; + SingleKeyingResult result_loc_b = key_strip.keyframe_insert( + binding, "location", 0, {5.0f, 47.1f}, INSERTKEY_NOFLAGS, settings); + 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( - 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."; + SingleKeyingResult result_rot = key_strip.keyframe_insert( + binding, "rotation_quaternion", 0, {1.0f, 0.25f}, INSERTKEY_NOFLAGS, settings); + 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 diff --git a/source/blender/animrig/intern/animdata.cc b/source/blender/animrig/intern/animdata.cc index aa2b215526a..f2246486a10 100644 --- a/source/blender/animrig/intern/animdata.cc +++ b/source/blender/animrig/intern/animdata.cc @@ -11,6 +11,7 @@ #include "BKE_action.h" #include "BKE_anim_data.hh" +#include "BKE_animation.hh" #include "BKE_fcurve.hh" #include "BKE_lib_id.hh" @@ -74,6 +75,33 @@ bAction *id_action_ensure(Main *bmain, ID *id) return adt->action; } +Animation *id_animation_ensure(Main *bmain, ID *id) +{ + BLI_assert(id != nullptr); + + AnimData *adt = BKE_animdata_ensure_id(id); + if (adt == nullptr) { + /* TODO: this printf is copied from the corresponding code in + * `id_action_ensure()`. Is this really the right way to handle reporting + * this? If it's never supposed to happen, an assert would be better. If + * it *is* supposed to happen, then printf doesn't seem like the right way + * to report it, if it should be reported at all. */ + printf("ERROR: Couldn't add AnimData (ID = %s)\n", (id) ? (id->name) : ""); + return nullptr; + } + + if (adt->animation != nullptr) { + return &adt->animation->wrap(); + } + + /* TODO: translate the default name. */ + ::Animation *anim = BKE_animation_add(bmain, "Animation"); + + DEG_relations_tag_update(bmain); + DEG_id_tag_update(&anim->id, ID_RECALC_ANIMATION_NO_FLUSH); + return &anim->wrap(); +} + void animdata_fcurve_delete(bAnimContext *ac, AnimData *adt, FCurve *fcu) { /* - If no AnimData, we've got nowhere to remove the F-Curve from diff --git a/source/blender/animrig/intern/evaluation_test.cc b/source/blender/animrig/intern/evaluation_test.cc index 670780bace4..2810f7826a1 100644 --- a/source/blender/animrig/intern/evaluation_test.cc +++ b/source/blender/animrig/intern/evaluation_test.cc @@ -140,10 +140,12 @@ TEST_F(AnimationEvaluationTest, evaluate_layer__keyframes) KeyframeStrip &key_strip = strip.as(); /* Set some keys. */ - key_strip.keyframe_insert(*binding, "location", 0, {1.0f, 47.1f}, settings); - key_strip.keyframe_insert(*binding, "location", 0, {5.0f, 47.5f}, settings); - key_strip.keyframe_insert(*binding, "rotation_euler", 1, {1.0f, 0.0f}, settings); - key_strip.keyframe_insert(*binding, "rotation_euler", 1, {5.0f, 3.14f}, settings); + key_strip.keyframe_insert(*binding, "location", 0, {1.0f, 47.1f}, INSERTKEY_NOFLAGS, settings); + key_strip.keyframe_insert(*binding, "location", 0, {5.0f, 47.5f}, INSERTKEY_NOFLAGS, settings); + key_strip.keyframe_insert( + *binding, "rotation_euler", 1, {1.0f, 0.0f}, INSERTKEY_NOFLAGS, settings); + key_strip.keyframe_insert( + *binding, "rotation_euler", 1, {5.0f, 3.14f}, INSERTKEY_NOFLAGS, settings); /* Set the animated properties to some values. These should not be overwritten * by the evaluation itself. */ @@ -181,9 +183,9 @@ TEST_F(AnimationEvaluationTest, strip_boundaries__single_strip) /* Set some keys. */ KeyframeStrip &key_strip = strip.as(); - key_strip.keyframe_insert(*binding, "location", 0, {1.0f, 47.0f}, settings); - key_strip.keyframe_insert(*binding, "location", 0, {5.0f, 327.0f}, settings); - key_strip.keyframe_insert(*binding, "location", 0, {10.0f, 48.0f}, settings); + key_strip.keyframe_insert(*binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); + key_strip.keyframe_insert(*binding, "location", 0, {5.0f, 327.0f}, INSERTKEY_NOFLAGS, settings); + key_strip.keyframe_insert(*binding, "location", 0, {10.0f, 48.0f}, INSERTKEY_NOFLAGS, settings); /* Evaluate the layer to see how it handles the boundaries + something in between. */ EXPECT_TRUE(test_evaluate_layer("location", 0, {1.0f, 47.0f})); @@ -205,15 +207,21 @@ TEST_F(AnimationEvaluationTest, strip_boundaries__nonoverlapping) /* Set some keys. */ { KeyframeStrip &key_strip1 = strip1.as(); - key_strip1.keyframe_insert(*binding, "location", 0, {1.0f, 47.0f}, settings); - key_strip1.keyframe_insert(*binding, "location", 0, {5.0f, 327.0f}, settings); - key_strip1.keyframe_insert(*binding, "location", 0, {10.0f, 48.0f}, settings); + key_strip1.keyframe_insert( + *binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); + key_strip1.keyframe_insert( + *binding, "location", 0, {5.0f, 327.0f}, INSERTKEY_NOFLAGS, settings); + key_strip1.keyframe_insert( + *binding, "location", 0, {10.0f, 48.0f}, INSERTKEY_NOFLAGS, settings); } { KeyframeStrip &key_strip2 = strip2.as(); - key_strip2.keyframe_insert(*binding, "location", 0, {1.0f, 47.0f}, settings); - key_strip2.keyframe_insert(*binding, "location", 0, {5.0f, 327.0f}, settings); - key_strip2.keyframe_insert(*binding, "location", 0, {10.0f, 48.0f}, settings); + key_strip2.keyframe_insert( + *binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); + key_strip2.keyframe_insert( + *binding, "location", 0, {5.0f, 327.0f}, INSERTKEY_NOFLAGS, settings); + key_strip2.keyframe_insert( + *binding, "location", 0, {10.0f, 48.0f}, INSERTKEY_NOFLAGS, settings); } /* Check Strip 1. */ @@ -245,15 +253,21 @@ TEST_F(AnimationEvaluationTest, strip_boundaries__overlapping_edge) /* Set some keys. */ { KeyframeStrip &key_strip1 = strip1.as(); - key_strip1.keyframe_insert(*binding, "location", 0, {1.0f, 47.0f}, settings); - key_strip1.keyframe_insert(*binding, "location", 0, {5.0f, 327.0f}, settings); - key_strip1.keyframe_insert(*binding, "location", 0, {10.0f, 48.0f}, settings); + key_strip1.keyframe_insert( + *binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); + key_strip1.keyframe_insert( + *binding, "location", 0, {5.0f, 327.0f}, INSERTKEY_NOFLAGS, settings); + key_strip1.keyframe_insert( + *binding, "location", 0, {10.0f, 48.0f}, INSERTKEY_NOFLAGS, settings); } { KeyframeStrip &key_strip2 = strip2.as(); - key_strip2.keyframe_insert(*binding, "location", 0, {1.0f, 47.0f}, settings); - key_strip2.keyframe_insert(*binding, "location", 0, {5.0f, 327.0f}, settings); - key_strip2.keyframe_insert(*binding, "location", 0, {10.0f, 48.0f}, settings); + key_strip2.keyframe_insert( + *binding, "location", 0, {1.0f, 47.0f}, INSERTKEY_NOFLAGS, settings); + key_strip2.keyframe_insert( + *binding, "location", 0, {5.0f, 327.0f}, INSERTKEY_NOFLAGS, settings); + key_strip2.keyframe_insert( + *binding, "location", 0, {10.0f, 48.0f}, INSERTKEY_NOFLAGS, settings); } /* Check Strip 1. */ diff --git a/source/blender/animrig/intern/fcurve.cc b/source/blender/animrig/intern/fcurve.cc index 57a3d28a79f..b83b4f178b0 100644 --- a/source/blender/animrig/intern/fcurve.cc +++ b/source/blender/animrig/intern/fcurve.cc @@ -6,12 +6,14 @@ * \ingroup animrig */ +#include #include #include #include "ANIM_animdata.hh" #include "ANIM_fcurve.hh" #include "BKE_fcurve.hh" +#include "BLI_math_base.h" #include "BLI_math_vector_types.hh" #include "BLI_string.h" #include "DNA_anim_types.h" @@ -271,11 +273,58 @@ void initialize_bezt(BezTriple *beztr, beztr->period = 4.1f; } -int insert_vert_fcurve(FCurve *fcu, - const float2 position, - const KeyframeSettings &settings, - eInsertKeyFlags flag) +/** + * Return whether the given fcurve already evaluates to the same value as the + * proposed keyframe at the keyframe's time. + * + * This is a helper function for determining whether to insert a keyframe or not + * when "only insert needed" is enabled. + * + * Note: this does *not* determine whether inserting the keyframe would change + * the fcurve at points other than the keyframe itself. For example, even if + * inserting the key wouldn't change the fcurve's value at the time of the + * keyframe, the resulting changes to bezier interpolation could change the + * fcurve on either side of it. This function intentionally does not account for + * that, since that's not how the "only insert needed" feature is supposed to + * work. + */ +static bool new_key_needed(FCurve *fcu, const float frame, const float value) { + if (fcu == nullptr) { + return true; + } + if (fcu->totvert == 0) { + return true; + } + + bool replace; + const int bezt_index = BKE_fcurve_bezt_binarysearch_index( + fcu->bezt, frame, fcu->totvert, &replace); + + if (replace) { + /* If there is already a key, we only need to modify it if the proposed value is different. */ + return fcu->bezt[bezt_index].vec[1][1] != value; + } + + const int diff_ulp = 32; + const float fcu_eval = evaluate_fcurve(fcu, frame); + /* No need to insert a key if the same value is already the value of the FCurve at that point. */ + if (compare_ff_relative(fcu_eval, value, FLT_EPSILON, diff_ulp)) { + return false; + } + + return true; +} + +SingleKeyingResult insert_vert_fcurve(FCurve *fcu, + const float2 position, + const KeyframeSettings &settings, + eInsertKeyFlags flag) +{ + if (flag & INSERTKEY_NEEDED && !new_key_needed(fcu, position[0], position[1])) { + return SingleKeyingResult::NO_KEY_NEEDED; + } + BezTriple beztr = {{{0}}}; initialize_bezt(&beztr, position, settings, eFCurve_Flags(fcu->flag)); @@ -286,9 +335,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 +374,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, @@ -492,7 +543,7 @@ void bake_fcurve_segments(FCurve *fcu) /* Add keyframes with these, tagging as 'breakdowns'. */ for (n = 1, fp = value_cache; n < range && fp; n++, fp++) { blender::animrig::insert_vert_fcurve( - fcu, {fp->frame, fp->val}, settings, eInsertKeyFlags(1)); + fcu, {fp->frame, fp->val}, settings, INSERTKEY_NOFLAGS); } MEM_freeN(value_cache); diff --git a/source/blender/animrig/intern/keyframing.cc b/source/blender/animrig/intern/keyframing.cc index b647be7b72e..665be5361cb 100644 --- a/source/blender/animrig/intern/keyframing.cc +++ b/source/blender/animrig/intern/keyframing.cc @@ -6,13 +6,13 @@ * \ingroup animrig */ -#include #include #include #include #include "ANIM_action.hh" +#include "ANIM_animation.hh" #include "ANIM_animdata.hh" #include "ANIM_fcurve.hh" #include "ANIM_keyframing.hh" @@ -58,6 +58,11 @@ void CombinedKeyingResult::add(const SingleKeyingResult result) result_counter[int(result)]++; } +void CombinedKeyingResult::add(const SingleKeyingResult result, int count) +{ + result_counter[int(result)] += count; +} + void CombinedKeyingResult::merge(const CombinedKeyingResult &other) { for (int i = 0; i < result_counter.size(); i++) { @@ -92,6 +97,17 @@ void CombinedKeyingResult::generate_reports(ReportList *reports) } Vector 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) { @@ -183,6 +199,48 @@ void CombinedKeyingResult::generate_reports(ReportList *reports) } } + if (this->get_count(SingleKeyingResult::NO_VALID_LAYER) > 0) { + const int error_count = this->get_count(SingleKeyingResult::NO_VALID_LAYER); + if (error_count == 1) { + errors.append( + fmt::format(RPT_("Due to there being no layer that can accept it, one " + "keyframe was not inserted."))); + } + else { + errors.append(fmt::format(RPT_("Due to there being no layer that can accept them, {:d} " + "keyframes were not inserted."), + error_count)); + } + } + + if (this->get_count(SingleKeyingResult::NO_VALID_STRIP) > 0) { + const int error_count = this->get_count(SingleKeyingResult::NO_VALID_STRIP); + if (error_count == 1) { + errors.append( + fmt::format(RPT_("Due to there being no strip that can accept it, one " + "keyframe was not inserted."))); + } + else { + errors.append(fmt::format(RPT_("Due to there being no strip that can accept them, {:d} " + "keyframes were not inserted."), + error_count)); + } + } + + if (this->get_count(SingleKeyingResult::NO_VALID_BINDING) > 0) { + const int error_count = this->get_count(SingleKeyingResult::NO_VALID_BINDING); + if (error_count == 1) { + errors.append( + fmt::format(RPT_("Due to a missing animation binding, one keyframe was not " + "inserted."))); + } + else { + errors.append(fmt::format(RPT_("Due to a missing animation binding, {:d} keyframes " + "were not inserted."), + error_count)); + } + } + if (errors.is_empty()) { BKE_report(reports, RPT_WARNING, "Encountered unhandled error during keyframing"); return; @@ -251,6 +309,11 @@ eInsertKeyFlags get_keyframing_flags(Scene *scene) return flag; } +bool can_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) { @@ -414,38 +477,6 @@ static eFCU_Cycle_Type remap_cyclic_keyframe_location(FCurve *fcu, float *px, fl return type; } -/** - * This helper function determines whether a new keyframe is needed. - * A keyframe doesn't get added when the FCurve already has the proposed value. - */ -static bool new_key_needed(FCurve *fcu, const float frame, const float value) -{ - if (fcu == nullptr) { - return true; - } - if (fcu->totvert == 0) { - return true; - } - - bool replace; - const int bezt_index = BKE_fcurve_bezt_binarysearch_index( - fcu->bezt, frame, fcu->totvert, &replace); - - if (replace) { - /* If there is already a key, we only need to modify it if the proposed value is different. */ - return fcu->bezt[bezt_index].vec[1][1] != value; - } - - const int diff_ulp = 32; - const float fcu_eval = evaluate_fcurve(fcu, frame); - /* No need to insert a key if the same value is already the value of the FCurve at that point. */ - if (compare_ff_relative(fcu_eval, value, FLT_EPSILON, diff_ulp)) { - return false; - } - - return true; -} - static float nla_time_remap(const AnimationEvalContext *anim_eval_context, PointerRNA *id_ptr, AnimData *adt, @@ -485,21 +516,7 @@ 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 (insert_vert_fcurve(fcu, {cfra, curval}, settings, flag) < 0) { - return SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE; - } - - return SingleKeyingResult::SUCCESS; + return insert_vert_fcurve(fcu, {cfra, curval}, settings, flag); } bool insert_keyframe_direct(ReportList *reports, @@ -600,8 +617,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 = can_create_fcurve(flag) ? action_fcurve_ensure(bmain, act, group, ptr, rna_path, array_index) : action_fcurve_find(act, rna_path, array_index); @@ -1042,6 +1059,95 @@ 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 eInsertKeyFlags insert_key_flags, + const KeyframeSettings &key_settings) +{ + if (layer.strips().size() == 0) { + layer.strip_add(Strip::Type::Keyframe); + } + Strip *strip = layer.strip(0); + if (strip == nullptr) { + return SingleKeyingResult::NO_VALID_STRIP; + } + + /* TODO: morph key data based on Layer position in stack and Strip offset. */ + return strip->as().keyframe_insert( + binding, rna_path, key_data.array_index, key_data.position, insert_key_flags, key_settings); +} + +static CombinedKeyingResult insert_key_anim(Animation &anim, + PointerRNA *rna_pointer, + const blender::Span rna_paths, + const float scene_frame, + const eInsertKeyFlags insert_key_flags, + const KeyframeSettings &key_settings) +{ + ID *id = rna_pointer->owner_id; + CombinedKeyingResult combined_result; + + Binding *binding = anim.binding_for_id(*id); + if (binding == nullptr) { + binding = &anim.binding_add(); + const bool success = anim.assign_id(binding, *id); + if (!success) { + /* TODO: count the rna paths properly (e.g. accounting for multi-element properties). */ + combined_result.add(SingleKeyingResult::NO_VALID_BINDING, rna_paths.size()); + return combined_result; + } + } + + Layer *layer; + if (anim.layers().size() == 0) { + layer = &anim.layer_add("Layer 0"); + } + else { + layer = anim.layer(0); + } + + if (layer == nullptr) { + /* TODO: count the rna paths properly (e.g. accounting for multi-element properties). */ + combined_result.add(SingleKeyingResult::NO_VALID_LAYER, rna_paths.size()); + return combined_result; + } + + for (const std::string &rna_path : rna_paths) { + const bool visual_keyframing = insert_key_flags & INSERTKEY_MATRIX; + PointerRNA ptr; + PropertyRNA *prop = nullptr; + const bool path_resolved = RNA_path_resolve_property( + rna_pointer, rna_path.c_str(), &ptr, &prop); + if (!path_resolved) { + combined_result.add(SingleKeyingResult::CANNOT_RESOLVE_PATH); + continue; + } + const std::optional rna_path_id_to_prop = RNA_path_from_ID_to_property(&ptr, + prop); + Vector rna_values = get_keyframe_values(&ptr, prop, visual_keyframing); + + for (int property_index : rna_values.index_range()) { + KeyInsertData key_data; + key_data.array_index = property_index; + key_data.position = {scene_frame, rna_values[property_index]}; + const SingleKeyingResult result = insert_key_layer( + *layer, *binding, rna_path_id_to_prop.value(), key_data, insert_key_flags, key_settings); + combined_result.add(result); + } + } + + DEG_id_tag_update(&anim.id, ID_RECALC_ANIMATION_NO_FLUSH); + + return combined_result; +} + CombinedKeyingResult insert_key_rna(PointerRNA *rna_pointer, const blender::Span rna_paths, const float scene_frame, @@ -1051,14 +1157,47 @@ 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) { + /* TODO: count the rna paths properly (e.g. accounting for multi-element properties). */ + combined_result.add(SingleKeyingResult::ID_NOT_ANIMATABLE, rna_paths.size()); return combined_result; } - AnimData *adt = BKE_animdata_from_id(id); + if (USER_EXPERIMENTAL_TEST(&U, use_animation_baklava) && (adt->animation || !adt->action)) { + Animation *anim = id_animation_ensure(bmain, id); + if (anim == nullptr) { + /* TODO: count the rna paths properly (e.g. accounting for multi-element + * properties). */ + /* TODO: is ID_NOT_ANIMATABLE the right result? If that were the case, + * presumably we would have returned on the failed attempt to get the + * AnimData above. But what *does* cause this case? */ + combined_result.add(SingleKeyingResult::ID_NOT_ANIMATABLE, rna_paths.size()); + return combined_result; + } + + /* 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_anim( + *anim, rna_pointer, rna_paths, scene_frame, insert_key_flags, key_settings); + } + + bAction *action = id_action_ensure(bmain, id); + if (action == nullptr) { + /* TODO: count the rna paths properly (e.g. accounting for multi-element + * properties). */ + /* TODO: is ID_NOT_ANIMATABLE the right result? If that were the case, + * presumably we would have returned on the failed attempt to get the + * AnimData above. But what *does* cause this case? */ + combined_result.add(SingleKeyingResult::ID_NOT_ANIMATABLE, rna_paths.size()); + return combined_result; + } /* Keyframing functions can deal with the nla_context being a nullptr. */ ListBase nla_cache = {nullptr, nullptr}; @@ -1079,6 +1218,7 @@ CombinedKeyingResult insert_key_rna(PointerRNA *rna_pointer, const bool path_resolved = RNA_path_resolve_property( rna_pointer, rna_path.c_str(), &ptr, &prop); if (!path_resolved) { + combined_result.add(SingleKeyingResult::CANNOT_RESOLVE_PATH); continue; } const std::optional rna_path_id_to_prop = RNA_path_from_ID_to_property(&ptr, diff --git a/source/blender/animrig/intern/keyframing_auto.cc b/source/blender/animrig/intern/keyframing_auto.cc index 5fd93d76e17..d2473829f07 100644 --- a/source/blender/animrig/intern/keyframing_auto.cc +++ b/source/blender/animrig/intern/keyframing_auto.cc @@ -43,6 +43,11 @@ static eInsertKeyFlags get_autokey_flags(Scene *scene) flag |= INSERTKEY_NEEDED; } + /* Only insert available. */ + if (is_keying_flag(scene, AUTOKEY_FLAG_INSERTAVAILABLE)) { + flag |= INSERTKEY_AVAILABLE; + } + /* Keyframing mode - only replace existing keyframes. */ if (is_autokey_mode(scene, AUTOKEY_MODE_EDITKEYS)) { flag |= INSERTKEY_REPLACE; @@ -132,43 +137,37 @@ void autokeyframe_object(bContext *C, Scene *scene, Object *ob, Spanadt; - ToolSettings *ts = scene->toolsettings; - Main *bmain = CTX_data_main(C); - - if (adt && adt->action) { - CombinedKeyingResult combined_result; - LISTBASE_FOREACH (FCurve *, fcu, &adt->action->curves) { - CombinedKeyingResult result = insert_keyframe(bmain, - *id, - (fcu->grp ? fcu->grp->name : nullptr), - fcu->rna_path, - fcu->array_index, - &anim_eval_context, - eBezTriple_KeyframeType(ts->keyframe_type), - flag); - combined_result.merge(result); - } - if (combined_result.get_count(SingleKeyingResult::SUCCESS) == 0) { - combined_result.generate_reports(reports); - } - } + /* Optimization: if there's no animation at all and "Only Insert Available" is + * enabled, we know nothing will get keyed anyway, so return early. */ + if (is_keying_flag(scene, AUTOKEY_FLAG_INSERTAVAILABLE) && + (!ob->adt || (!ob->adt->action && !ob->adt->animation))) + { + /* TODO: account for multi-element properties in the report. E.g. right now + * location will be reported as a single channel in the report. */ + CombinedKeyingResult result; + result.add(SingleKeyingResult::CANNOT_CREATE_FCURVE, rna_paths.size()); + result.generate_reports(reports); return; } const float scene_frame = BKE_scene_frame_get(scene); Main *bmain = CTX_data_main(C); + CombinedKeyingResult combined_result; for (PointerRNA ptr : sources) { - insert_key_rna(&ptr, - rna_paths, - scene_frame, - flag, - eBezTriple_KeyframeType(scene->toolsettings->keyframe_type), - bmain, - anim_eval_context); + const CombinedKeyingResult result = insert_key_rna( + &ptr, + rna_paths, + scene_frame, + flag, + eBezTriple_KeyframeType(scene->toolsettings->keyframe_type), + bmain, + anim_eval_context); + combined_result.merge(result); + } + + if (combined_result.get_count(SingleKeyingResult::SUCCESS) == 0) { + combined_result.generate_reports(reports); } } @@ -224,13 +223,13 @@ void autokeyframe_pose_channel(bContext *C, ID *id = &ob->id; AnimData *adt = ob->adt; bAction *act = (adt) ? adt->action : nullptr; + ::Animation *animation = (adt) ? adt->animation : nullptr; if (!blender::animrig::autokeyframe_cfra_can_key(scene, id)) { return; } ReportList *reports = CTX_wm_reports(C); - ToolSettings *ts = scene->toolsettings; KeyingSet *active_ks = ANIM_scene_get_active_keyingset(scene); Depsgraph *depsgraph = CTX_data_depsgraph_pointer(C); const float scene_frame = BKE_scene_frame_get(scene); @@ -260,46 +259,32 @@ void autokeyframe_pose_channel(bContext *C, return; } - /* only insert into available channels? */ - if (is_keying_flag(scene, AUTOKEY_FLAG_INSERTAVAILABLE)) { - if (!act) { - return; - } - LISTBASE_FOREACH (FCurve *, fcu, &act->curves) { - /* only insert keyframes for this F-Curve if it affects the current bone */ - char pchan_name[sizeof(pose_channel->name)]; - if (!BLI_str_quoted_substr(fcu->rna_path, "bones[", pchan_name, sizeof(pchan_name))) { - continue; - } - - /* only if bone name matches too... - * NOTE: this will do constraints too, but those are ok to do here too? - */ - if (STREQ(pchan_name, pose_channel->name)) { - CombinedKeyingResult result = insert_keyframe(bmain, - *id, - ((fcu->grp) ? (fcu->grp->name) : (nullptr)), - fcu->rna_path, - fcu->array_index, - &anim_eval_context, - eBezTriple_KeyframeType(ts->keyframe_type), - flag); - if (result.get_count(SingleKeyingResult::SUCCESS) == 0) { - result.generate_reports(reports); - } - } - } + /* Optimization: if there's no animation at all and "Only Insert Available" is + * enabled, we know nothing will get keyed anyway, so return early. */ + if (is_keying_flag(scene, AUTOKEY_FLAG_INSERTAVAILABLE) && !act && !animation) { + /* TODO: account for multi-element properties in the report. E.g. right now + * location will be reported as a single channel in the report. */ + CombinedKeyingResult result; + result.add(SingleKeyingResult::CANNOT_CREATE_FCURVE, rna_paths.size()); + result.generate_reports(reports); return; } + CombinedKeyingResult combined_result; for (PointerRNA &ptr : sources) { - insert_key_rna(&ptr, - rna_paths, - scene_frame, - flag, - eBezTriple_KeyframeType(scene->toolsettings->keyframe_type), - bmain, - anim_eval_context); + const CombinedKeyingResult result = insert_key_rna( + &ptr, + rna_paths, + scene_frame, + flag, + eBezTriple_KeyframeType(scene->toolsettings->keyframe_type), + bmain, + anim_eval_context); + combined_result.merge(result); + } + + if (combined_result.get_count(SingleKeyingResult::SUCCESS) == 0) { + combined_result.generate_reports(reports); } } diff --git a/source/blender/blenkernel/intern/fcurve_test.cc b/source/blender/blenkernel/intern/fcurve_test.cc index 5f397bfac3c..647db5d2e09 100644 --- a/source/blender/blenkernel/intern/fcurve_test.cc +++ b/source/blender/blenkernel/intern/fcurve_test.cc @@ -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. */ diff --git a/source/blender/editors/armature/pose_slide.cc b/source/blender/editors/armature/pose_slide.cc index 95b41c82f59..7647d2549f7 100644 --- a/source/blender/editors/armature/pose_slide.cc +++ b/source/blender/editors/armature/pose_slide.cc @@ -1714,7 +1714,7 @@ static void propagate_curve_values(ListBase /*tPChanFCurveLink*/ *pflinks, const float current_fcu_value = evaluate_fcurve(fcu, source_frame); LISTBASE_FOREACH (FrameLink *, target_frame, target_frames) { insert_vert_fcurve( - fcu, {target_frame->frame, current_fcu_value}, settings, INSERTKEY_NEEDED); + fcu, {target_frame->frame, current_fcu_value}, settings, INSERTKEY_NOFLAGS); } } } diff --git a/source/blender/makesrna/intern/rna_animation_id.cc b/source/blender/makesrna/intern/rna_animation_id.cc index da97aa94410..5455fbd2e06 100644 --- a/source/blender/makesrna/intern/rna_animation_id.cc +++ b/source/blender/makesrna/intern/rna_animation_id.cc @@ -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( - binding, rna_path, array_index, {time, value}, settings); + animrig::SingleKeyingResult result = key_strip.keyframe_insert( + binding, rna_path, array_index, {time, value}, INSERTKEY_NOFLAGS, 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); } } diff --git a/source/blender/makesrna/intern/rna_fcurve.cc b/source/blender/makesrna/intern/rna_fcurve.cc index a1083b16c51..f964340e77d 100644 --- a/source/blender/makesrna/intern/rna_fcurve.cc +++ b/source/blender/makesrna/intern/rna_fcurve.cc @@ -1065,11 +1065,14 @@ 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; }