toMercator changes polygons geometry

[Darune]

When using toMercator on a polygon that overlap the -180 or 180 longitude, without antimeridian cutting.

The current implementation wraps every point one by one which ends up returning a wrong polygon:

Here is a polygon overlapping the -180 longitude:

• https://gist.github.com/Darune/d73952fbe586d1e8116d6e779042ccb3
  Here is the result of a toMercator transformation reconverted using toWgs84:
• https://gist.github.com/Darune/dbd604f5b697cbbd36b918acab2dea94

code to reproduce the issue:

import { toWgs84, toMercator } from '@turf/projection';

toWgs84(toMercator({
  "type": "Feature",
  "properties": {}, 
  "geometry": {
    "type": "Polygon",
    "coordinates": [[
        [-181, 60],
        [-181, 61],
        [-179, 61],
        [-179, 60],
        [-181, 60]
    ]]
  }
}));

I think that when calling toMercator with a GeoJson, turf should not try to wrap longitude as it does.
if this is an acceptable behavior I could submit a PR

[stebogit]

@Darune you are right, also the fiji test, which should cover this case, does actually return a funky output that for some reason is considered OK.
Honestly I don't remember what happened here and why the issue was never really fixed properly. You are welcome to submit a PR anytime 😃

I think it would be useful also to add a set of tests like what you posted, where the double conversion should coincide with the original feature; something like:

t.deepEqual(feature, toWgs84(toMercator(feature)));

[Darune]

Great,

I didn't have time to handle this today, i'll do this on monday

[Darune]

Any specific reason to use proj4js for validation ?
Seems that the js version doesn't support the +over flag that is required to disable lon_wrap flag.

[stebogit]

@Darune I have no idea what you're talking about... 😅
I did not choose proj4 for any particular reason, however; at least none that I can remember.

[Darune]

The tests for toMercator https://github.com/Turfjs/turf/blob/master/packages/turf-projection/test.js#L31

Clearly use proj4 to validate the function output.
i'm free to change those right ? as the fidji.geojson transformed by proj4 is wrong aswell

[stebogit]

I have never used Mercator coordinates nor I know how to represent them directly on a map to tell if the output is right or not (i.e. if you have any suggestions or know any tools to do that I'd be glad to know 😄)

As long as it works I don't think what tool you use for validation matters. However I'm not the best person to give advice on this.
@morganherlocker @rowanwins ?

[morganherlocker]

I am not familiar with the @turf/projection source, but the toMercator function lives here if you want to debug the math: https://github.com/Turfjs/turf/blob/master/packages/turf-projection/index.ts#L87-L108

[rowanwins]

Yeah unfortunately the tests and the code are currently setup to run against individual coordinates rather than seeing them in context of a geometry that is crossing the anti-median, so a bit of thought needs to happen to get things behaving and tested properly.

[Darune]

I could retry writing tests for this, it has been stalled for way too long, but I might have time next week to digg this out of the dirt