[core] Coordinate wrapping fixes

- Make returning LatLngs unwrapped by default. - PointAnnotation and ShapeAnnotation are always wrapped so they can be selected via intersection from the visible tile boundaries. - Fixes LatLng::wrap() calculation. - Fixes LatLng::unwrapForShortestPath() calculation. The new unwrapForShortestPath algorithm unwraps the start coordinate either forwards or backwards depending on the end coordinate value, so we can always cross the antimeridian when needed and still obtain a wrapped end coordinate in the end. Fixes #4214.
mapbox · Mar 10, 2016 · 026b6d4 · 026b6d4
1 parent 808bf70
commit 026b6d4
Show file tree

Hide file tree

Showing 10 changed files with 87 additions and 61 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,6 +51,7 @@ Known issues:
 
 ## iOS master
 
+- Fixed a bounce-back effect when panning the map. ([#4214](https://github.com/mapbox/mapbox-gl-native/pull/4214))
 - An icon laid out along a line no longer appears if it would extend past the end of the line. Some one-way arrows no longer point the wrong way. ([#3839](https://github.com/mapbox/mapbox-gl-native/pull/3839))
 - Reduce slanted segments in dashed lines near corners. ([#3914](https://github.com/mapbox/mapbox-gl-native/pull/3914))
 - Telemetry location gathering now only occurs when the device is in motion. ([#4115](https://github.com/mapbox/mapbox-gl-native/pull/4115))

diff --git a/include/mbgl/annotation/point_annotation.hpp b/include/mbgl/annotation/point_annotation.hpp
@@ -9,9 +9,8 @@ namespace mbgl {
 
 class PointAnnotation {
 public:
-    inline PointAnnotation(const LatLng& position_, const std::string& icon_ = "")
-        : position(position_), icon(icon_) {
-    }
+    PointAnnotation(const LatLng& position_, const std::string& icon_ = "")
+        : position(position_.wrapped()), icon(icon_) {}
 
     const LatLng position;
     const std::string icon;

diff --git a/include/mbgl/annotation/shape_annotation.hpp b/include/mbgl/annotation/shape_annotation.hpp
@@ -33,11 +33,24 @@ class ShapeAnnotation {
         std::string>; // creates an annotation whose type and properties are sourced from a style layer
 
     ShapeAnnotation(const AnnotationSegments& segments_, const Properties& properties_)
-        : segments(segments_), properties(properties_) {
-    }
+        : segments(wrapCoordinates(segments_)), properties(properties_) {}
 
     const AnnotationSegments segments;
     const Properties properties;
+
+private:
+    AnnotationSegments wrapCoordinates(const AnnotationSegments& segments_) {
+        AnnotationSegments wrappedSegments;
+        // Wrap all segments coordinates.
+        for (const auto& segment_ : segments_) {
+            AnnotationSegment wrappedSegment;
+            for (const auto& latLng_ : segment_) {
+                wrappedSegment.push_back(latLng_.wrapped());
+            }
+            wrappedSegments.push_back(wrappedSegment);
+        }
+        return wrappedSegments;
+    }
 };
 
 } // namespace mbgl

diff --git a/include/mbgl/util/geo.hpp b/include/mbgl/util/geo.hpp
@@ -25,19 +25,17 @@ class LatLng {
     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) {
-            if (start.longitude > 0 && longitude < 0) {
-                longitude += util::DEGREES_MAX;
-            } else if (start.longitude < 0 && longitude > 0) {
-                longitude -= util::DEGREES_MAX;
-            }
+        if (longitude < -util::LONGITUDE_MAX) longitude = util::LONGITUDE_MAX + std::fmod(longitude + util::LONGITUDE_MAX, util::DEGREES_MAX);
+        if (longitude > util::LONGITUDE_MAX) longitude = -util::LONGITUDE_MAX + std::fmod(longitude + util::LONGITUDE_MAX, util::DEGREES_MAX);
+    }
+
+    // If we pass through the antimeridian, we update the start coordinate to make sure
+    // the end coordinate is always wrapped.
+    void unwrapForShortestPath(const LatLng& end) {
+        if (end.longitude < -util::LONGITUDE_MAX) {
+            longitude += util::DEGREES_MAX;
+        } else if (end.longitude > util::LONGITUDE_MAX) {
+            longitude -= util::DEGREES_MAX;
         }
     }
 

diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp
@@ -95,12 +95,17 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim
     if (camera.padding) {
         padding = *camera.padding;
     }
-    const LatLng startLatLng = getLatLng(padding);
+
+    LatLng startLatLng = getLatLng(padding);
+    startLatLng.unwrapForShortestPath(latLng);
+
+    // Make sure the end coordinate always remains valid.
+    latLng.wrap();
+
     const ScreenCoordinate startPoint = {
         state.lngX(startLatLng.longitude),
         state.latY(startLatLng.latitude),
     };
-    latLng.unwrapForShortestPath(getLatLng());
     const ScreenCoordinate endPoint = {
         state.lngX(latLng.longitude),
         state.latY(latLng.latitude),
@@ -136,7 +141,7 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim
         ScreenCoordinate framePoint = util::interpolate(startPoint, endPoint, t);
         LatLng frameLatLng = {
             state.yLat(framePoint.y, startWorldSize),
-            state.xLng(framePoint.x, startWorldSize),
+            state.xLng(framePoint.x, startWorldSize)
         };
         double frameScale = util::interpolate(startScale, scale, t);
         state.setLatLngZoom(frameLatLng, state.scaleZoom(frameScale));
@@ -178,12 +183,17 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima
     if (camera.padding) {
         padding = *camera.padding;
     }
-    const LatLng startLatLng = getLatLng(padding);
+
+    LatLng startLatLng = getLatLng(padding);
+    startLatLng.unwrapForShortestPath(latLng);
+
+    // Make sure the end coordinate always remains valid.
+    latLng.wrap();
+
     const ScreenCoordinate startPoint = {
         state.lngX(startLatLng.longitude),
         state.latY(startLatLng.latitude),
     };
-    latLng.unwrapForShortestPath(getLatLng());
     const ScreenCoordinate endPoint = {
         state.lngX(latLng.longitude),
         state.latY(latLng.latitude),

diff --git a/src/mbgl/map/transform_state.cpp b/src/mbgl/map/transform_state.cpp
@@ -89,38 +89,11 @@ ConstrainMode TransformState::getConstrainMode() const {
 #pragma mark - Position
 
 LatLng TransformState::getLatLng(LatLng::WrapMode wrapMode) const {
-    LatLng ll {
+    return {
         util::RAD2DEG * (2 * std::atan(std::exp(y / Cc)) - 0.5 * M_PI),
         -x / Bc,
         wrapMode
     };
-
-    if (wrapMode == LatLng::Unwrapped) return ll;
-
-    // adjust for date line
-    double w = worldSize() / 2;
-    double x_ = x;
-    if (x_ > w) {
-        while (x_ > w) {
-            x_ -= w;
-            if (ll.longitude < 0) {
-                ll.longitude += util::LONGITUDE_MAX;
-            } else if (ll.longitude > 0) {
-                ll.longitude -= util::LONGITUDE_MAX;
-            }
-        }
-    } else if (x_ < -w) {
-        while (x_ < -w) {
-            x_ += w;
-            if (ll.longitude < 0) {
-                ll.longitude -= util::LONGITUDE_MAX;
-            } else if (ll.longitude > 0) {
-                ll.longitude -= util::LONGITUDE_MAX;
-            }
-        }
-    }
-
-    return ll;
 }
 
 double TransformState::pixel_x() const {
@@ -350,7 +323,7 @@ void TransformState::moveLatLng(const LatLng& latLng, const ScreenCoordinate& an
 
     auto centerCoord = latLngToTileCoord(getLatLng(LatLng::Unwrapped));
     auto latLngCoord = latLngToTileCoord(latLng);
-    auto anchorCoord = latLngToTileCoord(screenCoordinateToLatLng(anchor, LatLng::Unwrapped));
+    auto anchorCoord = latLngToTileCoord(screenCoordinateToLatLng(anchor));
     setLatLngZoom(tileCoordToLatLng(centerCoord + latLngCoord - anchorCoord), getZoom());
 }
 

diff --git a/src/mbgl/map/transform_state.hpp b/src/mbgl/map/transform_state.hpp
@@ -37,7 +37,7 @@ class TransformState {
     ConstrainMode getConstrainMode() const;
 
     // Position
-    LatLng getLatLng(LatLng::WrapMode = LatLng::Wrapped) const;
+    LatLng getLatLng(LatLng::WrapMode = LatLng::Unwrapped) const;
     double pixel_x() const;
     double pixel_y() const;
 
@@ -65,7 +65,7 @@ class TransformState {
 
     // Conversion and projection
     ScreenCoordinate latLngToScreenCoordinate(const LatLng&) const;
-    LatLng screenCoordinateToLatLng(const ScreenCoordinate&, LatLng::WrapMode = LatLng::Wrapped) const;
+    LatLng screenCoordinateToLatLng(const ScreenCoordinate&, LatLng::WrapMode = LatLng::Unwrapped) const;
 
     double xLng(double x, double worldSize) const;
     double yLat(double y, double worldSize) const;

diff --git a/src/mbgl/util/tile_cover.cpp b/src/mbgl/util/tile_cover.cpp
@@ -23,7 +23,7 @@ class TileCoordinate {
     }
 
     static TileCoordinate fromScreenCoordinate(const TransformState& state, double zoom, const ScreenCoordinate& point) {
-        return fromLatLng(state, zoom, state.screenCoordinateToLatLng(point, LatLng::Unwrapped));
+        return fromLatLng(state, zoom, state.screenCoordinateToLatLng(point));
     }
 };
 

diff --git a/test/map/transform.cpp b/test/map/transform.cpp
@@ -158,13 +158,17 @@ TEST(Transform, UnwrappedLatLng) {
     ASSERT_NEAR(fromScreenCoordinate.latitude,   37.999999999999829, 0.0001); // 1.71E-13
     ASSERT_NEAR(fromScreenCoordinate.longitude, -76.999999999999773, 0.0001); // 2.27E-13
 
-    LatLng wrappedForwards = state.screenCoordinateToLatLng(state.latLngToScreenCoordinate({ 38, 283, LatLng::Wrapped }));
+    LatLng wrappedForwards = state.screenCoordinateToLatLng(state.latLngToScreenCoordinate({ 38, 283 }));
     ASSERT_NEAR(wrappedForwards.latitude, 37.999999999999716, 0.0001); // 2.84E-13
-    ASSERT_DOUBLE_EQ(wrappedForwards.longitude, fromScreenCoordinate.longitude);
-
-    LatLng wrappedBackwards = state.screenCoordinateToLatLng(state.latLngToScreenCoordinate({ 38, -437, LatLng::Wrapped }));
-    ASSERT_DOUBLE_EQ(wrappedBackwards.latitude, wrappedForwards.latitude);
-    ASSERT_DOUBLE_EQ(wrappedBackwards.longitude, fromScreenCoordinate.longitude);
+    ASSERT_NEAR(wrappedForwards.longitude, 282.99999999988751, 0.0001); // 1.1249E-11
+    wrappedForwards.wrap();
+    ASSERT_NEAR(wrappedForwards.longitude, -77.000000000112493, 0.001); // 1.1249E-11
+
+    LatLng wrappedBackwards = state.screenCoordinateToLatLng(state.latLngToScreenCoordinate({ 38, -437 }));
+    ASSERT_NEAR(wrappedBackwards.latitude, wrappedForwards.latitude, 0.001);
+    ASSERT_NEAR(wrappedBackwards.longitude, -436.99999999988728, 0.001); // 1.1272E-11
+    wrappedBackwards.wrap();
+    ASSERT_NEAR(wrappedBackwards.longitude, -76.99999999988728, 0.001); // 1.1272E-11
 }
 
 TEST(Transform, ConstrainHeightOnly) {

diff --git a/test/util/geo.cpp b/test/util/geo.cpp
@@ -105,6 +105,34 @@ TEST(LatLng, FromTileID) {
     }
 }
 
+TEST(LatLng, Boundaries) {
+    LatLng coordinate;
+    ASSERT_DOUBLE_EQ(0, coordinate.latitude);
+    ASSERT_DOUBLE_EQ(0, coordinate.longitude);
+
+    coordinate.longitude = -180.1;
+    ASSERT_DOUBLE_EQ(-180.1, coordinate.longitude);
+
+    coordinate.wrap();
+    ASSERT_DOUBLE_EQ(179.90000000000001, coordinate.longitude); // 1E-14
+
+    coordinate.longitude = 180.9;
+    coordinate.wrap();
+    ASSERT_DOUBLE_EQ(-179.09999999999999, coordinate.longitude);
+
+    coordinate.longitude = -360.5;
+    coordinate.wrap();
+    ASSERT_DOUBLE_EQ(-0.5, coordinate.longitude);
+
+    coordinate.longitude = 360.5;
+    coordinate.wrap();
+    ASSERT_DOUBLE_EQ(0.5, coordinate.longitude);
+
+    coordinate.longitude = 360000.5;
+    coordinate.wrap();
+    ASSERT_DOUBLE_EQ(0.5, coordinate.longitude);
+}
+
 TEST(LatLngBounds, FromTileID) {
     {
         const LatLngBounds bounds{ TileID(0, 0, 0, 0) };