From d0c421976c0822351a1a0401548677f3e5caa399 Mon Sep 17 00:00:00 2001
From: "Silc Lizard (Tokage) Renew"
<61938263+TokageItLab@users.noreply.github.com>
Date: Wed, 1 Jan 2025 10:58:25 +0900
Subject: [PATCH] Fix looking at with 180 degree arc
Co-authored-by: Fruitsalad <949631+fruitsalad@users.noreply.github.com>
---
core/math/basis.cpp | 7 ++++---
core/math/quaternion.h | 11 ++++++-----
core/math/vector3.h | 11 +++++++++++
doc/classes/Basis.xml | 3 ++-
doc/classes/Node3D.xml | 3 ++-
scene/3d/node_3d.cpp | 1 -
tests/core/math/test_quaternion.h | 30 ++++++++++++++++++++++++++++++
7 files changed, 55 insertions(+), 11 deletions(-)
diff --git a/core/math/basis.cpp b/core/math/basis.cpp
index 34ed1c2d855..e5f3431eef8 100644
--- a/core/math/basis.cpp
+++ b/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
- ERR_FAIL_COND_V_MSG(v_x.is_zero_approx(), Basis(), "The target vector and up vector can't be parallel to each other.");
-#endif
+ if (v_x.is_zero_approx()) {
+ WARN_PRINT("Target and up vectors are colinear. This is not advised as it may cause unwanted rotation around local Z axis.");
+ v_x = p_up.get_any_perpendicular(); // Vectors are almost parallel.
+ }
v_x.normalize();
Vector3 v_y = v_z.cross(v_x);
diff --git a/core/math/quaternion.h b/core/math/quaternion.h
index 655e55e0a20..605035c2143 100644
--- a/core/math/quaternion.h
+++ b/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) {
- x = 0;
- y = 1;
- z = 0;
+ if (c.is_zero_approx()) {
+ Vector3 axis = p_v0.get_any_perpendicular();
+ x = axis.x;
+ 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;
diff --git a/core/math/vector3.h b/core/math/vector3.h
index 14bc44c4e79..fd0dec3550d 100644
--- a/core/math/vector3.h
+++ b/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) {
diff --git a/doc/classes/Basis.xml b/doc/classes/Basis.xml
index 81f63addd4a..64afdff7ceb 100644
--- a/doc/classes/Basis.xml
+++ b/doc/classes/Basis.xml
@@ -214,7 +214,8 @@
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.
diff --git a/doc/classes/Node3D.xml b/doc/classes/Node3D.xml
index ae13af4b826..ba0272b02fe 100644
--- a/doc/classes/Node3D.xml
+++ b/doc/classes/Node3D.xml
@@ -127,7 +127,8 @@
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).
diff --git a/scene/3d/node_3d.cpp b/scene/3d/node_3d.cpp
index cd77a32455d..dc023340996 100644
--- a/scene/3d/node_3d.cpp
+++ b/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);
diff --git a/tests/core/math/test_quaternion.h b/tests/core/math/test_quaternion.h
index 40db43b88b4..003cb17ea98 100644
--- a/tests/core/math/test_quaternion.h
+++ b/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);