Add a function that returns a list of keyframe segments
A segment being a continuous selection of keyframes
As @Wayde Moss (GuiltyGhost) pointed out in D9374 this makes it easier to reuse code later on
Details
Details
- Reviewers
Sybren A. Stüvel (sybren) - Maniphest Tasks
- T81785: Implementation: Modal Key Manipulation Operators
- Commits
- rB8e31e53aa0ee: Function to return a list of keyframe segments
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
LGTM, apart from a few smaller bits. And one of those will make memory management a little easier, so I guess it's a good one ;-)
| source/blender/editors/animation/keyframes_general.c | ||
|---|---|---|
| 359 | Since a ListBase is just a container for two pointers, it might be simpler and faster to just use it as a value type. It may even be faster to work with then, as it's allocated on the stack (and thus likely to be locally cached). ListBase segments = {NULL, NULL}; | |
| 365 | Replacing sizeof(struct FCurveSegment) with sizeof(*segment) will remove some type duplication. It'll also automagically go well when the type of segment changes. It's unlikely that that'll happen, but in general it's preferred to use sizeof(some variable) instead of sizeof(type of that variable). | |
Comment Actions
- adress comments
@Sybren A. Stüvel (sybren) In order for the list to be allocated on the stack it needs to be passed in. right?
Let me know if I understood this correctly
Comment Actions
- move struct FCurveSegment to header file since it needs to be used from outside the keyframes_general.c file
Comment Actions
Nope, you can just do this:
ListBase find_fcurve_segments(FCurve *fcu) { ListBase segments; // do stuff BLI_addtail(&segments, segment); // more stuff return segments; } // ... in some other function: ListBase segments = find_fcurve_segments(fcu);
Comment Actions
Just the tiniest of things, no need to re-review after this.
| source/blender/editors/animation/keyframes_general.c | ||
|---|---|---|
| 359 | segment can be declared inside of the while-loop, as it's not necessary to access it outside. The compiler will deal with it smartly anyway. | |