From 026b6d4c01a3d96af9629cc1790373137ecab950 Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Mon, 7 Mar 2016 18:07:23 +0200 Subject: [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. --- CHANGELOG.md | 1 + include/mbgl/annotation/point_annotation.hpp | 5 ++--- include/mbgl/annotation/shape_annotation.hpp | 17 +++++++++++++-- include/mbgl/util/geo.hpp | 24 ++++++++++----------- src/mbgl/map/transform.cpp | 20 +++++++++++++----- src/mbgl/map/transform_state.cpp | 31 ++-------------------------- src/mbgl/map/transform_state.hpp | 4 ++-- src/mbgl/util/tile_cover.cpp | 2 +- test/map/transform.cpp | 16 ++++++++------ test/util/geo.cpp | 28 +++++++++++++++++++++++++ 10 files changed, 87 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05f3cdb4c8..d956fbc3ac 100644 --- 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 index e226673997..3dea9a3e3e 100644 --- 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 index fffa7dab6e..121cb7f1d7 100644 --- a/include/mbgl/annotation/shape_annotation.hpp +++ b/include/mbgl/annotation/shape_annotation.hpp @@ -33,11 +33,24 @@ public: 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 index 56d575ceaa..9e2d7a2bb1 100644 --- a/include/mbgl/util/geo.hpp +++ b/include/mbgl/util/geo.hpp @@ -25,19 +25,17 @@ public: 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 index 74e43a62fe..79b43f8901 100644 --- 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 index 006a8d5d3a..a23ee71f99 100644 --- 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 index 797a28692e..ef3a5dfefd 100644 --- a/src/mbgl/map/transform_state.hpp +++ b/src/mbgl/map/transform_state.hpp @@ -37,7 +37,7 @@ public: 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 @@ public: // 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 index 0fae940271..6efff4bb57 100644 --- a/src/mbgl/util/tile_cover.cpp +++ b/src/mbgl/util/tile_cover.cpp @@ -23,7 +23,7 @@ public: } 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 index 98fc1223e5..4412b78b04 100644 --- 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 index 86f448ebbc..bdf335dc47 100644 --- 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) }; -- cgit v1.2.1