Anim: Bake Channel operator #111263

Merged
Christoph Lendenfeld merged 49 commits from ChrisLend/blender:bake_channel_operator into main 2023-12-14 11:35:32 +01:00
6 changed files with 392 additions and 2 deletions

View File

@ -295,6 +295,7 @@ class GRAPH_MT_channel(Menu):
layout.operator("graph.keys_to_samples")
layout.operator("graph.samples_to_keys")
layout.operator("graph.sound_to_samples")
layout.operator("anim.channels_bake")
layout.separator()
layout.operator("graph.euler_filter", text="Discontinuity (Euler) Filter")

View File

@ -8,6 +8,7 @@
* \ingroup bke
*/
#include "BLI_math_vector_types.hh"
#include "DNA_curve_types.h"
#ifdef __cplusplus
@ -483,11 +484,31 @@ bool BKE_fcurve_bezt_subdivide_handles(struct BezTriple *bezt,
*/
void BKE_fcurve_bezt_shrink(struct FCurve *fcu, int new_totvert);
/**
nathanvegdahl marked this conversation as resolved Outdated

I think we should start holding ourselves to a bit of a higher standard for API documentation. And I say this for myself as well, not at all picking on you. With that in mind:

  • The first line should be a quick summary description of what the function does. Something like, "Merge keyframes from a and b together into a new bezier key array." It can feel redundant with the function name, but I think having a consistent flow for all documentation is useful, and I believe that's what Doxygen expects anyway.
  • Since return values aren't named, I think it makes sense to always add documentation for that unless it's extremely trivial. "Returns the newly allocated merged array." Or something like that.
  • Your documentation of the invariants is great! 👍 Nit: rather than "Caller is expected to free data," maybe "This function does not free a or b."
  • For potentially performance-sensitive functions like this, it wouldn't hurt to document its computational complexity, which I believe is O(N) with respect to the length of a and b in this case.
I think we should start holding ourselves to a bit of a higher standard for API documentation. And I say this for myself as well, not at all picking on you. With that in mind: - The first line should be a quick summary description of what the function does. Something like, "Merge keyframes from `a` and `b` together into a new bezier key array." It can feel redundant with the function name, but I think having a consistent flow for all documentation is useful, and I believe that's what Doxygen expects anyway. - Since return values aren't named, I think it makes sense to always add documentation for that unless it's *extremely* trivial. "Returns the newly allocated merged array." Or something like that. - Your documentation of the invariants is great! 👍 Nit: rather than "Caller is expected to free data," maybe "This function does not free `a` or `b`." - For potentially performance-sensitive functions like this, it wouldn't hurt to document its computational complexity, which I believe is O(N) with respect to the length of `a` and `b` in this case.
* Merge the two given BezTriple arrays `a` and `b` into a newly allocated BezTriple array of size
* `r_merged_size`. In case of keys on identical frames, `a` takes precedence.
* Does not free `a` or `b`.
* Assumes that both arrays are sorted for the x-position.
nathanvegdahl marked this conversation as resolved Outdated

Can a and b be const here?

Also, IIRC Blender uses the convention of prefixing "out" arguments with r_ (for "return"). So merged_size -> r_merged_size.

Can `a` and `b` be `const` here? Also, IIRC Blender uses the convention of prefixing "out" arguments with `r_` (for "return"). So `merged_size` -> `r_merged_size`.
* Has a complexity of O(N) with respect to the length of `size_a` + `size_b`.
*
* \return The merged BezTriple array of length `r_merged_size`.
*/
BezTriple *BKE_bezier_array_merge(
const BezTriple *a, int size_a, const BezTriple *b, int size_b, int *r_merged_size);

Since this is for performance, I think it makes sense to document the performance characteristics of this as well: performance is independent of the number of keys being removed, and on average O(N) with respect to the total number of keys remaining after removal.

Since this is for performance, I think it makes sense to document the performance characteristics of this as well: performance is independent of the number of keys being removed, and on average O(N) with respect to the total number of keys remaining after removal.

I think the performance analysis you added is mistaken:

Has a complexity of O(N) with respect to the amount of keys removed.

The number of keys removed doesn't affect performance at all, I don't think. The thing that affects the performance linearly is the number of keys that follow the removed range of keys, since those have to be shifted backwards to fill the gap.

So I think it's fine to say "O(N) with respect to the number of keys in fcu", because that's roughly true. In practice it's a little more involved than that, but it's a good enough summary analysis to inform the caller of the main impacting factor.

I think the performance analysis you added is mistaken: > Has a complexity of O(N) with respect to the amount of keys removed. The number of keys removed doesn't affect performance at all, I don't think. The thing that affects the performance linearly is the number of keys that follow the removed range of keys, since those have to be shifted backwards to fill the gap. So I think it's fine to say "O(N) with respect to the number of keys in `fcu`", because that's roughly true. In practice it's a little more involved than that, but it's a good enough summary analysis to inform the caller of the main impacting factor.

that is true, changed it :)

that is true, changed it :)
/**
* Delete a keyframe from an F-curve at a specific index.
*/
void BKE_fcurve_delete_key(struct FCurve *fcu, int index);
/** Delete an index range of keyframes from an F-curve. This is more performant than individually
* removing keys.
* Has a complexity of O(N) with respect to number of keys in `fcu`.
*
* \param index_range is right exclusive.
nathanvegdahl marked this conversation as resolved Outdated

Very minor nit: I'm pretty sure anyone will already know what you mean, but "right exclusive" might be slightly clearer, emphasizing that the range is only exclusive on the right.

Very minor nit: I'm pretty sure anyone will already know what you mean, but "right exclusive" might be slightly clearer, emphasizing that the range is only exclusive on the right.

yeah that's true, changed now

yeah that's true, changed now
*/
void BKE_fcurve_delete_keys(FCurve *fcu, blender::uint2 index_range);
/**
* Delete selected keyframes from an F-curve.
*/

View File

@ -22,6 +22,7 @@
#include "BLI_easing.h"
#include "BLI_ghash.h"
#include "BLI_math_vector.h"
#include "BLI_math_vector_types.hh"
#include "BLI_sort_utils.h"
#include "BLI_string_utils.hh"
@ -1705,6 +1706,81 @@ void BKE_fcurve_delete_key(FCurve *fcu, int index)
}
}
void BKE_fcurve_delete_keys(FCurve *fcu, blender::uint2 index_range)
{
BLI_assert(fcu != nullptr);
BLI_assert(fcu->bezt != nullptr);
BLI_assert(index_range[1] > index_range[0]);
BLI_assert(index_range[1] <= fcu->totvert);
const int removed_index_count = index_range[1] - index_range[0];
memmove(&fcu->bezt[index_range[0]],
&fcu->bezt[index_range[1]],
sizeof(BezTriple) * (fcu->totvert - index_range[1]));
fcu->totvert -= removed_index_count;
if (fcu->totvert == 0) {
fcurve_bezt_free(fcu);
}
}
BezTriple *BKE_bezier_array_merge(
const BezTriple *a, const int size_a, const BezTriple *b, const int size_b, int *r_merged_size)
{
BezTriple *large_array = static_cast<BezTriple *>(
MEM_callocN((size_a + size_b) * sizeof(BezTriple), "beztriple"));
int iterator_a = 0;
int iterator_b = 0;
*r_merged_size = 0;
/* For comparing if keyframes are at the same x-value. */
const int max_ulps = 32;
while (iterator_a < size_a || iterator_b < size_b) {
nathanvegdahl marked this conversation as resolved Outdated

If I understand correctly, this case and the one immediately below are when one or the other of the two arrays are completely merged in already, and the remaining keys in the other just need to be appended to the end. If that's correct, then I think these can be made a bit faster by just memcopying the the remaining keys in one go, rather than one at a time.

If I understand correctly, this case and the one immediately below are when one or the other of the two arrays are completely merged in already, and the remaining keys in the other just need to be appended to the end. If that's correct, then I think these can be made a bit faster by just memcopying the the remaining keys in one go, rather than one at a time.

good point, changed that.
This also improves readability a bit because now I can break from the if statement

good point, changed that. This also improves readability a bit because now I can `break` from the if statement
if (iterator_a >= size_a) {
const int remaining_keys = size_b - iterator_b;
memcpy(&large_array[*r_merged_size], &b[iterator_b], sizeof(BezTriple) * remaining_keys);
(*r_merged_size) += remaining_keys;
break;
}
nathanvegdahl marked this conversation as resolved Outdated

Should we be using some kind of epsilon threshold for the equality comparison here? Elsewhere in Blender's code I seem to recall there being some limit of how close keys can be before they're considered the same, and we should probably try to be consistent with that.

Should we be using some kind of epsilon threshold for the equality comparison here? Elsewhere in Blender's code I seem to recall there being some limit of how close keys can be before they're considered the same, and we should probably try to be consistent with that.

good point, used compare_ff_relative with BEZT_BINARYSEARCH_THRESH

good point, used `compare_ff_relative` with `BEZT_BINARYSEARCH_THRESH`

Looks good.

A side-thought: in the future, I wonder if we should track down all the places where key x-coordinates are compared for equality, and make a single function for that, to ensure it gets done consistently everywhere (which I strongly suspect it currently isn't). But certainly not something for this PR.

Looks good. A side-thought: in the future, I wonder if we should track down all the places where key x-coordinates are compared for equality, and make a single function for that, to ensure it gets done consistently everywhere (which I strongly suspect it currently isn't). But certainly not something for this PR.
if (iterator_b >= size_b) {
const int remaining_keys = size_a - iterator_a;
memcpy(&large_array[*r_merged_size], &a[iterator_a], sizeof(BezTriple) * remaining_keys);
(*r_merged_size) += remaining_keys;
break;
}
if (compare_ff_relative(
a[iterator_a].vec[1][0], b[iterator_b].vec[1][0], BEZT_BINARYSEARCH_THRESH, max_ulps))
{
memcpy(&large_array[*r_merged_size], &a[iterator_a], sizeof(BezTriple));
iterator_a++;
iterator_b++;
}
else if (a[iterator_a].vec[1][0] < b[iterator_b].vec[1][0]) {
memcpy(&large_array[*r_merged_size], &a[iterator_a], sizeof(BezTriple));
iterator_a++;
}
else {
memcpy(&large_array[*r_merged_size], &b[iterator_b], sizeof(BezTriple));
iterator_b++;
}
(*r_merged_size)++;
}
BezTriple *minimal_array;
if (*r_merged_size < size_a + size_b) {
minimal_array = static_cast<BezTriple *>(
MEM_reallocN(large_array, sizeof(BezTriple) * (*r_merged_size)));
}
else {
minimal_array = large_array;
}
return minimal_array;
}
bool BKE_fcurve_delete_keys_selected(FCurve *fcu)
{
if (fcu->bezt == nullptr) { /* ignore baked curves */

View File

@ -4310,6 +4310,189 @@ static void ANIM_OT_channel_view_pick(wmOperatorType *ot)
"Ignore frames outside of the preview range");
}
static const EnumPropertyItem channel_bake_key_options[] = {
{BEZT_IPO_BEZ, "BEZIER", 0, "Bezier", "New keys will be beziers"},
{BEZT_IPO_LIN, "LIN", 0, "Linear", "New keys will be linear"},
{BEZT_IPO_CONST, "CONST", 0, "Constant", "New keys will be constant"},
{0, nullptr, 0, nullptr, nullptr},
};
static const EnumPropertyItem channel_bake_remove_options[] = {
{int(BakeCurveRemove::REMOVE_NONE), "NONE", 0, "None", "Keep all keys"},
{int(BakeCurveRemove::REMOVE_IN_RANGE),
"IN_RANGE",
0,
"In Range",
"Remove all keys within the defined range"},
{int(BakeCurveRemove::REMOVE_OUT_RANGE),
"OUT_RANGE",
0,
"Outside Range",
nathanvegdahl marked this conversation as resolved Outdated

I'd prefer new constants for this enum, rather than awkwardly having just one that's new and defined arbitrarily high to avoid conflicting with the others. It's not as convenient in other code, because then you have to map between the two sets of constants later. But it feels cleaner to me that way.

I'd prefer new constants for this enum, rather than awkwardly having just one that's new and defined arbitrarily high to avoid conflicting with the others. It's not as convenient in other code, because then you have to map between the two sets of constants later. But it feels cleaner to me that way.

not sure I agree with this. It would add a level of indirection to the enum which I think isn't needed. Atm it is quite clear what key types the options correspond to since they use the same constants.

I agree it does feel a bit weird, but I feel like it is the better option

not sure I agree with this. It would add a level of indirection to the enum which I think isn't needed. Atm it is quite clear what key types the options correspond to since they use the same constants. I agree it does feel a bit weird, but I feel like it is the better option

That's fair. Let's leave it as-is, then. Thanks!

That's fair. Let's leave it as-is, then. Thanks!
"Remove all keys outside the defined range"},
{int(BakeCurveRemove::REMOVE_ALL), "ALL", 0, "All", "Remove all existing keys"},
{0, nullptr, 0, nullptr, nullptr},
};
static int channels_bake_exec(bContext *C, wmOperator *op)
{
bAnimContext ac;
/* Get editor data. */
if (ANIM_animdata_get_context(C, &ac) == 0) {
return OPERATOR_CANCELLED;
}
ListBase anim_data = {nullptr, nullptr};
const int filter = (ANIMFILTER_SEL | ANIMFILTER_NODUPLIS | ANIMFILTER_DATA_VISIBLE |
ANIMFILTER_LIST_VISIBLE | ANIMFILTER_FCURVESONLY);
size_t anim_data_length = ANIM_animdata_filter(
&ac, &anim_data, eAnimFilter_Flags(filter), ac.data, eAnimCont_Types(ac.datatype));
if (anim_data_length == 0) {
WM_report(RPT_WARNING, "No channels to operate on");
return OPERATOR_CANCELLED;
}
Scene *scene = CTX_data_scene(C);
/* The range will default to the scene or preview range, but only if it hasn't been set before.
* If a range is set here, the redo panel wouldn't work properly because the range would
* constantly be overridden. */
blender::int2 frame_range;
RNA_int_get_array(op->ptr, "range", frame_range);
if (frame_range[1] < frame_range[0]) {
frame_range[1] = frame_range[0];
}
const float step = RNA_float_get(op->ptr, "step");
if (frame_range[0] == 0 && frame_range[1] == 0) {
if (scene->r.flag & SCER_PRV_RANGE) {
frame_range = {scene->r.psfra, scene->r.pefra};
}
else {
frame_range = {scene->r.sfra, scene->r.efra};
}
RNA_int_set_array(op->ptr, "range", frame_range);
}
const bool remove_outside_range = RNA_boolean_get(op->ptr, "remove_outside_range");
const BakeCurveRemove remove_existing = remove_outside_range ? BakeCurveRemove::REMOVE_ALL :
BakeCurveRemove::REMOVE_IN_RANGE;
const int interpolation_type = RNA_enum_get(op->ptr, "interpolation_type");
const bool bake_modifiers = RNA_boolean_get(op->ptr, "bake_modifiers");
LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) {
FCurve *fcu = static_cast<FCurve *>(ale->data);
if (!fcu->bezt) {
continue;
}
AnimData *adt = ANIM_nla_mapping_get(&ac, ale);
blender::int2 nla_mapped_range;
nla_mapped_range[0] = int(BKE_nla_tweakedit_remap(adt, frame_range[0], NLATIME_CONVERT_UNMAP));
nla_mapped_range[1] = int(BKE_nla_tweakedit_remap(adt, frame_range[1], NLATIME_CONVERT_UNMAP));
/* Save current state of modifier flags so they can be reapplied after baking. */
blender::Vector<short> modifier_flags;
if (!bake_modifiers) {
LISTBASE_FOREACH (FModifier *, modifier, &fcu->modifiers) {
modifier_flags.append(modifier->flag);
modifier->flag |= FMODIFIER_FLAG_MUTED;
}
}
bool replace;
const int last_index = BKE_fcurve_bezt_binarysearch_index(
fcu->bezt, nla_mapped_range[1], fcu->totvert, &replace);
/* Since the interpolation of a key defines the curve following it, the last key in the baked
* segment needs to keep the interpolation mode that existed previously so the curve isn't
* changed. */
const char segment_end_interpolation = fcu->bezt[min_ii(last_index, fcu->totvert - 1)].ipo;
bake_fcurve(fcu, nla_mapped_range, step, remove_existing);
if (bake_modifiers) {
free_fmodifiers(&fcu->modifiers);
}
else {
int modifier_index = 0;
LISTBASE_FOREACH (FModifier *, modifier, &fcu->modifiers) {
modifier->flag = modifier_flags[modifier_index];
modifier_index++;
}
}
for (int i = 0; i < fcu->totvert; i++) {
BezTriple *key = &fcu->bezt[i];
if (key->vec[1][0] < nla_mapped_range[0]) {
continue;
}
if (key->vec[1][0] > nla_mapped_range[1]) {
fcu->bezt[max_ii(i - 1, 0)].ipo = segment_end_interpolation;
break;
}
key->ipo = interpolation_type;
}
}
ANIM_animdata_freelist(&anim_data);
WM_event_add_notifier(C, NC_ANIMATION | ND_KEYFRAME | NA_SELECTED, nullptr);
return OPERATOR_FINISHED;
}
static void ANIM_OT_channels_bake(wmOperatorType *ot)
{
/* Identifiers */
ot->name = "Bake Channels";
ot->idname = "ANIM_OT_channels_bake";
ot->description =
"Create keyframes following the current shape of F-Curves of selected channels";
/* API callbacks */
ot->exec = channels_bake_exec;
ot->poll = channel_view_poll;
nathanvegdahl marked this conversation as resolved Outdated

This description feels non-specific to me. There are a lot of operators that create key frames.

On the other hand, I'm struggling to think of a succinct alternative that doesn't suffer similar problems.

This description feels non-specific to me. There are a lot of operators that create key frames. On the other hand, I'm struggling to think of a succinct alternative that doesn't suffer similar problems.

I think I made it better
Create keyframes following the current shape of F-Curves of selected channels
The distinction to other keyframe adding operators is that this one looks at the shape of the current curve, so I think it makes sense to mention that

I think I made it better `Create keyframes following the current shape of F-Curves of selected channels` The distinction to other keyframe adding operators is that this one looks at the shape of the current curve, so I think it makes sense to mention that

Yeah, that's great. Thanks!

Yeah, that's great. Thanks!
ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO;
RNA_def_int_array(ot->srna,
"range",
2,
nullptr,
INT_MIN,
INT_MAX,
"Frame Range",
"The range in which to create new keys",
0,
INT_MAX);
RNA_def_float(ot->srna,
"step",
1.0f,
0.01f,
FLT_MAX,
"Frame Step",
"At which interval to add keys",
1.0f,
16.0f);
RNA_def_boolean(ot->srna,
"remove_outside_range",
false,
"Remove Outside Range",
nathanvegdahl marked this conversation as resolved Outdated

It might just be me, but "Remove Existing Keys" makes me feel like no keys will be removed if it's unchecked, when in reality the inside-range keys are always removed.

I think "Remove Keys Outside Range" might make it more clear that the only behavior that changes is what happens to outside-range keys. Unfortunately, that does leave it ambiguous what happens to the inside-range keys. But at least it leaves the user with the question of what happens to those keys, which they can then find out via testing or documentation. Whereas "Remove Existing Keys" feels to me like it answers that question, but incorrectly.

Then again, I could be the odd one out here.

It might just be me, but "Remove Existing Keys" makes me feel like no keys will be removed if it's unchecked, when in reality the inside-range keys are always removed. I think "Remove Keys Outside Range" might make it more clear that the only behavior that changes is what happens to outside-range keys. Unfortunately, that does leave it ambiguous what happens to the inside-range keys. But at least it leaves the user with the question of what happens to those keys, which they can then find out via testing or documentation. Whereas "Remove Existing Keys" feels to me like it answers that question, but incorrectly. Then again, I could be the odd one out here.

That was my first idea, but the label is so long it is cut off in the redo panel. So I tried finding something more concise.

I think the same logic of "let the user play with it to learn" applies to "Remove Existing Keys" as well. By default it will keep keys outside the given range and quickly toggling the property should be very obvious.

Let me know what you think

That was my first idea, but the label is so long it is cut off in the redo panel. So I tried finding something more concise. I think the same logic of "let the user play with it to learn" applies to "Remove Existing Keys" as well. By default it will keep keys outside the given range and quickly toggling the property should be very obvious. Let me know what you think

but the label is so long it is cut off in the redo panel

Oh, that makes sense. Nuts. How do you feel about just "Remove Outside Range" or "Clear Outside Range"? Those don't include "keys" in the label, but that can be made clear in the tool tip, or just by toggling it.

Alternatively, can we make the redo panel wider for this operator?

> but the label is so long it is cut off in the redo panel Oh, that makes sense. Nuts. How do you feel about just "Remove Outside Range" or "Clear Outside Range"? Those don't include "keys" in the label, but that can be made clear in the tool tip, or just by toggling it. Alternatively, can we make the redo panel wider for this operator?

renamed to "Remove Outside Range" :)

renamed to "Remove Outside Range" :)
"Removes keys outside the given range, leaving only the newly baked");
RNA_def_enum(ot->srna,
"interpolation_type",
nathanvegdahl marked this conversation as resolved Outdated

If we do keep these options as-are, I think REMOVE_IN_RANGE is a better default. I don't think people usually want to leave behind the old keys in the range being baked.

If we do keep these options as-are, I think `REMOVE_IN_RANGE` is a better default. I don't think people usually want to leave behind the old keys in the range being baked.
channel_bake_key_options,
BEZT_IPO_BEZ,
nathanvegdahl marked this conversation as resolved Outdated

This description reads like it's a boolean option.

This description reads like it's a boolean option.

indeed it used to be a boolean. changed it to be more representative of the enum

indeed it used to be a boolean. changed it to be more representative of the enum
"Interpolation Type",
"Choose the interpolation type with which new keys will be added");
RNA_def_boolean(ot->srna,
"bake_modifiers",
true,
"Bake Modifiers",
"Bake Modifiers into keyframes and delete them after");
}
/* Find a Graph Editor area and modify the given context to be the window region of it. */
static bool move_context_to_graph_editor(bContext *C)
{
@ -4584,6 +4767,8 @@ void ED_operatortypes_animchannels()
WM_operatortype_append(ANIM_OT_channels_group);
WM_operatortype_append(ANIM_OT_channels_ungroup);
WM_operatortype_append(ANIM_OT_channels_bake);
}
void ED_keymap_animchannels(wmKeyConfig *keyconf)

View File

@ -15,6 +15,7 @@
#include "BLI_blenlib.h"
#include "BLI_math_vector.h"
#include "BLI_math_vector_types.hh"
#include "BLI_string_utils.hh"
#include "BLI_utildefines.h"
@ -1211,7 +1212,7 @@ struct TempFrameValCache {
void sample_fcurve_segment(FCurve *fcu,
const float start_frame,
const int sample_rate,
const float sample_rate,
float *samples,
const int sample_count)
{
@ -1221,6 +1222,104 @@ void sample_fcurve_segment(FCurve *fcu,
}
}
static void remove_fcurve_key_range(FCurve *fcu,
nathanvegdahl marked this conversation as resolved
Review

While that's true, this is also accidentally quadratic, because on every removal (BKE_fcurve_delete_key) it has to shift all the keys after the removed key back. If you iterate forward and keep track of the gap as it's created, you can shift keys over the gap as you go for linear runtime.

While that's true, this is also accidentally quadratic, because on every removal (BKE_fcurve_delete_key) it has to shift all the keys after the removed key back. If you iterate forward and keep track of the gap as it's created, you can shift keys over the gap as you go for linear runtime.

solved by adding a new function BKE_fcurve_delete_keys that removes a range of keys in one swoop

solved by adding a new function `BKE_fcurve_delete_keys` that removes a range of keys in one swoop
const blender::int2 range,
const BakeCurveRemove removal_mode)
{
switch (removal_mode) {
case BakeCurveRemove::REMOVE_ALL: {
BKE_fcurve_delete_keys_all(fcu);
break;
}
case BakeCurveRemove::REMOVE_OUT_RANGE: {
bool replace;
int before_index = BKE_fcurve_bezt_binarysearch_index(
fcu->bezt, range[0], fcu->totvert, &replace);
if (before_index > 0) {
BKE_fcurve_delete_keys(fcu, {0, uint(before_index)});
}
int after_index = BKE_fcurve_bezt_binarysearch_index(
fcu->bezt, range[1], fcu->totvert, &replace);
/* REMOVE_OUT_RANGE is treated as exlusive on both ends. */
if (replace) {
after_index++;
}
if (after_index < fcu->totvert) {
BKE_fcurve_delete_keys(fcu, {uint(after_index), fcu->totvert});
}
break;
}
case BakeCurveRemove::REMOVE_IN_RANGE: {
bool replace;
const int range_start_index = BKE_fcurve_bezt_binarysearch_index(
fcu->bezt, range[0], fcu->totvert, &replace);
int range_end_index = BKE_fcurve_bezt_binarysearch_index(
fcu->bezt, range[1], fcu->totvert, &replace);
if (replace) {
range_end_index++;
}
if (range_end_index > range_start_index) {

This is also accidentally quadratic (actually slightly worse, because it additionally does a binary search to find where to insert each key), and does an allocation and full copy of the f-curve's entire bezt data on every insert.

However, this one is a bit dicier (but certainly not impossible) to make linear. So maybe it's better to leave this one as an optimization for later.

This is also accidentally quadratic (actually slightly worse, because it additionally does a binary search to find where to insert each key), and does an allocation and full copy of the f-curve's entire bezt data on every insert. However, this one is a bit dicier (but certainly not impossible) to make linear. So maybe it's better to leave this one as an optimization for later.

I ran it through a profiler. 90% of the time was spent on BKE_fcurve_recalc_handles
I changed the flag to INSERTKEY_FAST so it doesn't recalculate on every key insert
Now 83% is spent on __memcpy_avx_unaligned_erms which I have no Idea where that comes from

I ran it through a profiler. 90% of the time was spent on `BKE_fcurve_recalc_handles` I changed the flag to `INSERTKEY_FAST` so it doesn't recalculate on every key insert Now 83% is spent on `__memcpy_avx_unaligned_erms` which I have no Idea where that comes from
BKE_fcurve_delete_keys(fcu, {uint(range_start_index), uint(range_end_index)});
}
break;
}
default:
break;
}
}
void bake_fcurve(FCurve *fcu,
const blender::int2 range,
const float step,
const BakeCurveRemove remove_existing)
{
using namespace blender::animrig;
BLI_assert(step > 0);
const int sample_count = (range[1] - range[0]) / step + 1;
float *samples = static_cast<float *>(
MEM_callocN(sample_count * sizeof(float), "Channel Bake Samples"));
const float sample_rate = 1.0f / step;
sample_fcurve_segment(fcu, range[0], sample_rate, samples, sample_count);
if (remove_existing != BakeCurveRemove::REMOVE_NONE) {
remove_fcurve_key_range(fcu, range, remove_existing);
}
BezTriple *baked_keys = static_cast<BezTriple *>(
MEM_callocN(sample_count * sizeof(BezTriple), "beztriple"));
const KeyframeSettings settings = get_keyframe_settings(true);
for (int i = 0; i < sample_count; i++) {
BezTriple *key = &baked_keys[i];
blender::float2 key_position = {range[0] + i * step, samples[i]};
initialize_bezt(key, key_position, settings, eFCurve_Flags(fcu->flag));
}
int merged_size;
BezTriple *merged_bezt = BKE_bezier_array_merge(
baked_keys, sample_count, fcu->bezt, fcu->totvert, &merged_size);
if (fcu->bezt != nullptr) {
/* Can happen if we removed all keys beforehand. */
MEM_freeN(fcu->bezt);
}
MEM_freeN(baked_keys);
fcu->bezt = merged_bezt;
fcu->totvert = merged_size;
MEM_freeN(samples);
BKE_fcurve_handles_recalc(fcu);
}
void bake_fcurve_segments(FCurve *fcu)
{
using namespace blender::animrig;

View File

@ -8,6 +8,7 @@
#pragma once
#include "BLI_math_vector_types.hh"
#include "ED_anim_api.hh" /* for enum eAnimFilter_Flags */
struct BezTriple;
@ -497,7 +498,14 @@ void bake_fcurve_segments(FCurve *fcu);
* \param sample_rate: indicates how many samples per frame should be generated.
*/
void sample_fcurve_segment(
FCurve *fcu, float start_frame, int sample_rate, float *r_samples, int sample_count);
FCurve *fcu, float start_frame, float sample_rate, float *r_samples, int sample_count);
enum class BakeCurveRemove { REMOVE_NONE, REMOVE_IN_RANGE, REMOVE_OUT_RANGE, REMOVE_ALL };
/** Creates keyframes in the given range at the given step interval.
* \param range: start and end frame to bake. Is inclusive on both ends.
* \param remove_existing: choice which keys to remove in relation to the given range.
*/
void bake_fcurve(FCurve *fcu, blender::int2 range, float step, BakeCurveRemove remove_existing);
/* ----------- */