Page MenuHome

rough draft of upgraded API for keyframe_insert returning vector of Keyframes (BezTriple*)
Open, NormalPublic

Description

I have been frustrated by the keyframe_insert() function not giving me an easy way to control the interpolation type. After a few discussions on IRC I decided that it would be nice if that function just returned the new points it created on the curve.

Here are some of my coding notes which you can probably skip, but I'm including them anyway:

notes on how to improve keyframe API to return the KeyFrame object
from the keyframe_points array:

ideasman42 patch to keyframe_insert()
https://developer.blender.org/rBa91c4ac99fd4d766834a42ace4a670dfa824813b

bpy_rna.c:
line 4679
keyframe_insert python mapping to pyrna_struct_keyframe_insert

bpy_rna_anim.c:
line 205
PyObject *pyrna_struct_keyframe_insert(BPy_StructRNA *self, PyObject *args, PyObject *kw)
calls insert_keyframe()
flags weird?

It converts the return value from insert_keyframe() into a bool.

editors/animation/keyframing.c:

line 490:
insert_keyframe(reports, id, act, group[], rna_path[], array_index, cfra, flag)
calls insert_keyframe_direct()

Since this can insert multiple channels (location.x,y,z, or rotation_euler) it accumulates the result of insert_keyframe_direct for the return value, so it might return 3 or 4 (quaternion channel)

The return value of insert_keyframe() is used by a few functions:
pyrna_struct_keyframe_insert() turns it into a bool.
insert_key_button_exec() accumulates the result, but uses it as a boolean
keyingsets.c:ANIM_apply_keyingset() accumulates the result and returns the accumulated result !!

ANIM_apply_keyingset() requires a little more analysis:
Its return value is usued by insert_key_exec() and delete_key_exec(). In both it is compared against MODIFYKEY_INVALID_CONTEXT or used as a boolean. This seems a little fragile since the return value of ANIM_apply_keyingset() could instead be MODIFYKEY_MISSING_TYPEINFO. In the future, it could be extended with another negative value. Changing from a boolean to a >0 would make insert_key_exec() and delete_key_exec() more resilient.

line 854
bool insert_keyframe_direct(ReportList *reports, PointerRNA ptr, PropertyRNA *prop, FCurve *fcu, float cfra, short flag)
calls insert_vert_fcurve()
returns boolean
This is complicated since the boolean return value is whether there was an effect. sometimes the flag means we won't replace an existing keyframe.

this return value is used by insert_keyframe(), insert_key_button_exec(), achannel_setting_slider_cb(), achannel_setting_slider_shapekey_cb(), and multiple functions in gpencil_edit.c

insert_keyframe() uses the return value as an increment (+=)
insert_key_button_exec() uses the return value as a boolean.
achannel_setting_slider_cb() uses the return value as a boolean.
achannel_setting_slider_shapekey_cb() uess the return value as a boolean.
gpencil_edit.c ignores the return value.

line 384
int insert_vert_fcurve(FCurve *fcu, float x, float y, short flag)
win!
returns the index at which the keyframe was added.

rna_fcurve.c:

line 1768
RNA definition of keyfram_points.insert(x,y)

line 739
static BezTriple *rna_FKeyframe_points_insert(FCurve *fcu, float frame, float value, int flag)
returns the KeyFrame inserted
calls insert_vert_fcurve()

So far I think it is possible to get the KeyFrame/BezTriple up the
call stack.

insert_vert_fcurve() currently returns an index.

insert_keyframe_direct() currently returns a bool, but could instead
return a BezTriple*. Functions that accumulate the return value will
have to be converted from +=rval to +=(0!=rval).

The hairball comes in when we end up creating multiple fcurves
(e.g. position or rotation_euler).

This can happen in the insert_keyframe() function. We would have to
modify this to return an array of BezTriple*, and deal with the
resulting memory managment issues. The large number of users of
insert_keyframe would necessitate a little bifurcation:

One function would provide the integer return value. The other
function would return the BezTriple references. My architectural
instinct says that insert_keyframe() should be the name of the
function that returns an integer and its guts should be relocated to a
new function: insert_keyframe_return_beztriples() which returns a
NULL-terminated heap-allocated array of BezTriple*.

The hollow insert_keyframe() would perform the memory management on
the BezTriple* array and return a count.

pyrna_struct_keyframe_insert() would call the
insert_keyframe_return_beztriples() directly and convert the
BezTriple* to a python vector as well as free the containing array.

Details

Type
Design

Event Timeline

Robert Forsman (mutantbob) set Type to Patch.
Robert Forsman (mutantbob) created this task.
Robert Forsman (mutantbob) raised the priority of this task from to Needs Triage by Developer.

Since I can't figure out how to drag&drop files into this from my linux UI I'll put my first draft of the patch on pasteall and let someone else drag&drop it into the tracker.

http://www.pasteall.org/51246/c

Is the closest I can get to accomplishing this idea. IT IS DEFINITELY INCOMPLETE. This is more of a rough draft / proof of concept.

The roadblock I am up against right now is I'm not sure how to convert the BezTriple* into a PyObject*.

PyObject* bacon = PyLong_FromLong( (long) (result[i]) );

is what I have coded at the moment which just gives me the pointers cast to a long. That bit at least seems to give me plausible results in the python console. If someone can tell me the proper call to get a Keyframe python object. I can resume making progress.

I also have not considered what effect this might have on existing python code that calls keyframe_insert() and expects it to return a bool. I would not be surprised if this turns out to be a critical problem necessitating the creation of a separate function (keyframe_insert2() ?).

http://www.pasteall.org/51274/c

That's a new patch that seems to actually return the BezTriple* vector. The Keyframe objects in the vector are life. They can be used to modify the curve as was my original goal.

Here's an example use case:

for kf in obj.keyframe_insert('location', frame=31):

kf.interpolation='LINEAR'

There are debugging printfs in the code, and I suspect some RNA definitions need to be adjusted. The previously-mentioned side effects on the rest of the system are still uncertain.

I quickly went over all official addons, and none seems to check the return value (boolean) of keyframe_insert(). Thus, I don't expect any problems on the python-side by this change.

I am gaining confidence in my patch. I spent a couple of hours fiddling with a simple animation inserting keyframes and working with the NLA editor. I did not experience any oddities. This session did use any custom python scripts to test the API, it was stricly UI interaction.

This is the latest version of the patch. By making INSERTKEY_REPLACE a valid option we reveal the strange behavior of this flag. It probably should not be part of the final patch unless this weird behavior is actually intended (and documented).

This file is a demonstration of the capabilities of my new keyframe_insert() API, demonstrating the ability to set the handles and interpolation type. Combine it with the next upload for a torture test.

This sample python uses the keyframeFor() function from torture-lib.py to build up a "sawtooth" fcurve from multiple interpolation types. It does it in random order to see if we can shake any bugs out.