-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
make Heading pitch roll counter clock wise again 2 #5809
Changes from 6 commits
b12bf9d
fced5d3
66e6da1
a02d32c
a8234af
9135155
f1c1337
0223afc
6281116
02f4046
7468635
db16f8f
fc9cb93
16cd536
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,21 @@ | ||
Change Log | ||
========== | ||
### 1.38 - 2017-10-01 | ||
|
||
* Breaking changes | ||
|
||
* Deprecated | ||
* `HeadingPitchRoll.fromDirectQuaternion` is deprecated and his behaviour will replace `HeadingPitchRoll.fromQuaternion` behaviour in 1.43. [#5666](https://github.com/AnalyticalGraphicsInc/cesium/issues/5666) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to deprecate this 6 months out? We normally do 1-3 releases. I think 3 releases would be appropriate for this case unless someone said otherwise. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, who decides? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kaktus40 usually the developer and whoever reviews decides. I agree with @hpinkos that 3 releases would be fine. Please submit a GitHub issue for the deprecation. For more on deprecation, see https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Documentation/Contributors/CodingGuide#deprecation-and-breaking-changes |
||
* `Matrix3.fromDirectHeadingPitchRoll` is deprecated and his behaviour will replace `Matrix3.fromHeadingPitchRoll` behaviour in 1.43. [#5666](https://github.com/AnalyticalGraphicsInc/cesium/issues/5666) | ||
* `Quaternion.fromDirectHeadingPitchRoll` is deprecated and his behaviour will replace `Quaternion.fromHeadingPitchRoll` behaviour in 1.43. [#5666](https://github.com/AnalyticalGraphicsInc/cesium/issues/5666) | ||
* `Transforms.directHeadingPitchRollToFixedFrame` is deprecated and his behaviour will replace `Transforms.headingPitchRollToFixedFrame` behaviour in 1.43. [#5666](https://github.com/AnalyticalGraphicsInc/cesium/issues/5666) | ||
* `Transforms.directHeadingPitchRollQuaternion` is deprecated and his behaviour will replace `Transforms.headingPitchRollQuaternion` behaviour in 1.43. [#5666](https://github.com/AnalyticalGraphicsInc/cesium/issues/5666) | ||
* Added HeadingPitchRoll.fromDirectQuaternion that works with classical orientation of heading and pitch [#5666](https://github.com/AnalyticalGraphicsInc/cesium/issues/5666) | ||
* Added Matrix3.fromDirectHeadingPitchRoll that works with classical orientation of heading and pitch [#5666](https://github.com/AnalyticalGraphicsInc/cesium/issues/5666) | ||
* Added Quaternion.fromDirectHeadingPitchRoll that works with classical orientation of heading and pitch [#5666](https://github.com/AnalyticalGraphicsInc/cesium/issues/5666) | ||
* Added Transforms.directHeadingPitchRollToFixedFrame that works with classical orientation of heading and pitch [#5666](https://github.com/AnalyticalGraphicsInc/cesium/issues/5666) | ||
* Added Transforms.directHeadingPitchRollQuaternion that works with classical orientation of heading and pitch [#5666](https://github.com/AnalyticalGraphicsInc/cesium/issues/5666) | ||
|
||
### 1.37 - 2017-09-01 | ||
|
||
* Breaking changes | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,14 @@ define([ | |
'./defaultValue', | ||
'./defined', | ||
'./DeveloperError', | ||
'./Math' | ||
'./Math', | ||
'./deprecationWarning' | ||
], function( | ||
defaultValue, | ||
defined, | ||
DeveloperError, | ||
CesiumMath) { | ||
CesiumMath, | ||
deprecationWarning) { | ||
'use strict'; | ||
|
||
/** | ||
|
@@ -40,6 +42,7 @@ define([ | |
throw new DeveloperError('quaternion is required'); | ||
} | ||
//>>includeEnd('debug'); | ||
deprecationWarning('HeadingPitchRoll.fromQuaternion', 'This HeadingPitchRoll.fromQuaternion works in the Cesium legacy fashion which means that heading and pitch is opposite of the classical interpretation used in mathematics. This behavior will be corrected in 1.43 in order to be classical. The new behavior can be evaluate with HeadingPitchRoll.fromDirectQuaternion'); | ||
if (!defined(result)) { | ||
result = new HeadingPitchRoll(); | ||
} | ||
|
@@ -54,6 +57,34 @@ define([ | |
return result; | ||
}; | ||
|
||
/** | ||
* Computes the heading, pitch and roll from a quaternion (see http://en.wikipedia.org/wiki/Conversion_between_quaternions_and_Euler_angles ) in the mathematical common sense | ||
* | ||
* @param {Quaternion} quaternion The quaternion from which to retrieve heading, pitch, and roll, all expressed in radians. | ||
* @param {HeadingPitchRoll} [result] The object in which to store the result. If not provided, a new instance is created and returned. | ||
* @returns {HeadingPitchRoll} The modified result parameter or a new HeadingPitchRoll instance if one was not provided. | ||
*/ | ||
HeadingPitchRoll.fromDirectQuaternion = function(quaternion, result) { | ||
//>>includeStart('debug', pragmas.debug); | ||
if (!defined(quaternion)) { | ||
throw new DeveloperError('quaternion is required'); | ||
} | ||
//>>includeEnd('debug'); | ||
deprecationWarning('HeadingPitchRoll.fromDirectQuaternion', 'This HeadingPitchRoll.fromDirectQuaternion works in the classical interpretation used in mathematics. This function will replaced HeadingPitchRoll.fromQuaternion in 1.43.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You shouldn't include the deprecation warning in the new functions. Instead include this additional information about how it's been calculated in the documentation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just updated following your recommandations |
||
if (!defined(result)) { | ||
result = new HeadingPitchRoll(); | ||
} | ||
var test = 2 * (quaternion.w * quaternion.y - quaternion.z * quaternion.x); | ||
var denominatorRoll = 1 - 2 * (quaternion.x * quaternion.x + quaternion.y * quaternion.y); | ||
var numeratorRoll = 2 * (quaternion.w * quaternion.x + quaternion.y * quaternion.z); | ||
var denominatorHeading = 1 - 2 * (quaternion.y * quaternion.y + quaternion.z * quaternion.z); | ||
var numeratorHeading = 2 * (quaternion.w * quaternion.z + quaternion.x * quaternion.y); | ||
result.heading = Math.atan2(numeratorHeading, denominatorHeading); | ||
result.roll = Math.atan2(numeratorRoll, denominatorRoll); | ||
result.pitch = Math.asin(test); | ||
return result; | ||
}; | ||
|
||
/** | ||
* Returns a new HeadingPitchRoll instance from angles given in degrees. | ||
* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For
directHeadingPitchRollToFixedFrane
, do we need to modify the parameters for these two functions at all like you are doing everywhere else you're using the new functions?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
directHeadingPitchRollToFixedFrame
takes the same parameters thanheadingPitchRollToFixedFrame
. Internally, it usesQuaternion.fromDirectHeadingPitchRoll
instead ofQuaternion.fromHeadingPitchRoll
. For now in this PR we have two groups of functions that we shouldn't mix. The first group, that should be deprecated, has the next functions:HeadingPitchRoll.fromQuaternion
,Matrix3.fromHeadingPitchRoll
,Quaternion.fromHeadingPitchRoll
,Transforms.HeadingPitchRollToFixedFrame
.The newcomer group, that should replace the first group, has the next functions:
HeadingPitchRoll.fromDirectQuaternion
,Matrix3.fromDirectHeadingPitchRoll
,Quaternion.fromDirectHeadingPitchRoll
,Transforms.directHeadingPitchRollToFixedFrame
.A function in the first group and its equivalent in the second group takes the same parameters in the same orders and return the same type of result (HeadingPitchRoll, Matrix3, Quaternion or Matrix4 instances respectively). The only difference is the sense of rotation for heading and pitch used internally.
Normally, I checked in my IDE that each instance of the first group functions and if needed, I changed for the equivalent in the second group. Also I didn't replace the functions when the parameters where heading=0, pitch=0 because there will be no change in their behaviour.