godot: slerp between two normalized colinear vectors return not normalized error

I confirm Vector3::slerp() isn’t implemented properly: https://github.com/godotengine/godot/blob/468b987aa38b21b55c1cd8a8d4c03b8e1b2a1373/core/math/vector3.h#L217-L220

This issue specifically seems to be caused by not handling collinear vectors properly (in such cases their cross product is a zero vector (which can’t be normalized)):

# Both would throw errors:
Vector3.FORWARD.slerp(Vector3.FORWARD, 0.5)
Vector3.FORWARD.slerp(-Vector3.FORWARD, 0.5)

I would just add something like this in all relevant methods:

diff --git a/doc/classes/Vector3.xml b/doc/classes/Vector3.xml
index ead08d86df..2ec496fde0 100644
--- a/doc/classes/Vector3.xml
+++ b/doc/classes/Vector3.xml
@@ -289,7 +289,7 @@
 			<argument index="0" name="to" type="Vector3" />
 			<argument index="1" name="weight" type="float" />
 			<description>
-				Returns the result of spherical linear interpolation between this vector and [code]to[/code], by amount [code]weight[/code]. [code]weight[/code] is on the range of 0.0 to 1.0, representing the amount of interpolation.
+				Returns the result of spherical linear interpolation between this vector and [code]to[/code], by amount [code]weight[/code]. [code]weight[/code] is on the range of 0.0 to 1.0, representing the amount of interpolation. If this vector and [code]to[/code] are colinear, the interpolation is undefined and the method returns an empty vector.
 				This method also handles interpolating the lengths if the input vectors have different lengths. For the special case of one or both input vectors having zero length, this method behaves like [method lerp].
 			</description>
 		</method>

Maybe we should do something like something like:

 Vector3 Vector3::slerp(const Vector3 &p_to, const real_t p_weight) const { 
 	real_t theta = angle_to(p_to); 
 	return is_approx_zero(theta) ? rotated(cross(p_to).normalized(), theta * p_weight) : p_to; 
 } 

When angle between vectors is basically zero then just return second vector

@JestemStefan It won’t work for 180 degrees.

---

Presumably it should change to a lerp at small angles (with a possible normalize afterwards, but may not be necessary).

@lawnjelly Only because of the simpler calculations or is there some other reason? And in case of lerping I think normalizing would be needed no matter what threshold is choosen for deciding whether we lerp or no. Imagine that two input unit vectors are almost equal and their lengths equal the smallest possible length satysfying is_normalized() check. Then lerping between two such vectors (using 0.0 < weight < 1.0) would likely output a vector with length not satysfying is_normalized() check.

The solution is also undefined at 180 degrees, so I guess it’s a case of picking a random axis of rotation there (I guess quaternion slerp has the same problem).

We’d need to pick any valid axis of rotation, but not a random one. Results of slerp for the same arguments should be deterministic.

I also notice that angle_to is also doing the same cross product so suffering the same (and perhaps this could be done as a once off, I don’t know if this will be optimized out).

angle_to is fine, it’s calculating the same cross product but it uses it differently. Calculating cross product of collinear vectors is fine on its own, the problem is trying to use its result (a zero vector, even if normalized()) as an axis of rotation in slerp.

It is possible that Vector2::slerp() could also be affected.

Gave it a brief look and it seems fine. It’s much simpler since there’s only one plane to deal with and thus the same axis of rotation can be used no matter what.

I think this works (gdscript), @kleonc can give a look. I just made this up, because I couldn’t find good example code that dealt with the special cases.

func myslerp2(var from : Vector3, var to : Vector3, var weight : float):
	# assumes normalized vector input
	var dp = from.dot(to);

	# special cases, colinear, where dp is close to 1.0 or -1.0
	if (abs(dp) > 0.999):
		# near 0 degrees
		if (dp > 0.0):
			return from + ((to - from) * weight);
		
		# deal with 180 degree case .. pick a random cross
		# we use a swizzle here (hope this should always work)
		var rand_vec = Vector3(from.y, from.z, from.x)
		
		# we need 2 cross products to get one 90 degrees from the line
		var cross = from.cross(rand_vec)
		cross = from.cross(cross)
		
		return from.rotated(cross, PI * weight)
	
	# old routine, slightly optimized
	var cross = from.cross(to);
	var cross_length = cross.length()
	var theta = atan2(cross_length, dp);
	cross *= 1.0 / cross_length;
	
	return from.rotated(cross, theta * weight);

Ah I just realised there is a case the swizzling won’t work with: Vector (1,1,1) normalized. Because after the swizzling the vector will be the same. So that would be better to deal with too. I don’t know if there’s any simple standard way of coming up with a random vector that is guaranteed not to be the same that doesn’t involve these gymnastics.

About picking axis of rotation in 180 degrees case: here are some nice comments on the subject. According to this comment it’s impossible to pick such axis without considering different cases (branching is unavoidable). According to this comment we could for example use something like this:

# ...
# 180 degrees case
var axis_of_rotation: Vector3
if from.x == 0:
	axis_of_rotation = Vector3(1, 0, 0)
elif from.y == 0:
	axis_of_rotation = Vector3(0, 1, 0)
elif from.z == 0:
	axis_of_rotation = Vector3(0, 0, 1)
else:
	# could be simplified to not use `normalized()`
	axis_of_rotation = Vector3(1, 1, -(from.x + from.y) / from.z).normalized()

I’m also not sure about using PI in return from.rotated(cross, PI * weight). When calling slerp(a, b, 1.0) you’d expect it to return b. So I wonder if using PI instead of the result of a.angle_to(b) in there could result in less expectable results (more different from b) in some cases. However, if we’d use the result of a.angle_to(b) then it would probably matter whether we pick some_axis_of_rotation or -some_axis_of_rotation as we would want to rotate in the direction where the angle is sligthly less than 180 degrees, not slightly bigger. So if we’d pick wrong axis an error would be even bigger. So yeah, picking PI is probably just fine (just thinking out loud).

Alternatively this code also appears on google search quite a bit and seems a bit more elegant (especially using acos on the dot product for the angle, but doesn’t deal with the 180 degree case:

https://keithmaggio.wordpress.com/2011/02/15/math-magician-lerp-slerp-and-nlerp/

// Special Thanks to Johnathan, Shaun and Geof!
Vector3 Slerp(Vector3 start, Vector3 end, float percent)
{
     // Dot product - the cosine of the angle between 2 vectors.
     float dot = Vector3.Dot(start, end);
     // Clamp it to be in the range of Acos()
     // This may be unnecessary, but floating point
     // precision can be a fickle mistress.
     Mathf.Clamp(dot, -1.0f, 1.0f);
     // Acos(dot) returns the angle between start and end,
     // And multiplying that by percent returns the angle between
     // start and the final result.
     float theta = Mathf.Acos(dot)*percent;
     Vector3 RelativeVec = end - start*dot;
     RelativeVec.Normalize();
     // Orthonormal basis
     // The final result.
     return ((start*Mathf.Cos(theta)) + (RelativeVec*Mathf.Sin(theta)));
}

Yeah, I’ve searched a little too and most implementations I found seems similar to that one (for example libGDX).

Actually given that the behaviour is undefined at 180 degrees because of the singularity, it could be argued not to deal with it and flag an error (this is what happens currently with the look_at function). On the other hand in 99% of cases choosing a random axis might work ok, at risk of some flickering if the epsilon is too large.

Yeah, not handling such case is also a viable solution. Anyway, it will need to be documented (whether it won’t be handled or the axis of rotation will be chosen in an arbitrarily choosen way).

For vectors pointing in the same direction, I tried adding these test cases and it passes:

	CHECK_MESSAGE(
			Vector2(1, 1).slerp(Vector2(2, 2), 0.5).is_equal_approx(Vector2(1.5, 1.5)),
			"Vector2 slerp with colinear inputs should behave as expected.");

	CHECK_MESSAGE(
			Vector3(1, 1, 1).slerp(Vector3(2, 2, 2), 0.5).is_equal_approx(Vector3(1.5, 1.5, 1.5)),
			"Vector3 slerp with colinear inputs should behave as expected.");

For vectors pointing in opposite directions, I don’t know what you expect to happen. If we interpolate over the surface of a sphere, there is no preferred direction if the vectors are pointing in exactly opposite directions. I guess the best we can do is add a meaningful error message for debug builds?

Currently errors are shown for collinear vectors but slerping vectors with the same direction is fine, simply no rotation needs to be done.

Meaning the 0 degrees case (collinear vectors with the same direction) currently errors out but according to the behavior introduced in #55877 it should just lerp the vectors.

And yeah, not supporting the 180 degrees case (collinear vectors with the opposite direction) is fine. But besides documenting it maybe it should also result in a meaningful error, not like the current one? 🤔