-
Notifications
You must be signed in to change notification settings - Fork 951
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
Fixes 180th meridian crossing for @turf/rhumb-destination #771
Conversation
…ion; added tests;
@@ -51,5 +51,8 @@ module.exports = function (origin, distance, bearing, units) { | |||
var pt = new GeodesyLatLon(coords[1], coords[0]); | |||
var destination = pt.rhumbDestinationPoint(distanceInMeters, bearing); | |||
|
|||
// compensate the crossing of the 180th meridian (https://macwright.org/2016/09/26/the-180th-meridian.html) | |||
// solution from https://github.com/mapbox/mapbox-gl-js/issues/3250#issuecomment-294887678 | |||
destination.lon += (destination.lon - coords[0] > 180) ? -360 : (coords[0] - destination.lon > 180) ? 360 : 0; |
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.
👍 patch, but what if the input is [-720, 65]
.
I wrote a full library for this type of fix, this is "simple" but really annoying to implement.
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.
Example Point at -539
longitude
{
"type": "Feature",
"properties": {
"bearing": -90,
"dist": 100
},
"geometry": {
"type": "Point",
"coordinates": [
-539.5,
-16.5
]
}
}
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.
No we should keep the input coordinates the same, there's merit to having coordinates with +180 longitude, it's really up to the GIS application to handle those coordinates, Leaflet & MapboxGL can handle it just fine.
Yep I think we should apply this to these modules as well, this isn't a breaking change since this only affects Fiji or calculations crossing the 180th Meridian. CC: @morganherlocker @mourner Feel free to jump in if you think otherwise. |
This fix reflects @tmcw's excellent The 180th Meridian article/blog. |
Fixes #770, returning if necessary longitude greater than 180 degrees.
Please fill free to add more tests.
I'd see this as a bug fix rather than a breaking compatibility with previous version.
Modules depending on
@turf/rhumb-destination
:@turf/transform-translate
@turf/transform-rotate
@turf/transform-scale
(still under-construction)@DenisCarriere we might want to mention this behaviour to the README?
Also, I was wondering if we needed now to check if the input has longitude greater than 180 degrees, so that it's normalized between -180..180 before the computation.
Finally, do you think we should add the same correction to
@turf/destination
and other modules?cc: @dpmcmlxxvi