Skip to content

Conversation

@TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented May 13, 2025

For the Quaternion element in the Godot editor, the input precision of the float is 0.001. However, the accumulation of precision error in synthesizing four floats is not taken into account.

This PR ensures that the epsilon is determined taking into account the accumulation of errors. From a mathematical point of view, the worst-case error should be 0.004004, so the epsilon is 0.005 by ceil.

Calculation of accumulated error from 4 floats
Public callers are allowed an absolute component error of ±0.001.

For a unit quaternion q₀ = (x, y, z, w) and perturbation δᵢ:

‖q‖² = Σ(xᵢ + δᵢ)² = 1 + 2 Σ xᵢ δᵢ + Σ δᵢ²

with |δᵢ| ≤ 0.001,   Σ|xᵢ| ≤ 2  ⇒  |2 Σ xᵢ δᵢ| ≤ 0.004  

Σ δᵢ² ≤ 0.000004

Worst-case deviation ≈ 0.004004.

The following code will be identified as not normalized in most cases before the application of this PR, but will pass the normalization check after the PR is applied.

test_quat_compose.gd

extends Node3D

func _process(delta: float) -> void:
	var q: Quaternion = Quaternion(randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1)).normalized()
	q = Quaternion(snappedf(q.x, 0.001), snappedf(q.y, 0.001), snappedf(q.z, 0.001), snappedf(q.w, 0.001))

	if !q.is_normalized():
		print(str(q))

test_quat.zip

However, errors may still accumulate due to multiplication between quaternions.

This means that the result of multiplication between normalized quaternions may not be normalized. If the multiplication exceeds three times, normalize may collapse. This can be checked with the following gdscript.

test_quat_accum.gd

extends Node3D

func _process(delta: float) -> void:
	for i in range(100000):
		var q1: Quaternion = Quaternion(randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1)).normalized()
		q1 = Quaternion(snappedf(q1.x, 0.001), snappedf(q1.y, 0.001), snappedf(q1.z, 0.001), snappedf(q1.w, 0.001))
		var q2: Quaternion = Quaternion(randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1)).normalized()
		q2 = Quaternion(snappedf(q2.x, 0.001), snappedf(q2.y, 0.001), snappedf(q2.z, 0.001), snappedf(q2.w, 0.001))
		var q3: Quaternion = q1 * q2
		var q4: Quaternion = q3 * q1

		if q1.is_normalized() && q2.is_normalized() && q3.is_normalized() && !q4.is_normalized():
			print(str(q1), str(q2), str(q3), str(q4))

To solve this, it is safe to normalize the result of multiplication between normalized quaternions.

constexpr void Quaternion::operator*=(const Quaternion &p_q) {
	bool is_safe = is_normalized() && p_q.is_normalized();
	real_t xx = w * p_q.x + x * p_q.w + y * p_q.z - z * p_q.y;
	real_t yy = w * p_q.y + y * p_q.w + z * p_q.x - x * p_q.z;
	real_t zz = w * p_q.z + z * p_q.w + x * p_q.y - y * p_q.x;
	w = w * p_q.w - x * p_q.x - y * p_q.y - z * p_q.z;
	x = xx;
	y = yy;
	z = zz;
	if (is_safe) {
		normalize();
	}
}

But I believe that this probably should not be done from a performance standpoint.

What we should find with care here is a function that directly returns the result of normalized Quaternion multiplication like return q1 * q2;. If anyone find that, I think it should be fixed accordingly as follow up this PR.

Also the fact that the multiplications between normalized Quaternion results may not be normalized, so that normalization is necessary may need to be documented, e.g., in the https://docs.godotengine.org/en/latest/tutorials/math/matrices_and_transforms.html.

@TokageItLab TokageItLab added this to the 4.5 milestone May 13, 2025
@TokageItLab TokageItLab requested review from a team and aaronfranke May 13, 2025 03:19
@TokageItLab TokageItLab requested a review from a team as a code owner May 13, 2025 03:19
@TokageItLab TokageItLab changed the title Fix Quaternion tolerance taken into account accumulated 4 float errors Fix Quaternion tolerance take into account accumulated 4 float errors May 13, 2025
@TokageItLab TokageItLab changed the title Fix Quaternion tolerance take into account accumulated 4 float errors Fix Quaternion tolerance to take into account accumulated 4 float errors May 13, 2025
@aaronfranke
Copy link
Member

I don't know about changing a core math function to fix a problem caused by the editor. Perhaps we need to increase the precision in the editor instead?

@TokageItLab
Copy link
Member Author

TokageItLab commented May 13, 2025

Do you mean to precise the unit precision of the editor only for Quaternion? Although I don't think quaternions should be directly manipulated by humans, even if they were, it seems impractical to display and manipulate values in the editor with a precision more detail than 0.001.

Also, the unit precision of 0.001 is commonly used for properties of all classes; as long as the Vector3 editor's default unit is 0.001, it is meaningless to be only precise in the precision of the Quaternion elements. As commented in #106337, an error may occur within the constructor when generating a Quaternion from a non-Quaternion (although I think that should be normalized in the constructor, as in that PR #106337). So, I think it makes no sense to normalize() only the values retrieved from the Quaternion editor as well.

It seems that replacing length_squared() with length() is alternative like #98308, but there is a performance concern. It is also more vulnerable to error accumulation.

If you set the epsilon to 0.005, the multiplication between quaternions immediately after input (snapping) will fall within the is_normalized() range. However, if you multiply by a third one, normalization is no longer guaranteed.

func _process(delta: float) -> void:
	var q1: Quaternion = Quaternion(randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1)).normalized()
	q1 = Quaternion(snappedf(q1.x, 0.001), snappedf(q1.y, 0.001), snappedf(q1.z, 0.001), snappedf(q1.w, 0.001))
	var q2: Quaternion = Quaternion(randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1)).normalized()
	q2 = Quaternion(snappedf(q2.x, 0.001), snappedf(q2.y, 0.001), snappedf(q2.z, 0.001), snappedf(q2.w, 0.001))
	var q3: Quaternion = q1 * q2

	if q3.is_normalized(): # <- always true
		print(str(q3))

	var q4: Quaternion = q1 * q2 * q1

	if q4.is_normalized(): # <- can be false
		print(str(q4))

When length_squared() is length(), there is no guarantee that the multiplication of quaternions immediately after input will fall within the is_normalized() range.

func _process(delta: float) -> void:
	var q1: Quaternion = Quaternion(randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1)).normalized()
	q1 = Quaternion(snappedf(q1.x, 0.001), snappedf(q1.y, 0.001), snappedf(q1.z, 0.001), snappedf(q1.w, 0.001))
	var q2: Quaternion = Quaternion(randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1), randf_range(-1, 1)).normalized()
	q2 = Quaternion(snappedf(q2.x, 0.001), snappedf(q2.y, 0.001), snappedf(q2.z, 0.001), snappedf(q2.w, 0.001))
	var q3: Quaternion = q1 * q2

	if q3.is_normalized(): # <- can be false
		print(str(q3))

If performance is not a concern, I think it would be most accurate to adopt length() and guarantee normalization in the multiplication as shown at the end of the description, but benchmark measurements would be needed.

@TokageItLab
Copy link
Member Author

Superseded by #106352

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Changing bone euler rotation sets non-normalized quaternion

3 participants