Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quaternion.fromHeadingPitchRoll uses wrong axis? #3256

Closed
xtassin opened this issue Nov 30, 2015 · 13 comments
Closed

Quaternion.fromHeadingPitchRoll uses wrong axis? #3256

xtassin opened this issue Nov 30, 2015 · 13 comments

Comments

@xtassin
Copy link
Contributor

xtassin commented Nov 30, 2015

Hi,

Testing the fromHeadingPitchRoll method, I got erratic results until I spotted what I (humbly) believe is a wrong use of axis.

Considering a eastNorthUp frame, the original method applies roll value on X axis and pitch on Y where I would expect the opposite.

When overriding the method with my own (swapped X and Y), I get the expected results.

I may be mistaken on assuming the eastNorthUp frame though.

If I am wrong, I would be happy to hear the proper reasoning for using this method.

Thanks,

Xavier.

@hpinkos
Copy link
Contributor

hpinkos commented Nov 30, 2015

Thanks @xtassin, I'm pretty sure you're right. I tried to look into this before but never got a definitive answer.
@bagnell? Do you have time to check this?

@speigg
Copy link
Contributor

speigg commented Jul 25, 2016

I just noticed this.. it does feel very unintuitive to me. However, the tests seem to be written assuming these axes as well (obviously, or they wouldn't pass). And the documentation also states that this is how it works, so I suppose it is intentional. (Regardless, I would also like to hear the reasoning behind why this method is written this way 😄)

@kaktus40
Copy link
Contributor

kaktus40 commented Aug 5, 2016

Hello,
check wikipedia.With this article, I can confirm that heading is a rotation around Z axis, pitch is a rotation around Y axis and roll is a rotation around X axis. Quaternion is only a representation of rotation. This is not linked to a frame. Commonly in Cesium, the frame most used is East North Up. But when we talk about heading pitch roll in aviation, the frame should be North East Down or North West Up. That is why I made a this pull.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 2, 2017

@kaktus40 can we close this or perhaps does it just need a doc update?

@hpinkos
Copy link
Contributor

hpinkos commented Jan 2, 2017

@pjcozzi here's where the confusion is: If you have heading = 0; the model points east. Then if you apply a pitch, it rotates about the north axis pointing the airplane up, and roll rotates about the east axis tilting the plane from side to side.
I would expect the model to point north at heading = 0. That seems to be the standard in aviation at least. In this case pitch should rotate about the east axis and roll would rotate about the north axis (the opposite of what we currently do)

So it's up to you if you think it's something we need to change or just something we need to document somewhere.

@kaktus40
Copy link
Contributor

kaktus40 commented Jan 2, 2017

Hello,
Sorry I didn't have the occasion to work on this.
This issue is deeply linked with the default reference frame used in Cesium. When I can turn the idea suggest here into a pull request, this issue should be resolved.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 3, 2017

Sounds great, thanks @kaktus40!

@hpinkos
Copy link
Contributor

hpinkos commented Jan 5, 2017

@ghost
Copy link

ghost commented Jan 20, 2017

@hpinkos Read the google group conversation. Until a PR is submitted and approved for this, is there a workaround for using the VelocityOrientationProperty with sampled positions other than recreating the model to face east? The model we are using points north, but slides around sideways.

@hpinkos
Copy link
Contributor

hpinkos commented Jan 23, 2017

@Zizekftw For now, I would recommend re-orienting your source model and converting that t glTF. I believe it should be forward on the X axis.

@kaktus40
Copy link
Contributor

kaktus40 commented Sep 3, 2017

Hello, now that localFrameToFixedFrameGenerator has been implemented in this pull, maybe this issue can be closed? Or maybe we should wait that issue to be resolved before closing it?

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 6, 2017

Perhaps it is OK to close this now as a duplicate of #5666.

@cesium-concierge
Copy link

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/forum/?hl=en#!topic/cesium-dev/bsPTytXzNqc

If this issue affects any of these threads, please post a comment like the following:

The issue at #3256 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.

I am a bot who helps you make Cesium awesome! Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants