From 295df944786b6d0050d3dce8ea22d430edf4789a Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Tue, 18 Jun 2024 18:29:25 +0200 Subject: [PATCH] Anim: add "legacy behavior" option to Limit Rotation constraint This adds a "Legacy Behavior" option to the Limit Rotation constraint that makes it behave how Limit Rotation constraints did prior to ed2408400d53e943cb6c97db10f12cf6729ef089. Newly created constraints have this option disabled, but versioning code enables the option on constraints from older files to ensure that the behavior of e.g. existing rigs is not altered. This is one part of a two-part fix for #123105. The other part is in PR extensions/rigify#4. Pull Request: https://projects.blender.org/blender/blender/pulls/123361 --- .../startup/bl_ui/properties_constraint.py | 1 + .../blender/blenkernel/BKE_blender_version.h | 2 +- .../blender/blenkernel/intern/constraint.cc | 46 +++++++++++++++---- .../blenloader/intern/versioning_400.cc | 26 +++++++++++ .../blender/makesdna/DNA_constraint_types.h | 5 ++ .../blender/makesrna/intern/rna_constraint.cc | 8 ++++ 6 files changed, 77 insertions(+), 11 deletions(-) diff --git a/scripts/startup/bl_ui/properties_constraint.py b/scripts/startup/bl_ui/properties_constraint.py index 66f2195d348..1a01714b525 100644 --- a/scripts/startup/bl_ui/properties_constraint.py +++ b/scripts/startup/bl_ui/properties_constraint.py @@ -233,6 +233,7 @@ class ConstraintButtonsPanel: layout.prop(con, "euler_order", text="Order") layout.prop(con, "use_transform_limit") + layout.prop(con, "use_legacy_behavior") self.space_template(layout, con, target=False, owner=True) self.draw_influence(layout, con) diff --git a/source/blender/blenkernel/BKE_blender_version.h b/source/blender/blenkernel/BKE_blender_version.h index 30c6f3f823f..678124d8a87 100644 --- a/source/blender/blenkernel/BKE_blender_version.h +++ b/source/blender/blenkernel/BKE_blender_version.h @@ -29,7 +29,7 @@ extern "C" { /* Blender file format version. */ #define BLENDER_FILE_VERSION BLENDER_VERSION -#define BLENDER_FILE_SUBVERSION 59 +#define BLENDER_FILE_SUBVERSION 60 /* Minimum Blender version that supports reading file written with the current * version. Older Blender versions will test this and cancel loading the file, showing a warning to diff --git a/source/blender/blenkernel/intern/constraint.cc b/source/blender/blenkernel/intern/constraint.cc index 7e575b5111c..a5a8cfa5ade 100644 --- a/source/blender/blenkernel/intern/constraint.cc +++ b/source/blender/blenkernel/intern/constraint.cc @@ -1758,17 +1758,43 @@ static void rotlimit_evaluate(bConstraint *con, bConstraintOb *cob, ListBase * / mat4_to_eulO(eul, rot_order, cob->matrix); - /* constraint data uses radians internally */ - - /* limiting of euler values... */ - if (data->flag & LIMIT_XROT) { - eul[0] = clamp_angle(eul[0], data->xmin, data->xmax); + /* Limit the euler values. */ + if (data->flag & LIMIT_ROT_LEGACY_BEHAVIOR) { + /* The legacy behavior, which just does a naive clamping of the angles as + * simple numbers. Since the input angles are always in the range [-180, + * 180] degrees due to being derived from matrix decomposition, this naive + * approach causes problems when rotations cross 180 degrees. Specifically, + * it results in unpredictable and unwanted rotation flips of the + * constrained objects/bones, especially when the constraint isn't in local + * space. + * + * The correct thing to do is a more sophisticated form of clamping that + * treats the angles as existing on a continuous loop, which is what the + * non-legacy behavior further below does. However, for backwards + * compatibility we are preserving this old behavior behind an option. + * + * See issues #117927 and #123105 for additional background. */ + if (data->flag & LIMIT_XROT) { + eul[0] = clamp_f(eul[0], data->xmin, data->xmax); + } + if (data->flag & LIMIT_YROT) { + eul[1] = clamp_f(eul[1], data->ymin, data->ymax); + } + if (data->flag & LIMIT_ZROT) { + eul[2] = clamp_f(eul[2], data->zmin, data->zmax); + } } - if (data->flag & LIMIT_YROT) { - eul[1] = clamp_angle(eul[1], data->ymin, data->ymax); - } - if (data->flag & LIMIT_ZROT) { - eul[2] = clamp_angle(eul[2], data->zmin, data->zmax); + else { + /* The correct, non-legacy behavior. */ + if (data->flag & LIMIT_XROT) { + eul[0] = clamp_angle(eul[0], data->xmin, data->xmax); + } + if (data->flag & LIMIT_YROT) { + eul[1] = clamp_angle(eul[1], data->ymin, data->ymax); + } + if (data->flag & LIMIT_ZROT) { + eul[2] = clamp_angle(eul[2], data->zmin, data->zmax); + } } loc_eulO_size_to_mat4(cob->matrix, loc, eul, size, rot_order); diff --git a/source/blender/blenloader/intern/versioning_400.cc b/source/blender/blenloader/intern/versioning_400.cc index 5b1a89aee52..478f376d073 100644 --- a/source/blender/blenloader/intern/versioning_400.cc +++ b/source/blender/blenloader/intern/versioning_400.cc @@ -17,6 +17,7 @@ #include "DNA_anim_types.h" #include "DNA_brush_types.h" #include "DNA_camera_types.h" +#include "DNA_constraint_types.h" #include "DNA_curve_types.h" #include "DNA_defaults.h" #include "DNA_light_types.h" @@ -4165,6 +4166,31 @@ void blo_do_versions_400(FileData *fd, Library * /*lib*/, Main *bmain) FOREACH_NODETREE_END; } + if (!MAIN_VERSION_FILE_ATLEAST(bmain, 402, 60)) { + /* Limit Rotation constraints from old files should use the legacy Limit + * Rotation behavior. */ + LISTBASE_FOREACH (Object *, obj, &bmain->objects) { + LISTBASE_FOREACH (bConstraint *, constraint, &obj->constraints) { + if (constraint->type != CONSTRAINT_TYPE_ROTLIMIT) { + continue; + } + static_cast(constraint->data)->flag |= LIMIT_ROT_LEGACY_BEHAVIOR; + } + + if (!obj->pose) { + continue; + } + LISTBASE_FOREACH (bPoseChannel *, pbone, &obj->pose->chanbase) { + LISTBASE_FOREACH (bConstraint *, constraint, &pbone->constraints) { + if (constraint->type != CONSTRAINT_TYPE_ROTLIMIT) { + continue; + } + static_cast(constraint->data)->flag |= LIMIT_ROT_LEGACY_BEHAVIOR; + } + } + } + } + /** * Always bump subversion in BKE_blender_version.h when adding versioning * code here, and wrap it inside a MAIN_VERSION_FILE_ATLEAST check. diff --git a/source/blender/makesdna/DNA_constraint_types.h b/source/blender/makesdna/DNA_constraint_types.h index 5a36d2f0b66..e4b0e94b408 100644 --- a/source/blender/makesdna/DNA_constraint_types.h +++ b/source/blender/makesdna/DNA_constraint_types.h @@ -1067,6 +1067,11 @@ typedef enum eRotLimit_Flags { LIMIT_XROT = (1 << 0), LIMIT_YROT = (1 << 1), LIMIT_ZROT = (1 << 2), + + /* Use the legacy behavior of the Limit Rotation constraint. See the + * implementation of `rotlimit_evaluate()` in constraint.cc for more + * details. */ + LIMIT_ROT_LEGACY_BEHAVIOR = (1 << 3), } eRotLimit_Flags; /* distance limit constraint */ diff --git a/source/blender/makesrna/intern/rna_constraint.cc b/source/blender/makesrna/intern/rna_constraint.cc index 18b54ef216a..45eb79e5c30 100644 --- a/source/blender/makesrna/intern/rna_constraint.cc +++ b/source/blender/makesrna/intern/rna_constraint.cc @@ -2690,6 +2690,14 @@ static void rna_def_constraint_rotation_limit(BlenderRNA *brna) prop, "Affect Transform", "Transform tools are affected by this constraint as well"); RNA_def_property_update(prop, NC_OBJECT | ND_CONSTRAINT, "rna_Constraint_update"); + prop = RNA_def_property(srna, "use_legacy_behavior", PROP_BOOLEAN, PROP_NONE); + RNA_def_property_boolean_sdna(prop, nullptr, "flag", LIMIT_ROT_LEGACY_BEHAVIOR); + RNA_def_property_ui_text( + prop, + "Legacy Behavior", + "Use the old semi-broken behavior that doesn't understand that rotations loop around"); + RNA_def_property_update(prop, NC_OBJECT | ND_CONSTRAINT, "rna_Constraint_update"); + RNA_define_lib_overridable(false); }