[Bf-blender-cvs] [a7650c62069] master: BLI_math: ensure non-negative matrices for mat3_to_quat calculations
Campbell Barton
noreply at git.blender.org
Thu Aug 25 05:50:34 CEST 2022
Commit: a7650c6206908a8865d6140a310809ec5ab0c770
Author: Campbell Barton
Date: Thu Aug 25 12:45:43 2022 +1000
Branches: master
https://developer.blender.org/rBa7650c6206908a8865d6140a310809ec5ab0c770
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.
===================================================================
M source/blender/blenlib/BLI_math_matrix.h
M source/blender/blenlib/BLI_math_rotation.h
M source/blender/blenlib/intern/math_matrix.c
M source/blender/blenlib/intern/math_rotation.c
M source/blender/editors/space_view3d/view3d_utils.c
M source/blender/editors/space_view3d/view3d_view.c
M source/blender/python/mathutils/mathutils_Matrix.c
M source/blender/python/mathutils/mathutils_Quaternion.c
===================================================================
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);
More information about the Bf-blender-cvs
mailing list