Page MenuHome
Differential D6379 Diff 30207 source/blender/blenkernel/intern/fcurve.c

Changeset View

Standalone View

View Options

source/blender/blenkernel/intern/fcurve.c

Show First 20 Lines • Show All 904 Lines • ▼ Show 20 Lines
* \{ */ * \{ */
/* Add a BezTriple to a column. */ /* Add a BezTriple to a column. */
static void UNUSED_FUNCTION(bezt_add_to_cfra_elem)(ListBase *lb, BezTriple *bezt) static void UNUSED_FUNCTION(bezt_add_to_cfra_elem)(ListBase *lb, BezTriple *bezt)
{ {
CfraElem *ce, *cen; CfraElem *ce, *cen;
for (ce = lb->first; ce; ce = ce->next) { for (ce = lb->first; ce; ce = ce->next) {
/* Double key? */ /* Double key? */
sybrenUnsubmitted
Done
Inline Actions

start and end can be const.

sybren: `start` and `end` can be `const`.
zeddbAuthorUnsubmitted
Done
Inline Actions

I can't as I increment start in the code.
I think it would be wasteful do to a const as the int is not a reference and I would need to delare and alternate start variable if I declare it as const.

zeddb: I can't as I increment start in the code. I think it would be wasteful do to a const as the int…
if (IS_EQT(ce->cfra, bezt->vec[1][0], BEZT_BINARYSEARCH_THRESH)) { if (IS_EQT(ce->cfra, bezt->vec[1][0], BEZT_BINARYSEARCH_THRESH)) {
if (bezt->f2 & SELECT) { if (bezt->f2 & SELECT) {
ce->sel = bezt->f2; ce->sel = bezt->f2;
sybrenUnsubmitted
Done
Inline Actions

Comments should be full sentences, so start with capital and use punctuation. That includes copy-pasted comments.

sybren: Comments should be full sentences, so start with capital and use punctuation. That includes…
} }
sybrenUnsubmitted
Done
Inline Actions

TODOs should have the name of the person who knows more about this.

sybren: TODOs should have the name of the person who knows more about this.
return; return;
} }
/* Should key be inserted before this column? */ /* Should key be inserted before this column? */
if (ce->cfra > bezt->vec[1][0]) { if (ce->cfra > bezt->vec[1][0]) {
break; break;
} }
} }
/* Create a new column */ /* Create a new column */
cen = MEM_callocN(sizeof(CfraElem), "add_to_cfra_elem"); cen = MEM_callocN(sizeof(CfraElem), "add_to_cfra_elem");
if (ce) { if (ce) {
BLI_insertlinkbefore(lb, ce, cen); BLI_insertlinkbefore(lb, ce, cen);
} }
else { else {
BLI_addtail(lb, cen); BLI_addtail(lb, cen);
} }
cen->cfra = bezt->vec[1][0]; cen->cfra = bezt->vec[1][0];
cen->sel = bezt->f2; cen->sel = bezt->f2;
} }
/** \} */ /** \} */
/* -------------------------------------------------------------------- */ /* -------------------------------------------------------------------- */
/** \name Samples Utilities /** \name Samples Utilities
sybrenUnsubmitted
Done
Inline Actions

tot_kf and tot_sp can be declared const.

sybren: `tot_kf` and `tot_sp` can be declared `const`.
zeddbAuthorUnsubmitted
Done
Inline Actions

They can't be declared as const as they are decremented in the code.

zeddb: They can't be declared as const as they are decremented in the code.
sybrenUnsubmitted
Done
Inline Actions

Try to avoid overly abbreviated names.

sybren: Try to avoid overly abbreviated names.
* \{ */ * \{ */
/* Some utilities for working with FPoints (i.e. 'sampled' animation curve data, such as /* Some utilities for working with FPoints (i.e. 'sampled' animation curve data, such as
* data imported from BVH/Mocap files), which are specialized for use with high density datasets, * data imported from BVH/Mocap files), which are specialized for use with high density datasets,
* which BezTriples/Keyframe data are ill equipped to do. * which BezTriples/Keyframe data are ill equipped to do.
*/ */
sybrenUnsubmitted
Done
Inline Actions

Why the (float) cast? I don't think that's necessary here.

sybren: Why the `(float)` cast? I don't think that's necessary here.
/* Basic sampling callback which acts as a wrapper for evaluate_fcurve() /* Basic sampling callback which acts as a wrapper for evaluate_fcurve()
* 'data' arg here is unneeded here... * 'data' arg here is unneeded here...
*/ */
sybrenUnsubmitted
Done
Inline Actions

The comment below says that the points are linear, which means that they won't have handles. Since that is the case, what does "flat points" mean? What are "dummy" points, and why are they needed at all?

sybren: The comment below says that the points are linear, which means that they won't have handles.
zeddbAuthorUnsubmitted
Done
Inline Actions

This is so that points are added if there are no data before the start frame.

So if for example the bakes keys start at frame 10 but we start at frame 5.
Then keys are inserted from 5-10 with the "height" of the key on frame 10.
This will then create a "flat" line in this range.

I'm guessing this is so that there is always data on the unbake frame range.
Not really sure why either, this is simply copy pasted, so I guess there was a good reason as some point at least.

zeddb: This is so that points are added if there are no data before the start frame. So if for…
float fcurve_samplingcb_evalcurve(FCurve *fcu, void *UNUSED(data), float evaltime) float fcurve_samplingcb_evalcurve(FCurve *fcu, void *UNUSED(data), float evaltime)
sybrenUnsubmitted
Done
Inline Actions

This skips the fpt->vec[0] == start case, which is also skipped by the above for-loop.

sybren: This skips the `fpt->vec[0] == start` case, which is also skipped by the above for-loop.
zeddbAuthorUnsubmitted
Done
Inline Actions

Yes?
If we are at the correct point already then the functions should do nothing in those two functions.
So this is correct.

zeddb: Yes? If we are at the correct point already then the functions should do nothing in those two…
{ {
/* Assume any interference from drivers on the curve is intended... */ /* Assume any interference from drivers on the curve is intended... */
return evaluate_fcurve(fcu, evaltime); return evaluate_fcurve(fcu, evaltime);
} }
/* Main API function for creating a set of sampled curve data, given some callback function /* Main API function for creating a set of sampled curve data, given some callback function
* used to retrieve the values to store. * used to retrieve the values to store.
*/ */
void fcurve_store_samples(FCurve *fcu, void *data, int start, int end, FcuSampleFunc sample_cb) void fcurve_store_samples(FCurve *fcu, void *data, int start, int end, FcuSampleFunc sample_cb)
{ {
sybrenUnsubmitted
Done
Inline Actions

'leading' and 'heading' mean the same thing. Maybe 'leading' and 'trailing'? I know it's just a copy-paste, but it's an excellent opportunity to make things actually correct.

sybren: 'leading' and 'heading' mean the same thing. Maybe 'leading' and 'trailing'? I know it's just a…
FPoint *fpt, *new_fpt; FPoint *fpt, *new_fpt;
int cfra; int cfra;
/* Sanity checks. */ /* Sanity checks. */
/* TODO: make these tests report errors using reports not CLOG's (Joshua Leung 2009) */ /* TODO: make these tests report errors using reports not CLOG's (Joshua Leung 2009) */
if (ELEM(NULL, fcu, sample_cb)) { if (ELEM(NULL, fcu, sample_cb)) {
CLOG_ERROR(&LOG, "No F-Curve with F-Curve Modifiers to Bake"); CLOG_ERROR(&LOG, "No F-Curve with F-Curve Modifiers to Bake");
return; return;
} }
if (start > end) { if (start > end) {
CLOG_ERROR(&LOG, "Error: Frame range for Sampled F-Curve creation is inappropriate"); CLOG_ERROR(&LOG, "Error: Frame range for Sampled F-Curve creation is inappropriate");
return; return;
} }
sybrenUnsubmitted
Done
Inline Actions

This exact same code is duplicated twice. Better to turn it into a small function and call it three times.

sybren: This exact same code is duplicated twice. Better to turn it into a small function and call it…
/* Set up sample data. */ /* Set up sample data. */
fpt = new_fpt = MEM_callocN(sizeof(FPoint) * (end - start + 1), "FPoint Samples"); fpt = new_fpt = MEM_callocN(sizeof(FPoint) * (end - start + 1), "FPoint Samples");
/* Use the sampling callback at 1-frame intervals from start to end frames. */ /* Use the sampling callback at 1-frame intervals from start to end frames. */
for (cfra = start; cfra <= end; cfra++, fpt++) { for (cfra = start; cfra <= end; cfra++, fpt++) {
fpt->vec[0] = (float)cfra; fpt->vec[0] = (float)cfra;
fpt->vec[1] = sample_cb(fcu, data, (float)cfra); fpt->vec[1] = sample_cb(fcu, data, (float)cfra);
} }
/* Free any existing sample/keyframe data on curve. */ /* Free any existing sample/keyframe data on curve. */
if (fcu->bezt) { if (fcu->bezt) {
MEM_freeN(fcu->bezt); MEM_freeN(fcu->bezt);
} }
if (fcu->fpt) { if (fcu->fpt) {
MEM_freeN(fcu->fpt); MEM_freeN(fcu->fpt);
} }
/* Store the samples. */ /* Store the samples. */
fcu->bezt = NULL; fcu->bezt = NULL;
fcu->fpt = new_fpt; fcu->fpt = new_fpt;
fcu->totvert = end - start + 1; fcu->totvert = end - start + 1;
} }
static void init_unbaked_bezt_data(BezTriple *bezt)
{
bezt->f1 = bezt->f2 = bezt->f3 = SELECT;
/* Use linear interpolation so we match the baked curve shape. */
sybrenUnsubmitted
Done
Inline Actions

"We match" is ambiguous. My grammar terminology is failing me here; I can read "we match the baked curve shape" as "we make sure [the unbaked curve shape] matches the baked curve shape", but also as "our shape matches the baked curve shape". Are you saying I should go to the gym more often? ;-)

I think "Baked FCurve points always use linear interpolation." would be clearer.

sybren: "We match" is ambiguous. My grammar terminology is failing me here; I can read "we match the…
bezt->ipo = BEZT_IPO_LIN;
bezt->h1 = bezt->h2 = HD_AUTO_ANIM;
}
/* Convert baked/sampled fcurves into bezt/regular fcurves. */
void fcurve_samples_to_keyframes(FCurve *fcu, int start, int end)
{
/* Sanity checks. */
/* TODO: make these tests report errors using reports not CLOG's (Joshua Leung 2009). */
sybrenUnsubmitted
Done
Inline Actions

The name is supposed to be in the form TODO(aligorith). But, given upcoming changes to the commenting guidelines (T81452, most importantly Brecht's comment T81452#1029052), I'm not even sure what will be the final verdict on/form of this. I think I agree with Brecht in that the comment should be so complete that it doesn't require any knowledge of who added it or who to ask for more information. So in that light, and sorry for the inconsistency, I withdraw my earlier "add names to TODOs" note.

sybren: The name is supposed to be in the form `TODO(aligorith)`. But, given upcoming changes to the…
zeddbAuthorUnsubmitted
Done
Inline Actions

This is consistent with the rest of the file now.
If we are going to change it again, that should be in an other commit.

zeddb: This is consistent with the rest of the file now. If we are going to change it again, that…
sybrenUnsubmitted
Done
Inline Actions

👍

sybren: :+1:
if (fcu == NULL) {
CLOG_ERROR(&LOG, "No F-Curve with F-Curve Modifiers to Un-Bake");
return;
}
if (start > end) {
CLOG_ERROR(&LOG, "Error: Frame range to unbake F-Curve is inappropriate");
return;
}
if (fcu->fpt == NULL) {
/* No data to unbake. */
CLOG_ERROR(&LOG, "Error: Curve containts no baked keyframes");
return;
}
/* Free any existing sample/keyframe data on the curve. */
if (fcu->bezt) {
MEM_freeN(fcu->bezt);
}
BezTriple *bezt;
FPoint *fpt = fcu->fpt;
int tot_keyframes = end - start;
int tot_curve_points = fcu->totvert;
bezt = fcu->bezt = MEM_callocN(sizeof(*fcu->bezt) * (size_t)tot_keyframes, __func__);
fcu->totvert = tot_keyframes;
/* Get first sample point to 'copy' as keyframe. */
for (; tot_curve_points && (fpt->vec[0] < start); fpt++, tot_curve_points--) {
/* pass */
}
/* Add leading dummy flat points if needed. */
for (; tot_keyframes && (fpt->vec[0] > start); start++, bezt++, tot_keyframes--) {
init_unbaked_bezt_data(bezt);
bezt->vec[1][0] = (float)start;
bezt->vec[1][1] = fpt->vec[1];
}
/* Copy actual sample points. */
for (; tot_keyframes && tot_curve_points;
start++, bezt++, tot_keyframes--, fpt++, tot_curve_points--) {
init_unbaked_bezt_data(bezt);
copy_v2_v2(bezt->vec[1], fpt->vec);
}
/* Add trailing dummy flat points if needed. */
for (fpt--; tot_keyframes; start++, bezt++, tot_keyframes--) {
init_unbaked_bezt_data(bezt);
bezt->vec[1][0] = (float)start;
bezt->vec[1][1] = fpt->vec[1];
}
MEM_SAFE_FREE(fcu->fpt);
/* Not strictly needed since we use linear interpolation, but better be consistent here. */
calchandles_fcurve(fcu);
}
/* ***************************** F-Curve Sanity ********************************* */ /* ***************************** F-Curve Sanity ********************************* */
/* The functions here are used in various parts of Blender, usually after some editing /* The functions here are used in various parts of Blender, usually after some editing
* of keyframe data has occurred. They ensure that keyframe data is properly ordered and * of keyframe data has occurred. They ensure that keyframe data is properly ordered and
* that the handles are correct. * that the handles are correct.
*/ */
/* Checks if the F-Curve has a Cycles modifier, and returns the type of the cycle behavior. */ /* Checks if the F-Curve has a Cycles modifier, and returns the type of the cycle behavior. */
eFCU_Cycle_Type BKE_fcurve_get_cycle_type(FCurve *fcu) eFCU_Cycle_Type BKE_fcurve_get_cycle_type(FCurve *fcu)
▲ Show 20 Lines • Show All 992 Lines • Show Last 20 Lines