Cartographic cleanup. #109
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Cartographic3 now has static versions of it's prototype methods. 2. Cartographic3 methods now work on object literals that expose longitude/latitude/height properties. 3. Cartographic3 now has proper error handling and full test coverage.
1. Remove all usage of creating Cartographic instances using degrees as internal representation, Cartographics are always represented as radians. 2. Add fromDegrees helper function to aid in creating Cartographics from degrees. 3. Reaname toCartesian(s) and toCartographic(s) methods to cartesianToCartographic/cartesianArrayToCartographicArray and cartographicToCartesian and cartographicArrayToCartesianArray respectively. 4. Fix comment in Quaternion.fromAxisAngle to specify the correct unit.
…n code with new helper function.
|
This all looks good to me. Given the major API changes, I think @pjcozzi should look at it too. |
Awesome - less is more. |
Source/Core/Cartographic.js
Outdated
There was a problem hiding this comment.
Perhaps say "longitude, latitude, and height" since that is the order in the constructor.
|
@mramato Just that one comment and this is ready to go. |
|
Thanks @pjcozzi, all done. |
pjcozzi
added a commit
that referenced
this pull request
Oct 22, 2015
Fixed crash when computing sensor horizon crossings.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Unlike the recent Cartesian cleanup, Cartographic cleanup was a little more all over the place. The biggest aspect of this change is making sure we no longer construct Cartographic instances that store their internal values as degrees. Instead we always require Cartographic instances to be expressed in radians. This will clean-up the API and make using Cartographics much less error-proned. Obviously users like degrees, so we make it easy to create Cartographics from degrees, we just don't store them that way. There's still plenty of cleanup to be done on other core types that use Cartographic, like Ellipsoid, but that will be done in another pull request. This once concentrates on just Cartographic itself. That being said, I did rename some Ellipsoid methods because they were very unclear.
Cartographic3has been renamed toCartographic.Cartographic2use been removed as it is redundant, usCartographicwith height 0.0 instead.Cartographicnow has static versions of all prototype methods and can now perform operations with object literals as long as they havelongitude,latitude, andheight properties.Cartographic.fromDegreesto make it easier to create Cartographic instances from values in degrees. This simple converts the components to radians before calling the constructor.Cartographicand brought test coverage up to 100%.Ellipsoid.toCartesianwas renamed toEllipsoid.cartographicToCartesianEllipsoid.toCartesianswas renamed toEllipsoid.cartographicArrayToCartesianArrayEllipsoid.toCartographic2was renamed toEllipsoid.cartesianToCartographicEllipsoid.toCartographic2swas renamed toEllipsoid.cartesianArrayToCartographicArrayEllipsoid.toCartographic3was renamed toEllipsoid.cartesianToCartographicEllipsoid.toCartographic3swas renamed toEllipsoid.cartesianArrayToCartographicArrayEllipsoid.cartographicDegreesToCartesian,Math.cartographic3ToRadians,Math.cartographic2ToRadians,Math.cartographic2ToDegrees, andMath.cartographic3ToDegreeswere removed. These functions are no longer needed because Cartographic instances are always represented in radians.CustomSensorVolumewith calls toCartesian3.fromSpherical.EDIT: Forgot to mention that these changes are related to issues #71, #84, #86 (those issues will remain open until we address them for every Core type).