From 11de777628e9e2c4b219bae3f1a6eabd86f2a3c5 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Fri, 8 Mar 2019 15:53:25 +0200 Subject: [core] Remove optional from Map::setLatLngBounds() This is a first step into grouping together bounds related Map methods into one that takes a "BoundOptions" object. LatLngBounds::unbounded() replaces an undefined optional. v2: Document LatLngBounds::unbounded() --- include/mbgl/map/map.hpp | 4 ++-- include/mbgl/util/geo.hpp | 15 ++++++++++++++- platform/glfw/glfw_view.cpp | 40 +++++++++++++++++++--------------------- src/mbgl/map/map.cpp | 6 +++--- src/mbgl/map/transform.cpp | 10 +++++----- src/mbgl/map/transform.hpp | 2 +- src/mbgl/map/transform_state.cpp | 11 +++++------ src/mbgl/map/transform_state.hpp | 6 +++--- src/mbgl/util/geo.cpp | 4 ++++ test/map/transform.test.cpp | 4 ++-- 10 files changed, 58 insertions(+), 44 deletions(-) diff --git a/include/mbgl/map/map.hpp b/include/mbgl/map/map.hpp index 0c979f3e19..93e5190f99 100644 --- a/include/mbgl/map/map.hpp +++ b/include/mbgl/map/map.hpp @@ -77,8 +77,8 @@ public: LatLngBounds latLngBoundsForCamera(const CameraOptions&) const; // Bounds - void setLatLngBounds(optional); - optional getLatLngBounds() const; + void setLatLngBounds(LatLngBounds); + LatLngBounds getLatLngBounds() const; void setMinZoom(double); double getMinZoom() const; void setMaxZoom(double); diff --git a/include/mbgl/util/geo.hpp b/include/mbgl/util/geo.hpp index 0943c64d5f..a9c1112fd1 100644 --- a/include/mbgl/util/geo.hpp +++ b/include/mbgl/util/geo.hpp @@ -105,6 +105,15 @@ public: return bounds; } + /// Returns an infinite bound, a bound for which the constrain method returns its + /// input unmodified. + /// + /// Note: this is different than LatLngBounds::world() since arbitrary unwrapped + /// coordinates are also inside the bounds. + static LatLngBounds unbounded() { + return {}; + } + // Constructs a LatLngBounds object with the tile's exact boundaries. LatLngBounds(const CanonicalTileID&); @@ -159,15 +168,19 @@ public: private: LatLng sw; LatLng ne; + bool bounded = true; LatLngBounds(LatLng sw_, LatLng ne_) : sw(std::move(sw_)), ne(std::move(ne_)) {} + LatLngBounds() + : sw({-90, -180}), ne({90, 180}), bounded(false) {} + 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; + return (!a.bounded && !b.bounded) || (a.bounded && b.bounded && a.sw == b.sw && a.ne == b.ne); } friend bool operator!=(const LatLngBounds& a, const LatLngBounds& b) { diff --git a/platform/glfw/glfw_view.cpp b/platform/glfw/glfw_view.cpp index dc5e6b1ecd..86833da6ee 100644 --- a/platform/glfw/glfw_view.cpp +++ b/platform/glfw/glfw_view.cpp @@ -288,7 +288,7 @@ void GLFWView::onKey(GLFWwindow *window, int key, int /*scancode*/, int action, mbgl::LatLngBounds::hull(mbgl::LatLng { -45.0, -170.0 }, mbgl::LatLng { 45.0, 170.0 }), // inside mbgl::LatLngBounds::hull(mbgl::LatLng { -45.0, -200.0 }, mbgl::LatLng { 45.0, -160.0 }), // left IDL mbgl::LatLngBounds::hull(mbgl::LatLng { -45.0, 160.0 }, mbgl::LatLng { 45.0, 200.0 }), // right IDL - mbgl::LatLngBounds::world() + mbgl::LatLngBounds::unbounded() }; static size_t nextBound = 0u; static mbgl::AnnotationID boundAnnotationID = std::numeric_limits::max(); @@ -296,29 +296,27 @@ void GLFWView::onKey(GLFWwindow *window, int key, int /*scancode*/, int action, mbgl::LatLngBounds bound = bounds[nextBound++]; nextBound = nextBound % bounds.size(); - if (bound == mbgl::LatLngBounds::world()) { - view->map->setLatLngBounds({}); + view->map->setLatLngBounds(bound); + + if (bound == mbgl::LatLngBounds::unbounded()) { view->map->removeAnnotation(boundAnnotationID); boundAnnotationID = std::numeric_limits::max(); - break; - } else { - view->map->setLatLngBounds(bound); - } - - mbgl::Polygon rect; - rect.push_back({ - mbgl::Point{ bound.west(), bound.north() }, - mbgl::Point{ bound.east(), bound.north() }, - mbgl::Point{ bound.east(), bound.south() }, - mbgl::Point{ bound.west(), bound.south() }, - }); - - auto boundAnnotation = mbgl::FillAnnotation { rect, 0.5f, { view->makeRandomColor() }, { view->makeRandomColor() } }; - - if (boundAnnotationID == std::numeric_limits::max()) { - boundAnnotationID = view->map->addAnnotation(boundAnnotation); } else { - view->map->updateAnnotation(boundAnnotationID, boundAnnotation); + mbgl::Polygon rect; + rect.push_back({ + mbgl::Point{ bound.west(), bound.north() }, + mbgl::Point{ bound.east(), bound.north() }, + mbgl::Point{ bound.east(), bound.south() }, + mbgl::Point{ bound.west(), bound.south() }, + }); + + auto boundAnnotation = mbgl::FillAnnotation { rect, 0.5f, { view->makeRandomColor() }, { view->makeRandomColor() } }; + + if (boundAnnotationID == std::numeric_limits::max()) { + boundAnnotationID = view->map->addAnnotation(boundAnnotation); + } else { + view->map->updateAnnotation(boundAnnotationID, boundAnnotation); + } } } break; } diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index 0d2aa421f6..e45d4b85f2 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -275,13 +275,13 @@ LatLngBounds Map::latLngBoundsForCamera(const CameraOptions& camera) const { #pragma mark - Bounds -optional Map::getLatLngBounds() const { +LatLngBounds Map::getLatLngBounds() const { return impl->transform.getState().getLatLngBounds(); } -void Map::setLatLngBounds(optional bounds) { +void Map::setLatLngBounds(LatLngBounds bounds) { impl->cameraMutated = true; - impl->transform.setLatLngBounds(bounds); + impl->transform.setLatLngBounds(std::move(bounds)); impl->onUpdate(); } diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp index a90bf9871f..e97f0da3f1 100644 --- a/src/mbgl/map/transform.cpp +++ b/src/mbgl/map/transform.cpp @@ -85,7 +85,7 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim 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(); + const LatLng& latLng = state.bounds != LatLngBounds::unbounded() ? unwrappedLatLng : unwrappedLatLng.wrapped(); double zoom = camera.zoom.value_or(getZoom()); double bearing = camera.bearing ? -*camera.bearing * util::DEG2RAD : getBearing(); double pitch = camera.pitch ? *camera.pitch * util::DEG2RAD : getPitch(); @@ -94,7 +94,7 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim return; } - if (!state.bounds) { + if (state.bounds == LatLngBounds::unbounded()) { 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 @@ -344,11 +344,11 @@ double Transform::getZoom() const { #pragma mark - Bounds -void Transform::setLatLngBounds(optional bounds) { - if (bounds && !bounds->valid()) { +void Transform::setLatLngBounds(LatLngBounds bounds) { + if (!bounds.valid()) { throw std::runtime_error("failed to set bounds: bounds are invalid"); } - state.setLatLngBounds(bounds); + state.setLatLngBounds(std::move(bounds)); } void Transform::setMinZoom(const double minZoom) { diff --git a/src/mbgl/map/transform.hpp b/src/mbgl/map/transform.hpp index 13750c2ec0..29ca9bd14e 100644 --- a/src/mbgl/map/transform.hpp +++ b/src/mbgl/map/transform.hpp @@ -52,7 +52,7 @@ public: // Bounds - void setLatLngBounds(optional); + void setLatLngBounds(LatLngBounds); void setMinZoom(double); void setMaxZoom(double); diff --git a/src/mbgl/map/transform_state.cpp b/src/mbgl/map/transform_state.cpp index 73c61fdf0f..45bee7ac7f 100644 --- a/src/mbgl/map/transform_state.cpp +++ b/src/mbgl/map/transform_state.cpp @@ -9,7 +9,8 @@ namespace mbgl { TransformState::TransformState(ConstrainMode constrainMode_, ViewportMode viewportMode_) - : constrainMode(constrainMode_) + : bounds(LatLngBounds::unbounded()) + , constrainMode(constrainMode_) , viewportMode(viewportMode_) { } @@ -185,14 +186,14 @@ double TransformState::getZoomFraction() const { #pragma mark - Bounds -void TransformState::setLatLngBounds(optional bounds_) { +void TransformState::setLatLngBounds(LatLngBounds bounds_) { if (bounds_ != bounds) { bounds = bounds_; setLatLngZoom(getLatLng(LatLng::Unwrapped), getZoom()); } } -optional TransformState::getLatLngBounds() const { +LatLngBounds TransformState::getLatLngBounds() const { return bounds; } @@ -379,9 +380,7 @@ void TransformState::moveLatLng(const LatLng& latLng, const ScreenCoordinate& an void TransformState::setLatLngZoom(const LatLng& latLng, double zoom) { LatLng constrained = latLng; - if (bounds) { - constrained = bounds->constrain(latLng); - } + constrained = bounds.constrain(latLng); double newScale = util::clamp(zoomScale(zoom), min_scale, max_scale); const double newWorldSize = newScale * util::tileSize; diff --git a/src/mbgl/map/transform_state.hpp b/src/mbgl/map/transform_state.hpp index 44eb1a8a3e..811fa253af 100644 --- a/src/mbgl/map/transform_state.hpp +++ b/src/mbgl/map/transform_state.hpp @@ -55,8 +55,8 @@ public: double getZoomFraction() const; // Bounds - void setLatLngBounds(optional); - optional getLatLngBounds() const; + void setLatLngBounds(LatLngBounds); + LatLngBounds getLatLngBounds() const; void setMinZoom(double); double getMinZoom() const; void setMaxZoom(double); @@ -93,7 +93,7 @@ private: bool rotatedNorth() const; void constrain(double& scale, double& x, double& y) const; - optional bounds; + LatLngBounds bounds; // Limit the amount of zooming possible on the map. double min_scale = std::pow(2, 0); diff --git a/src/mbgl/util/geo.cpp b/src/mbgl/util/geo.cpp index 19510e2039..1106779474 100644 --- a/src/mbgl/util/geo.cpp +++ b/src/mbgl/util/geo.cpp @@ -92,6 +92,10 @@ bool LatLngBounds::intersects(const LatLngBounds area, LatLng::WrapMode wrap /*= } LatLng LatLngBounds::constrain(const LatLng& p) const { + if (!bounded) { + return p; + } + double lat = p.latitude(); double lng = p.longitude(); diff --git a/test/map/transform.test.cpp b/test/map/transform.test.cpp index 441794886d..33d325dfb2 100644 --- a/test/map/transform.test.cpp +++ b/test/map/transform.test.cpp @@ -583,7 +583,7 @@ TEST(Transform, LatLngBounds) { transform.jumpTo(CameraOptions().withCenter(LatLng()).withZoom(transform.getState().getMaxZoom())); // Default bounds. - ASSERT_EQ(transform.getState().getLatLngBounds(), optional {}); + ASSERT_EQ(transform.getState().getLatLngBounds(), LatLngBounds::unbounded()); ASSERT_EQ(transform.getLatLng(), nullIsland); // Invalid bounds. @@ -591,7 +591,7 @@ TEST(Transform, LatLngBounds) { transform.setLatLngBounds(LatLngBounds::empty()); ASSERT_TRUE(false) << "Should throw"; } catch (...) { - ASSERT_EQ(transform.getState().getLatLngBounds(), optional {}); + ASSERT_EQ(transform.getState().getLatLngBounds(), LatLngBounds::unbounded()); } transform.jumpTo(CameraOptions().withCenter(sanFrancisco)); -- cgit v1.2.1