[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<LatLngBounds>.

v2: Document LatLngBounds::unbounded()

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<LatLngBounds>);
-    optional<LatLngBounds> 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<mbgl::AnnotationID>::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<mbgl::AnnotationID>::max();
-                break;
-            } else {
-                view->map->setLatLngBounds(bound);
-            }
-
-            mbgl::Polygon<double> rect;
-            rect.push_back({
-                mbgl::Point<double>{ bound.west(), bound.north() },
-                mbgl::Point<double>{ bound.east(), bound.north() },
-                mbgl::Point<double>{ bound.east(), bound.south() },
-                mbgl::Point<double>{ bound.west(), bound.south() },
-            });
-
-            auto boundAnnotation = mbgl::FillAnnotation { rect, 0.5f, { view->makeRandomColor() }, { view->makeRandomColor() } };
-
-            if (boundAnnotationID == std::numeric_limits<mbgl::AnnotationID>::max()) {
-                boundAnnotationID = view->map->addAnnotation(boundAnnotation);
             } else {
-                view->map->updateAnnotation(boundAnnotationID, boundAnnotation);
+                mbgl::Polygon<double> rect;
+                rect.push_back({
+                    mbgl::Point<double>{ bound.west(), bound.north() },
+                    mbgl::Point<double>{ bound.east(), bound.north() },
+                    mbgl::Point<double>{ bound.east(), bound.south() },
+                    mbgl::Point<double>{ bound.west(), bound.south() },
+                });
+
+                auto boundAnnotation = mbgl::FillAnnotation { rect, 0.5f, { view->makeRandomColor() }, { view->makeRandomColor() } };
+
+                if (boundAnnotationID == std::numeric_limits<mbgl::AnnotationID>::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<LatLngBounds> Map::getLatLngBounds() const {
+LatLngBounds Map::getLatLngBounds() const {
     return impl->transform.getState().getLatLngBounds();
 }
 
-void Map::setLatLngBounds(optional<LatLngBounds> 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<LatLngBounds> 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<LatLngBounds>);
+    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<LatLngBounds> bounds_) {
+void TransformState::setLatLngBounds(LatLngBounds bounds_) {
     if (bounds_ != bounds) {
         bounds = bounds_;
         setLatLngZoom(getLatLng(LatLng::Unwrapped), getZoom());
     }
 }
 
-optional<LatLngBounds> 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<LatLngBounds>);
-    optional<LatLngBounds> 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<LatLngBounds> 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<LatLngBounds> {});
+    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<LatLngBounds> {});
+        ASSERT_EQ(transform.getState().getLatLngBounds(), LatLngBounds::unbounded());
     }
 
     transform.jumpTo(CameraOptions().withCenter(sanFrancisco));