Cleanup: Anim, simplify animviz_verify_motionpaths()

Simplify `animviz_verify_motionpaths()` as the current code structure
got in the way of me reviewing an addition to it.

Some conditions and calculations are reordered so that they don't have
to be repeated. This allowed me to reduce nesting of the code, and make
it easier to understand when exactly a motion path is reused
cache-and-all, when it's reused but gets a new cache, and when it's
freshly allocated.

I also think this removes a theoretical memory leak, as there was a code
path that would allocate a new cache without freeing the old one. This
would require a somewhat invalid data structure to begin with, but the
code path now simply doesn't exist any more. I don't think this
problematic code path was ever hit in normal use, though.

No actual functional changes.
This commit is contained in:
Sybren A. Stüvel
2024-01-30 10:54:31 +01:00
parent 5a6034654c
commit 4d3bb6a38a
@@ -141,39 +141,33 @@ bMotionPath *animviz_verify_motionpaths(ReportList *reports,
return nullptr;
}
/* if there is already a motionpath, just return that,
* provided its settings are ok (saves extra free+alloc)
*/
if (*dst != nullptr) {
int expected_length = avs->path_ef - avs->path_sf;
const int expected_length = avs->path_ef - avs->path_sf;
BLI_assert(expected_length > 0); /* Because the `if` above. */
/* If there is already a motionpath, just return that, provided its settings
* are ok (saves extra free+alloc). */
if (*dst != nullptr) {
mpath = *dst;
/* Path is "valid" if length is valid,
* but must also be of the same length as is being requested. */
if ((mpath->start_frame != mpath->end_frame) && (mpath->length > 0)) {
/* outer check ensures that we have some curve data for this path */
if (mpath->length == expected_length) {
mpath->start_frame = avs->path_sf;
mpath->end_frame = avs->path_ef;
/* return/use this as it is already valid length */
return mpath;
}
/* clear the existing path (as the range has changed), and reallocate below */
animviz_free_motionpath_cache(mpath);
/* Only reuse a path if it was already a valid path, and of the expected length. */
if (mpath->start_frame != mpath->end_frame && mpath->length == expected_length) {
mpath->start_frame = avs->path_sf;
mpath->end_frame = avs->path_ef;
return mpath;
}
/* Clear the existing cache, to allocate a new one below. */
animviz_free_motionpath_cache(mpath);
}
else {
/* create a new motionpath, and assign it */
mpath = static_cast<bMotionPath *>(MEM_callocN(sizeof(bMotionPath), "bMotionPath"));
*dst = mpath;
}
/* set settings from the viz settings */
/* Copy mpath settings from the viz settings. */
mpath->start_frame = avs->path_sf;
mpath->end_frame = avs->path_ef;
mpath->length = mpath->end_frame - mpath->start_frame;
mpath->length = expected_length;
if (avs->path_bakeflag & MOTIONPATH_BAKE_HEADS) {
mpath->flag |= MOTIONPATH_FLAG_BHEAD;
@@ -182,22 +176,21 @@ bMotionPath *animviz_verify_motionpaths(ReportList *reports,
mpath->flag &= ~MOTIONPATH_FLAG_BHEAD;
}
/* set default custom values */
mpath->color[0] = 1.0; /* Red */
/* Set default custom values (RGB). */
mpath->color[0] = 1.0;
mpath->color[1] = 0.0;
mpath->color[2] = 0.0;
mpath->line_thickness = 2;
mpath->flag |= MOTIONPATH_FLAG_LINES; /* draw lines by default */
mpath->flag |= MOTIONPATH_FLAG_LINES;
/* allocate a cache */
/* Allocate a cache. */
mpath->points = static_cast<bMotionPathVert *>(
MEM_callocN(sizeof(bMotionPathVert) * mpath->length, "bMotionPathVerts"));
/* tag viz settings as currently having some path(s) which use it */
/* Tag viz settings as currently having some path(s) which use it. */
avs->path_bakeflag |= MOTIONPATH_BAKE_HAS_PATHS;
/* return it */
return mpath;
}