Merge pull request #100991 from TokageItLab/180deg-arc

Fix looking at with 180 degree arc

core/math/basis.cpp

@@ -1049,9 +1049,10 @@ Basis Basis::looking_at(const Vector3 &p_target, const Vector3 &p_up, bool p_use
 		v_z = -v_z;
 	}
 	Vector3 v_x = p_up.cross(v_z);
-#ifdef MATH_CHECKS
+	if (v_x.is_zero_approx()) {
-	ERR_FAIL_COND_V_MSG(v_x.is_zero_approx(), Basis(), "The target vector and up vector can't be parallel to each other.");
+		WARN_PRINT("Target and up vectors are colinear. This is not advised as it may cause unwanted rotation around local Z axis.");
-#endif
+		v_x = p_up.get_any_perpendicular(); // Vectors are almost parallel.
+	}
 	v_x.normalize();
 	Vector3 v_y = v_z.cross(v_x);
 

core/math/quaternion.h

@@ -142,14 +142,15 @@ struct [[nodiscard]] Quaternion {
 
 	Quaternion(const Vector3 &p_v0, const Vector3 &p_v1) { // Shortest arc.
 		Vector3 c = p_v0.cross(p_v1);
-		real_t d = p_v0.dot(p_v1);
 
-		if (d < -1.0f + (real_t)CMP_EPSILON) {
+		if (c.is_zero_approx()) {
-			x = 0;
+			Vector3 axis = p_v0.get_any_perpendicular();
-			y = 1;
+			x = axis.x;
-			z = 0;
+			y = axis.y;
+			z = axis.z;
 			w = 0;
 		} else {
+			real_t d = p_v0.dot(p_v1);
 			real_t s = Math::sqrt((1.0f + d) * 2.0f);
 			real_t rs = 1.0f / s;
 

core/math/vector3.h

@@ -130,6 +130,7 @@ struct [[nodiscard]] Vector3 {
 	_FORCE_INLINE_ Vector3 cross(const Vector3 &p_with) const;
 	_FORCE_INLINE_ real_t dot(const Vector3 &p_with) const;
 	Basis outer(const Vector3 &p_with) const;
+	_FORCE_INLINE_ Vector3 get_any_perpendicular() const;
 
 	_FORCE_INLINE_ Vector3 abs() const;
 	_FORCE_INLINE_ Vector3 floor() const;
@@ -326,6 +327,16 @@ Vector3 Vector3::direction_to(const Vector3 &p_to) const {
 	return ret;
 }
 
+Vector3 Vector3::get_any_perpendicular() const {
+	// Return the any perpendicular vector by cross product with the Vector3.RIGHT or Vector3.UP,
+	// whichever has the greater angle to the current vector with the sign of each element positive.
+	// The only essence is "to avoid being parallel to the current vector", and there is no mathematical basis for using Vector3.RIGHT and Vector3.UP,
+	// since it could be a different vector depending on the prior branching code Math::abs(x) <= Math::abs(y) && Math::abs(x) <= Math::abs(z).
+	// However, it would be reasonable to use any of the axes of the basis, as it is simpler to calculate.
+	ERR_FAIL_COND_V_MSG(is_zero_approx(), Vector3(0, 0, 0), "The Vector3 must not be zero.");
+	return cross((Math::abs(x) <= Math::abs(y) && Math::abs(x) <= Math::abs(z)) ? Vector3(1, 0, 0) : Vector3(0, 1, 0)).normalized();
+}
+
 /* Operators */
 
 Vector3 &Vector3::operator+=(const Vector3 &p_v) {

doc/classes/Basis.xml

@@ -214,7 +214,8 @@
 			<description>
 				Creates a new [Basis] with a rotation such that the forward axis (-Z) points towards the [param target] position.
 				By default, the -Z axis (camera forward) is treated as forward (implies +X is right). If [param use_model_front] is [code]true[/code], the +Z axis (asset front) is treated as forward (implies +X is left) and points toward the [param target] position.
-				The up axis (+Y) points as close to the [param up] vector as possible while staying perpendicular to the forward axis. The returned basis is orthonormalized (see [method orthonormalized]). The [param target] and [param up] vectors cannot be [constant Vector3.ZERO], and cannot be parallel to each other.
+				The up axis (+Y) points as close to the [param up] vector as possible while staying perpendicular to the forward axis. The returned basis is orthonormalized (see [method orthonormalized]).
+				The [param target] and the [param up] cannot be [constant Vector3.ZERO], and shouldn't be colinear to avoid unintended rotation around local Z axis.
 			</description>
 		</method>
 		<method name="orthonormalized" qualifiers="const">

doc/classes/Node3D.xml

@@ -127,7 +127,8 @@
 			<description>
 				Rotates the node so that the local forward axis (-Z, [constant Vector3.FORWARD]) points toward the [param target] position.
 				The local up axis (+Y) points as close to the [param up] vector as possible while staying perpendicular to the local forward axis. The resulting transform is orthogonal, and the scale is preserved. Non-uniform scaling may not work correctly.
-				The [param target] position cannot be the same as the node's position, the [param up] vector cannot be zero, and the direction from the node's position to the [param target] vector cannot be parallel to the [param up] vector.
+				The [param target] position cannot be the same as the node's position, the [param up] vector cannot be zero.
+				The [param target] and the [param up] cannot be [constant Vector3.ZERO], and shouldn't be colinear to avoid unintended rotation around local Z axis.
 				Operations take place in global space, which means that the node must be in the scene tree.
 				If [param use_model_front] is [code]true[/code], the +Z axis (asset front) is treated as forward (implies +X is left) and points toward the [param target] position. By default, the -Z axis (camera forward) is treated as forward (implies +X is right).
 			</description>

scene/3d/node_3d.cpp

@@ -1066,7 +1066,6 @@ void Node3D::look_at_from_position(const Vector3 &p_pos, const Vector3 &p_target
 	ERR_THREAD_GUARD;
 	ERR_FAIL_COND_MSG(p_pos.is_equal_approx(p_target), "Node origin and target are in the same position, look_at() failed.");
 	ERR_FAIL_COND_MSG(p_up.is_zero_approx(), "The up vector can't be zero, look_at() failed.");
-	ERR_FAIL_COND_MSG(p_up.cross(p_target - p_pos).is_zero_approx(), "Up vector and direction between node origin and target are aligned, look_at() failed.");
 
 	Vector3 forward = p_target - p_pos;
 	Basis lookat_basis = Basis::looking_at(forward, p_up, p_use_model_front);

tests/core/math/test_quaternion.h

@@ -235,6 +235,36 @@ TEST_CASE("[Quaternion] Construct Basis Axes") {
 	CHECK(q[3] == doctest::Approx(0.8582598));
 }
 
+TEST_CASE("[Quaternion] Construct Shortest Arc For 180 Degree Arc") {
+	Vector3 up(0, 1, 0);
+	Vector3 down(0, -1, 0);
+	Vector3 left(-1, 0, 0);
+	Vector3 right(1, 0, 0);
+	Vector3 forward(0, 0, -1);
+	Vector3 back(0, 0, 1);
+
+	// When we have a 180 degree rotation quaternion which was defined as
+	// A to B, logically when we transform A we expect to get B.
+	Quaternion left_to_right(left, right);
+	Quaternion right_to_left(right, left);
+	CHECK(left_to_right.xform(left).is_equal_approx(right));
+	CHECK(Quaternion(right, left).xform(right).is_equal_approx(left));
+	CHECK(Quaternion(up, down).xform(up).is_equal_approx(down));
+	CHECK(Quaternion(down, up).xform(down).is_equal_approx(up));
+	CHECK(Quaternion(forward, back).xform(forward).is_equal_approx(back));
+	CHECK(Quaternion(back, forward).xform(back).is_equal_approx(forward));
+
+	// With (arbitrary) opposite vectors that are not axis-aligned as parameters.
+	Vector3 diagonal_up = Vector3(1.2, 2.3, 4.5).normalized();
+	Vector3 diagonal_down = -diagonal_up;
+	Quaternion q1(diagonal_up, diagonal_down);
+	CHECK(q1.xform(diagonal_down).is_equal_approx(diagonal_up));
+	CHECK(q1.xform(diagonal_up).is_equal_approx(diagonal_down));
+
+	// For the consistency of the rotation direction, they should be symmetrical to the plane.
+	CHECK(left_to_right.is_equal_approx(right_to_left.inverse()));
+}
+
 TEST_CASE("[Quaternion] Get Euler Orders") {
 	double x = Math::deg_to_rad(30.0);
 	double y = Math::deg_to_rad(45.0);