Fix longitudes outside of [-180,180] range#194
Conversation
mourner
left a comment
mourner
left a comment
There was a problem hiding this comment.
Thank you for the fix! Can you also add a minimal test for this?
| // longitude/latitude to spherical mercator in [0..1] range | ||
| function lngX(lng) { | ||
| return lng / 360 + 0.5; | ||
| return lng / 360 + 0.5 + (lng > 180 || lng < -180 ? -Math.sign(lng) : 0); |
There was a problem hiding this comment.
A slightly simpler / more reliable way to wrap the value:
| return lng / 360 + 0.5 + (lng > 180 || lng < -180 ? -Math.sign(lng) : 0); | |
| return ((lng / 360 + 0.5) % 1 + 1) % 1; |
There was a problem hiding this comment.
Thanks for the tip on the alternative math. I tested it and unfortunately, it breaks the tests.
With the suggested approach, the returned values from lngX are different from the values from the first solution at their ~14-15 decimal digit, which causes the tests to fail. So I kept the original solution for now.
I've also added a test for this change in 99c877f.
There was a problem hiding this comment.
@mourner just a quick follow-up on this PR to see if there are any other changes you'd like me to include. :)
2 participants
This is not necessarily an issue with supercluster (assuming it expects all longitudes to be within the [-180, 180] range), but it causes issues with
maplibre-glandmapbox-gl.For example, a point with coordinates [-181, 0] renders fine in a non-cluster mode in the two mentioned libraries, but it doesn't render in a clustered geojson source. Here's an example showing this issue: https://codepen.io/kaveh/pen/mdBqrYq. The red layer is for non-clustered points (shows 2 rendered points) and green is for clustered (shows only 1).
I could fix my coordinates before adding them to source in those libraries, but I thought if the same data works in non-clustered mode, it should work with clustered mode too and that's why I submitted the fix here.