These functions are now named BKE_{verb}_{submodule}_{noun}. I think it's more common to have BKE_{submodule}_{verb}_{noun} or `BKE_{submodule}_{noun}_{verb}, so that alphabetical ordering (like in IDE autocompletion) will group related functions together.

To me there is little difference between "length" and "total distance", so these names could be improved. I'm guessing that len indicates "number of segments in the curve" and tot_dist indicates "arc length of the curve", but "length" could just as well mean "arc length". Having "distance" as a name for "arc length" would be weird though, so my guess could be wrong.

typo

"distance" → "length" or "arc length". A distance is defined between two things, and is not a propery of a single thing.

"len_data" should be "the length of the data", f.e. the length of an array. anim_path_accum_length would be better.

Why is this assumed?

typo

Here we are under a "last segment" comment in a "cyclic" condition, yet "Last segment in a cyclic curve" is handled below. This is confusing.

This is missing "and idx does not refer to the first or last segment".

That's clear from the code. Why is this enough, though?

I think this can be rewritten so that it can become a const. Assigning false and then conditionally assigning true simply means assigning the condition.

By renaming the variable to is_cyclic the comment can also be removed, as then the code is clear enough by itself. With all these rewrites, I don't see the benefit of keeping this cryptic name.

By comparing using >= 0 the "is non-negative" is clearer to me at first glance.

In other words: const bool is_cyclic = bl->poly >= 0.

const

const

const

const

p1 and p2 are already in use for the final result points. I'd just name them left_len and right_len so that they match a typical binary search. It also makes the relation cleare between these and the /* Go to the right. */ and /* Else, go to the left. */ comments.

Unnecessary parentheses.

Shouldn't this check appear either before cur_step is used, or after it has been assigned a new value? It's strange to have it here in the middle.

Unnecessary parentheses.

Change this to BLI_assert(!"Couldn't find any valid point on the animation path!") so that the assertion states the reason for it failling.

This "go to the next half" code is a bit strange. It seems to spread its code between this bit and the first line of the next loop iteration (cur_idx = cur_base + (cur_step / 2);).

A traditional binary search maintains an index of the left and right element of the interval, and then computes the midpoint from that; to go to the next iteration, either the left or the right point is moved to the mid point ± 1. This implementation instead only keeps track of the left point and the distance to the right point. To me it's not obviously clear that dividing this distance won't skip certain right-side points when the array length is not a power of two. Furthermore, this requires two divisions by two for each iteration, instead of just one.

What is the reason to choose this implementation over a more traditional one?

I don't see why it would take 9-10 arguments. The inputs seem to be accum_len_arr and seg_len, and the outputs frac and cur_idx. From what I can see, the call to get_curve_points_from_idx(cur_idx, bl, cycl, &p0, &p1, &p2, &p3); can be done outside of that function.

I'd prefer cur_step /= 2 instead, as it makes it clearer at first glance that the current value is divided by two. Otherwise you need to carefully read the second cur_step to ensure it's not one of the other cur_xxxx variables.

const