blender
/
blender
134
425
Fork 606
Code Issues 6.3k Pull Requests 772 Projects 19 Wiki Activity

Refactor: remove ReportList argument from insert_key #120784

Merged
Christoph Lendenfeld merged 4 commits from ChrisLend/blender:refactor_remove_reports_from_key_fn into main 2024-04-23 10:10:34 +02:00
Conversation 2 Commits 4 Files Changed 8 +173 -154
8 changed files with 173 additions and 154 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 7fad0782d5 - Show all commits

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

@ -37,6 +37,9 @@ enum class SingleKeyingResult {
FCURVE_NOT_KEYFRAMEABLE,
NO_KEY_NEEDED,
UNABLE_TO_INSERT_TO_NLA_STACK,
ID_NOT_EDITABLE,
ID_NOT_ANIMATABLE,
CANNOT_RESOLVE_PATH,
/* Make sure to always keep this at the end of the enum. */
_KEYING_RESULT_MAX,
};
@ -85,15 +88,14 @@ void update_autoflags_fcurve_direct(FCurve *fcu, PropertyRNA *prop);
* \param array_index: The index to key or -1 keys all array indices.
* \return The number of key-frames inserted.
*/
int insert_keyframe(Main *bmain,
ReportList *reports,
ID *id,
const char group[],
const char rna_path[],
int array_index,
const AnimationEvalContext *anim_eval_context,
eBezTriple_KeyframeType keytype,
eInsertKeyFlags flag);
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);
/**
* \brief Secondary Insert Key-framing API call.

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

@ -556,54 +556,53 @@ static SingleKeyingResult insert_keyframe_fcurve_value(Main *bmain,
return result;
}
int insert_keyframe(Main *bmain,
ReportList *reports,
ID *id,
const char group[],
const char rna_path[],
int array_index,
const AnimationEvalContext *anim_eval_context,
eBezTriple_KeyframeType keytype,
eInsertKeyFlags flag)
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)
{
if (id == nullptr) {
BKE_reportf(reports, RPT_ERROR, "No ID block to insert keyframe in (path = %s)", rna_path);
return 0;
}
CombinedKeyingResult combined_result;
if (!BKE_id_is_editable(bmain, id)) {
BKE_reportf(reports, RPT_ERROR, "'%s' on %s is not editable", rna_path, id->name + 2);
return 0;
if (!BKE_id_is_editable(bmain, &id)) {
// BKE_reportf(reports, RPT_ERROR, "'%s' on %s is not editable", rna_path, id->name + 2);
combined_result.add(SingleKeyingResult::ID_NOT_EDITABLE);
return combined_result;
}
PointerRNA ptr;
PropertyRNA *prop = nullptr;
PointerRNA id_ptr = RNA_id_pointer_create(id);
PointerRNA id_ptr = RNA_id_pointer_create(&id);
if (RNA_path_resolve_property(&id_ptr, rna_path, &ptr, &prop) == false) {
BKE_reportf(
/* BKE_reportf(
reports,
RPT_ERROR,
"Could not insert keyframe, as RNA path is invalid for the given ID (ID = %s, path = %s)",
(id) ? id->name : RPT_("<Missing ID block>"),
rna_path);
return 0;
rna_path); */
combined_result.add(SingleKeyingResult::CANNOT_RESOLVE_PATH);
return combined_result;
}
bAction *act = id_action_ensure(bmain, id);
bAction *act = id_action_ensure(bmain, &id);
if (act == nullptr) {
BKE_reportf(reports,
/* BKE_reportf(reports,
RPT_ERROR,
"Could not insert keyframe, as this type does not support animation data (ID = "
"%s, path = %s)",
id->name,
rna_path);
return 0;
rna_path); */
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);
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);
@ -611,17 +610,15 @@ int insert_keyframe(Main *bmain,
Vector<float> values = get_keyframe_values(&ptr, prop, visual_keyframing);
bool force_all;
BitVector<> successful_remaps = nla_map_keyframe_values_and_generate_reports(
values.as_mutable_span(),
array_index,
ptr,
*prop,
nla_context,
anim_eval_context,
reports,
&force_all);
CombinedKeyingResult combined_result;
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;
@ -738,11 +735,7 @@ int insert_keyframe(Main *bmain,
}
}
if (key_count == 0) {
combined_result.generate_reports(reports);
}
return key_count;
return combined_result;
}
/* ************************************************** */

77
source/blender/animrig/intern/keyframing_auto.cc
View File

@ -100,6 +100,10 @@ bool autokeyframe_cfra_can_key(const Scene *scene, ID *id)
void autokeyframe_object(bContext *C, Scene *scene, Object *ob, Span<std::string> rna_paths)
{
ID *id = &ob->id;
if (id == nullptr) {
nathanvegdahl marked this conversation as resolved Outdated
Nathan Vegdahl commented 2024-04-19 11:36:46 +02:00
Outdated
Review
Copy Link

I think(?) this code path will never actually be taken, unless via some weird undefined behavior: the id field is just a struct directly inside the Object type, so taking its address should never be null.

However, in theory ob itself could be null. So we can check for that. But it looks like that isn't supposed to ever happen, so I would just make an assert for it (and possibly document it as an invariant in the function's doc comment, if it isn't already). Same with the C and scene parameters, for that matter.

I think(?) this code path will never actually be taken, unless via some weird undefined behavior: the `id` field is just a struct directly inside the `Object` type, so taking its address should never be null. However, in theory `ob` itself could be null. So we can check for that. But it looks like that isn't supposed to ever happen, so I would just make an assert for it (and possibly document it as an invariant in the function's doc comment, if it isn't already). Same with the `C` and `scene` parameters, for that matter.
Christoph Lendenfeld commented 2024-04-19 13:45:13 +02:00
Outdated
Review
Copy Link

yeah you are right. In that case I'll remove the check.

yeah you are right. In that case I'll remove the check.
return;
}
if (!autokeyframe_cfra_can_key(scene, id)) {
return;
}
@ -134,16 +138,20 @@ void autokeyframe_object(bContext *C, Scene *scene, Object *ob, Span<std::string
Main *bmain = CTX_data_main(C);
if (adt && adt->action) {
CombinedKeyingResult combined_result;
LISTBASE_FOREACH (FCurve *, fcu, &adt->action->curves) {
insert_keyframe(bmain,
reports,
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_keyframe(bmain,
*id,
(fcu->grp ? fcu->grp->name : nullptr),
fcu->rna_path,
fcu->array_index,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag);
combined_result.merge(result);
}
if (combined_result.get_count(SingleKeyingResult::SUCCESS) == 0) {
combined_result.generate_reports(reports);
}
}
return;
@ -211,6 +219,10 @@ void autokeyframe_pose_channel(bContext *C,
AnimData *adt = ob->adt;
bAction *act = (adt) ? adt->action : nullptr;
if (id == nullptr) {
nathanvegdahl marked this conversation as resolved Outdated
Nathan Vegdahl commented 2024-04-19 12:03:04 +02:00
Outdated
Review
Copy Link

Same here: I don't think(?) this code path is possible. Should be replaced by an assert that ob is non-null.

Same here: I don't think(?) this code path is possible. Should be replaced by an assert that `ob` is non-null.
👍 1
return;
}
if (!blender::animrig::autokeyframe_cfra_can_key(scene, id)) {
return;
}
@ -234,12 +246,12 @@ void autokeyframe_pose_channel(bContext *C,
flag |= INSERTKEY_MATRIX;
}
blender::Vector<PointerRNA> sources;
Vector<PointerRNA> sources;
/* Add data-source override for the camera object. */
ANIM_relative_keyingset_add_source(sources, id, &RNA_PoseBone, pose_channel);
/* only insert into active keyingset? */
if (blender::animrig::is_keying_flag(scene, AUTOKEY_FLAG_ONLYKEYINGSET) && (active_ks)) {
if (is_keying_flag(scene, AUTOKEY_FLAG_ONLYKEYINGSET) && (active_ks)) {
/* Run the active Keying Set on the current data-source. */
ANIM_apply_keyingset(
C, &sources, active_ks, MODIFYKEY_MODE_INSERT, anim_eval_context.eval_time);
@ -247,7 +259,7 @@ void autokeyframe_pose_channel(bContext *C,
}
/* only insert into available channels? */
if (blender::animrig::is_keying_flag(scene, AUTOKEY_FLAG_INSERTAVAILABLE)) {
if (is_keying_flag(scene, AUTOKEY_FLAG_INSERTAVAILABLE)) {
if (!act) {
return;
}
@ -262,15 +274,14 @@ void autokeyframe_pose_channel(bContext *C,
* NOTE: this will do constraints too, but those are ok to do here too?
*/
if (STREQ(pchan_name, pose_channel->name)) {
blender::animrig::insert_keyframe(bmain,
reports,
id,
((fcu->grp) ? (fcu->grp->name) : (nullptr)),
fcu->rna_path,
fcu->array_index,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag);
insert_keyframe(bmain,
nathanvegdahl marked this conversation as resolved Outdated
Nathan Vegdahl commented 2024-04-19 12:06:42 +02:00
Outdated
Review
Copy Link

Do we not need to accumulate and handle the returned CombinedKeyingResult here?

Do we not need to accumulate and handle the returned `CombinedKeyingResult` here?
Christoph Lendenfeld commented 2024-04-19 13:49:54 +02:00
Outdated
Review
Copy Link

good point, the function didn't handle the return argument before, but of course now we would be missing the messages

good point, the function didn't handle the return argument before, but of course now we would be missing the messages
*id,
((fcu->grp) ? (fcu->grp->name) : (nullptr)),
fcu->rna_path,
fcu->array_index,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag);
}
}
return;
@ -302,7 +313,6 @@ bool autokeyframe_property(bContext *C,
bAction *action;
bool driven;
bool special;
bool changed = false;
/* For entire array buttons we check the first component, it's not perfect
* but works well enough in typical cases. */
@ -313,13 +323,14 @@ bool autokeyframe_property(bContext *C,
/* Only early out when we actually want an existing F-curve already
* (e.g. auto-keyframing from buttons). */
if (fcu == nullptr && (driven || special || only_if_property_keyed)) {
return changed;
return false;
}
if (driven) {
return false;
}
bool changed = false;
if (special) {
/* NLA Strip property. */
if (is_autokey_on(scene)) {
@ -354,16 +365,16 @@ bool autokeyframe_property(bContext *C,
* E.g., color wheels (see #42567). */
BLI_assert((fcu->array_index == rnaindex) || (rnaindex == -1));
}
changed = insert_keyframe(bmain,
reports,
id,
(fcu && fcu->grp) ? fcu->grp->name : nullptr,
fcu ? fcu->rna_path : (path ? path->c_str() : nullptr),
rnaindex,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag) != 0;
CombinedKeyingResult result = insert_keyframe(bmain,
*id,
(fcu && fcu->grp) ? fcu->grp->name : nullptr,
fcu ? fcu->rna_path :
(path ? path->c_str() : nullptr),
rnaindex,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag);
changed = result.get_count(SingleKeyingResult::SUCCESS) != 0;
WM_event_add_notifier(C, NC_ANIMATION | ND_KEYFRAME | NA_EDITED, nullptr);
}
}

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

@ -962,6 +962,7 @@ void ANIM_OT_keyframe_delete_v3d(wmOperatorType *ot)
static int insert_key_button_exec(bContext *C, wmOperator *op)
{
using namespace blender::animrig;
Main *bmain = CTX_data_main(C);
Scene *scene = CTX_data_scene(C);
ToolSettings *ts = scene->toolsettings;
@ -992,15 +993,14 @@ static int insert_key_button_exec(bContext *C, wmOperator *op)
FCurve *fcu = BKE_fcurve_find(&strip->fcurves, RNA_property_identifier(prop), index);
if (fcu) {
changed = blender::animrig::insert_keyframe_direct(
op->reports,
ptr,
prop,
fcu,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
nullptr,
eInsertKeyFlags(0));
changed = insert_keyframe_direct(op->reports,
ptr,
prop,
fcu,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
nullptr,
eInsertKeyFlags(0));
}
else {
BKE_report(op->reports,
@ -1017,19 +1017,18 @@ static int insert_key_button_exec(bContext *C, wmOperator *op)
C, &ptr, prop, index, nullptr, nullptr, &driven, &special);
if (fcu && driven) {
const float driver_frame = blender::animrig::evaluate_driver_from_rna_pointer(
const float driver_frame = evaluate_driver_from_rna_pointer(
&anim_eval_context, &ptr, prop, fcu);
AnimationEvalContext remapped_context = BKE_animsys_eval_context_construct(
CTX_data_depsgraph_pointer(C), driver_frame);
changed = blender::animrig::insert_keyframe_direct(
op->reports,
ptr,
prop,
fcu,
&remapped_context,
eBezTriple_KeyframeType(ts->keyframe_type),
nullptr,
INSERTKEY_NOFLAGS);
changed = insert_keyframe_direct(op->reports,
ptr,
prop,
fcu,
&remapped_context,
eBezTriple_KeyframeType(ts->keyframe_type),
nullptr,
INSERTKEY_NOFLAGS);
}
}
else {
@ -1064,15 +1063,15 @@ static int insert_key_button_exec(bContext *C, wmOperator *op)
index = -1;
}
changed = (blender::animrig::insert_keyframe(bmain,
op->reports,
ptr.owner_id,
group,
path->c_str(),
index,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag) != 0);
CombinedKeyingResult result = insert_keyframe(bmain,
*ptr.owner_id,
group,
path->c_str(),
index,
&anim_eval_context,
eBezTriple_KeyframeType(ts->keyframe_type),
flag);
changed = result.get_count(SingleKeyingResult::SUCCESS) != 0;
}
else {
BKE_report(op->reports,

40
source/blender/editors/animation/keyingsets.cc
View File

@ -1020,6 +1020,7 @@ static int insert_key_to_keying_set_path(bContext *C,
const eModifyKey_Modes mode,
const float frame)
{
using namespace blender::animrig;
/* Since keying settings can be defined on the paths too,
* apply the settings for this path first. */
const eInsertKeyFlags path_insert_key_flags = keyingset_apply_keying_flags(
@ -1074,29 +1075,36 @@ static int insert_key_to_keying_set_path(bContext *C,
const AnimationEvalContext anim_eval_context = BKE_animsys_eval_context_construct(depsgraph,
frame);
int keyed_channels = 0;
CombinedKeyingResult combined_result;
for (; array_index < array_length; array_index++) {
if (mode == MODIFYKEY_MODE_INSERT) {
keyed_channels += blender::animrig::insert_keyframe(bmain,
reports,
keyingset_path->id,
groupname,
keyingset_path->rna_path,
array_index,
&anim_eval_context,
keytype,
path_insert_key_flags);
CombinedKeyingResult result = insert_keyframe(bmain,
*keyingset_path->id,
groupname,
keyingset_path->rna_path,
array_index,
&anim_eval_context,
keytype,
path_insert_key_flags);
keyed_channels += result.get_count(SingleKeyingResult::SUCCESS);
combined_result.merge(result);
}
else if (mode == MODIFYKEY_MODE_DELETE) {
keyed_channels += blender::animrig::delete_keyframe(bmain,
reports,
keyingset_path->id,
nullptr,
keyingset_path->rna_path,
array_index,
frame);
keyed_channels += delete_keyframe(bmain,
reports,
keyingset_path->id,
nullptr,
keyingset_path->rna_path,
array_index,
frame);
}
}
if (combined_result.get_count(SingleKeyingResult::SUCCESS) == 0) {
combined_result.generate_reports(reports);
}
switch (GS(keyingset_path->id->name)) {
case ID_OB: /* Object (or Object-Related) Keyframes */
{

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

@ -837,15 +837,17 @@ static void insert_fcurve_key(bAnimContext *ac,
* (TODO: add the full-blown PointerRNA relative parsing case here...)
*/
if (ale->id && !ale->owner) {
insert_keyframe(ac->bmain,
reports,
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_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);
if (result.get_count(SingleKeyingResult::SUCCESS) == 0) {
result.generate_reports(reports);
}
}
else {
AnimData *adt = ANIM_nla_mapping_get(ac, ale);

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

@ -206,15 +206,17 @@ 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) {
insert_keyframe(ac->bmain,
reports,
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_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);
if (result.get_count(SingleKeyingResult::SUCCESS) == 0) {
result.generate_reports(reports);
}
}
else {
AnimData *adt = ANIM_nla_mapping_get(ac, ale);

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

@ -404,15 +404,17 @@ 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));
result = (blender::animrig::insert_keyframe(G_MAIN,
&reports,
id,
group_name,
path_full,
index,
&anim_eval_context,
eBezTriple_KeyframeType(keytype),
eInsertKeyFlags(options)) != 0);
blender::animrig::CombinedKeyingResult combined_result = blender::animrig::insert_keyframe(
G_MAIN,
*id,
group_name,
path_full,
index,
&anim_eval_context,
eBezTriple_KeyframeType(keytype),
eInsertKeyFlags(options));
combined_result.generate_reports(&reports);
result = combined_result.get_count(blender::animrig::SingleKeyingResult::SUCCESS) != 0;
}
MEM_freeN((void *)path_full);