Bug 636 - Quaternion multiplication unexpected behavior
Summary: Quaternion multiplication unexpected behavior
Status: RESOLVED FIXED
Alias: None
Product: Jogl
Classification: JogAmp
Component: graph (show other bugs)
Version: 2
Hardware: All all
: --- critical
Assignee: Sven Gothel
URL:
Depends on:
Blocks:
 
Reported: 2012-11-06 23:29 CET by Tek
Modified: 2012-11-11 09:14 CET (History)
2 users (show)

See Also:
Type: ---
SCM Refs:
jogl 944562a9600598dfa8a23f96f568fde999e1eca3 jogl 5fafc1ac360333645b807dcd8dff0c0a655ea439
Workaround: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tek 2012-11-06 23:29:07 CET
I am using JOGL 2.0 rc 10.

I tried using Quaternion class that was implemented in jogamp.graph.math.MathFloat.Quaternion and I noticed an error in mult(Quaternion).

I am not a Quaternion expert but according to all math site online quaternion multiplication is done as follows:

Let Q1 and Q2 be two quaternions, which are defined, respectively, as (w1, x1, y1, z1) and (w2, x2, y2, z2).
(Q1 * Q2).w = (w1w2 - x1x2 - y1y2 - z1z2)
(Q1 * Q2).x = (w1x2 + x1w2 + y1z2 - z1y2)
(Q1 * Q2).y = (w1y2 - x1z2 + y1w2 + z1x2)
(Q1 * Q2).z = (w1z2 + x1y2 - y1x2 + z1w2

http://en.wikipedia.org/wiki/Quaternion
http://www.cprogramming.com/tutorial/3d/quaternions.html
http://mathworld.wolfram.com/Quaternion.html

In Quaternion class, mult(Quaternion) is currently defined as:

    public void mult(Quaternion q)
    {
        float w1 = w*q.w - (x*q.x + y*q.y + z*q.z);

        float x1 = w*q.z + q.w*z + y*q.z - z*q.y;
        float y1 = w*q.x + q.w*x + z*q.x - x*q.z;
        float z1 = w*q.y + q.w*y + x*q.y - y*q.x;

        w = w1;
        x = x1;
        y = y1;
        z = z1; 
    }
I believe this multiplication is incorrect and it rotated my jogl model unpredictively.  Also, according to quaternion definition, quaternion multiplication are not commutative!!! (ie: q.w*z is different from z*q.w)

I override the mult(Quaternion) with the definition of quaternion multiplication and it behaves as expected:

    public void mult(Quaternion q)
    {
        float w1 = w*q.w - x*q.x - y*q.y - z*q.z;

        float x1 = w*q.x + x*q.w + y*q.z - z*q.y;
        float y1 = w*q.y - x*q.z + y*q.w + x*q.x;
        float z1 = w*q.z + x*q.y - y*q.x + y*q.w;

        w = w1;
        x = x1;
        y = y1;
        z = z1; 
    }

Thank you in advance
Comment 1 Sven Gothel 2012-11-07 01:57:38 CET
Thank you for your detailed bug report, inclusive references and fix.

Assigned to the original author, Rami Santina.

Let's wait until Rami has a chance to reply.
Comment 2 Sven Gothel 2012-11-07 01:59:04 CET
@Rami: We may want to merge math util stuff (FloatUtil, ..) somehow more central ?
Comment 3 Rami Santina 2012-11-07 12:04:30 CET
Yup thats correct, the fix is verified. Its good that we havent used this method in graph yet :) Great Catch. Thanks.

PS: its true quaternions are not commutative, but thats regarding q1*12 vs q2*q1, thus has nothing to do with quat. premitives. since w1*w2 will always give same result :)

@Sven, can you merge in this fix?
Comment 4 Rami Santina 2012-11-07 15:59:41 CET
Yup thats correct, the fix is verified. Its good that we havent used this method in graph yet :) Great Catch. Thanks.

PS: its true quaternions are not commutative, but thats regarding q1*12 vs q2*q1, thus has nothing to do with quat. premitives. since w1*w2 will always give same result :)

@Sven, can you merge in this fix?
Comment 5 Sven Gothel 2012-11-07 16:32:02 CET
(In reply to comment #4)
> @Sven, can you merge in this fix?
Yes.
Comment 6 Sven Gothel 2012-11-07 16:33:15 CET
BTW ... can we have a unit test for this ? :)

I will also validate the package location of out float/math stuff.
Comment 7 Tek 2012-11-07 18:32:34 CET
(In reply to comment #4)
> PS: its true quaternions are not commutative, but thats regarding q1*12 vs
> q2*q1, thus has nothing to do with quat. premitives. since w1*w2 will always
> give same result :)

Thank you for clearing that up.
Comment 8 Sven Gothel 2012-11-11 09:14:03 CET
Fixed w/ 944562a9600598dfa8a23f96f568fde999e1eca3 - setting Tek as author of commit.

Also went fwd to reorg. math util locations, see commit 5fafc1ac360333645b807dcd8dff0c0a655ea439,
etc ..