[core] Moved wrapping to LatLng scope

Fixes a precision loss when converting unwrapped LatLngs.

4 files changed, 47 insertions, 21 deletions

diff --git a/include/mbgl/util/geo.hpp b/include/mbgl/util/geo.hpp
index fa36896869..5636cd0bdf 100644
--- a/include/mbgl/util/geo.hpp
+++ b/include/mbgl/util/geo.hpp
@@ -2,6 +2,7 @@
 #define MBGL_UTIL_GEO
 
 #include <mbgl/util/vec.hpp>
+#include <mbgl/util/constants.hpp>
 
 #include <cmath>
 
@@ -13,11 +14,28 @@ using ScreenCoordinate = vec2<double>;
 
 class LatLng {
 public:
-    double latitude = 0;
-    double longitude = 0;
+    double latitude;
+    double longitude;
 
-    LatLng(double lat = 0, double lon = 0)
-        : latitude(lat), longitude(lon) {}
+    enum WrapMode : bool { Unwrapped, Wrapped };
+
+    LatLng(double lat = 0, double lon = 0, WrapMode mode = Unwrapped)
+        : latitude(lat), longitude(lon) { if (mode == Wrapped) wrap(); }
+
+    LatLng wrapped() const { return { latitude, longitude, Wrapped }; }
+
+    void wrap() {
+        if (longitude < -util::LONGITUDE_MAX) longitude = std::fmod(longitude, util::LONGITUDE_MAX * 2);
+        if (longitude > util::LONGITUDE_MAX) longitude = -util::LONGITUDE_MAX + std::fmod(longitude, util::LONGITUDE_MAX);
+    }
+
+    /** If a path crossing the antemeridian would be shorter, extend the final
+      coordinate so that interpolating between the two endpoints will cross it. */
+    void unwrapForShortestPath(const LatLng& start) {
+        if (std::abs(start.longitude) + std::abs(longitude) > util::LONGITUDE_MAX) {
+            longitude += (start.longitude > 0 && longitude < 0) ? 360 : -360;
+        }
+    }
 
     explicit operator bool() const {
         return !(std::isnan(latitude) || std::isnan(longitude));
diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp
index 7e7436b1dc..d8cbfd23c2 100644
--- a/src/mbgl/map/transform.cpp
+++ b/src/mbgl/map/transform.cpp
@@ -101,7 +101,7 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim
         state.lngX(startLatLng.longitude),
         state.latY(startLatLng.latitude),
     };
-    unwrapLatLng(latLng);
+    latLng.unwrapForShortestPath(getLatLng());
     const ScreenCoordinate endPoint = {
         state.lngX(latLng.longitude),
         state.latY(latLng.latitude),
@@ -184,7 +184,7 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima
         state.lngX(startLatLng.longitude),
         state.latY(startLatLng.latitude),
     };
-    unwrapLatLng(latLng);
+    latLng.unwrapForShortestPath(getLatLng());
     const ScreenCoordinate endPoint = {
         state.lngX(latLng.longitude),
         state.latY(latLng.latitude),
@@ -329,19 +329,6 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima
     }, duration);
 }
 
-/** If a path crossing the antemeridian would be shorter, extend the final
- coordinate so that interpolating between the two endpoints will cross it. */
-void Transform::unwrapLatLng(LatLng& latLng) {
-    LatLng startLatLng = getLatLng();
-    if (std::abs(startLatLng.longitude) + std::abs(latLng.longitude) > util::LONGITUDE_MAX) {
-        if (startLatLng.longitude > 0 && latLng.longitude < 0) {
-            latLng.longitude += 360;
-        } else if (startLatLng.longitude < 0 && latLng.longitude > 0) {
-            latLng.longitude -= 360;
-        }
-    }
-}
-
 #pragma mark - Position
 
 void Transform::moveBy(const ScreenCoordinate& offset, const Duration& duration) {
diff --git a/src/mbgl/map/transform.hpp b/src/mbgl/map/transform.hpp
index d7f9e6c51a..638eead5f1 100644
--- a/src/mbgl/map/transform.hpp
+++ b/src/mbgl/map/transform.hpp
@@ -144,8 +144,6 @@ public:
     LatLng screenCoordinateToLatLng(const ScreenCoordinate&) const;
 
 private:
-    void unwrapLatLng(LatLng&);
-    
     View &view;
 
     TransformState state;
diff --git a/test/map/transform.cpp b/test/map/transform.cpp
index f606418826..15b86f3d30 100644
--- a/test/map/transform.cpp
+++ b/test/map/transform.cpp
@@ -140,6 +140,29 @@ TEST(Transform, PerspectiveProjection) {
     ASSERT_NEAR(point.y, 0, 0.02);
 }
 
+TEST(Transform, UnwrappedLatLng) {
+    MockView view;
+    Transform transform(view, ConstrainMode::HeightOnly);
+    transform.resize({{ 1000, 1000 }});
+    transform.setScale(2 << 9);
+    transform.setPitch(0.9);
+    transform.setLatLng(LatLng(38, -77));
+
+    const TransformState& state = transform.getState();
+
+    LatLng fromScreenCoordinate = state.screenCoordinateToLatLng({ 500, 500 });
+    ASSERT_NEAR(fromScreenCoordinate.latitude,   37.999999999999829, 0.0001); // 1.71E-13
+    ASSERT_NEAR(fromScreenCoordinate.longitude, -76.999999999999773, 0.0001); // 2.27E-13
+
+    LatLng unwrappedForwards = state.screenCoordinateToLatLng(state.latLngToScreenCoordinate({ 38, 283 }));
+    ASSERT_NEAR(unwrappedForwards.latitude, 37.999999999999716, 0.0001); // 2.84E-13
+    ASSERT_DOUBLE_EQ(unwrappedForwards.longitude, fromScreenCoordinate.longitude);
+
+    LatLng unwrappedBackwards = state.screenCoordinateToLatLng(state.latLngToScreenCoordinate({ 38, -437 }));
+    ASSERT_DOUBLE_EQ(unwrappedBackwards.latitude, unwrappedForwards.latitude);
+    ASSERT_DOUBLE_EQ(unwrappedBackwards.longitude, fromScreenCoordinate.longitude);
+}
+
 TEST(Transform, ConstrainHeightOnly) {
     MockView view;
     LatLng loc;