search

rive-app / rive-cpp-legacy

C++ runtime for Rive
MIT License
287 stars 43 forks source link

Clarify ownership for keyframes #278

Closed mikerreed closed 2 years ago

mikerreed commented 2 years ago

happiness = collapsing a destructor

mikerreed commented 2 years ago

LGTM!

As a general comment so you can understand my reticence on smart pointers... I'm always wary of the overhead of such abstractions for parts of the code that aren't client-facing and have large volume (some files have lots of keyframes) as that overhead usually varies greatly on different platforms/compilers but I think this one really should be 0 cost.

Understood. But I also agree that unique_ptr is almost always zero overhead. I like its documentation aspects as much as the ease-of-getting-it-right.

mikerreed commented 2 years ago

Related -- I've seen a few places where we allocate an array of objects (like KeyFrames) and the objects are all-the-same-type (and we don't edit the array later on. Sure seems like an opportunity to do a bulk allocation rather than one-at-a-time. I know the import process is (I presume) more gather than bulk, but we might look for opportunities for arenas in the future.