blender/blender
172
533
Fork 807
Code Issues 6.5k Pull Requests 969 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 +4 -3
1 changed files with 4 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit c8bd2e109f - Show all commits

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

@ -823,7 +823,7 @@ CombinedKeyingResult insert_keyframes(Main *bmain,
const std::optional<float> scene_frame,
const AnimationEvalContext &anim_eval_context,
const eBezTriple_KeyframeType key_type,
eInsertKeyFlags insert_key_flags)
const eInsertKeyFlags insert_key_flags)
{
ID *id = struct_pointer->owner_id;
@ -887,6 +887,7 @@ CombinedKeyingResult insert_keyframes(Main *bmain,
/* Handle the `force_all` condition mentioned above, ensuring the
* "all-or-nothing" behavior if needed. */
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
eInsertKeyFlags insert_key_flags_adjusted = insert_key_flags;
if (force_all && rna_values.size() > 0 &&
(insert_key_flags & (INSERTKEY_REPLACE | INSERTKEY_AVAILABLE)) != 0)
{
@ -913,7 +914,7 @@ 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);
insert_key_flags_adjusted &= ~(INSERTKEY_REPLACE | INSERTKEY_AVAILABLE);
break;
}
}
@ -926,7 +927,7 @@ CombinedKeyingResult insert_keyframes(Main *bmain,
rna_path_id_to_prop->c_str(),
nla_frame,
rna_values.as_span(),
insert_key_flags,
insert_key_flags_adjusted,
key_type,
elements_to_key);
combined_result.merge(result);