Issue in quaternion log function

[kschoice]

In GLM version 0.9.5.3, in the quaternion log function, there is a return line as follows :
return detail::tquat<T, P>(t * q.x, t * q.y, t * q.z, log(QuatLen));

where I guess it should obviously be
return detail::tquat<T, P>(log(QuatLen), t * q.x, t * q.y, t * q.z);

as the quaternion constructor asks for (w,x,y,z) in that order.
I must say I only realized the prototype then, as it doesn't seem at all straightforward to me. I would have expected it to use order (x,y,z,w). I guess the person who wrote the log function made the same mistake.

[kschoice]

I shall add that after the following code :
glm::quat q; // q (1, 0, 0, 0)
glm::quat p = glm::log(q);
qlm::quat r = glm::exp(p);
r should have same value as q, but is filled with undefined values after division by zero inside glm::exp.

[Groovounet]

Erm, yes that code seems wrong. I have fixed the parameter inversion.

The glm::log function returns a quaternion that is quite a NaN for the quaternion I think which explain the glm::exp behavior on that result.

This said log(0) is -inf so the result you see might actually make sense here.

[kschoice]

Thanks for taking in account the bug described in my first message.

"This said log(0) is -inf so the result you see might actually make sense here."

Sorry, but I think you misunderstood the problem in my second message. The exp function is the one producing NaNs :
glm::quat q; // q (1, 0, 0, 0)
glm::quat p = glm::log(q); // After this, p = (0, 0, 0, 0), which is fine (log (1 + 0_i + 0_j + 0*k) = log(1) = 0)
qlm::quat r = glm::exp(p); // After this, r = (NaN, NaN, NaN, NaN) and should be (1, 0, 0, 0)
The problem is glm::exp makes a division by the norm of vec3(r.x, r.y, r.z). This happens for any quaternion argument with no imaginary part (e.g., equivalent to a plain number).

[Groovounet]

Erm true. I understood correctly your explanation but my analysis was too quick.

[kschoice]

Good. I hope this can benefit others, though I'm guessing not much people use log and exp for quaternions.

[kschoice]

Another problem with quaternion log :
if((q.x == T(0)) && (q.y == T(0)) && (q.z == T(0)))
{
...
}
else
{
T Vec3Len = sqrt(q.x * q.x + q.y * q.y + q.z * q.z);
T QuatLen = sqrt(Vec3Len * Vec3Len + q.w * q.w);
T t = atan(Vec3Len, T(q.w)) / Vec3Len;
}

Problem is (it just happened to me with Visual Studio 2012), Vec3Len can compute to a strict 0, even if q.x, q.y and q.z aren't all strict 0s. Thus raising division by zero.

I fixed it this way (I realize it may be a waste to compute Vec3Len in all cases, but I am more concerned with consistency) :
T Vec3Len = sqrt(q.x * q.x + q.y * q.y + q.z * q.z);
if (abs(Vec3Len) < std::numeric_limits::epsilon())
{
...
}
else
{
T QuatLen = sqrt(Vec3Len * Vec3Len + q.w * q.w);
T t = atan(Vec3Len, T(q.w)) / Vec3Len;
}

Please let me know if you think I disregarded something.

[Nemikolh]

T Vec3Len = sqrt(q.x * q.x + q.y * q.y + q.z * q.z);
if (abs(Vec3Len) < std::numeric_limits::epsilon())

I think you can remove the abs call. :)

[kschoice]

Good point lol !

[Groovounet]

Do you have a fix for this log function?

If yes, a pull request would be appreciated. :D

[kschoice]

Do you have a fix for this log function?
If yes, a pull request would be appreciated. :D

Sorry, I'm afraid I don't understand what you're asking for :-s

[Groovounet]

If you want the log function for quaternion to be fix in GLM, it would be better to submit an implementation.

[kschoice]

Sorry, I'm not used to GitHub or collaborative coding in general. I thought submitting my modification in this discussion was enough. What am I supposed to do ?

[Groovounet]

glm [at] g-truc.net would do then, thanks!

[Groovounet]

I have applied your changes in GLM 0.9.5 branch and they will be included in GLM 0.9.5.4 release.

Thanks for contributing!
Christophe