blender/blender
170
528
Fork 805
Code Issues 6.6k Pull Requests 957 Projects 21 Wiki Activity

Refactor: combine insert_keyframe() and insert_key_rna() into a single function #122053

Merged
Nathan Vegdahl merged 49 commits from nathanvegdahl/blender:combine_keying_functions into main 2024-06-11 16:43:08 +02:00
Conversation 8 Commits 49 Files Changed 9 +14 -15
1 changed files with 14 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 6161a4d77a - Show all commits

29
source/blender/animrig/intern/keyframing.cc
View File

@ -865,11 +865,15 @@ CombinedKeyingResult insert_keyframes(Main *bmain,
Vector<float> rna_values = get_keyframe_values(&ptr, prop, visual_keyframing);
bool force_all;
BitVector<> elements_to_key(rna_values.size(), false);
/* NOTE: in addition to remapping, this also marks what elements of an array
* property will actually get keyed by `insert_key_action()` below via
* `elements_to_key`. Importantly, for quaternion properties it may choose
* to key all array elements regardless of the passed index parameter,
* depending on the NLA setup. */
/* NOTE: this function call is complex with interesting effects. Of
* particular note is that in addition to doing time remapping, it also:
* - Fills in `elements_to_key`, flagging which elements of an array
* property should actually get keyed.
* - Sets `force_all`, which if true means that an array property should be
* treated as all-or-nothing: either all elements get keyed or none do,
* regardless of how keying flags might have otherwise treated different
* elements differently. */
BKE_animsys_nla_remap_keyframe_values(nla_context,
&id_pointer,
prop,
@ -879,19 +883,12 @@ CombinedKeyingResult insert_keyframes(Main *bmain,
&force_all,
elements_to_key);
/* There is a conflict between forcing all elements to be keyed and
* replace-only or only-insert-available being enabled when not all elements
* meet the prerequisites to be keyed with those flags.
*
* One choice would be to fail keying all elements if any element isn't
* keyable with those flags. Another is to override the flags and insert
* keys on all elements anyway if any element *is* keyable with those flags.
* We take the latter approach here. */
/* Handle the `force_all` condition mentioned above, ensuring the
* "all-or-nothing" behavior if needed. */
if (force_all && rna_values.size() > 0 &&
(insert_key_flags & (INSERTKEY_REPLACE | INSERTKEY_AVAILABLE)) != 0)
nathanvegdahl marked this conversation as resolved Outdated
Christoph Lendenfeld commented 2024-05-23 17:03:21 +02:00
Outdated
Review
Copy Link

In what case would the action from the AnimData and the action we get from id_action_ensure not be the same?

In what case would the action from the AnimData and the action we get from `id_action_ensure` not be the same?
Nathan Vegdahl commented 2024-05-23 17:38:36 +02:00
Outdated
Review
Copy Link

That's a great question. I'm not sure myself, but insert_keyframe() checked for this, so I'm keeping it in for now just in case. (At first I thought it might be related to the NLA, but IIRC if there's an NLA action for keying then it's temporarily stored as the animdata action anyway.) It's something I think we can probably investigate and remove later, but for now it's a "don't remove code you don't understand" kind of situation.

That's a great question. I'm not sure myself, but `insert_keyframe()` checked for this, so I'm keeping it in for now just in case. (At first I thought it might be related to the NLA, but IIRC if there's an NLA action for keying then it's temporarily stored as the animdata action anyway.) It's something I think we can probably investigate and remove later, but for now it's a "don't remove code you don't understand" kind of situation.
Christoph Lendenfeld commented 2024-05-23 17:39:55 +02:00
Outdated
Review
Copy Link

Maybe add your findings as a comment then so it's easier to investigate later when we actually want to remove it.

Maybe add your findings as a comment then so it's easier to investigate later when we actually want to remove it.
👍 1
{
/* Determine if at least one element would succeed getting keyed given the
* state of the `INSERTKEY_REPLACE` and `INSERTKEY_AVAILABLE` flags. */
/* Determine if at least one element would succeed getting keyed. */
bool at_least_one_would_succeed = false;
for (int i = 0; i < rna_values.size(); i++) {
const FCurve *fcu = action_fcurve_find(action, rna_path_id_to_prop->c_str(), i);
@ -911,6 +908,8 @@ CombinedKeyingResult insert_keyframes(Main *bmain,
}
}
/* If at least one would succeed, then we disable all keying flags that
* would prevent the other elements from getting keyed as well. */
if (at_least_one_would_succeed) {
insert_key_flags &= ~(INSERTKEY_REPLACE | INSERTKEY_AVAILABLE);
break;