From b5bb48edaa0a553a4b91c3632148bf86ac9e2bf6 Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Sun, 13 Mar 2016 19:05:15 +0200 Subject: [PATCH] [core] Simplify LatLng::unwrapForShortestPath usage Simplify LatLng::{wrap,unwrapForShortestPath} code, avoiding duplicated code between Transform::{latLngToScreenCoordinate,easeTo,flyTo}. Added unit tests for camera usage in Transform to detect cases like e.g. crossing the antimeridian as a shortest path between two coordinates. Transform::flyTo precision loss to be handled in #4298. --- include/mbgl/util/geo.hpp | 17 +++++---- src/mbgl/map/transform.cpp | 60 +++++++++++-------------------- src/mbgl/map/transform.hpp | 2 ++ test/map/transform.cpp | 72 ++++++++++++++++++++++++++++++++++++-- 4 files changed, 100 insertions(+), 51 deletions(-) diff --git a/include/mbgl/util/geo.hpp b/include/mbgl/util/geo.hpp index 0b6ee1750..84128caae 100644 --- a/include/mbgl/util/geo.hpp +++ b/include/mbgl/util/geo.hpp @@ -1,6 +1,7 @@ #ifndef MBGL_UTIL_GEO #define MBGL_UTIL_GEO +#include #include #include @@ -29,18 +30,16 @@ public: LatLng wrapped() const { return { latitude, longitude, Wrapped }; } void wrap() { - 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); + longitude = util::wrap(longitude, -util::LONGITUDE_MAX, util::LONGITUDE_MAX); } - // If we pass through the antimeridian, we update the start coordinate to make sure - // the end coordinate is always wrapped. + // If the distance from start to end longitudes is between half and full + // world, unwrap the start longitude to ensure the shortest path is taken. 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; - } + const double delta = std::abs(end.longitude - longitude); + if (delta < util::LONGITUDE_MAX || delta > util::DEGREES_MAX) return; + if (longitude > 0 && end.longitude < 0) longitude -= util::DEGREES_MAX; + else if (longitude < 0 && end.longitude > 0) longitude += util::DEGREES_MAX; } explicit operator bool() const { diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp index 26245c86e..29fa7c9eb 100644 --- a/src/mbgl/map/transform.cpp +++ b/src/mbgl/map/transform.cpp @@ -77,27 +77,21 @@ void Transform::jumpTo(const CameraOptions& camera) { * not included in `options`. */ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& animation) { - LatLng latLng = camera.center ? *camera.center : getLatLng(); - double zoom = camera.zoom ? *camera.zoom : getZoom(); - double angle = camera.angle ? *camera.angle : getAngle(); - double pitch = camera.pitch ? *camera.pitch : getPitch(); - + const LatLng latLng = camera.center.value_or(getLatLng()).wrapped(); + double zoom = camera.zoom.value_or(getZoom()); + double angle = camera.angle.value_or(getAngle()); + double pitch = camera.pitch.value_or(getPitch()); + if (!latLng || std::isnan(zoom)) { return; } - + // Determine endpoints. EdgeInsets padding; - if (camera.padding) { - padding = *camera.padding; - } - - LatLng startLatLng = getLatLng(padding); + if (camera.padding) padding = *camera.padding; + LatLng startLatLng = getLatLng(padding).wrapped(); startLatLng.unwrapForShortestPath(latLng); - // Make sure the end coordinate always remains valid. - latLng.wrap(); - const ScreenCoordinate startPoint = { state.lngX(startLatLng.longitude), state.latY(startLatLng.latitude), @@ -165,10 +159,10 @@ 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) { - LatLng latLng = camera.center ? *camera.center : getLatLng(); - double zoom = camera.zoom ? *camera.zoom : getZoom(); - double angle = camera.angle ? *camera.angle : getAngle(); - double pitch = camera.pitch ? *camera.pitch : getPitch(); + const LatLng latLng = camera.center.value_or(getLatLng()).wrapped(); + double zoom = camera.zoom.value_or(getZoom()); + double angle = camera.angle.value_or(getAngle()); + double pitch = camera.pitch.value_or(getPitch()); if (!latLng || std::isnan(zoom)) { return; @@ -176,16 +170,10 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima // Determine endpoints. EdgeInsets padding; - if (camera.padding) { - padding = *camera.padding; - } - - LatLng startLatLng = getLatLng(padding); + if (camera.padding) padding = *camera.padding; + LatLng startLatLng = getLatLng(padding).wrapped(); startLatLng.unwrapForShortestPath(latLng); - // Make sure the end coordinate always remains valid. - latLng.wrap(); - const ScreenCoordinate startPoint = { state.lngX(startLatLng.longitude), state.latY(startLatLng.latitude), @@ -653,20 +641,12 @@ void Transform::setGestureInProgress(bool inProgress) { ScreenCoordinate Transform::latLngToScreenCoordinate(const LatLng& latLng) const { if (!latLng) return ScreenCoordinate::null(); - // If the center and point coordinates are not in the same side of the - // antimeridian, we need to unwrap the point longitude to make sure it can - // still be seen from the visible side of the antimeridian that is opposite - // to the center side. - double longitude = latLng.longitude; - const double centerLng = getLatLng().longitude; - if (std::abs(centerLng - latLng.longitude) > std::abs(util::LONGITUDE_MAX)) { - if (centerLng > 0 && latLng.longitude < 0) { - longitude += util::DEGREES_MAX; - } else if (centerLng < 0 && latLng.longitude > 0) { - longitude -= util::DEGREES_MAX; - } - } - ScreenCoordinate point = state.latLngToScreenCoordinate({ latLng.latitude, longitude }); + // If the center and point longitudes are not in the same side of the + // antimeridian, we unwrap the point longitude so it would be seen if + // e.g. the next antimeridian side is visible. + LatLng unwrappedLatLng = latLng.wrapped(); + unwrappedLatLng.unwrapForShortestPath(getLatLng()); + ScreenCoordinate point = state.latLngToScreenCoordinate(unwrappedLatLng); point.y = state.height - point.y; return point; } diff --git a/src/mbgl/map/transform.hpp b/src/mbgl/map/transform.hpp index 1e84b013c..980a19829 100644 --- a/src/mbgl/map/transform.hpp +++ b/src/mbgl/map/transform.hpp @@ -138,6 +138,8 @@ public: // Transitions bool inTransition() const; Update updateTransitions(const TimePoint& now); + TimePoint getTransitionStart() const { return transitionStart; } + Duration getTransitionDuration() const { return transitionDuration; } void cancelTransitions(); // Gesture diff --git a/test/map/transform.cpp b/test/map/transform.cpp index d07a8d388..d26df51fd 100644 --- a/test/map/transform.cpp +++ b/test/map/transform.cpp @@ -291,8 +291,8 @@ TEST(Transform, Anchor) { ASSERT_DOUBLE_EQ(latLng.latitude, transform.getLatLng().latitude); ASSERT_DOUBLE_EQ(latLng.longitude, transform.getLatLng().longitude); - transform.setAngle(-45 * util::DEG2RAD, anchorPoint); - ASSERT_NEAR(-45 / util::RAD2DEG, transform.getAngle(), 0.000001); + transform.setAngle(45 * util::DEG2RAD, anchorPoint); + ASSERT_NEAR(45 / util::RAD2DEG, transform.getAngle(), 0.000001); ASSERT_NEAR(anchorLatLng.latitude, transform.getLatLng().latitude, 1); ASSERT_NEAR(anchorLatLng.longitude, transform.getLatLng().longitude, 1); @@ -416,3 +416,71 @@ TEST(Transform, Antimeridian) { ASSERT_DOUBLE_EQ(pixelWaikiriForwards.x, pixelWaikiriBackwards.x); ASSERT_DOUBLE_EQ(pixelWaikiriForwards.y, pixelWaikiriBackwards.y); } + +TEST(Transform, Camera) { + MockView view; + Transform transform(view, ConstrainMode::HeightOnly); + transform.resize({{ 1000, 1000 }}); + + const LatLng latLng1 { 45, 135 }; + CameraOptions cameraOptions1; + cameraOptions1.zoom = 20; + cameraOptions1.center = latLng1; + transform.jumpTo(cameraOptions1); + + ASSERT_DOUBLE_EQ(latLng1.latitude, transform.getLatLng().latitude); + ASSERT_DOUBLE_EQ(latLng1.longitude, transform.getLatLng().longitude); + ASSERT_DOUBLE_EQ(20, transform.getZoom()); + + const LatLng latLng2 { -45, -135 }; + CameraOptions cameraOptions2; + cameraOptions2.zoom = 10; + cameraOptions2.center = latLng2; + + transform.jumpTo(cameraOptions2); + ASSERT_DOUBLE_EQ(latLng2.latitude, transform.getLatLng().latitude); + ASSERT_DOUBLE_EQ(latLng2.longitude, transform.getLatLng().longitude); + ASSERT_DOUBLE_EQ(10, transform.getZoom()); + + AnimationOptions easeOptions(Seconds(1)); + easeOptions.transitionFrameFn = [&](double t) { + ASSERT_TRUE(t >= 0 && t <= 1); + ASSERT_GE(latLng1.latitude, transform.getLatLng().latitude); + ASSERT_LE(latLng1.longitude, transform.getLatLng().longitude); + }; + easeOptions.transitionFinishFn = [&]() { + ASSERT_NEAR(latLng1.latitude, transform.getLatLng().latitude, 0.000001); + ASSERT_NEAR(latLng1.longitude, transform.getLatLng().longitude, 0.000001); + ASSERT_NEAR(20, transform.getZoom(), 0.000001); + }; + + transform.easeTo(cameraOptions1, easeOptions); + ASSERT_TRUE(transform.inTransition()); + transform.updateTransitions(transform.getTransitionStart() + Milliseconds(250)); + transform.updateTransitions(transform.getTransitionStart() + Milliseconds(500)); + transform.updateTransitions(transform.getTransitionStart() + Milliseconds(750)); + transform.updateTransitions(transform.getTransitionStart() + transform.getTransitionDuration()); + ASSERT_FALSE(transform.inTransition()); + + AnimationOptions flyOptions(Seconds(1)); + flyOptions.transitionFrameFn = [&](double t) { + ASSERT_TRUE(t >= 0 && t <= 1); + ASSERT_LE(latLng2.latitude, transform.getLatLng().latitude); + ASSERT_GE(latLng2.longitude, transform.getLatLng().longitude); + }; + flyOptions.transitionFinishFn = [&]() { + // XXX Fix precision loss in flyTo: + // https://github.com/mapbox/mapbox-gl-native/issues/4298 + ASSERT_NEAR(latLng2.latitude, transform.getLatLng().latitude, 0.001); + ASSERT_NEAR(latLng2.longitude, transform.getLatLng().longitude, 0.001); + ASSERT_NEAR(10, transform.getZoom(), 0.00001); + }; + + transform.flyTo(cameraOptions2, flyOptions); + ASSERT_TRUE(transform.inTransition()); + transform.updateTransitions(transform.getTransitionStart() + Milliseconds(250)); + transform.updateTransitions(transform.getTransitionStart() + Milliseconds(500)); + transform.updateTransitions(transform.getTransitionStart() + Milliseconds(750)); + transform.updateTransitions(transform.getTransitionStart() + transform.getTransitionDuration()); + ASSERT_FALSE(transform.inTransition()); +}