diff options
| -rw-r--r-- | source/blender/blenkernel/BKE_fcurve.h | 4 | ||||
| -rw-r--r-- | source/blender/blenkernel/intern/fcurve.c | 8 | ||||
| -rw-r--r-- | source/blender/blenkernel/intern/fcurve_test.cc | 31 | ||||
| -rw-r--r-- | source/blender/editors/space_graph/graph_edit.c | 238 | ||||
| -rw-r--r-- | tests/python/bl_animation_fcurves.py | 96 |
5 files changed, 309 insertions, 68 deletions
diff --git a/source/blender/blenkernel/BKE_fcurve.h b/source/blender/blenkernel/BKE_fcurve.h index f527f40d0d7..4569e68ea4a 100644 --- a/source/blender/blenkernel/BKE_fcurve.h +++ b/source/blender/blenkernel/BKE_fcurve.h @@ -248,6 +248,10 @@ bool BKE_fcurve_calc_bounds(struct FCurve *fcu, void BKE_fcurve_active_keyframe_set(struct FCurve *fcu, const struct BezTriple *active_bezt); int BKE_fcurve_active_keyframe_index(const struct FCurve *fcu); +/* Move the indexed keyframe to the given value, and move the handles with it to ensure the slope + * remains the same. */ +void BKE_fcurve_keyframe_move_value_with_handles(struct BezTriple *keyframe, float new_value); + /* .............. */ /* Are keyframes on F-Curve of any use (to final result, and to show in editors)? */ diff --git a/source/blender/blenkernel/intern/fcurve.c b/source/blender/blenkernel/intern/fcurve.c index e6d3696b198..bcd2fe098b7 100644 --- a/source/blender/blenkernel/intern/fcurve.c +++ b/source/blender/blenkernel/intern/fcurve.c @@ -884,6 +884,14 @@ int BKE_fcurve_active_keyframe_index(const FCurve *fcu) /** \} */ +void BKE_fcurve_keyframe_move_value_with_handles(struct BezTriple *keyframe, const float new_value) +{ + const float value_delta = new_value - keyframe->vec[1][1]; + keyframe->vec[0][1] += value_delta; + keyframe->vec[1][1] = new_value; + keyframe->vec[2][1] += value_delta; +} + /* -------------------------------------------------------------------- */ /** \name Status Checks * \{ */ diff --git a/source/blender/blenkernel/intern/fcurve_test.cc b/source/blender/blenkernel/intern/fcurve_test.cc index fb6ce02d146..7d6b5fd88be 100644 --- a/source/blender/blenkernel/intern/fcurve_test.cc +++ b/source/blender/blenkernel/intern/fcurve_test.cc @@ -331,4 +331,35 @@ TEST(fcurve_active_keyframe, ActiveKeyframe) BKE_fcurve_free(fcu); } +TEST(BKE_fcurve, BKE_fcurve_keyframe_move_value_with_handles) +{ + FCurve *fcu = BKE_fcurve_create(); + + insert_vert_fcurve(fcu, 1.0f, 7.5f, BEZT_KEYTYPE_KEYFRAME, INSERTKEY_NO_USERPREF); + insert_vert_fcurve(fcu, 8.0f, 15.0f, BEZT_KEYTYPE_KEYFRAME, INSERTKEY_NO_USERPREF); + insert_vert_fcurve(fcu, 14.0f, 8.2f, BEZT_KEYTYPE_KEYFRAME, INSERTKEY_NO_USERPREF); + + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[0][0], 5.2671194f); + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[0][1], 15.0f); + + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[1][0], 8.0f); + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[1][1], 15.0f); + + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[2][0], 10.342469f); + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[2][1], 15.0f); + + BKE_fcurve_keyframe_move_value_with_handles(&fcu->bezt[1], 47.0f); + + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[0][0], 5.2671194f) << "Left handle should not move in time"; + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[0][1], 47.0f) << "Left handle value should have been updated"; + + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[1][0], 8.0f) << "Frame should not move in time"; + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[1][1], 47.0f) << "Frame value should have been updated"; + + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[2][0], 10.342469f) << "Right handle should not move in time"; + EXPECT_FLOAT_EQ(fcu->bezt[1].vec[2][1], 47.0f) << "Right handle value should have been updated"; + + BKE_fcurve_free(fcu); +} + } // namespace blender::bke::tests diff --git a/source/blender/editors/space_graph/graph_edit.c b/source/blender/editors/space_graph/graph_edit.c index 09440ee7d98..bcd3adec1ab 100644 --- a/source/blender/editors/space_graph/graph_edit.c +++ b/source/blender/editors/space_graph/graph_edit.c @@ -2610,6 +2610,14 @@ typedef struct tEulerFilter { const char *rna_path; } tEulerFilter; +static bool keyframe_time_differs(BezTriple *keyframes[3]) +{ + const float precision = 1e-5; + return fabs(keyframes[0]->vec[1][0] - keyframes[1]->vec[1][0]) > precision || + fabs(keyframes[1]->vec[1][0] - keyframes[2]->vec[1][0]) > precision || + fabs(keyframes[0]->vec[1][0] - keyframes[2]->vec[1][0]) > precision; +} + /* Find groups of `rotation_euler` channels. */ static ListBase /*tEulerFilter*/ euler_filter_group_channels( const ListBase /*bAnimListElem*/ *anim_data, ReportList *reports, int *r_num_groups) @@ -2666,68 +2674,151 @@ static ListBase /*tEulerFilter*/ euler_filter_group_channels( return euler_groups; } -static int euler_filter_perform_filter(ListBase /*tEulerFilter*/ *eulers, ReportList *reports) +/* Perform discontinuity filter based on conversion to matrix and back. + * Return true if the curves were filtered (which may have been a no-op), false otherwise. */ +static bool euler_filter_multi_channel(tEulerFilter *euf, ReportList *reports) +{ + /* Sanity check: ensure that there are enough F-Curves to work on in this group. */ + if (ELEM(NULL, euf->fcurves[0], euf->fcurves[1], euf->fcurves[2])) { + /* Report which components are missing. */ + BKE_reportf(reports, + RPT_INFO, + "Missing %s%s%s component(s) of euler rotation for ID='%s' and RNA-Path='%s'", + (euf->fcurves[0] == NULL) ? "X" : "", + (euf->fcurves[1] == NULL) ? "Y" : "", + (euf->fcurves[2] == NULL) ? "Z" : "", + euf->id->name, + euf->rna_path); + return false; + } + + FCurve *fcu_rot_x = euf->fcurves[0]; + FCurve *fcu_rot_y = euf->fcurves[1]; + FCurve *fcu_rot_z = euf->fcurves[2]; + if (fcu_rot_x->totvert != fcu_rot_y->totvert || fcu_rot_y->totvert != fcu_rot_z->totvert) { + BKE_reportf(reports, + RPT_INFO, + "XYZ rotations not equally keyed for ID='%s' and RNA-Path='%s'", + euf->id->name, + euf->rna_path); + return false; + } + + if (fcu_rot_x->totvert < 2) { + /* Empty curves and single keyframes are trivially "filtered". */ + return false; + } + + float filtered_euler[3] = { + fcu_rot_x->bezt[0].vec[1][1], + fcu_rot_y->bezt[0].vec[1][1], + fcu_rot_z->bezt[0].vec[1][1], + }; + + for (int keyframe_index = 1; keyframe_index < fcu_rot_x->totvert; ++keyframe_index) { + BezTriple *keyframes[3] = { + &fcu_rot_x->bezt[keyframe_index], + &fcu_rot_y->bezt[keyframe_index], + &fcu_rot_z->bezt[keyframe_index], + }; + + if (keyframe_time_differs(keyframes)) { + /* The X-coordinates of the keyframes are different, so we cannot correct this key. */ + continue; + } + + const float unfiltered_euler[3] = { + keyframes[0]->vec[1][1], + keyframes[1]->vec[1][1], + keyframes[2]->vec[1][1], + }; + + /* The conversion back from matrix to Euler angles actually performs the filtering. */ + float matrix[3][3]; + eul_to_mat3(matrix, unfiltered_euler); + mat3_normalized_to_compatible_eul(filtered_euler, filtered_euler, matrix); + + /* TODO(Sybren): it might be a nice touch to compare `filtered_euler` with `unfiltered_euler`, + * to see if there was actually a change. This could improve reporting for the artist. */ + + BKE_fcurve_keyframe_move_value_with_handles(keyframes[0], filtered_euler[0]); + BKE_fcurve_keyframe_move_value_with_handles(keyframes[1], filtered_euler[1]); + BKE_fcurve_keyframe_move_value_with_handles(keyframes[2], filtered_euler[2]); + } + + return true; +} + +/* Remove 360-degree flips from a single FCurve. + * Return true if the curve was modified, false otherwise. */ +static bool euler_filter_single_channel(FCurve *fcu) { - int failed = 0; + /* Simple method: just treat any difference between keys of greater than 180 degrees as being a + * flip. */ + BezTriple *bezt, *prev; + uint i; - LISTBASE_FOREACH (tEulerFilter *, euf, eulers) { - /* Sanity check: ensure that there are enough F-Curves to work on in this group. */ - /* TODO: also enforce assumption that there be a full set of keyframes - * at each position by ensuring that totvert counts are same? (Joshua Leung 2011) */ - if (ELEM(NULL, euf->fcurves[0], euf->fcurves[1], euf->fcurves[2])) { - /* Report which components are missing. */ - BKE_reportf(reports, - RPT_WARNING, - "Missing %s%s%s component(s) of euler rotation for ID='%s' and RNA-Path='%s'", - (euf->fcurves[0] == NULL) ? "X" : "", - (euf->fcurves[1] == NULL) ? "Y" : "", - (euf->fcurves[2] == NULL) ? "Z" : "", - euf->id->name, - euf->rna_path); - - /* Keep track of number of failed sets, and carry on to next group. */ - failed++; + /* Skip if not enough verts to do a decent analysis. */ + if (fcu->totvert <= 2) { + return false; + } + + /* Prev follows bezt, bezt = "current" point to be fixed. */ + /* Our method depends on determining a "difference" from the previous vert. */ + bool is_modified = false; + for (i = 1, prev = fcu->bezt, bezt = fcu->bezt + 1; i < fcu->totvert; i++, prev = bezt++) { + const float sign = (prev->vec[1][1] > bezt->vec[1][1]) ? 1.0f : -1.0f; + + /* >= 180 degree flip? */ + if ((sign * (prev->vec[1][1] - bezt->vec[1][1])) < (float)M_PI) { continue; } - /* Simple method: just treat any difference between - * keys of greater than 180 degrees as being a flip. */ - /* FIXME: there are more complicated methods that - * will be needed to fix more cases than just some */ - for (int f = 0; f < 3; f++) { - FCurve *fcu = euf->fcurves[f]; - BezTriple *bezt, *prev; - uint i; - - /* Skip if not enough vets to do a decent analysis of.... */ - if (fcu->totvert <= 2) { - continue; - } + /* 360 degrees to add/subtract frame value until difference + * is acceptably small that there's no more flip. */ + const float fac = sign * 2.0f * (float)M_PI; - /* Prev follows bezt, bezt = "current" point to be fixed. */ - /* Our method depends on determining a "difference" from the previous vert. */ - for (i = 1, prev = fcu->bezt, bezt = fcu->bezt + 1; i < fcu->totvert; i++, prev = bezt++) { - const float sign = (prev->vec[1][1] > bezt->vec[1][1]) ? 1.0f : -1.0f; + while ((sign * (prev->vec[1][1] - bezt->vec[1][1])) >= (float)M_PI) { + bezt->vec[0][1] += fac; + bezt->vec[1][1] += fac; + bezt->vec[2][1] += fac; + } - /* >= 180 degree flip? */ - if ((sign * (prev->vec[1][1] - bezt->vec[1][1])) < (float)M_PI) { - continue; - } + is_modified = true; + } - /* 360 degrees to add/subtract frame value until difference - * is acceptably small that there's no more flip. */ - const float fac = sign * 2.0f * (float)M_PI; + return is_modified; +} - while ((sign * (prev->vec[1][1] - bezt->vec[1][1])) >= (float)M_PI) { - bezt->vec[0][1] += fac; - bezt->vec[1][1] += fac; - bezt->vec[2][1] += fac; - } +static void euler_filter_perform_filter(ListBase /*tEulerFilter*/ *eulers, + ReportList *reports, + int *r_curves_filtered, + int *r_curves_seen) +{ + *r_curves_filtered = 0; + *r_curves_seen = 0; + + LISTBASE_FOREACH (tEulerFilter *, euf, eulers) { + int curves_filtered_this_group = 0; + + if (euler_filter_multi_channel(euf, reports)) { + curves_filtered_this_group = 3; + } + + for (int channel_index = 0; channel_index < 3; channel_index++) { + FCurve *fcu = euf->fcurves[channel_index]; + if (fcu == NULL) { + continue; + } + ++*r_curves_seen; + + if (euler_filter_single_channel(fcu)) { + ++curves_filtered_this_group; } } - } - return failed; + *r_curves_filtered += min_ii(3, curves_filtered_this_group); + } } static int graphkeys_euler_filter_exec(bContext *C, wmOperator *op) @@ -2764,30 +2855,43 @@ static int graphkeys_euler_filter_exec(bContext *C, wmOperator *op) /* Step 2: go through each set of curves, processing the values at each keyframe. * - It is assumed that there must be a full set of keyframes at each keyframe position. */ - const int failed = euler_filter_perform_filter(&eulers, op->reports); - BLI_freelistN(&eulers); + int curves_filtered; + int curves_seen; + euler_filter_perform_filter(&eulers, op->reports, &curves_filtered, &curves_seen); + BLI_freelistN(&eulers); ANIM_animdata_update(&ac, &anim_data); ANIM_animdata_freelist(&anim_data); - /* Updates + finishing warnings. */ - if (failed == groups) { - BKE_report( - op->reports, - RPT_ERROR, - "No Euler Rotations could be corrected, ensure each rotation has keys for all components, " - "and that F-Curves for these are in consecutive XYZ order and selected"); + if (curves_filtered == 0) { + if (curves_seen < 3) { + /* Showing the entire error message makes no sense when the artist is only trying to filter + * one or two curves. */ + BKE_report(op->reports, RPT_WARNING, "No Euler Rotations could be corrected."); + } + else { + BKE_report(op->reports, + RPT_ERROR, + "No Euler Rotations could be corrected, ensure each rotation has keys for all " + "components, " + "and that F-Curves for these are in consecutive XYZ order and selected"); + } return OPERATOR_CANCELLED; } - if (failed) { - BKE_report( - op->reports, - RPT_ERROR, - "Some Euler Rotations could not be corrected due to missing/unselected/out-of-order " - "F-Curves, " - "ensure each rotation has keys for all components, and that F-Curves for these are in " - "consecutive XYZ order and selected"); + if (curves_filtered != curves_seen) { + BLI_assert(curves_filtered < curves_seen); + BKE_reportf(op->reports, + RPT_INFO, + "%d of %d rotation channels were filtered. See the Info window for details.", + curves_filtered, + curves_seen); + } + else if (curves_seen == 1) { + BKE_report(op->reports, RPT_INFO, "The rotation channel was filtered."); + } + else { + BKE_reportf(op->reports, RPT_INFO, "All %d rotation channels were filtered.", curves_seen); } /* Set notifier that keyframes have changed. */ diff --git a/tests/python/bl_animation_fcurves.py b/tests/python/bl_animation_fcurves.py index b5772b8d335..2ec04749d70 100644 --- a/tests/python/bl_animation_fcurves.py +++ b/tests/python/bl_animation_fcurves.py @@ -25,11 +25,13 @@ blender -b -noaudio --factory-startup --python tests/python/bl_animation_fcurves import pathlib import sys import unittest +from math import degrees, radians +from typing import List import bpy -class FCurveEvaluationTest(unittest.TestCase): +class AbstractAnimationTest: @classmethod def setUpClass(cls): cls.testdir = args.testdir @@ -38,6 +40,7 @@ class FCurveEvaluationTest(unittest.TestCase): self.assertTrue(self.testdir.exists(), 'Test dir %s should exist' % self.testdir) +class FCurveEvaluationTest(AbstractAnimationTest, unittest.TestCase): def test_fcurve_versioning_291(self): # See D8752. bpy.ops.wm.open_mainfile(filepath=str(self.testdir / "fcurve-versioning-291.blend")) @@ -73,6 +76,97 @@ class FCurveEvaluationTest(unittest.TestCase): self.assertAlmostEqual(1.0, fcurve.evaluate(10)) +class EulerFilterTest(AbstractAnimationTest, unittest.TestCase): + def setUp(self): + super().setUp() + bpy.ops.wm.open_mainfile(filepath=str(self.testdir / "euler-filter.blend")) + + def test_multi_channel_filter(self): + """Test fixing discontinuities that require all X/Y/Z rotations to work.""" + + self.activate_object('Three-Channel-Jump') + fcu_rot = self.active_object_rotation_channels() + + ## Check some pre-filter values to make sure the file is as we expect. + # Keyframes before the "jump". These shouldn't be touched by the filter. + self.assertEqualAngle(-87.5742, fcu_rot[0], 22) + self.assertEqualAngle(69.1701, fcu_rot[1], 22) + self.assertEqualAngle(-92.3918, fcu_rot[2], 22) + # Keyframes after the "jump". These should be updated by the filter. + self.assertEqualAngle(81.3266, fcu_rot[0], 23) + self.assertEqualAngle(111.422, fcu_rot[1], 23) + self.assertEqualAngle(76.5996, fcu_rot[2], 23) + + bpy.ops.graph.euler_filter(self.get_context()) + + # Keyframes before the "jump". These shouldn't be touched by the filter. + self.assertEqualAngle(-87.5742, fcu_rot[0], 22) + self.assertEqualAngle(69.1701, fcu_rot[1], 22) + self.assertEqualAngle(-92.3918, fcu_rot[2], 22) + # Keyframes after the "jump". These should be updated by the filter. + self.assertEqualAngle(-98.6734, fcu_rot[0], 23) + self.assertEqualAngle(68.5783, fcu_rot[1], 23) + self.assertEqualAngle(-103.4, fcu_rot[2], 23) + + def test_single_channel_filter(self): + """Test fixing discontinuities in single channels.""" + + self.activate_object('One-Channel-Jumps') + fcu_rot = self.active_object_rotation_channels() + + ## Check some pre-filter values to make sure the file is as we expect. + # Keyframes before the "jump". These shouldn't be touched by the filter. + self.assertEqualAngle(360, fcu_rot[0], 15) + self.assertEqualAngle(396, fcu_rot[1], 21) # X and Y are keyed on different frames. + # Keyframes after the "jump". These should be updated by the filter. + self.assertEqualAngle(720, fcu_rot[0], 16) + self.assertEqualAngle(72, fcu_rot[1], 22) + + bpy.ops.graph.euler_filter(self.get_context()) + + # Keyframes before the "jump". These shouldn't be touched by the filter. + self.assertEqualAngle(360, fcu_rot[0], 15) + self.assertEqualAngle(396, fcu_rot[1], 21) # X and Y are keyed on different frames. + # Keyframes after the "jump". These should be updated by the filter. + self.assertEqualAngle(360, fcu_rot[0], 16) + self.assertEqualAngle(432, fcu_rot[1], 22) + + def assertEqualAngle(self, angle_degrees: float, fcurve: bpy.types.FCurve, frame: int) -> None: + self.assertAlmostEqual( + radians(angle_degrees), + fcurve.evaluate(frame), + 4, + "Expected %.3f degrees, but FCurve %s[%d] evaluated to %.3f on frame %d" % ( + angle_degrees, fcurve.data_path, fcurve.array_index, degrees(fcurve.evaluate(frame)), frame, + ) + ) + + @staticmethod + def get_context(): + ctx = bpy.context.copy() + + for area in bpy.context.window.screen.areas: + if area.type != 'GRAPH_EDITOR': + continue + + ctx['area'] = area + ctx['space'] = area.spaces.active + break + + return ctx + + @staticmethod + def activate_object(object_name: str) -> None: + ob = bpy.data.objects[object_name] + bpy.context.view_layer.objects.active = ob + + @staticmethod + def active_object_rotation_channels() -> List[bpy.types.FCurve]: + ob = bpy.context.view_layer.objects.active + action = ob.animation_data.action + return [action.fcurves.find('rotation_euler', index=idx) for idx in range(3)] + + def main(): global args import argparse |