Normalization is needed after quaternion multiply

[panxinmiao]

I find that sometimes when the model moves too fast, the physical engine may break the model.

Because of the rounding error of floating-point numbers, the modulus of quaternion is not strictly equal to 1, and the error will accumulate continuously in the "multiply" operation. Ultimately, the four components of the quaternion will overflow and become NAN, and then the model will break down.

Therefore, after each quaternion multiply, it needs to be normalized.

[WestLangley]

@Mugen87 Exactly what is the line that causes the problem?

Is the bone matrix not a pure rotation matrix? If so, this fix is likely not the correct one.

/ping @takahirox

[takahirox]

Confirmed that quaternion values can be too huge, it can break model, and this change solves the problem (not sure what the root issue is tho). I think good to go so far. We can investigate more later.

[takahirox]

Might not need .normalize() here? I tested locally, it seems ok to remove.

[WestLangley]

Confirmed that quaternion values can be too huge, it can break model, and this change solves the problem (not sure what the root issue is tho). I think good to go so far. We can investigate more later.

three.js quaternions must have unit length. I think you need to figure out what line of code is causing the problem.

[takahirox]

I'm too busy to investigate. So I'd like to merge this change so far and look into later because I know this change is just one or two lines and actually solves the problem. Or I'm glad if someone tries to figure out the root issue.

[WestLangley]

Maybe add an inline comment, then? I expect we are all wary of fixes that camouflage an issue.

[takahirox]

Yeah, I think inline comment is a good idea.

[WestLangley]

Merging then, so @takahirox can add the inline comments as needed.

[yomboprime]

Just a note: when multiplying successively unit quaternions it is normal that the results degrade progressively if the code doesn't normalize them. The same happens if you make succesive cross products on unit Vector3s or products on Matrix3.

Little by little, floating point errors accumulate over the frames. The result is a quaternion or vector with length not equal to 1, or matrix with not unit main vectors or not orthogonal.

[WestLangley]

@yomboprime Seems reasonable. Would you like to file a PR with your recommended changes to the math library?

[garyo]

I've seen the same thing as @yomboprime in my own graphics libs. You have to periodically renormalize if you're repeatedly transforming a matrix or quaternion due to accumulating floating point error (but not if you're always transforming from a fixed source). Of course the renormalize has a cost so you don't want to overdo it.

[yomboprime]

@yomboprime Seems reasonable. Would you like to file a PR with your recommended changes to the math library?

I've reviewed the code and I think @takahirox is right, only the second call to normalize() is needed. When performing calculations normally it is needed only to normalize at the end.

[takahirox]

Wondering which we should call .normalize() in the math library or out of the library.

Doing in the math library like this maybe useful to users because they don't need to care floating point error but might be costly.

// Quaternion.js
multiplyQuaternions: function ( a, b ) {

	// from http://www.euclideanspace.com/maths/algebra/realNormedAlgebra/quaternions/code/index.htm

	var qax = a._x, qay = a._y, qaz = a._z, qaw = a._w;
	var qbx = b._x, qby = b._y, qbz = b._z, qbw = b._w;

	this._x = qax * qbw + qaw * qbx + qay * qbz - qaz * qby;
	this._y = qay * qbw + qaw * qby + qaz * qbx - qax * qbz;
	this._z = qaz * qbw + qaw * qbz + qax * qby - qay * qbx;
	this._w = qaw * qbw - qax * qbx - qay * qby - qaz * qbz;

	this.normalize(); // adding here

	this.onChangeCallback();

	return this;

},

[garyo]

Doing in the math library like this maybe useful to users because they don't need to care floating point error but might be costly.

IMHO that's definitely too costly. Renormalization is only needed when repeatedly transforming the same quaternion/matrix. If you're doing something like this pseudocode you don't need it at all:

for (float amt = 0; amt < 360; amt+=dtheta) {
  model_transform = fixed_orig.rotate(amt, axis);
}

i.e. if you're always rotating starting from a fixed matrix/quaternion, you don't need this since the error never builds up.

[takahirox]

I thought this type of problems may be difficult for many users to figure out the root issue if they face, so renormalizing in the math lib may be helpful even if it's costly. But yeah I agree it may be "too" costly. Adding note to the document instead?

[garyo]

If three.js has a "debug" mode, then in that mode it could check at various times that quaternions are "nearly normalized" and log a warning if not. That would be really nice. But other than that I think a doc note is appropriate. Every graphics lib has this same issue. (BTW I'm an old school graphics guy but quite new to three.js, so take everything I say as an "outsider view"!)

[yomboprime]

Oh, definitely IMHO renormalization has to be done by the application. Another related example: If a user adds a value to an angle successively, a trig function on the angle will eventually return wrong numbers, even NaN. The solution in this case is for example clamp the angle to [0, 2*PI) after all the calculations on each frame. The users have to take care of their floating points.

[takahirox]

OK, renormalization in application side looks right way. Made PR #16207 to remove redundant renormalization and add inline comment. Thanks.