tmaq
/
tornavis
1
0
Fork 0
Code Issues 2 Pull Requests 29 Packages Projects 24 Releases Wiki Activity

Refactor: keyframing functions return more detailed status 

The keyframing functions up to this point created many
messages through the `ReportList` about what went wrong
when inserting keys.

This has 2 issues:
* Everyone that wants to insert keyframes needs to have
a `ReportList` which comes from `bContext`
* The keyframing functions are very verbose.
Providing the user with errors when there is actually nothing wrong.

To combat that, return a status enum from functions that deal
with a single FCurve.
Functions that deal with more than one FCurve collect those statuses
into a new class `CombinedKeyingResult`
Based on that we can create a single well worded error message,
instead of potentially flooding the user with individual messages.

The downside is that some info is lost, e.g. we do not
know which exact FCurve has failed.

This will show a formatted error when inserting keys with a keying
set or when pressing `I` over the property to keyframe.
But not when pressing `I` in the viewport. The latter can be fixed in a later patch.

Pull Request: https://projects.blender.org/blender/blender/pulls/117449
This commit is contained in:
Christoph Lendenfeld 2024-03-08 09:56:40 +01:00 committed by Christoph Lendenfeld
parent a444a5eeba
commit 57419c9653
2 changed files with 182 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
source/blender/animrig/CMakeLists.txt
View File

@ -11,6 +11,7 @@ set(INC
../editors/include
../makesrna
../windowmanager
../../../extern/fmtlib/include
# RNA_prototypes.h
${CMAKE_BINARY_DIR}/source/blender/makesrna
)

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

@ -10,6 +10,8 @@
#include <cmath>
#include <string>
#include <fmt/format.h>
#include "ANIM_action.hh"
#include "ANIM_animdata.hh"
#include "ANIM_fcurve.hh"
@ -46,6 +48,45 @@
namespace blender::animrig {
enum class SingleKeyingResult {
SUCCESS = 0,
CANNOT_CREATE_FCURVE,
FCURVE_NOT_KEYFRAMEABLE,
NO_KEY_NEEDED,
/* Make sure to always keep this at the end of the enum. */
_KEYING_RESULT_MAX,
};
class CombinedKeyingResult {
private:
/* The index to the array maps a `SingleKeyingResult` to the number of times this result has
* occurred. */
std::array<int, int(SingleKeyingResult::_KEYING_RESULT_MAX)> result_counter{0};
public:
void add(const SingleKeyingResult result)
{
result_counter[int(result)]++;
}
int get_count(const SingleKeyingResult result) const
{
return result_counter[int(result)];
}
bool has_errors() const
{
/* For loop starts at 1 to skip the SUCCESS flag. Assumes that SUCCESS is 0 and the rest of the
* enum are sequential values. */
for (int i = 1; i < result_counter.size(); i++) {
if (result_counter[i] > 0) {
return true;
}
}
return false;
}
};
void update_autoflags_fcurve_direct(FCurve *fcu, PropertyRNA *prop)
{
/* Set additional flags for the F-Curve (i.e. only integer values). */
@ -309,11 +350,11 @@ static float nla_time_remap(const AnimationEvalContext *anim_eval_context,
}
/* Insert the specified keyframe value into a single F-Curve. */
static bool insert_keyframe_value(
static SingleKeyingResult insert_keyframe_value(
FCurve *fcu, float cfra, float curval, eBezTriple_KeyframeType keytype, eInsertKeyFlags flag)
{
if (!BKE_fcurve_is_keyframable(fcu)) {
return false;
return SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE;
}
/* Adjust coordinates for cycle aware insertion. */
@ -329,17 +370,19 @@ static bool insert_keyframe_value(
if (flag & INSERTKEY_NEEDED) {
if (!new_key_needed(fcu, cfra, curval)) {
return false;
return SingleKeyingResult::NO_KEY_NEEDED;
}
if (insert_vert_fcurve(fcu, {cfra, curval}, settings, flag) < 0) {
return false;
return SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE;
}
return true;
return SingleKeyingResult::SUCCESS;
}
return insert_vert_fcurve(fcu, {cfra, curval}, settings, flag) >= 0;
if (insert_vert_fcurve(fcu, {cfra, curval}, settings, flag) < 0) {
return SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE;
}
return SingleKeyingResult::SUCCESS;
}
bool insert_keyframe_direct(ReportList *reports,
@ -401,9 +444,9 @@ bool insert_keyframe_direct(ReportList *reports,
}
const float cfra = anim_eval_context->eval_time;
const bool success = insert_keyframe_value(fcu, cfra, current_value, keytype, flag);
const SingleKeyingResult result = insert_keyframe_value(fcu, cfra, current_value, keytype, flag);
if (!success) {
if (result != SingleKeyingResult::SUCCESS) {
BKE_reportf(reports,
RPT_ERROR,
"Failed to insert keys on F-Curve with path '%s[%d]', ensure that it is not "
@ -411,22 +454,21 @@ bool insert_keyframe_direct(ReportList *reports,
fcu->rna_path,
fcu->array_index);
}
return success;
return result == SingleKeyingResult::SUCCESS;
}
/** Find or create the FCurve based on the given path, and insert the specified value into it. */
static bool insert_keyframe_fcurve_value(Main *bmain,
ReportList *reports,
PointerRNA *ptr,
PropertyRNA *prop,
bAction *act,
const char group[],
const char rna_path[],
int array_index,
const float fcurve_frame,
float curval,
eBezTriple_KeyframeType keytype,
eInsertKeyFlags flag)
static SingleKeyingResult insert_keyframe_fcurve_value(Main *bmain,
PointerRNA *ptr,
PropertyRNA *prop,
bAction *act,
const char group[],
const char rna_path[],
int array_index,
const float fcurve_frame,
float curval,
eBezTriple_KeyframeType keytype,
eInsertKeyFlags flag)
{
/* Make sure the F-Curve exists.
* - if we're replacing keyframes only, DO NOT create new F-Curves if they do not exist yet
@ -439,7 +481,7 @@ static bool insert_keyframe_fcurve_value(Main *bmain,
/* We may not have a F-Curve when we're replacing only. */
if (!fcu) {
return false;
return SingleKeyingResult::CANNOT_CREATE_FCURVE;
}
const bool is_new_curve = (fcu->totvert == 0);
@ -454,23 +496,51 @@ static bool insert_keyframe_fcurve_value(Main *bmain,
/* Update F-Curve flags to ensure proper behavior for property type. */
update_autoflags_fcurve_direct(fcu, prop);
const bool success = insert_keyframe_value(fcu, fcurve_frame, curval, keytype, flag);
if (!success && reports != nullptr) {
BKE_reportf(reports,
RPT_ERROR,
"Failed to insert keys on F-Curve with path '%s[%d]', ensure that it is not "
"locked or sampled, and try removing F-Modifiers",
fcu->rna_path,
fcu->array_index);
}
const SingleKeyingResult result = insert_keyframe_value(
fcu, fcurve_frame, curval, keytype, flag);
/* If the curve is new, make it cyclic if appropriate. */
if (is_cyclic_action && is_new_curve) {
make_new_fcurve_cyclic(fcu, {act->frame_start, act->frame_end});
}
return success;
return result;
}
static void generate_keyframe_reports_from_result(ReportList *reports,
const CombinedKeyingResult &result)
{
std::string error = "Inserting keyframes failed due to the following reasons:";
if (result.get_count(SingleKeyingResult::CANNOT_CREATE_FCURVE) > 0) {
const int error_count = result.get_count(SingleKeyingResult::CANNOT_CREATE_FCURVE);
error.append(
fmt::format("\n- Could not create {} F-Curve{}. This can happen when only inserting to "
"available F-Curves.",
error_count,
error_count > 1 ? "s" : ""));
}
if (result.get_count(SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE) > 0) {
const int error_count = result.get_count(SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE);
if (error_count == 1) {
error.append("\n- One F-Curve is not keyframeable. It might be locked or sampled.");
}
else {
error.append(fmt::format(
"\n- {} F-Curves are not keyframeable. They might be locked or sampled.", error_count));
}
}
if (result.get_count(SingleKeyingResult::NO_KEY_NEEDED) > 0) {
const int error_count = result.get_count(SingleKeyingResult::NO_KEY_NEEDED);
error.append(fmt::format(
"\n- Due to the setting 'Only Insert Needed', {} keyframe{} not been inserted.",
error_count,
error_count > 1 ? "s have" : " has"));
}
BKE_reportf(reports, RPT_ERROR, "%s", error.c_str());
}
int insert_keyframe(Main *bmain,
@ -540,6 +610,8 @@ int insert_keyframe(Main *bmain,
&force_all,
successful_remaps);
CombinedKeyingResult combined_result;
/* Key the entire array. */
int key_count = 0;
if (array_index == -1 || force_all) {
@ -551,20 +623,19 @@ int insert_keyframe(Main *bmain,
if (!successful_remaps[array_index]) {
continue;
}
if (insert_keyframe_fcurve_value(bmain,
reports,
&ptr,
prop,
act,
group,
rna_path,
array_index,
nla_mapped_frame,
values[array_index],
keytype,
flag))
{
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;
@ -580,18 +651,21 @@ int insert_keyframe(Main *bmain,
}
if (array_index != exclude) {
key_count += insert_keyframe_fcurve_value(bmain,
reports,
&ptr,
prop,
act,
group,
rna_path,
array_index,
nla_mapped_frame,
values[array_index],
keytype,
flag);
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++;
}
}
}
}
@ -603,36 +677,42 @@ int insert_keyframe(Main *bmain,
continue;
}
key_count += insert_keyframe_fcurve_value(bmain,
reports,
&ptr,
prop,
act,
group,
rna_path,
array_index,
nla_mapped_frame,
values[array_index],
keytype,
flag);
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]) {
key_count += insert_keyframe_fcurve_value(bmain,
reports,
&ptr,
prop,
act,
group,
rna_path,
array_index,
nla_mapped_frame,
values[array_index],
keytype,
flag);
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++;
}
}
}
@ -647,6 +727,10 @@ int insert_keyframe(Main *bmain,
}
}
if (key_count == 0) {
generate_keyframe_reports_from_result(reports, combined_result);
}
return key_count;
}
@ -872,19 +956,18 @@ int insert_key_action(Main *bmain,
property_array_index++;
continue;
}
const bool inserted_key = insert_keyframe_fcurve_value(bmain,
nullptr,
ptr,
prop,
action,
group.c_str(),
rna_path.c_str(),
property_array_index,
frame,
value,
key_type,
insert_key_flag);
if (inserted_key) {
const SingleKeyingResult inserted_key = insert_keyframe_fcurve_value(bmain,
ptr,
prop,
action,
group.c_str(),
rna_path.c_str(),
property_array_index,
frame,
value,
key_type,
insert_key_flag);
if (inserted_key == SingleKeyingResult::SUCCESS) {
inserted_keys++;
}
property_array_index++;