blender
/
blender
137
417
Fork 599
Code Issues 6.3k Pull Requests 775 Projects 19 Wiki Activity

Refactor: fcurve bounds functions #105177

Merged
Christoph Lendenfeld merged 16 commits from ChrisLend/blender:refactor_fcurve_bounds into main 2023-03-10 11:33:21 +01:00
Conversation 0 Commits 16 Files Changed 9 +34 -2
2 changed files with 34 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 69053a6421 - Show all commits

4
source/blender/blenkernel/intern/fcurve.c
View File

@ -710,8 +710,8 @@ static bool calculate_fpt_bounds(const FCurve *fcu, const float frame_range[2],
if (frame_range != NULL) {
/* Start index can be calculated because fpt has a key on every full frame. */
const int start_index = (int)(frame_range[0] - fcu->fpt[0].vec[0]);
const int end_index = start_index + (int)(frame_range[1] - frame_range[0]);
const float start_index = frame_range[0] - fcu->fpt[0].vec[0];
const float end_index = start_index + frame_range[1] - frame_range[0];
dr.sybren marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-27 15:39:28 +01:00
Review
Copy Link

Since there are no handles, you can use the ordering of the points and do a binary search for the start of the range. You can then immediately handle the xmin, and when the last point in the range is found, update the xmax. Per point you then only need to consider the y-coordinates, cutting the number of operations in half.

Since there are no handles, you can use the ordering of the points and do a binary search for the start of the range. You can then immediately handle the `xmin`, and when the last point in the range is found, update the `xmax`. Per point you then only need to consider the y-coordinates, cutting the number of operations in half.
Christoph Lendenfeld commented 2023-03-02 12:55:15 +01:00
Review
Copy Link

instead of doing a binary search i calculate the index based on the assumption that a baked fcurve has a sample on every full frame.

instead of doing a binary search i calculate the index based on the assumption that a baked fcurve has a sample on every full frame.
if (start_index > fcu->totvert - 1 || end_index < 0) {
/* Range is outside of keyframe samples. */
ChrisLend marked this conversation as resolved
Sybren A. Stüvel commented 2023-02-27 15:41:49 +01:00
Review
Copy Link

Initialize r_bounds to ±∞ (so all minimum values to ∞ and all maximum values to -∞). Those are valid IEEE 754 floating point values, and work well with the min/max functions. That'll remove a boolean, a condition, and half of the lines of code.

Initialize `r_bounds` to `±∞` (so all minimum values to `∞` and all maximum values to `-∞`). Those are valid IEEE 754 floating point values, and work well with the min/max functions. That'll remove a boolean, a condition, and half of the lines of code.

32
source/blender/blenkernel/intern/fcurve_test.cc
View File

@ -496,6 +496,38 @@ TEST(BKE_fcurve, BKE_fcurve_calc_bounds)
EXPECT_FLOAT_EQ(fcu->bezt[3].vec[2][1], bounds.ymin);
EXPECT_FLOAT_EQ(fcu->bezt[3].vec[0][1], bounds.ymax);
/* Curve samples. */
const int sample_start = 1;
const int sample_end = 20;
fcurve_store_samples(fcu, NULL, sample_start, sample_end, fcurve_samplingcb_evalcurve);
success = BKE_fcurve_calc_bounds(
fcu, false /* sel only */, false /* include handles */, NULL /* frame range */, &bounds);
EXPECT_TRUE(success) << "FCurve samples should have a range.";
EXPECT_FLOAT_EQ(sample_start, bounds.xmin);
EXPECT_FLOAT_EQ(sample_end, bounds.xmax);
EXPECT_FLOAT_EQ(-20.0f, bounds.ymin);
EXPECT_FLOAT_EQ(15.0f, bounds.ymax);
range[0] = 8.0f;
range[1] = 20.0f;
success = BKE_fcurve_calc_bounds(
fcu, false /* sel only */, false /* include handles */, range /* frame range */, &bounds);
EXPECT_TRUE(success) << "FCurve samples should have a range.";
EXPECT_FLOAT_EQ(range[0], bounds.xmin);
EXPECT_FLOAT_EQ(range[1], bounds.xmax);
EXPECT_FLOAT_EQ(-20.0f, bounds.ymin);
EXPECT_FLOAT_EQ(15.0f, bounds.ymax);
range[0] = 20.1f;
range[1] = 30.0f;
success = BKE_fcurve_calc_bounds(
fcu, false /* sel only */, false /* include handles */, range /* frame range */, &bounds);
EXPECT_FALSE(success)
<< "A frame range outside the range of keyframe samples should not have bounds.";
BKE_fcurve_free(fcu);
}