Fix Basis is_orthogonal and is_rotation methods, add is_orthonormal

core/math/basis.cpp

@@ -89,13 +89,26 @@ Basis Basis::orthogonalized() const {
 	return c;
 }
 
+// Returns true if the basis vectors are orthogonal (perpendicular), so it has no skew or shear, and can be decomposed into rotation and scale.
+// See https://en.wikipedia.org/wiki/Orthogonal_basis
 bool Basis::is_orthogonal() const {
-	Basis identity;
-	Basis m = (*this) * transposed();
+	const Vector3 x = get_column(0);
+	const Vector3 y = get_column(1);
+	const Vector3 z = get_column(2);
+	return Math::is_zero_approx(x.dot(y)) && Math::is_zero_approx(x.dot(z)) && Math::is_zero_approx(y.dot(z));
+}
 
-	return m.is_equal_approx(identity);
+// Returns true if the basis vectors are orthonormal (orthogonal and normalized), so it has no scale, skew, or shear.
+// See https://en.wikipedia.org/wiki/Orthonormal_basis
+bool Basis::is_orthonormal() const {
+	const Vector3 x = get_column(0);
+	const Vector3 y = get_column(1);
+	const Vector3 z = get_column(2);
+	return Math::is_equal_approx(x.length_squared(), 1) && Math::is_equal_approx(y.length_squared(), 1) && Math::is_equal_approx(z.length_squared(), 1) && Math::is_zero_approx(x.dot(y)) && Math::is_zero_approx(x.dot(z)) && Math::is_zero_approx(y.dot(z));
 }
 
+// Returns true if the basis is conformal (orthogonal, uniform scale, preserves angles and distance ratios).
+// See https://en.wikipedia.org/wiki/Conformal_linear_transformation
 bool Basis::is_conformal() const {
 	const Vector3 x = get_column(0);
 	const Vector3 y = get_column(1);
@@ -104,6 +117,7 @@ bool Basis::is_conformal() const {
 	return Math::is_equal_approx(x_len_sq, y.length_squared()) && Math::is_equal_approx(x_len_sq, z.length_squared()) && Math::is_zero_approx(x.dot(y)) && Math::is_zero_approx(x.dot(z)) && Math::is_zero_approx(y.dot(z));
 }
 
+// Returns true if the basis only has diagonal elements, so it may only have scale or flip, but no rotation, skew, or shear.
 bool Basis::is_diagonal() const {
 	return (
 			Math::is_zero_approx(rows[0][1]) && Math::is_zero_approx(rows[0][2]) &&
@@ -111,8 +125,9 @@ bool Basis::is_diagonal() const {
 			Math::is_zero_approx(rows[2][0]) && Math::is_zero_approx(rows[2][1]));
 }
 
+// Returns true if the basis is a pure rotation matrix, so it has no scale, skew, shear, or flip.
 bool Basis::is_rotation() const {
-	return Math::is_equal_approx(determinant(), 1, (real_t)UNIT_EPSILON) && is_orthogonal();
+	return is_conformal() && Math::is_equal_approx(determinant(), 1, (real_t)UNIT_EPSILON);
 }
 
 #ifdef MATH_CHECKS

core/math/basis.h

@@ -138,6 +138,7 @@ struct _NO_DISCARD_ Basis {
 	_FORCE_INLINE_ Basis operator*(const real_t p_val) const;
 
 	bool is_orthogonal() const;
+	bool is_orthonormal() const;
 	bool is_conformal() const;
 	bool is_diagonal() const;
 	bool is_rotation() const;

tests/core/math/test_basis.h

@@ -324,6 +324,100 @@ TEST_CASE("[Basis] Is conformal checks") {
 	CHECK_FALSE_MESSAGE(
 			Basis(Vector3(Math_SQRT12, Math_SQRT12, 0), Vector3(0, 1, 0), Vector3(0, 0, 1)).is_conformal(),
 			"Basis with the X axis skewed 45 degrees should not be conformal.");
+
+	CHECK_MESSAGE(
+			Basis(0, 0, 0, 0, 0, 0, 0, 0, 0).is_conformal(),
+			"Edge case: Basis with all zeroes should return true for is_conformal (because a 0 scale is uniform).");
+}
+
+TEST_CASE("[Basis] Is orthogonal checks") {
+	CHECK_MESSAGE(
+			Basis().is_orthogonal(),
+			"Identity Basis should be orthogonal.");
+
+	CHECK_MESSAGE(
+			Basis::from_euler(Vector3(1.2, 3.4, 5.6)).is_orthogonal(),
+			"Basis with only rotation should be orthogonal.");
+
+	CHECK_MESSAGE(
+			Basis::from_scale(Vector3(-1, -1, -1)).is_orthogonal(),
+			"Basis with only a flip should be orthogonal.");
+
+	CHECK_MESSAGE(
+			Basis::from_scale(Vector3(1.2, 3.4, 5.6)).is_orthogonal(),
+			"Basis with only scale should be orthogonal.");
+
+	CHECK_MESSAGE(
+			Basis(Vector3(3, 4, 0), Vector3(4, -3, 0), Vector3(0, 0, 5)).is_orthogonal(),
+			"Basis with a flip, rotation, and uniform scale should be orthogonal.");
+
+	CHECK_FALSE_MESSAGE(
+			Basis(Vector3(Math_SQRT12, Math_SQRT12, 0), Vector3(0, 1, 0), Vector3(0, 0, 1)).is_orthogonal(),
+			"Basis with the X axis skewed 45 degrees should not be orthogonal.");
+
+	CHECK_MESSAGE(
+			Basis(0, 0, 0, 0, 0, 0, 0, 0, 0).is_orthogonal(),
+			"Edge case: Basis with all zeroes should return true for is_orthogonal, since zero vectors are orthogonal to all vectors.");
+}
+
+TEST_CASE("[Basis] Is orthonormal checks") {
+	CHECK_MESSAGE(
+			Basis().is_orthonormal(),
+			"Identity Basis should be orthonormal.");
+
+	CHECK_MESSAGE(
+			Basis::from_euler(Vector3(1.2, 3.4, 5.6)).is_orthonormal(),
+			"Basis with only rotation should be orthonormal.");
+
+	CHECK_MESSAGE(
+			Basis::from_scale(Vector3(-1, -1, -1)).is_orthonormal(),
+			"Basis with only a flip should be orthonormal.");
+
+	CHECK_FALSE_MESSAGE(
+			Basis::from_scale(Vector3(1.2, 3.4, 5.6)).is_orthonormal(),
+			"Basis with only scale should not be orthonormal.");
+
+	CHECK_FALSE_MESSAGE(
+			Basis(Vector3(3, 4, 0), Vector3(4, -3, 0), Vector3(0, 0, 5)).is_orthonormal(),
+			"Basis with a flip, rotation, and uniform scale should not be orthonormal.");
+
+	CHECK_FALSE_MESSAGE(
+			Basis(Vector3(Math_SQRT12, Math_SQRT12, 0), Vector3(0, 1, 0), Vector3(0, 0, 1)).is_orthonormal(),
+			"Basis with the X axis skewed 45 degrees should not be orthonormal.");
+
+	CHECK_FALSE_MESSAGE(
+			Basis(0, 0, 0, 0, 0, 0, 0, 0, 0).is_orthonormal(),
+			"Edge case: Basis with all zeroes should return false for is_orthonormal, since the vectors do not have a length of 1.");
+}
+
+TEST_CASE("[Basis] Is rotation checks") {
+	CHECK_MESSAGE(
+			Basis().is_rotation(),
+			"Identity Basis should be a rotation (a rotation of zero).");
+
+	CHECK_MESSAGE(
+			Basis::from_euler(Vector3(1.2, 3.4, 5.6)).is_rotation(),
+			"Basis with only rotation should be a rotation.");
+
+	CHECK_FALSE_MESSAGE(
+			Basis::from_scale(Vector3(-1, -1, -1)).is_rotation(),
+			"Basis with only a flip should not be a rotation.");
+
+	CHECK_FALSE_MESSAGE(
+			Basis::from_scale(Vector3(1.2, 3.4, 5.6)).is_rotation(),
+			"Basis with only scale should not be a rotation.");
+
+	CHECK_FALSE_MESSAGE(
+			Basis(Vector3(2, 0, 0), Vector3(0, 0.5, 0), Vector3(0, 0, 1)).is_rotation(),
+			"Basis with a squeeze should not be a rotation.");
+
+	CHECK_FALSE_MESSAGE(
+			Basis(Vector3(Math_SQRT12, Math_SQRT12, 0), Vector3(0, 1, 0), Vector3(0, 0, 1)).is_rotation(),
+			"Basis with the X axis skewed 45 degrees should not be a rotation.");
+
+	CHECK_FALSE_MESSAGE(
+			Basis(0, 0, 0, 0, 0, 0, 0, 0, 0).is_rotation(),
+			"Edge case: Basis with all zeroes should return false for is_rotation, because it is not just a rotation (has a scale of 0).");
 }
 
 } // namespace TestBasis