Refactor storage usage during fcurve modifier evaluation
ClosedPublic
Actions

Authored by Jacques Lucke (JacquesLucke) on Apr 17 2019, 4:03 PM.

Details

Reviewers

Brecht Van Lommel (brecht)
Alexander Gavrilov (angavrilov)

Commits

rBS81ce3801bf0a: Animation: Refactor storage usage during fcurve modifier evaluation
rB81ce3801bf0a: Animation: Refactor storage usage during fcurve modifier evaluation

Summary

Previously, when a fcurve modifier used storage,
many heap allocations were done.
This caused major slowdowns as described in T63656.

Furthermore, the storage usage was a special case only
used by the Cycles modifier. This refactor makes
storage usage the "normal" case.
That reduces the overall complexity.

The store is stack allocated now.

The framerate on the provided test scene went up from ~5 fps to ~16 fps.

Diff Detail

Repository

rB Blender

Branch

refactor-fcm-storage (branched from master)

Build Status

Buildable 3353
Build 3353: arc lint + arc unit

Event Timeline

Jacques Lucke (JacquesLucke) created this revision.Apr 17 2019, 4:03 PM

Harbormaster completed remote builds in B3351: Diff 14780.Apr 17 2019, 4:03 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Apr 17 2019, 4:06 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)

Seems OK overall, just some nitpicks about naming and blank lines :)

source/blender/blenkernel/BKE_fcurve.h
200	modifier_count
202	Maybe call this 'data' or 'buffer' to avoid storage->storage?
source/blender/blenkernel/intern/anim_sys.c
2991	<add blank line here>
source/blender/blenkernel/intern/fmodifier.c
1294–1296	<blank line>
1303	Using a max macro would probably be more obvious: max_size = MAX2(max_size, fmi->storage_size);
1405	Maybe use the POINTER_OFFSET macro to make this more clear. Also, maybe move this before nval and don't remove blank lines surrounding the call below. The same for the function below.