blender/blender
166
522
Fork 797
Code Issues 6.8k Pull Requests 948 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 +5 -3
1 changed files with 5 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit a1fce09b3f - Show all commits

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

@ -860,11 +860,10 @@ CombinedKeyingResult insert_keyframes(Main *bmain,
combined_result.add(SingleKeyingResult::CANNOT_RESOLVE_PATH);
continue;
}
const std::optional<std::string> rna_path_id_to_prop = RNA_path_from_ID_to_property(&ptr,
prop);
Vector<float> rna_values = get_keyframe_values(&ptr, prop, visual_keyframing);
bool force_all;
BitVector<> elements_to_key(rna_values.size(), false);
bool force_all;
/* NOTE: this function call is complex with interesting effects. Of
* particular note is that in addition to doing time remapping, it also:
@ -883,6 +882,9 @@ CombinedKeyingResult insert_keyframes(Main *bmain,
&force_all,
elements_to_key);
const std::optional<std::string> rna_path_id_to_prop = RNA_path_from_ID_to_property(&ptr,
prop);
/* 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
if (force_all && rna_values.size() > 0 &&