Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

look_at() can't look up #57457

Open
prominentdetail opened this issue Jan 30, 2022 · 4 comments
Open

look_at() can't look up #57457

prominentdetail opened this issue Jan 30, 2022 · 4 comments

Comments

@prominentdetail
Copy link
Contributor

Godot version

3.4.2

System information

windows 10

Issue description

b.look_at( b.transform.origin+Vector3.UP, Vector3.UP )

When I try to make my object look upwards, it fails to rotate at all.
Based on the documentation, it should end up with the -z towards the target position.

Steps to reproduce

b.look_at( b.transform.origin+Vector3.UP, Vector3.UP )

Minimal reproduction project

No response

@Zylann
Copy link
Contributor

Zylann commented Jan 30, 2022

I believe this is gimbal lock. You are providing two directions, both of which are the same, relatively to the node. So yeah, it could look up, but there is an infinite amount of possible rotations around that axis that fulfill that direction... so it cannot work because the node has no idea which one to take. Usually it can figure it out because the two directions are not the same, but here they are.

It should even tell you in the debugger:

E 0:00:01.670   look_at_from_position: Up vector and direction between node origin and target are aligned, look_at() failed.
  <C++ Error>   Condition "p_up.cross(p_target - p_pos) == Vector3()" is true.
  <C++ Source>  scene/3d/spatial.cpp:671 @ look_at_from_position()
  <Stack Trace> main.gd:9 @ _process()

Maybe you need to specify another vector instead of UP for the second argument, to eliminate this ambiguity, IF you want to look up:

var kinda_up = (_node.global_transform.basis.y + _node.global_transform.basis.z).normalized()
_node.look_at(_node.transform.origin + Vector3(0, 1, 0), kinda_up)

Usually I really just don't use look_at alone, I have some variables keeping track of the looking state (as pitch+yaw angles for example) so that I can't end up with ambiguous cases to handle like that. Or I just clamp the angles such that they never exactly reach up or down.

@lawnjelly
Copy link
Member

See also #53793.
Has a lot of similarities with the situation in #53928.

@krisutofu
Copy link

krisutofu commented Jun 27, 2024

We discussed a solution for the Quaternion 180° rotation and this issue was mentioned.

For me, the solution looks simple. If the cross product between Up and looking-at direction is 0, it should re-use the X-direction basis-vector from the node's transform where the Basis::looking_at() function was called. (EDIT: it requires a negation if the target and the original looking direction are opposite.) Otherwise, the code behaves like it does at the moment: normalizing the cross product and use it as X-Axis. You shouldn't need to manipulate the up vector because the nodes current ground direction does not depend on where it wants to look at.

The singularity in this case is caused by a loss of information in the Basis::looking_at() function arguments. The up vector is used to represent a linear mapping to a perpendicular vector but there is no linear mapping which produces a non-zero perpendicular vector for all input vectors. With the given arguments it's therefore impossible to produce situation-appropriate expected behaviour for every case.

The idea behind reusing the previous vector in the edge case is, because it minimizes the change between the previous and the next vector while preserving the original angles between the basis vectors.

Option 1:

// choose a sensible default value for all cases and only use this function where it doesn't matter
Basis Basis::looking_at(const Vector3 &p_target, const Vector3 &p_up, bool p_use_model_front)
{
	return Basis::looking_at(p_target, p_up, p_use_model_front, std::nullopt);
}

// the p_base_x value is the x column of the previous basis
Basis Basis::looking_at(const Vector3 &p_target, const Vector3 &p_up, bool p_use_model_front, const Vector3 &p_lateral, std::optional<Vector3> p_base_x) {
#ifdef MATH_CHECKS
	ERR_FAIL_COND_V_MSG(p_target.is_zero_approx(), Basis(), "The target vector can't be zero.");
	ERR_FAIL_COND_V_MSG(p_up.is_zero_approx(), Basis(), "The up vector can't be zero.");
#endif
	Vector3 v_z = p_target.normalized();
	if (!p_use_model_front) {
		v_z = -v_z;
	}
	Vector3 v_x = p_up.cross(v_z);
	if (v_x.is_zero_approx()) {
		if (p_base_x)  v_x = p_base_x * sign(p_up.x * v_z.x);
		else v_x = ::default_perpendicular_vector(v_z);  // explanation below
	}
	else {
		v_x.normalize();
	}

	Vector3 v_y = v_z.cross(v_x);

	Basis basis;
	basis.set_columns(v_x, v_y, v_z);
	return basis;
}

The ::default_perpendicular_vector() function (or whatever it would be named) could reuse new code to determine the default perpendicular vector of another vector, as discussed in #93653 .

Option 2:

Do not change Basis::looking_at() and if the returned X column is zero, just replace the X and Y vector in the returned Basis object that was obtained in Node3D::look_at_from_position() with the original vectors. It would also preserve the original y vector. (Taking care of required sign changes.)

The original bug is actually, that the Basis::looking_at() function is used for implementing Node3D::look_at() or Node3D::look_at_from_position() because the static Basis produces a fresh node-independent basis, throwing away the old Basis information.

@krisutofu
Copy link

Another problem is, if the basis was distorted or non-uniformly scaled. The scale and the skewness will be lost after applying this function. Ideally, the scale and angles would be preserved in the basis.

The typical and efficient way of handling these looking_at operations is a matrix multiplication with the basis to obtain a new basis. Computing a rotation matrix on the fly is not that expensive actually if efficiently computed. The corner case is just the 0° or 180° rotation which can be handled specifically by using a defined default rotation axis, such as Y axis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants