blender/blender
170
528
Fork 804
Code Issues 6.6k Pull Requests 955 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 +81 -247
6 changed files with 81 additions and 247 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 23bb1ddd8a - Show all commits

54
source/blender/animrig/ANIM_keyframing.hh
View File

@ -81,25 +81,33 @@ class CombinedKeyingResult {
void update_autoflags_fcurve_direct(FCurve *fcu, PropertyRNA *prop);
/**
* \brief Main Insert Key-framing API call.
* \brief Main key-frame insertion API.
*
* Use this to create any necessary animation data, and then insert a keyframe
* using the current value being keyframed, in the relevant place.
* Insert keys for `id` for all paths in `rna_paths`. It creates any necessary
* animation data (AnimData, Action, ...) if it doesn't already exist in order
* to do this.
*
* \param flag: Used for special settings that alter the behavior of the keyframe insertion.
* These include the 'visual' key-framing modes, quick refresh, and extra keyframe filtering.
* \param rna_paths: the RNA paths to insert keys for. Note that an empty array
* index is treated as "key all elements" for array properties.
*
* \param array_index: The index to key or -1 keys all array indices.
* \return The number of key-frames inserted.
* \param channel_group: the channel group to put any newly created fcurves
* under. If not given, the standard groups for each RNA path are used.
*
* \param scene_frame: the frame to insert the keys at. It should be in scene
* time, not NLA mapped (NLA mapping is already handled internally by this
* function). If not given, the current scene time is used.
*
* \returns A summary of the successful and failed keyframe insertions, with
* reasons for the failures.
*/
CombinedKeyingResult insert_keyframe(Main *bmain,
ID &id,
const char group[],
const char rna_path[],
int array_index,
const AnimationEvalContext *anim_eval_context,
eBezTriple_KeyframeType keytype,
eInsertKeyFlags flag);
CombinedKeyingResult insert_key_rna(Main *bmain,
ID &id,
std::optional<std::string> channel_group,
const blender::Span<RNAPath> rna_paths,
std::optional<float> scene_frame,
const AnimationEvalContext &anim_eval_context,
eBezTriple_KeyframeType key_type,
eInsertKeyFlags insert_key_flags);
/**
* \brief Secondary Insert Key-framing API call.
@ -254,20 +262,4 @@ CombinedKeyingResult insert_key_action(Main *bmain,
eInsertKeyFlags insert_key_flag,
eBezTriple_KeyframeType key_type,
BitSpan keying_mask);
/**
* Insert keys to the ID of the given PointerRNA for the given RNA paths. Tries to create an
* action if none exists yet.
* \param scene_frame: is expected to be not NLA mapped as that happens within the function.
* \returns How often keyframe insertion was successful and how often it failed / for which reason.
*/
CombinedKeyingResult insert_key_rna(Main *bmain,
ID &id,
std::optional<std::string> channel_group,
const blender::Span<RNAPath> rna_paths,
std::optional<float> scene_frame,
const AnimationEvalContext &anim_eval_context,
eBezTriple_KeyframeType key_type,
eInsertKeyFlags insert_key_flags);
} // namespace blender::animrig

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

@ -377,7 +377,8 @@ static eFCU_Cycle_Type remap_cyclic_keyframe_location(FCurve *fcu, float *px, fl
return type;
}
static float nla_time_remap(const AnimationEvalContext *anim_eval_context,
static float nla_time_remap(float time,
const AnimationEvalContext *anim_eval_context,
PointerRNA *id_ptr,
AnimData *adt,
bAction *act,
@ -388,13 +389,12 @@ static float nla_time_remap(const AnimationEvalContext *anim_eval_context,
*r_nla_context = BKE_animsys_get_nla_keyframing_context(
nla_cache, id_ptr, adt, anim_eval_context);
const float remapped_frame = BKE_nla_tweakedit_remap(
adt, anim_eval_context->eval_time, NLATIME_CONVERT_UNMAP);
const float remapped_frame = BKE_nla_tweakedit_remap(adt, time, NLATIME_CONVERT_UNMAP);
return remapped_frame;
}
*r_nla_context = nullptr;
return anim_eval_context->eval_time;
return time;
}
/* Insert the specified keyframe value into a single F-Curve. */
@ -550,175 +550,6 @@ static SingleKeyingResult insert_keyframe_fcurve_value(Main *bmain,
return result;
}
CombinedKeyingResult insert_keyframe(Main *bmain,
ID &id,
const char group[],
const char rna_path[],
int array_index,
const AnimationEvalContext *anim_eval_context,
eBezTriple_KeyframeType keytype,
eInsertKeyFlags flag)
{
CombinedKeyingResult combined_result;
if (!BKE_id_is_editable(bmain, &id)) {
combined_result.add(SingleKeyingResult::ID_NOT_EDITABLE);
return combined_result;
}
PointerRNA ptr;
PropertyRNA *prop = nullptr;
PointerRNA id_ptr = RNA_id_pointer_create(&id);
if (RNA_path_resolve_property(&id_ptr, rna_path, &ptr, &prop) == false) {
combined_result.add(SingleKeyingResult::CANNOT_RESOLVE_PATH);
return combined_result;
}
bAction *act = id_action_ensure(bmain, &id);
if (act == nullptr) {
combined_result.add(SingleKeyingResult::ID_NOT_ANIMATABLE);
return combined_result;
}
/* Apply NLA-mapping to frame to use (if applicable). */
NlaKeyframingContext *nla_context = nullptr;
ListBase nla_cache = {nullptr, nullptr};
AnimData *adt = BKE_animdata_from_id(&id);
const float nla_mapped_frame = nla_time_remap(
anim_eval_context, &id_ptr, adt, act, &nla_cache, &nla_context);
const bool visual_keyframing = flag & INSERTKEY_MATRIX;
Vector<float> values = get_keyframe_values(&ptr, prop, visual_keyframing);
bool force_all;
BitVector<> successful_remaps(values.size(), false);
BKE_animsys_nla_remap_keyframe_values(nla_context,
&ptr,
prop,
values,
array_index,
anim_eval_context,
&force_all,
successful_remaps);
/* Key the entire array. */
int key_count = 0;
if (array_index == -1 || force_all) {
/* In force mode, if any of the curves succeeds, drop the replace mode and restart. */
if (force_all && (flag & (INSERTKEY_REPLACE | INSERTKEY_AVAILABLE)) != 0) {
int exclude = -1;
for (array_index = 0; array_index < values.size(); array_index++) {
if (!successful_remaps[array_index]) {
continue;
}
const SingleKeyingResult result = insert_keyframe_fcurve_value(bmain,
&ptr,
prop,
act,
group,
rna_path,
array_index,
nla_mapped_frame,
values[array_index],
keytype,
flag);
combined_result.add(result);
if (result == SingleKeyingResult::SUCCESS) {
key_count++;
exclude = array_index;
break;
}
}
if (exclude != -1) {
flag &= ~(INSERTKEY_REPLACE | INSERTKEY_AVAILABLE);
for (array_index = 0; array_index < values.size(); array_index++) {
if (!successful_remaps[array_index]) {
continue;
}
if (array_index != exclude) {
const SingleKeyingResult result = insert_keyframe_fcurve_value(bmain,
&ptr,
prop,
act,
group,
rna_path,
array_index,
nla_mapped_frame,
values[array_index],
keytype,
flag);
combined_result.add(result);
if (result == SingleKeyingResult::SUCCESS) {
key_count++;
}
}
}
}
}
/* Simply insert all channels. */
else {
for (array_index = 0; array_index < values.size(); array_index++) {
if (!successful_remaps[array_index]) {
continue;
}
const SingleKeyingResult result = insert_keyframe_fcurve_value(bmain,
&ptr,
prop,
act,
group,
rna_path,
array_index,
nla_mapped_frame,
values[array_index],
keytype,
flag);
combined_result.add(result);
if (result == SingleKeyingResult::SUCCESS) {
key_count++;
}
}
}
}
/* Key a single index. */
else {
if (array_index >= 0 && array_index < values.size() && successful_remaps[array_index]) {
const SingleKeyingResult result = insert_keyframe_fcurve_value(bmain,
&ptr,
prop,
act,
group,
rna_path,
array_index,
nla_mapped_frame,
values[array_index],
keytype,
flag);
combined_result.add(result);
if (result == SingleKeyingResult::SUCCESS) {
key_count++;
}
}
}
BKE_animsys_free_nla_keyframing_context_cache(&nla_cache);
if (key_count > 0) {
if (act != nullptr) {
DEG_id_tag_update(&act->id, ID_RECALC_ANIMATION_NO_FLUSH);
}
if (adt != nullptr && adt->action != nullptr && adt->action != act) {
DEG_id_tag_update(&adt->action->id, ID_RECALC_ANIMATION_NO_FLUSH);
}
}
return combined_result;
}
/* ************************************************** */
/* KEYFRAME DELETION */
@ -973,55 +804,55 @@ CombinedKeyingResult insert_key_rna(Main *bmain,
const eInsertKeyFlags insert_key_flags)
{
PointerRNA rna_pointer = RNA_id_pointer_create(&id);
bAction *action = id_action_ensure(bmain, &id);
PointerRNA id_pointer = RNA_id_pointer_create(&id);
CombinedKeyingResult combined_result;
const float frame = scene_frame.value_or(anim_eval_context.eval_time);
bAction *action = id_action_ensure(bmain, &id);
if (action == nullptr) {
combined_result.add(SingleKeyingResult::ID_NOT_ANIMATABLE);
return combined_result;
}
AnimData *adt = BKE_animdata_from_id(&id);
BLI_assert(adt != nullptr);
/* Keyframing functions can deal with the nla_context being a nullptr. */
/* NOTE: keyframing functions can deal with the nla_context being a nullptr. */
ListBase nla_cache = {nullptr, nullptr};
NlaKeyframingContext *nla_context = nullptr;
if (adt && adt->action == action) {
PointerRNA id_pointer = RNA_id_pointer_create(&id);
nla_context = BKE_animsys_get_nla_keyframing_context(
&nla_cache, &id_pointer, adt, &anim_eval_context);
}
const float nla_frame = BKE_nla_tweakedit_remap(adt, frame, NLATIME_CONVERT_UNMAP);
const float nla_frame = nla_time_remap(scene_frame.value_or(anim_eval_context.eval_time),
&anim_eval_context,
&id_pointer,
adt,
action,
&nla_cache,
&nla_context);
const bool visual_keyframing = insert_key_flags & INSERTKEY_MATRIX;
for (const RNAPath &rna_path : rna_paths) {
PointerRNA ptr;
PropertyRNA *prop = nullptr;
const bool path_resolved = RNA_path_resolve_property(
&rna_pointer, rna_path.path.c_str(), &ptr, &prop);
&id_pointer, rna_path.path.c_str(), &ptr, &prop);
if (!path_resolved) {
combined_result.add(SingleKeyingResult::CANNOT_RESOLVE_PATH);
continue;
}
nathanvegdahl marked this conversation as resolved Outdated
Christoph Lendenfeld commented 2024-05-23 17:00:16 +02:00
Outdated
Review
Copy Link

I think it would make sense to pass in the scene_frame and a Depsgraph pointer, both required, instead of sometimes using the time from the eval context.
Considering an AnimEvalContext is exactly that it would make things a lot clearer. We can always construct an AnimEvalContext from that data if needed.

Edit: or just pass an AnimEvalContext and use that exclusively

I think it would make sense to pass in the scene_frame and a Depsgraph pointer, both required, instead of sometimes using the time from the eval context. Considering an AnimEvalContext is exactly that it would make things a lot clearer. We can always construct an AnimEvalContext from that data if needed. Edit: or just pass an AnimEvalContext and use that exclusively
Nathan Vegdahl commented 2024-05-28 14:53:39 +02:00
Outdated
Review
Copy Link

(I've clarified this in the PR description now.)

I agree that we should move toward that. For this PR I'm just giving the function all the capabilities of the two it was created from, and trying to make minimal changes to the code that called each function. In this case insert_keyframe() implicitly used scene time whereas insert_key_rna() took the time explicitly. So this line accommodates both behaviors.

I was thinking we could strip this function down in those ways in a separate PR.

(I've clarified this in the PR description now.) I agree that we should move toward that. For this PR I'm just giving the function all the capabilities of the two it was created from, and trying to make minimal changes to the code that called each function. In this case `insert_keyframe()` implicitly used scene time whereas `insert_key_rna()` took the time explicitly. So this line accommodates both behaviors. I was thinking we could strip this function down in those ways in a separate PR.
Christoph Lendenfeld commented 2024-05-28 15:54:34 +02:00
Outdated
Review
Copy Link

sounds good. Got nothing against reworking this in a separate PR

sounds good. Got nothing against reworking this in a separate PR
👍 1
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);
BitVector<> successful_remaps(rna_values.size(), false);
BKE_animsys_nla_remap_keyframe_values(nla_context,
&rna_pointer,
&id_pointer,
prop,
rna_values.as_mutable_span(),
rna_path.index.value_or(-1),
&anim_eval_context,
nullptr,
successful_remaps);
const CombinedKeyingResult result = insert_key_action(bmain,
action,
&rna_pointer,
&id_pointer,
prop,
channel_group,
rna_path_id_to_prop->c_str(),
@ -1032,8 +863,16 @@ CombinedKeyingResult insert_key_rna(Main *bmain,
successful_remaps);
combined_result.merge(result);
}
BKE_animsys_free_nla_keyframing_context_cache(&nla_cache);
if (combined_result.get_count(SingleKeyingResult::SUCCESS) > 0) {
DEG_id_tag_update(&action->id, ID_RECALC_ANIMATION_NO_FLUSH);
if (adt->action != nullptr && adt->action != action) {
DEG_id_tag_update(&adt->action->id, ID_RECALC_ANIMATION_NO_FLUSH);
}
}
return combined_result;
}

4
source/blender/editors/animation/keyframing.cc
View File

@ -1049,8 +1049,8 @@ static int insert_key_button_exec(bContext *C, wmOperator *op)
group = "Object Transforms";
}
/* NOTE: `index == -1` is a magic number, meaning "operate on all elements"
* or "not an array property". */
/* NOTE: `index == -1` is a magic number, meaning either "operate on all
* elements" or "not an array property". */
const std::optional<int> array_index = (all || index < 0) ? std::nullopt :
std::optional(index);

17
source/blender/editors/space_action/action_edit.cc
View File

@ -851,14 +851,15 @@ static void insert_fcurve_key(bAnimContext *ac,
* (TODO: add the full-blown PointerRNA relative parsing case here...)
*/
if (ale->id && !ale->owner) {
CombinedKeyingResult result = insert_keyframe(ac->bmain,
*ale->id,
((fcu->grp) ? (fcu->grp->name) : (nullptr)),
fcu->rna_path,
fcu->array_index,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag);
CombinedKeyingResult result = insert_key_rna(ac->bmain,
*ale->id,
fcu->grp ? std::optional(fcu->grp->name) :
std::nullopt,
{{fcu->rna_path, {}, fcu->array_index}},
std::nullopt,
anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag);
if (result.get_count(SingleKeyingResult::SUCCESS) == 0) {
result.generate_reports(reports);
}

17
source/blender/editors/space_graph/graph_edit.cc
View File

@ -206,14 +206,15 @@ static void insert_graph_keys(bAnimContext *ac, eGraphKeys_InsertKey_Types mode)
* up adding the keyframes on a new F-Curve in the action data instead.
*/
if (ale->id && !ale->owner && !fcu->driver) {
CombinedKeyingResult result = insert_keyframe(ac->bmain,
*ale->id,
((fcu->grp) ? (fcu->grp->name) : (nullptr)),
fcu->rna_path,
fcu->array_index,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag);
CombinedKeyingResult result = insert_key_rna(ac->bmain,
*ale->id,
fcu->grp ? std::optional(fcu->grp->name) :
std::nullopt,
{{fcu->rna_path, {}, fcu->array_index}},
std::nullopt,
anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag);
if (result.get_count(SingleKeyingResult::SUCCESS) == 0) {
result.generate_reports(reports);
}

17
source/blender/python/intern/bpy_rna_anim.cc
View File

@ -405,14 +405,15 @@ PyObject *pyrna_struct_keyframe_insert(BPy_StructRNA *self, PyObject *args, PyOb
ID *id = self->ptr.owner_id;
BLI_assert(BKE_id_is_in_global_main(id));
CombinedKeyingResult combined_result = insert_keyframe(G_MAIN,
*id,
group_name,
path_full,
index,
&anim_eval_context,
eBezTriple_KeyframeType(keytype),
eInsertKeyFlags(options));
CombinedKeyingResult combined_result = insert_key_rna(G_MAIN,
*id,
group_name ? std::optional(group_name) :
std::nullopt,
{{path_full, {}, index}},
std::nullopt,
anim_eval_context,
eBezTriple_KeyframeType(keytype),
eInsertKeyFlags(options));
const int success_count = combined_result.get_count(SingleKeyingResult::SUCCESS);
if (success_count == 0) {
combined_result.generate_reports(&reports);