[core] Use unwrapped longitude for bounds check

2019-02-23 13:22:45 +02:00
parent 7d407175e0
commit e5ed9d9ddf
5 changed files with 212 additions and 54 deletions
--- a/include/mbgl/util/geo.hpp
+++ b/include/mbgl/util/geo.hpp
@@ -127,15 +127,7 @@ public:
                      (sw.longitude() + ne.longitude()) / 2);
    }

-    LatLng constrain(const LatLng& p) const {
-        if (contains(p)) {
-            return p;
-        }
-        return LatLng {
-            util::clamp(p.latitude(), sw.latitude(), ne.latitude()),
-            util::clamp(p.longitude(), sw.longitude(), ne.longitude())
-        };
-    }
+    LatLng constrain(const LatLng& p) const;

    void extend(const LatLng& point) {
        sw = LatLng(std::min(point.latitude(), sw.latitude()),
@@ -171,6 +163,9 @@ private:
    LatLngBounds(LatLng sw_, LatLng ne_)
        : sw(std::move(sw_)), ne(std::move(ne_)) {}

+    bool containsLatitude(double latitude) const;
+    bool containsLongitude(double longitude, LatLng::WrapMode wrap) const;
+
    friend bool operator==(const LatLngBounds& a, const LatLngBounds& b) {
        return a.sw == b.sw && a.ne == b.ne;
    }
--- a/src/mbgl/map/transform.cpp
+++ b/src/mbgl/map/transform.cpp
@@ -82,8 +82,10 @@ void Transform::jumpTo(const CameraOptions& camera) {
 * not included in `options`.
 */
 void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& animation) {
-    const LatLng unwrappedLatLng = camera.center.value_or(getLatLng());
-    const LatLng latLng = unwrappedLatLng.wrapped();
+    const EdgeInsets& padding = camera.padding;
+    LatLng startLatLng = getLatLng(padding, LatLng::Unwrapped);
+    const LatLng& unwrappedLatLng = camera.center.value_or(startLatLng);
+    const LatLng& latLng = state.bounds ? unwrappedLatLng : unwrappedLatLng.wrapped();
    double zoom = camera.zoom.value_or(getZoom());
    double angle = camera.angle ? -*camera.angle * util::DEG2RAD : getAngle();
    double pitch = camera.pitch ? *camera.pitch * util::DEG2RAD : getPitch();
@@ -92,17 +94,18 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim
        return;
    }

-    // Determine endpoints.
-    EdgeInsets padding = camera.padding;
-    LatLng startLatLng = getLatLng(padding);
-    // If gesture in progress, we transfer the world rounds from the end
-    // longitude into start, so we can guarantee the "scroll effect" of rounding
-    // the world while assuring the end longitude remains wrapped.
-    if (isGestureInProgress()) {
-        startLatLng = LatLng(startLatLng.latitude(), startLatLng.longitude() - (unwrappedLatLng.longitude() - latLng.longitude()));
+    if (!state.bounds) {
+        if (isGestureInProgress()) {
+            // If gesture in progress, we transfer the wrap rounds from the end longitude into
+            // start, so the "scroll effect" of rounding the world is the same while assuring the
+            // end longitude remains wrapped.
+            const double wrap = unwrappedLatLng.longitude() - latLng.longitude();
+            startLatLng = LatLng(startLatLng.latitude(), startLatLng.longitude() - wrap);
+        } else {
+            // Find the shortest path otherwise.
+            startLatLng.unwrapForShortestPath(latLng);
+        }
    }
-    // Find the shortest path otherwise.
-    else startLatLng.unwrapForShortestPath(latLng);

    const Point<double> startPoint = Projection::project(startLatLng, state.scale);
    const Point<double> endPoint = Projection::project(latLng, state.scale);
@@ -156,7 +159,8 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim
    Where applicable, local variable documentation begins with the associated
    variable or function in van Wijk (2003). */
 void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &animation) {
-    const LatLng latLng = camera.center.value_or(getLatLng()).wrapped();
+    const EdgeInsets& padding = camera.padding;
+    const LatLng& latLng = camera.center.value_or(getLatLng(padding, LatLng::Unwrapped)).wrapped();
    double zoom = camera.zoom.value_or(getZoom());
    double angle = camera.angle ? -*camera.angle * util::DEG2RAD : getAngle();
    double pitch = camera.pitch ? *camera.pitch * util::DEG2RAD : getPitch();
@@ -166,8 +170,7 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima
    }

    // Determine endpoints.
-    EdgeInsets padding = camera.padding;
-    LatLng startLatLng = getLatLng(padding).wrapped();
+    LatLng startLatLng = getLatLng(padding, LatLng::Unwrapped).wrapped();
    startLatLng.unwrapForShortestPath(latLng);

    const Point<double> startPoint = Projection::project(startLatLng, state.scale);
@@ -316,9 +319,9 @@ void Transform::moveBy(const ScreenCoordinate& offset, const AnimationOptions& a
    easeTo(CameraOptions().withCenter(state.screenCoordinateToLatLng(centerPoint)), animation);
 }

-LatLng Transform::getLatLng(const EdgeInsets& padding) const {
+LatLng Transform::getLatLng(const EdgeInsets& padding, LatLng::WrapMode wrap) const {
    if (padding.isFlush()) {
-        return state.getLatLng();
+        return state.getLatLng(wrap);
    } else {
        return screenCoordinateToLatLng(padding.getCenter(state.size.width, state.size.height));
    }
--- a/src/mbgl/map/transform.hpp
+++ b/src/mbgl/map/transform.hpp
@@ -46,7 +46,7 @@ public:
        @param offset The distance to pan the map by, measured in pixels from
            top to bottom and from left to right. */
    void moveBy(const ScreenCoordinate& offset, const AnimationOptions& = {});
-    LatLng getLatLng(const EdgeInsets& = {}) const;
+    LatLng getLatLng(const EdgeInsets& = {}, LatLng::WrapMode = LatLng::Wrapped) const;
    ScreenCoordinate getScreenCoordinate(const EdgeInsets& = {}) const;

    // Bounds
--- a/src/mbgl/util/geo.cpp
+++ b/src/mbgl/util/geo.cpp
@@ -39,35 +39,12 @@ bool LatLngBounds::contains(const CanonicalTileID& tileID) const {
 }

 bool LatLngBounds::contains(const LatLng& point, LatLng::WrapMode wrap /*= LatLng::Unwrapped*/) const {
-    bool containsLatitude = point.latitude() >= sw.latitude() &&
-                            point.latitude() <= ne.latitude();
-    if (!containsLatitude) {
-        return false;
-    }
-
-    bool containsUnwrappedLongitude = point.longitude() >= sw.longitude() &&
-                                      point.longitude() <= ne.longitude();
-    if (containsUnwrappedLongitude) {
-        return true;
-    } else if (wrap == LatLng::Wrapped) {
-        LatLngBounds wrapped(sw.wrapped(), ne.wrapped());
-        auto ptLon = point.wrapped().longitude();
-        if (crossesAntimeridian()) {
-            return (ptLon >= wrapped.sw.longitude() &&
-                    ptLon <= util::LONGITUDE_MAX) ||
-                   (ptLon <= wrapped.ne.longitude() &&
-                    ptLon >= -util::LONGITUDE_MAX);
-        } else {
-            return (ptLon >= wrapped.sw.longitude() &&
-                    ptLon <= wrapped.ne.longitude());
-        }
-    }
-    return false;
+    return containsLatitude(point.latitude()) && containsLongitude(point.longitude(), wrap);
 }

 bool LatLngBounds::contains(const LatLngBounds& area, LatLng::WrapMode wrap /*= LatLng::Unwrapped*/) const {
-    bool containsLatitude = area.north() <= north() && area.south() >= south();
-    if (!containsLatitude) {
+    bool containsAreaLatitude = area.north() <= north() && area.south() >= south();
+    if (!containsAreaLatitude) {
        return false;
    }

@@ -114,6 +91,44 @@ bool LatLngBounds::intersects(const LatLngBounds area, LatLng::WrapMode wrap /*=
    return false;
 }

+LatLng LatLngBounds::constrain(const LatLng& p) const {
+    double lat = p.latitude();
+    double lng = p.longitude();
+
+    if (!containsLatitude(lat)) {
+        lat = util::clamp(lat, south(), north());
+    }
+
+    if (!containsLongitude(lng, LatLng::Unwrapped)) {
+        lng = util::clamp(lng, west(), east());
+    }
+
+    return LatLng { lat, lng };
+}
+
+bool LatLngBounds::containsLatitude(double latitude) const {
+    return latitude >= south() && latitude <= north();
+}
+
+bool LatLngBounds::containsLongitude(double longitude, LatLng::WrapMode wrap) const {
+    if (longitude >= west() && longitude <= east()) {
+        return true;
+    }
+
+    if (wrap == LatLng::Wrapped) {
+        LatLngBounds wrapped(sw.wrapped(), ne.wrapped());
+        longitude = LatLng(0.0, longitude).wrapped().longitude();
+        if (crossesAntimeridian()) {
+            return (longitude >= -util::LONGITUDE_MAX && longitude <= wrapped.east()) ||
+                   (longitude >= wrapped.west() && longitude <= util::LONGITUDE_MAX);
+        }
+
+        return longitude >= wrapped.west() && longitude <= wrapped.east();
+    }
+
+    return false;
+}
+
 ScreenCoordinate EdgeInsets::getCenter(uint16_t width, uint16_t height) const {
    return {
        (width - left() - right()) / 2.0 + left(),
--- a/test/map/transform.test.cpp
+++ b/test/map/transform.test.cpp
@@ -436,7 +436,7 @@ TEST(Transform, Camera) {
    flyOptions.transitionFrameFn = [&](double t) {
        ASSERT_TRUE(t >= 0 && t <= 1);
        ASSERT_LE(latLng2.latitude(), transform.getLatLng().latitude());
-        ASSERT_GE(latLng2.longitude(), transform.getLatLng().longitude());
+        ASSERT_GE(latLng2.longitude(), transform.getLatLng({}, LatLng::Unwrapped).longitude());
    };
    flyOptions.transitionFinishFn = [&]() {
        // XXX Fix precision loss in flyTo:
@@ -586,20 +586,165 @@ TEST(Transform, LatLngBounds) {
    transform.setLatLngBounds(LatLngBounds::singleton(sanFrancisco));
    ASSERT_EQ(transform.getLatLng(), sanFrancisco);

+    //    -1   |   0   |  +1
+    // ┌───┬───┰───┬───┰───┬───┐
+    // │   │   ┃•  │   ┃   │   │
+    // ├───┼───╂───┼───╂───┼───┤
+    // │   │   ┃▓▓▓│▓▓▓┃   │   │
+    // └───┴───┸───┴───┸───┴───┘
    transform.setLatLngBounds(LatLngBounds::hull({ -90.0, -180.0 }, { 0.0, 180.0 }));
    transform.jumpTo(CameraOptions().withCenter(sanFrancisco));
    ASSERT_EQ(transform.getLatLng().latitude(), 0.0);
    ASSERT_EQ(transform.getLatLng().longitude(), sanFrancisco.longitude());

+    // Try crossing the antimeridian from the left.
+    transform.jumpTo(CameraOptions().withCenter(LatLng { 0.0, -200.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), -180.0);
+
+    // Try crossing the antimeridian from the right.
+    transform.jumpTo(CameraOptions().withCenter(LatLng { 0.0, 200.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng({}, LatLng::Unwrapped).longitude(), 180.0);
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), -180.0);
+
+    //    -1   |   0   |  +1
+    // ┌───┬───┰───┬───┰───┬───┐
+    // │   │   ┃•  │▓▓▓┃   │   │
+    // ├───┼───╂───┼───╂───┼───┤
+    // │   │   ┃   │▓▓▓┃   │   │
+    // └───┴───┸───┴───┸───┴───┘
    transform.setLatLngBounds(LatLngBounds::hull({ -90.0, 0.0 }, { 90.0, 180.0 }));
    transform.jumpTo(CameraOptions().withCenter(sanFrancisco));
    ASSERT_EQ(transform.getLatLng().latitude(), sanFrancisco.latitude());
    ASSERT_EQ(transform.getLatLng().longitude(), 0.0);

+    //    -1   |   0   |  +1
+    // ┌───┬───┰───┬───┰───┬───┐
+    // │   │   ┃•  │   ┃   │   │
+    // ├───┼───╂───┼───╂───┼───┤
+    // │   │   ┃   │▓▓▓┃   │   │
+    // └───┴───┸───┴───┸───┴───┘
    transform.setLatLngBounds(LatLngBounds::hull({ -90.0, 0.0 }, { 0.0, 180.0 }));
    transform.jumpTo(CameraOptions().withCenter(sanFrancisco));
    ASSERT_EQ(transform.getLatLng().latitude(), 0.0);
    ASSERT_EQ(transform.getLatLng().longitude(), 0.0);
+
+    //    -1   |   0   |  +1
+    // ┌───┬───┰───┬───┰───┬───┐
+    // │   │   ┃   │  ▓┃▓  │   │
+    // ├───┼───╂───┼───╂───┼───┤
+    // │   │   ┃   │   ┃   │   │
+    // └───┴───┸───┴───┸───┴───┘
+    LatLng inside { 45.0, 150.0 };
+    transform.setLatLngBounds(LatLngBounds::hull({ 0.0, 120.0 }, { 90.0, 240.0 }));
+    transform.jumpTo(CameraOptions().withCenter(inside));
+    ASSERT_EQ(transform.getLatLng().latitude(), inside.latitude());
+    ASSERT_EQ(transform.getLatLng().longitude(), inside.longitude());
+
+    transform.jumpTo(CameraOptions().withCenter(LatLng { 0.0, 140.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), 140.0);
+
+    transform.jumpTo(CameraOptions().withCenter(LatLng { 0.0, 160.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), 160.0);
+
+    // Constrain latitude only.
+    transform.jumpTo(CameraOptions().withCenter(LatLng { -45.0, inside.longitude() }));
+    ASSERT_EQ(transform.getLatLng().latitude(), 0.0);
+    ASSERT_EQ(transform.getLatLng().longitude(), inside.longitude());
+
+    // Crossing the antimeridian, within bounds.
+    transform.jumpTo(CameraOptions().withCenter(LatLng { inside.latitude(), 181.0 }));
+    ASSERT_EQ(transform.getLatLng().longitude(), -179.0);
+
+    // Crossing the antimeridian, outside bounds.
+    transform.jumpTo(CameraOptions().withCenter(inside));
+    transform.jumpTo(CameraOptions().withCenter(LatLng { inside.latitude(), 250.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), -120.0);
+
+    // Constrain to the left edge.
+    transform.jumpTo(CameraOptions().withCenter(LatLng { inside.latitude(), 119.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), 120.0);
+
+    // Simulate swipe to the left.
+    mbgl::AnimationOptions easeOptions(mbgl::Seconds(1));
+    easeOptions.transitionFrameFn = [&](double /* t */) {
+        ASSERT_NEAR(transform.getLatLng().longitude(), 120.0, 1e-4);
+    };
+    easeOptions.transitionFinishFn = [&]() {
+        ASSERT_NEAR(transform.getLatLng().longitude(), 120.0, 1e-4);
+    };
+    transform.moveBy(ScreenCoordinate { -500, -500 }, easeOptions);
+
+    transform.updateTransitions(transform.getTransitionStart() + Milliseconds(0));
+    transform.updateTransitions(transform.getTransitionStart() + Milliseconds(250));
+    transform.updateTransitions(transform.getTransitionStart() + Milliseconds(500));
+    transform.updateTransitions(transform.getTransitionStart() + Milliseconds(750));
+    transform.updateTransitions(transform.getTransitionStart() + transform.getTransitionDuration());
+
+    // Constrain to the right edge.
+    transform.jumpTo(CameraOptions().withCenter(LatLng { inside.latitude(), 241.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), -120.0);
+
+    // Simulate swipe to the right.
+    easeOptions.transitionFrameFn = [&](double /* t */) {
+        ASSERT_NEAR(transform.getLatLng().longitude(), -120.0, 1e-4);
+    };
+    easeOptions.transitionFinishFn = [&]() {
+        ASSERT_NEAR(transform.getLatLng().longitude(), -120.0, 1e-4);
+    };
+    transform.moveBy(ScreenCoordinate { 500, 500 }, easeOptions);
+
+    transform.updateTransitions(transform.getTransitionStart() + Milliseconds(0));
+    transform.updateTransitions(transform.getTransitionStart() + Milliseconds(250));
+    transform.updateTransitions(transform.getTransitionStart() + Milliseconds(500));
+    transform.updateTransitions(transform.getTransitionStart() + Milliseconds(750));
+    transform.updateTransitions(transform.getTransitionStart() + transform.getTransitionDuration());
+
+    //    -1   |   0   |  +1
+    // ┌───┬───┰───┬───┰───┬───┐
+    // │   │   ┃   │   ┃   │   │
+    // ├───┼───╂───┼───╂───┼───┤
+    // │   │  ▓┃▓  │   ┃   │   │
+    // └───┴───┸───┴───┸───┴───┘
+    inside = LatLng{ -45.0, -150.0 };
+    transform.setLatLngBounds(LatLngBounds::hull({ -90.0, -240.0 }, { 0.0, -120.0 }));
+    transform.jumpTo(CameraOptions().withCenter(inside));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().latitude(), inside.latitude());
+    ASSERT_EQ(transform.getLatLng().longitude(), inside.longitude());
+
+    transform.jumpTo(CameraOptions().withCenter(LatLng { 0.0, -140.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), -140.0);
+
+    transform.jumpTo(CameraOptions().withCenter(LatLng { 0.0, -160.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), -160.0);
+
+    // Constrain latitude only.
+    transform.jumpTo(CameraOptions().withCenter(LatLng { 45.0, inside.longitude() }));
+    ASSERT_EQ(transform.getLatLng().latitude(), 0.0);
+    ASSERT_EQ(transform.getLatLng().longitude(), inside.longitude());
+
+    // Crossing the antimeridian, within bounds.
+    transform.jumpTo(CameraOptions().withCenter(LatLng { inside.latitude(), -181.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().latitude(), inside.latitude());
+    ASSERT_EQ(transform.getLatLng().longitude(), 179.0);
+
+    // Crossing the antimeridian, outside bounds.
+    transform.jumpTo(CameraOptions().withCenter(inside));
+    transform.jumpTo(CameraOptions().withCenter(LatLng { inside.latitude(), -250.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), 120.0);
+
+    // Constrain to the left edge.
+    transform.jumpTo(CameraOptions().withCenter(LatLng { inside.latitude(), -119.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), -120.0);
+
+    transform.moveBy(ScreenCoordinate { -500, 0 });
+    ASSERT_NEAR(transform.getLatLng().longitude(), -120.0, 1e-4);
+
+    // Constrain to the right edge.
+    transform.jumpTo(CameraOptions().withCenter(LatLng { inside.latitude(), -241.0 }));
+    ASSERT_DOUBLE_EQ(transform.getLatLng().longitude(), 120.0);
+
+    transform.moveBy(ScreenCoordinate { 500, 0 });
+    ASSERT_NEAR(transform.getLatLng().longitude(), 120.0, 1e-4);
 }

 TEST(Transform, PitchBounds) {