Page MenuHome

Remove unnecessary fields from FPoint
Needs ReviewPublic

Authored by Steffen Ohrendorf (sto) on Oct 13 2020, 8:39 PM.

Details

Summary

Problem: an "FPoint" contains besides the sampled curve coordinates also a single boolean flag and also padding. The "selection flag" is only exposed through the Python API and used nowhere else; thus, the FPoint struct is twice as big as really needed.

Proposed solution: Remove selection flag field and the required padding caused by this.

Alternative solution: N/A.

Limitations: None.

User interface: Removes the "select" flag from the FCurveSample struct in Python, thus it's a breaking change.

Diff Detail

Repository
rB Blender

Event Timeline

Steffen Ohrendorf (sto) requested review of this revision.Oct 13 2020, 8:39 PM
Steffen Ohrendorf (sto) created this revision.

Taking this further (leading to a non-minor refactoring), one could even reduce a lot of redundancy by storing the points' X values separately on group or even action level and just reference it from the fcurves. The idea is that you usually have loc/rot/scale keyframes for all axes at the same frame, which means there's redundancy in the X values for all those fcurves referencing the same animatable object. Furthermore, loc/rot/scale is usually keyframed all together, which means that 9 fcurves share the same X coordinates. Even further, the variation of X values of a whole action is probably very low, or even non-existent, which means the time coordinates of all keyframes of an action can probably be stored in a few arrays of floating point values, while there are many more fcurves compared to the number of distinct keyframe time coordinates. For a large number of fcurves per action sharing the same timing information, this would lead to an overall reduction of memory usage of up to 75%, as fcurves would only store their Y values and reference a shared array of X coordinates.

Sybren A. Stüvel (sybren) requested changes to this revision.Oct 15 2020, 10:30 AM

Please take a look at Ingredients of a Patch. The patch description should at least be updated to explain what the patch is doing. It shouldn't be necessary to read through the patched code to get an understanding of the changes, and how it impacts Blender users.

This revision now requires changes to proceed.Oct 15 2020, 10:30 AM
Steffen Ohrendorf (sto) retitled this revision from Add "frozen fcurve samples" to preserve half of the memory to Add "compressed fcurve samples" to preserve half of the memory.Oct 15 2020, 11:20 AM
Steffen Ohrendorf (sto) edited the summary of this revision. (Show Details)

Ah, didn't know about that page, I've changed the description as best as I could, including the current state of the patch as well as where it will lead to once someone says I should continue development.

The patch's complexity roughly reflects the solution proposed in the description, but the general idea is already in the patch.

Steffen Ohrendorf (sto) edited the summary of this revision. (Show Details)

Use much simpler approach

After finding out that I misunderstood some properties of FCurves, I got a much much simpler solution.

Steffen Ohrendorf (sto) retitled this revision from Add "compressed fcurve samples" to preserve half of the memory to Remove unnecessary fields from FPoint.Oct 16 2020, 1:24 AM