Repository: godotengine/godot (language: cpp)

Issue:
Potential problem in look_at()
### Godot version

3.3.2.stable.official

### System information

macOS 11.3.1 GLES3

### Issue description

When using the `look_at()` function in a script attached to a camera, there seems to be some sort of discretization effect occurring in my tests; for a camera positioned at e.g. `(0,90,0)` looking towards `(0,0,0)`, I see rendering artifacts. These artifacts persist for a small region around `(0,0,0)` on the `x,z` plane (approx. +/- 0.1 units).

This effect is particularly jarring when it manifests as part of an animated movement, producing an effect almost like a magnet snapping the camera view direction onto the target location at `(0,0,0)`.

I've attached an example project to demonstrate this effect; a camera is located at `(0,90,0)` above a simple plane, and the `look_at()` method is invoked in the camera's `_process()` method. The target position is slowly moved left and right via a `sin()` function, and the effect is visible when the camera looks in the vicinity of `(0,0,0)`.

Am I missing something important, here?

### Steps to reproduce

See issue description, and attached project file.

### Minimal reproduction project

[Weird.zip](https://github.com/godotengine/godot/files/6697007/Weird.zip)



Discussion / hints:
Looks more like a problem with `OmniLight`'s shadow. Duplicate (_edit: only the shadow visual artifact part_) of #25750?

- Plane's depth/width subdivision = 0:
![Godot_v3 3 2-stable_win64_sgiu2cLQ8G](https://user-images.githubusercontent.com/9283098/122996666-6c03f880-d3ab-11eb-96bb-c74e6370a841.png)

- Plane's depth/width subdivision = 3:
![Godot_v3 3 2-stable_win64_nV6YpehB9n](https://user-images.githubusercontent.com/9283098/122998137-fa2cae80-d3ac-11eb-9b4f-908c5732ce47.png)





https://user-images.githubusercontent.com/4098291/123001028-e74ab780-d375-11eb-8122-51d5fcdd2211.mov

It's not just a rendering artifact, I think there's also a "jump" or "snap" effect on the camera view vector. At least there is for me! :)
I can reproduce this on 3.3.2 on Linux, so this is not an OS-specific issue. I have no idea where the issue is coming from though…

Can you reproduce this if you move everything away from the origin? Visually, it should look identical but without the bug occurring.
> It's not just a rendering artifact, I think there's also a "jump" or "snap" effect on the camera view vector. At least there is for me! :)

You're right, the rendering artifact is gone when shadow is disabled in `OmniLight` but for `targetPos.x == targetPos.y == 0.0` and `abs(targetPos.z) < 0.403` it looks (incorrectly) like we're looking at `targetPos == (0, 0, 0)`.
Simply moving the camera from `(0,90,0)` to `(1,90,0)` (but keeping the target as `(0,0,f(sin t))`) seems to fix the "snapping magnet" issue.

If I then change the dynamic camera target from `(0,0,f(sin t))` to `(1,0,f(sin t))` (i.e., the camera view vector is back along `(0,-1,0)`) someone puts the pesky magnet back onto the camera ...
Smells like a numerical issue when calculating the inverse of a transform matrix, maybe?

Or perhaps a pesky `if (x < epilson) { y = constant; } /* prevent DBZ */` sort of issue somewhere?

Fix patch:
diff --git a/core/math/basis.cpp b/core/math/basis.cpp
index e5f3431eef84..18959f3c0162 100644
--- a/core/math/basis.cpp
+++ b/core/math/basis.cpp
@@ -455,6 +455,11 @@ void Basis::get_rotation_axis_angle_local(Vector3 &p_axis, real_t &p_angle) cons
 }
 
 Vector3 Basis::get_euler(EulerOrder p_order) const {
+	// This epsilon value results in angles within a +/- 0.04 degree range being simplified/truncated.
+	// Based on testing, this is the largest the epsilon can be without the angle truncation becoming
+	// visually noticeable.
+	const real_t epsilon = 0.00000025;
+
 	switch (p_order) {
 		case EulerOrder::XYZ: {
 			// Euler angles in XYZ convention.
@@ -466,8 +471,8 @@ Vector3 Basis::get_euler(EulerOrder p_order) const {
 
 			Vector3 euler;
 			real_t sy = rows[0][2];
-			if (sy < (1.0f - (real_t)CMP_EPSILON)) {
-				if (sy > -(1.0f - (real_t)CMP_EPSILON)) {
+			if (sy < (1.0f - epsilon)) {
+				if (sy > -(1.0f - epsilon)) {
 					// is this a pure Y rotation?
 					if (rows[1][0] == 0 && rows[0][1] == 0 && rows[1][2] == 0 && rows[2][1] == 0 && rows[1][1] == 1) {
 						// return the simplest form (human friendlier in editor and scripts)
@@ -501,8 +506,8 @@ Vector3 Basis::get_euler(EulerOrder p_order) const {
 
 			Vector3 euler;
 			real_t sz = rows[0][1];
-			if (sz < (1.0f - (real_t)CMP_EPSILON)) {
-				if (sz > -(1.0f - (real_t)CMP_EPSILON)) {
+			if (sz < (1.0f - epsilon)) {
+				if (sz > -(1.0f - epsilon)) {
 					euler.x = Math::atan2(rows[2][1], rows[1][1]);
 					euler.y = Math::atan2(rows[0][2], rows[0][0]);
 					euler.z = Math::asin(-sz);
@@ -532,8 +537,8 @@ Vector3 Basis::get_euler(EulerOrder p_order) const {
 
 			real_t m12 = rows[1][2];
 
-			if (m12 < (1 - (real_t)CMP_EPSILON)) {
-				if (m12 > -(1 - (real_t)CMP_EPSILON)) {
+			if (m12 < (1 - epsilon)) {
+				if (m12 > -(1 - epsilon)) {
 					// is this a pure X rotation?
 					if (rows[1][0] == 0 && rows[0][1] == 0 && rows[0][2] == 0 && rows[2][0] == 0 && rows[0][0] == 1) {
 						// return the simplest form (human friendlier in editor and scripts)
@@ -568,8 +573,8 @@ Vector3 Basis::get_euler(EulerOrder p_order) const {
 
 			Vector3 euler;
 			real_t sz = rows[1][0];
-			if (sz < (1.0f - (real_t)CMP_EPSILON)) {
-				if (sz > -(1.0f - (real_t)CMP_EPSILON)) {
+			if (sz < (1.0f - epsilon)) {
+				if (sz > -(1.0f - epsilon)) {
 					euler.x = Math::atan2(-rows[1][2], rows[1][1]);
 					euler.y = Math::atan2(-rows[2][0], rows[0][0]);
 					euler.z = Math::asin(sz);
@@ -596,8 +601,8 @@ Vector3 Basis::get_euler(EulerOrder p_order) const {
 			//        -cx*sy            sx                    cx*cy
 			Vector3 euler;
 			real_t sx = rows[2][1];
-			if (sx < (1.0f - (real_t)CMP_EPSILON)) {
-				if (sx > -(1.0f - (real_t)CMP_EPSILON)) {
+			if (sx < (1.0f - epsilon)) {
+				if (sx > -(1.0f - epsilon)) {
 					euler.x = Math::asin(sx);
 					euler.y = Math::atan2(-rows[2][0], rows[2][2]);
 					euler.z = Math::atan2(-rows[0][1], rows[1][1]);
@@ -624,8 +629,8 @@ Vector3 Basis::get_euler(EulerOrder p_order) const {
 			//        -sy               cy*sx                 cy*cx
 			Vector3 euler;
 			real_t sy = rows[2][0];
-			if (sy < (1.0f - (real_t)CMP_EPSILON)) {
-				if (sy > -(1.0f - (real_t)CMP_EPSILON)) {
+			if (sy < (1.0f - epsilon)) {
+				if (sy > -(1.0f - epsilon)) {
 					euler.x = Math::atan2(rows[2][1], rows[2][2]);
 					euler.y = Math::asin(-sy);
 					euler.z = Math::atan2(rows[1][0], rows[0][0]);


Test patch:
diff --git a/tests/core/math/test_basis.h b/tests/core/math/test_basis.h
index f8c5ef279dcd..6166986286e4 100644
--- a/tests/core/math/test_basis.h
+++ b/tests/core/math/test_basis.h
@@ -93,9 +93,9 @@ void test_rotation(Vector3 deg_original_euler, EulerOrder rot_order) {
 
 	Basis res = to_rotation.inverse() * rotation_from_computed_euler;
 
-	CHECK_MESSAGE((res.get_column(0) - Vector3(1.0, 0.0, 0.0)).length() <= 0.1, vformat("Fail due to X %s\n", String(res.get_column(0))));
-	CHECK_MESSAGE((res.get_column(1) - Vector3(0.0, 1.0, 0.0)).length() <= 0.1, vformat("Fail due to Y %s\n", String(res.get_column(1))));
-	CHECK_MESSAGE((res.get_column(2) - Vector3(0.0, 0.0, 1.0)).length() <= 0.1, vformat("Fail due to Z %s\n", String(res.get_column(2))));
+	CHECK_MESSAGE((res.get_column(0) - Vector3(1.0, 0.0, 0.0)).length() <= 0.001, vformat("Fail due to X %s\n", String(res.get_column(0))));
+	CHECK_MESSAGE((res.get_column(1) - Vector3(0.0, 1.0, 0.0)).length() <= 0.001, vformat("Fail due to Y %s\n", String(res.get_column(1))));
+	CHECK_MESSAGE((res.get_column(2) - Vector3(0.0, 0.0, 1.0)).length() <= 0.001, vformat("Fail due to Z %s\n", String(res.get_column(2))));
 
 	// Double check `to_rotation` decomposing with XYZ rotation order.
 	const Vector3 euler_xyz_from_rotation = to_rotation.get_euler(EulerOrder::XYZ);
@@ -103,9 +103,9 @@ void test_rotation(Vector3 deg_original_euler, EulerOrder rot_order) {
 
 	res = to_rotation.inverse() * rotation_from_xyz_computed_euler;
 
-	CHECK_MESSAGE((res.get_column(0) - Vector3(1.0, 0.0, 0.0)).length() <= 0.1, vformat("Double check with XYZ rot order failed, due to X %s\n", String(res.get_column(0))));
-	CHECK_MESSAGE((res.get_column(1) - Vector3(0.0, 1.0, 0.0)).length() <= 0.1, vformat("Double check with XYZ rot order failed, due to Y %s\n", String(res.get_column(1))));
-	CHECK_MESSAGE((res.get_column(2) - Vector3(0.0, 0.0, 1.0)).length() <= 0.1, vformat("Double check with XYZ rot order failed, due to Z %s\n", String(res.get_column(2))));
+	CHECK_MESSAGE((res.get_column(0) - Vector3(1.0, 0.0, 0.0)).length() <= 0.001, vformat("Double check with XYZ rot order failed, due to X %s\n", String(res.get_column(0))));
+	CHECK_MESSAGE((res.get_column(1) - Vector3(0.0, 1.0, 0.0)).length() <= 0.001, vformat("Double check with XYZ rot order failed, due to Y %s\n", String(res.get_column(1))));
+	CHECK_MESSAGE((res.get_column(2) - Vector3(0.0, 0.0, 1.0)).length() <= 0.001, vformat("Double check with XYZ rot order failed, due to Z %s\n", String(res.get_column(2))));
 
 	INFO(vformat("Rotation order: %s\n.", get_rot_order_name(rot_order)));
 	INFO(vformat("Original Rotation: %s\n", String(deg_original_euler)));
@@ -176,6 +176,12 @@ TEST_CASE("[Basis] Euler conversions") {
 	vectors_to_test.push_back(Vector3(120.0, -150.0, -130.0));
 	vectors_to_test.push_back(Vector3(120.0, 150.0, -130.0));
 	vectors_to_test.push_back(Vector3(120.0, 150.0, 130.0));
+	vectors_to_test.push_back(Vector3(89.9, 0.0, 0.0));
+	vectors_to_test.push_back(Vector3(-89.9, 0.0, 0.0));
+	vectors_to_test.push_back(Vector3(0.0, 89.9, 0.0));
+	vectors_to_test.push_back(Vector3(0.0, -89.9, 0.0));
+	vectors_to_test.push_back(Vector3(0.0, 0.0, 89.9));
+	vectors_to_test.push_back(Vector3(0.0, 0.0, -89.9));
 
 	for (int h = 0; h < euler_order_to_test.size(); h += 1) {
 		for (int i = 0; i < vectors_to_test.size(); i += 1) {