diff options
author | Campbell Barton <campbell@blender.org> | 2022-08-25 05:45:43 +0300 |
---|---|---|
committer | Campbell Barton <campbell@blender.org> | 2022-08-25 06:48:31 +0300 |
commit | a7650c6206908a8865d6140a310809ec5ab0c770 (patch) | |
tree | 63137fb4cb772daafe5683a86b5496361e83deae | |
parent | 8593228a13d38057a5d849f46d5cc0ab23fb1405 (diff) |
BLI_math: ensure non-negative matrices for mat3_to_quat calculations
Making the callers responsible for this isn't practical as matrices are
often passed indirectly to a functions such as mat3_to_axis_angle,
BKE_object_mat3_to_rot & BKE_pchan_mat3_to_rot.
Or the matrix is combined from other matrices which could be negative.
Given quaternions calculated from negative matrices are completely
invalid and checking only needs to negate matrices with a negative
determinant, move the check into mat3_to_quat and related functions.
Add mat3_normalized_to_quat_fast for cases no error checking on the
input matrix is needed such as blending rotations.
-rw-r--r-- | source/blender/blenlib/BLI_math_matrix.h | 12 | ||||
-rw-r--r-- | source/blender/blenlib/BLI_math_rotation.h | 5 | ||||
-rw-r--r-- | source/blender/blenlib/intern/math_matrix.c | 15 | ||||
-rw-r--r-- | source/blender/blenlib/intern/math_rotation.c | 52 | ||||
-rw-r--r-- | source/blender/editors/space_view3d/view3d_utils.c | 11 | ||||
-rw-r--r-- | source/blender/editors/space_view3d/view3d_view.c | 6 | ||||
-rw-r--r-- | source/blender/python/mathutils/mathutils_Matrix.c | 13 | ||||
-rw-r--r-- | source/blender/python/mathutils/mathutils_Quaternion.c | 8 |
8 files changed, 64 insertions, 58 deletions
diff --git a/source/blender/blenlib/BLI_math_matrix.h b/source/blender/blenlib/BLI_math_matrix.h index 87a01e0c264..467e6db4805 100644 --- a/source/blender/blenlib/BLI_math_matrix.h +++ b/source/blender/blenlib/BLI_math_matrix.h @@ -454,8 +454,20 @@ void rescale_m4(float mat[4][4], const float scale[3]); */ void transform_pivot_set_m4(float mat[4][4], const float pivot[3]); +/** + * \param rot: A 3x3 rotation matrix, normalized never negative. + */ void mat4_to_rot(float rot[3][3], const float wmat[4][4]); + +/** + * \param rot: A 3x3 rotation matrix, normalized never negative. + * \param size: The scale, negative if `mat3` is negative. + */ void mat3_to_rot_size(float rot[3][3], float size[3], const float mat3[3][3]); +/** + * \param rot: A 3x3 rotation matrix, normalized never negative. + * \param size: The scale, negative if `mat3` is negative. + */ void mat4_to_loc_rot_size(float loc[3], float rot[3][3], float size[3], const float wmat[4][4]); void mat4_to_loc_quat(float loc[3], float quat[4], const float wmat[4][4]); void mat4_decompose(float loc[3], float quat[4], float size[3], const float wmat[4][4]); diff --git a/source/blender/blenlib/BLI_math_rotation.h b/source/blender/blenlib/BLI_math_rotation.h index d4b97b85134..1fa088d7128 100644 --- a/source/blender/blenlib/BLI_math_rotation.h +++ b/source/blender/blenlib/BLI_math_rotation.h @@ -118,6 +118,11 @@ void quat_to_mat4(float m[4][4], const float q[4]); */ void quat_to_compatible_quat(float q[4], const float a[4], const float old[4]); +/** + * A version of #mat3_normalized_to_quat that skips error checking. + */ +void mat3_normalized_to_quat_fast(float q[4], const float mat[3][3]); + void mat3_normalized_to_quat(float q[4], const float mat[3][3]); void mat4_normalized_to_quat(float q[4], const float mat[4][4]); void mat3_to_quat(float q[4], const float mat[3][3]); diff --git a/source/blender/blenlib/intern/math_matrix.c b/source/blender/blenlib/intern/math_matrix.c index c4c9b9e3d01..e96b12033a9 100644 --- a/source/blender/blenlib/intern/math_matrix.c +++ b/source/blender/blenlib/intern/math_matrix.c @@ -2239,11 +2239,6 @@ void mat4_to_loc_quat(float loc[3], float quat[4], const float wmat[4][4]) copy_m3_m4(mat3, wmat); normalize_m3_m3(mat3_n, mat3); - /* So scale doesn't interfere with rotation T24291. */ - if (is_negative_m3(mat3)) { - negate_m3(mat3_n); - } - mat3_normalized_to_quat(quat, mat3_n); copy_v3_v3(loc, wmat[3]); } @@ -2252,7 +2247,7 @@ void mat4_decompose(float loc[3], float quat[4], float size[3], const float wmat { float rot[3][3]; mat4_to_loc_rot_size(loc, rot, size, wmat); - mat3_normalized_to_quat(quat, rot); + mat3_normalized_to_quat_fast(quat, rot); } /** @@ -2391,8 +2386,8 @@ void blend_m3_m3m3(float out[3][3], mat3_to_rot_size(drot, dscale, dst); mat3_to_rot_size(srot, sscale, src); - mat3_normalized_to_quat(dquat, drot); - mat3_normalized_to_quat(squat, srot); + mat3_normalized_to_quat_fast(dquat, drot); + mat3_normalized_to_quat_fast(squat, srot); /* do blending */ interp_qt_qtqt(fquat, dquat, squat, srcweight); @@ -2417,8 +2412,8 @@ void blend_m4_m4m4(float out[4][4], mat4_to_loc_rot_size(dloc, drot, dscale, dst); mat4_to_loc_rot_size(sloc, srot, sscale, src); - mat3_normalized_to_quat(dquat, drot); - mat3_normalized_to_quat(squat, srot); + mat3_normalized_to_quat_fast(dquat, drot); + mat3_normalized_to_quat_fast(squat, srot); /* do blending */ interp_v3_v3v3(floc, dloc, sloc, srcweight); diff --git a/source/blender/blenlib/intern/math_rotation.c b/source/blender/blenlib/intern/math_rotation.c index 7ecc271fa2a..bbea95514e9 100644 --- a/source/blender/blenlib/intern/math_rotation.c +++ b/source/blender/blenlib/intern/math_rotation.c @@ -269,13 +269,11 @@ void quat_to_mat4(float m[4][4], const float q[4]) m[3][3] = 1.0f; } -void mat3_normalized_to_quat(float q[4], const float mat[3][3]) +void mat3_normalized_to_quat_fast(float q[4], const float mat[3][3]) { BLI_ASSERT_UNIT_M3(mat); - /* Callers must ensure matrices have a positive determinant for valid results, see: T94231. */ - BLI_assert_msg(!is_negative_m3(mat), - "Matrix 'mat' must not be negative, the resulting quaternion will be invalid. " - "The caller should call negate_m3(mat) if is_negative_m3(mat) returns true."); + /* Caller must ensure matrices aren't negative for valid results, see: T24291, T94231. */ + BLI_assert(!is_negative_m3(mat)); /* Check the trace of the matrix - bad precision if close to -1. */ const float trace = mat[0][0] + mat[1][1] + mat[2][2]; @@ -336,30 +334,46 @@ void mat3_normalized_to_quat(float q[4], const float mat[3][3]) normalize_qt(q); } -void mat3_to_quat(float q[4], const float mat[3][3]) + +static void mat3_normalized_to_quat_with_checks(float q[4], float mat[3][3]) { - float unit_mat[3][3]; + const float det = determinant_m3_array(mat); + if (UNLIKELY(!isfinite(det))) { + unit_m3(mat); + } + else if (UNLIKELY(det < 0.0f)) { + negate_m3(mat); + } + mat3_normalized_to_quat_fast(q, mat); +} - /* work on a copy */ - /* this is needed AND a 'normalize_qt' in the end */ - normalize_m3_m3(unit_mat, mat); - mat3_normalized_to_quat(q, unit_mat); +void mat3_normalized_to_quat(float q[4], const float mat[3][3]) +{ + float unit_mat_abs[3][3]; + copy_m3_m3(unit_mat_abs, mat); + mat3_normalized_to_quat_with_checks(q, unit_mat_abs); } -void mat4_normalized_to_quat(float q[4], const float mat[4][4]) +void mat3_to_quat(float q[4], const float mat[3][3]) { - float mat3[3][3]; + float unit_mat_abs[3][3]; + normalize_m3_m3(unit_mat_abs, mat); + mat3_normalized_to_quat_with_checks(q, unit_mat_abs); +} - copy_m3_m4(mat3, mat); - mat3_normalized_to_quat(q, mat3); +void mat4_normalized_to_quat(float q[4], const float mat[4][4]) +{ + float unit_mat_abs[3][3]; + copy_m3_m4(unit_mat_abs, mat); + mat3_normalized_to_quat_with_checks(q, unit_mat_abs); } void mat4_to_quat(float q[4], const float mat[4][4]) { - float mat3[3][3]; - - copy_m3_m4(mat3, mat); - mat3_to_quat(q, mat3); + float unit_mat_abs[3][3]; + copy_m3_m4(unit_mat_abs, mat); + normalize_m3(unit_mat_abs); + mat3_normalized_to_quat_with_checks(q, unit_mat_abs); } void mat3_to_quat_is_ok(float q[4], const float wmat[3][3]) diff --git a/source/blender/editors/space_view3d/view3d_utils.c b/source/blender/editors/space_view3d/view3d_utils.c index 5154c2d4f52..cb716391fb2 100644 --- a/source/blender/editors/space_view3d/view3d_utils.c +++ b/source/blender/editors/space_view3d/view3d_utils.c @@ -1485,18 +1485,15 @@ void ED_view3d_from_m4(const float mat[4][4], float ofs[3], float quat[4], const negate_v3_v3(ofs, mat[3]); } - if (ofs && dist) { - madd_v3_v3fl(ofs, nmat[2], *dist); - } - /* Quat */ if (quat) { - if (is_negative_m3(nmat)) { - negate_m3(nmat); - } mat3_normalized_to_quat(quat, nmat); invert_qt_normalized(quat); } + + if (ofs && dist) { + madd_v3_v3fl(ofs, nmat[2], *dist); + } } void ED_view3d_to_m4(float mat[4][4], const float ofs[3], const float quat[4], const float dist) diff --git a/source/blender/editors/space_view3d/view3d_view.c b/source/blender/editors/space_view3d/view3d_view.c index 05922ba7a95..b8042a9f215 100644 --- a/source/blender/editors/space_view3d/view3d_view.c +++ b/source/blender/editors/space_view3d/view3d_view.c @@ -369,11 +369,7 @@ static void obmat_to_viewmat(RegionView3D *rv3d, Object *ob) invert_m4_m4(rv3d->viewmat, bmat); /* view quat calculation, needed for add object */ - copy_m4_m4(bmat, rv3d->viewmat); - if (is_negative_m4(bmat)) { - negate_m4(bmat); - } - mat4_normalized_to_quat(rv3d->viewquat, bmat); + mat4_normalized_to_quat(rv3d->viewquat, rv3d->viewmat); } void view3d_viewmatrix_set(Depsgraph *depsgraph, diff --git a/source/blender/python/mathutils/mathutils_Matrix.c b/source/blender/python/mathutils/mathutils_Matrix.c index de42b11c70b..8405b966a4e 100644 --- a/source/blender/python/mathutils/mathutils_Matrix.c +++ b/source/blender/python/mathutils/mathutils_Matrix.c @@ -1243,19 +1243,12 @@ static PyObject *Matrix_to_quaternion(MatrixObject *self) "inappropriate matrix size - expects 3x3 or 4x4 matrix"); return NULL; } - float mat3[3][3]; if (self->row_num == 3) { - copy_m3_m3(mat3, (const float(*)[3])self->matrix); + mat3_to_quat(quat, (const float(*)[3])self->matrix); } else { - copy_m3_m4(mat3, (const float(*)[4])self->matrix); + mat4_to_quat(quat, (const float(*)[4])self->matrix); } - normalize_m3(mat3); - if (is_negative_m3(mat3)) { - /* Without this, the results are invalid, see: T94231. */ - negate_m3(mat3); - } - mat3_normalized_to_quat(quat, mat3); return Quaternion_CreatePyObject(quat, NULL); } @@ -1894,7 +1887,7 @@ static PyObject *Matrix_decompose(MatrixObject *self) } mat4_to_loc_rot_size(loc, rot, size, (const float(*)[4])self->matrix); - mat3_to_quat(quat, rot); + mat3_normalized_to_quat_fast(quat, rot); ret = PyTuple_New(3); PyTuple_SET_ITEMS(ret, diff --git a/source/blender/python/mathutils/mathutils_Quaternion.c b/source/blender/python/mathutils/mathutils_Quaternion.c index 4972381d29e..a5ea09bef48 100644 --- a/source/blender/python/mathutils/mathutils_Quaternion.c +++ b/source/blender/python/mathutils/mathutils_Quaternion.c @@ -543,13 +543,7 @@ static PyObject *Quaternion_rotate(QuaternionObject *self, PyObject *value) length = normalize_qt_qt(tquat, self->quat); quat_to_mat3(self_rmat, tquat); mul_m3_m3m3(rmat, other_rmat, self_rmat); - normalize_m3(rmat); - /* This check could also be performed on `other_rmat`, use the final result instead to ensure - * float imprecision doesn't allow the multiplication to make `rmat` negative. */ - if (is_negative_m3(rmat)) { - negate_m3(rmat); - } - mat3_normalized_to_quat(self->quat, rmat); + mat3_to_quat(self->quat, rmat); mul_qt_fl(self->quat, length); /* maintain length after rotating */ (void)BaseMath_WriteCallback(self); |