From 8dbd167e80217dd0ad97113f16dd184224c52fa5 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Tue, 7 May 2024 15:06:57 +0200 Subject: [PATCH] Anim: add failure propagation to more lower-level keying functions This changes the following keyframing functions to return `SingleKeyingResult`, which is in turn used for better failure reporting in the higher-level functions that call them: - `KeyframeStrip::keyframe_insert()` - `insert_vert_fcurve()` As a side effect, this also means that `rna_KeyframeAnimationStrip_key_insert()` can no longer return an `FCurve *`, and now instead returns a bool indicating success. This is part of an ongoing progressive refactor to improve error messages and failure handling in the keyingframing code. Pull Request: https://projects.blender.org/blender/blender/pulls/121517 --- source/blender/animrig/ANIM_animation.hh | 10 +-- source/blender/animrig/ANIM_fcurve.hh | 14 ++-- source/blender/animrig/ANIM_keyframing.hh | 5 +- source/blender/animrig/intern/animation.cc | 30 ++------ .../blender/animrig/intern/animation_test.cc | 30 ++++---- source/blender/animrig/intern/fcurve.cc | 16 +++-- source/blender/animrig/intern/keyframing.cc | 23 ++++--- .../blender/blenkernel/intern/fcurve_test.cc | 69 ++++++++++++++----- .../makesrna/intern/rna_animation_id.cc | 30 ++++---- source/blender/makesrna/intern/rna_fcurve.cc | 8 ++- 10 files changed, 137 insertions(+), 98 deletions(-) diff --git a/source/blender/animrig/ANIM_animation.hh b/source/blender/animrig/ANIM_animation.hh index 0d349421b4c..36b7396c861 100644 --- a/source/blender/animrig/ANIM_animation.hh +++ b/source/blender/animrig/ANIM_animation.hh @@ -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"); diff --git a/source/blender/animrig/ANIM_fcurve.hh b/source/blender/animrig/ANIM_fcurve.hh index 654fdbb2ecd..32f8c5593fd 100644 --- a/source/blender/animrig/ANIM_fcurve.hh +++ b/source/blender/animrig/ANIM_fcurve.hh @@ -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. diff --git a/source/blender/animrig/ANIM_keyframing.hh b/source/blender/animrig/ANIM_keyframing.hh index d8940005f2d..d7a0e046686 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,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, diff --git a/source/blender/animrig/intern/animation.cc b/source/blender/animrig/intern/animation.cc index df85fc85749..ac7c6e4859e 100644 --- a/source/blender/animrig/intern/animation.cc +++ b/source/blender/animrig/intern/animation.cc @@ -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. */ diff --git a/source/blender/animrig/intern/animation_test.cc b/source/blender/animrig/intern/animation_test.cc index d5958705efa..aed093c8685 100644 --- a/source/blender/animrig/intern/animation_test.cc +++ b/source/blender/animrig/intern/animation_test.cc @@ -437,10 +437,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( + 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 diff --git a/source/blender/animrig/intern/fcurve.cc b/source/blender/animrig/intern/fcurve.cc index c47fb3c17d7..aaf70b9681b 100644 --- a/source/blender/animrig/intern/fcurve.cc +++ b/source/blender/animrig/intern/fcurve.cc @@ -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, diff --git a/source/blender/animrig/intern/keyframing.cc b/source/blender/animrig/intern/keyframing.cc index b647be7b72e..60ae8a9fff7 100644 --- a/source/blender/animrig/intern/keyframing.cc +++ b/source/blender/animrig/intern/keyframing.cc @@ -92,6 +92,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) { @@ -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; } 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/makesrna/intern/rna_animation_id.cc b/source/blender/makesrna/intern/rna_animation_id.cc index da97aa94410..24368e66978 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( + 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); } } diff --git a/source/blender/makesrna/intern/rna_fcurve.cc b/source/blender/makesrna/intern/rna_fcurve.cc index a1083b16c51..094d71522a1 100644 --- a/source/blender/makesrna/intern/rna_fcurve.cc +++ b/source/blender/makesrna/intern/rna_fcurve.cc @@ -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; }