Dav1dde / gl3n

OpenGL Maths for D (not glm for D).
http://dav1dde.github.com/gl3n/
Other
103 stars 49 forks source link

euler_rotation and pitch/yaw/roll broken #60

Closed extrawurst closed 9 years ago

extrawurst commented 9 years ago

it took me hours to finally realize those methods are broken instead of my code using them ^^

consider the following test case:

unittest{
    enum startPitch = 0.1;
    enum startYaw = -0.2;
    enum startRoll = 0.6;

    import std.stdio;
    writefln("%s,%s,%s",startPitch,startYaw,startRoll);

    auto q = quat.euler_rotation(startRoll,startPitch,startYaw);

    writefln("%s,%s,%s",q.pitch,q.yaw,q.roll);

    assert(almost_equal(q.pitch,startPitch),.format("%s != %s",q.pitch,startPitch));
    assert(almost_equal(q.yaw,startYaw),.format("%s != %s",q.yaw,startYaw));
    assert(almost_equal(q.roll,startRoll),.format("%s != %s",q.roll,startRoll));
}

essentially we take euler angles, create a quaternion of them and get the euler angles from that quaternion again. this should work without loosing any information. the above test case tries that. it fails.

then i examined the wiki page for euler angles and found that all four methods are defined different than in your implementation: http://en.wikipedia.org/wiki/Conversion_between_quaternions_and_Euler_angles

my implementation looks like this and solves the issue but breaks some of your unittests where u hardcoded what seems to be wrong results:

/// Returns the yaw.
    @property real yaw() const {
        return atan2(2.0*(w*z + x*y), 1.0 - 2.0*(y*y + z*z));
    }

    /// Returns the pitch.
    @property real pitch() const {
        return asin(2.0*(w*y - z*x));
    }

    /// Returns the roll.
    @property real roll() const {
        return atan2(2.0*(w*x + y*z), 1.0 - 2.0*(x*x + y*y));
    }

static Quaternion euler_rotation(real roll, real pitch, real yaw) {
        Quaternion ret;

        auto cr = cos(roll / 2.0);
        auto cp = cos(pitch / 2.0);
        auto cy = cos(yaw / 2.0);
        auto sr = sin(roll / 2.0);
        auto sp = sin(pitch / 2.0);
        auto sy = sin(yaw / 2.0);

        ret.w = cr * cp * cy + sr * sp * sy;
        ret.x = sr * cp * cy - cr * sp * sy;
        ret.y = cr * sp * cy + sr * cp * sy;
        ret.z = cr * cp * sy - sr * sp * cy;

        return ret;
    }
Dav1dde commented 9 years ago

This is a pretty huge issue :o. Good find!

I copied my stuff basically 1:1 from somewhere (I though I put a comment where I got it from ...). Let me look into that/your pull.

Dav1dde commented 9 years ago

Following: http://www.euclideanspace.com/maths/geometry/rotations/conversions/eulerToQuaternion/ the euler_rotation code is correct, but your usage isn't.

Heading = rotation about y axis
Attitude = rotation about z axis
Bank = rotation about x axis

static Quaternion euler_rotation(real heading, real attitude, real bank)

Heading, Attitude and Bank should probably be converted the roll, pitch, yaw.

The yaw, pitch, roll properties seem actually to be wrong, according to http://www.tinkerforge.com/en/doc/Software/Bricks/IMU_Brick_CSharp.html

It is also possible to calculate independent angles. You can calculate yaw, pitch and roll in a right-handed vehicle coordinate system according to DIN70000 with:

yaw   =  atan2(2*x*y + 2*w*z, w*w + x*x - y*y - z*z)
pitch = -asin(2*w*y - 2*x*z)
roll  = -atan2(2*y*z + 2*w*x, -w*w + x*x + y*y - z*z))
extrawurst commented 9 years ago

no time to look at your source but my stuff is from wikipedia, do you think it's wrong ?

extrawurst commented 9 years ago

@Dav1dde to be honest I dont care what the formulars are, I just wanna have them working so that when i create a quternion with euler angles and convert that quaternion back to eulers that they are matching.

extrawurst commented 9 years ago

from what i see the only question is how u translate heading/attitude/bank to yaw/roll/pitch and then your euler_rotation matches mine. but that is exactly the problem because they do not match the way back using the yaw/roll/pitch methods (besides that those were buggy).. so i dont see how my usage is wrong ?

extrawurst commented 9 years ago

seems to me heading/attitude/bank is simply another form of representing a rotation in angles and we should support both ways in the lib.

Dav1dde commented 9 years ago

I am sorry, I was really busy and then I forgot about the issue, I just checked your code, it seems to be correct! I will merge your pull, sorry about that, again.

Dav1dde commented 9 years ago

Fixed by #61