[fixed] quaternion::rotationFromTo copy&paste fail

[Agent D]

In the download-able version 1.7.2 and SVN revision 3702, the file quaternion.h contains a very interesting passage in the function "rotationFromTo", namely lines 633-638(651-656 in SVN):

Code: Select all

 
axis = axis.crossProduct(core::vector3df(X,Y,Z));
if (axis.getLength()==0)
{
    axis.set(0.f,1.f,0.f);
    axis.crossProduct(core::vector3df(X,Y,Z));
}
 

As you can see in the lines with the "crossProduct" calls, the quaternion's own components of unknown values are accessed. It appears a bit strange to me that the quaternion's original values are needed to compute a new unit quaternion rotation from one given vector to an other. Especially, imagine the quaternion this method is called on is an identity quaternion, in which case the computed axis would be of zero length.
This cannot be right.

After a little google search, I found the very same code(even with the same commentaries) in the Ogre repository, where it is part of the Vector3 class, which makes sense, because then the starting vector's components are used. So i propose the following fix:

Code: Select all

 
axis = axis.crossProduct(v0);
if (axis.getLength()==0)
{
    axis.set(0.f,1.f,0.f);
    axis.crossProduct(v0);
}
 

mfg

Agent D

[hybrid]

Sounds reasonable. I have to switch over to the other system where I still have the 1.7 branch up and running so it comes into that version as well - maybe tonight. Thanks for the fix.

[hybrid]

We already have some tests in SVN/trunk, and they seem to work correctly. Do you have some rotations which fail due this wrong code? I'd like to have some test case first, to make sure that no corner cases go through here. Maybe the test is always false anyway and the branch is never taken?
Edit: After googling about this code I found the original article and the statement there: Every axis perpendicular to v0 will do. Of course, this means your fix is correct and it hints me to possible problematic vectors. Let's see...

[Agent D]

The branch is executed when rotationFromTo is called with to vectors, pointing in opposing directions. In that case, a 180 degree rotation about a vector perpendicular to one of the two vectors is required. The original code would take the cross product of a unit vector pointing down either the x or the y axis with an unpredictable other vector. My proposition takes the cross product with v0, to get a vector perpendicular to v0.
Unfortunately I don't have a test case, as I discovered this purely by reading the source code.

[hybrid]

Ok, for now I've put it into SVN/trunk only. If there's some need, we could also backport it after some more testing, especially with skinned meshes. But as it worked well for now, it might be enouhg to fix these corner cases only in the next major version.