[core] Tighten LatLng and other geo.hpp classes

* Remove LatLng::null and enforce invariants
* Remove unnecessary operator bool()

5 files changed, 40 insertions, 97 deletions

diff --git a/include/mbgl/util/geo.hpp b/include/mbgl/util/geo.hpp
index 2cc9297aae..d0d0518c2a 100644
--- a/include/mbgl/util/geo.hpp
+++ b/include/mbgl/util/geo.hpp
@@ -9,6 +9,7 @@
 #include <mapbox/geometry/box.hpp>
 
 #include <cmath>
+#include <stdexcept>
 
 namespace mbgl {
 
@@ -21,17 +22,29 @@ using ScreenBox        = mapbox::geometry::box<double>;
 
 class LatLng {
 public:
-    struct null {};
-
     double latitude;
     double longitude;
 
     enum WrapMode : bool { Unwrapped, Wrapped };
 
-    LatLng(null) : latitude(std::numeric_limits<double>::quiet_NaN()), longitude(latitude) {}
-
     LatLng(double lat = 0, double lon = 0, WrapMode mode = Unwrapped)
-        : latitude(lat), longitude(lon) { if (mode == Wrapped) wrap(); }
+        : latitude(lat), longitude(lon) {
+        if (std::isnan(lat)) {
+            throw std::domain_error("latitude must not be NaN");
+        }
+        if (std::isnan(lon)) {
+            throw std::domain_error("longitude must not be NaN");
+        }
+        if (std::abs(lat) > 90.0) {
+            throw std::domain_error("latitude must be between -90 and 90");
+        }
+        if (!std::isfinite(lon)) {
+            throw std::domain_error("longitude must not be infinite");
+        }
+        if (mode == Wrapped) {
+            wrap();
+        }
+    }
 
     LatLng wrapped() const { return { latitude, longitude, Wrapped }; }
 
@@ -48,10 +61,6 @@ public:
         else if (longitude < 0 && end.longitude > 0) longitude += util::DEGREES_MAX;
     }
 
-    explicit operator bool() const {
-        return !(std::isnan(latitude) || std::isnan(longitude));
-    }
-
     // Constructs a LatLng object with the top left position of the specified tile.
     LatLng(const CanonicalTileID& id);
     LatLng(const UnwrappedTileID& id);
@@ -72,10 +81,6 @@ public:
 
     ProjectedMeters(double n = 0, double e = 0)
         : northing(n), easting(e) {}
-
-    explicit operator bool() const {
-        return !(std::isnan(northing) || std::isnan(easting));
-    }
 };
 
 constexpr bool operator==(const ProjectedMeters& a, const ProjectedMeters& b) {
@@ -197,10 +202,6 @@ public:
     EdgeInsets(const double t, const double l, const double b, const double r)
         : top(t), left(l), bottom(b), right(r) {}
 
-    explicit operator bool() const {
-        return !(std::isnan(top) || std::isnan(left) || std::isnan(bottom) || std::isnan(right))
-            && (top || left || bottom || right);
-    }
 
     void operator+=(const EdgeInsets& o) {
         top += o.top;
diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
index 8957f3272e..60113b5733 100644
--- a/src/mbgl/map/map.cpp
+++ b/src/mbgl/map/map.cpp
@@ -616,7 +616,7 @@ CameraOptions Map::cameraForLatLngs(const std::vector<LatLng>& latLngs, optional
     if (width > 0 || height > 0) {
         double scaleX = double(getSize().width) / width;
         double scaleY = double(getSize().height) / height;
-        if (padding && *padding) {
+        if (padding) {
             scaleX -= (padding->left + padding->right) / width;
             scaleY -= (padding->top + padding->bottom) / height;
         }
@@ -627,7 +627,7 @@ CameraOptions Map::cameraForLatLngs(const std::vector<LatLng>& latLngs, optional
 
     // Calculate the center point of a virtual bounds that is extended in all directions by padding.
     ScreenCoordinate centerPixel = nePixel + swPixel;
-    if (padding && *padding) {
+    if (padding) {
         ScreenCoordinate paddedNEPixel = {
             padding->right / minScale,
             padding->top / minScale,
diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp
index d325271388..71d216f1ed 100644
--- a/src/mbgl/map/transform.cpp
+++ b/src/mbgl/map/transform.cpp
@@ -93,13 +93,12 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim
     double angle = camera.angle.value_or(getAngle());
     double pitch = camera.pitch.value_or(getPitch());
 
-    if (!latLng || std::isnan(zoom)) {
+    if (std::isnan(zoom)) {
         return;
     }
 
     // Determine endpoints.
-    EdgeInsets padding;
-    if (camera.padding) padding = *camera.padding;
+    optional<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
@@ -168,13 +167,12 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima
     double angle = camera.angle.value_or(getAngle());
     double pitch = camera.pitch.value_or(getPitch());
 
-    if (!latLng || std::isnan(zoom)) {
+    if (std::isnan(zoom)) {
         return;
     }
 
     // Determine endpoints.
-    EdgeInsets padding;
-    if (camera.padding) padding = *camera.padding;
+    optional<EdgeInsets> padding = camera.padding;
     LatLng startLatLng = getLatLng(padding).wrapped();
     startLatLng.unwrapForShortestPath(latLng);
 
@@ -198,9 +196,9 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima
 
     /// w₀: Initial visible span, measured in pixels at the initial scale.
     /// Known henceforth as a <i>screenful</i>.
-    double w0 = padding ? std::max(state.size.width, state.size.height)
-                        : std::max(state.size.width - padding.left - padding.right,
-                                   state.size.height - padding.top - padding.bottom);
+    double w0 = padding ? std::max(state.size.width - padding->left - padding->right,
+                                   state.size.height - padding->top - padding->bottom)
+                        : std::max(state.size.width, state.size.height);
     /// w₁: Final visible span, measured in pixels with respect to the initial
     /// scale.
     double w1 = w0 / state.zoomScale(zoom - startZoom);
@@ -335,7 +333,6 @@ void Transform::setLatLng(const LatLng& latLng, const AnimationOptions& animatio
 }
 
 void Transform::setLatLng(const LatLng& latLng, optional<EdgeInsets> padding, const AnimationOptions& animation) {
-    if (!latLng) return;
     CameraOptions camera;
     camera.center = latLng;
     camera.padding = padding;
@@ -343,26 +340,20 @@ void Transform::setLatLng(const LatLng& latLng, optional<EdgeInsets> padding, co
 }
 
 void Transform::setLatLng(const LatLng& latLng, optional<ScreenCoordinate> anchor, const AnimationOptions& animation) {
-    if (!latLng) return;
     CameraOptions camera;
     camera.center = latLng;
     if (anchor) {
-        EdgeInsets padding;
-        padding.top = anchor->y;
-        padding.left = anchor->x;
-        padding.bottom = state.size.height - anchor->y;
-        padding.right = state.size.width - anchor->x;
-        if (padding) camera.padding = padding;
+        camera.padding = EdgeInsets(anchor->y, anchor->x, state.size.height - anchor->y, state.size.width - anchor->x);
     }
     easeTo(camera, animation);
 }
 
 void Transform::setLatLngZoom(const LatLng& latLng, double zoom, const AnimationOptions& animation) {
-    setLatLngZoom(latLng, zoom, EdgeInsets {}, animation);
+    setLatLngZoom(latLng, zoom, optional<EdgeInsets> {}, animation);
 }
 
 void Transform::setLatLngZoom(const LatLng& latLng, double zoom, optional<EdgeInsets> padding, const AnimationOptions& animation) {
-    if (!latLng || std::isnan(zoom)) return;
+    if (std::isnan(zoom)) return;
 
     CameraOptions camera;
     camera.center = latLng;
@@ -372,7 +363,7 @@ void Transform::setLatLngZoom(const LatLng& latLng, double zoom, optional<EdgeIn
 }
 
 LatLng Transform::getLatLng(optional<EdgeInsets> padding) const {
-    if (padding && *padding) {
+    if (padding) {
         return screenCoordinateToLatLng(padding->getCenter(state.size.width, state.size.height));
     } else {
         return state.getLatLng();
@@ -380,7 +371,7 @@ LatLng Transform::getLatLng(optional<EdgeInsets> padding) const {
 }
 
 ScreenCoordinate Transform::getScreenCoordinate(optional<EdgeInsets> padding) const {
-    if (padding && *padding) {
+    if (padding) {
         return padding->getCenter(state.size.width, state.size.height);
     } else {
         return { state.size.width / 2., state.size.height / 2. };
@@ -483,7 +474,7 @@ void Transform::setAngle(double angle, optional<ScreenCoordinate> anchor, const
 
 void Transform::setAngle(double angle, optional<EdgeInsets> padding, const AnimationOptions& animation) {
     optional<ScreenCoordinate> anchor;
-    if (padding && *padding) anchor = getScreenCoordinate(padding);
+    if (padding) anchor = getScreenCoordinate(padding);
     setAngle(angle, anchor, animation);
 }
 
diff --git a/test/map/transform.test.cpp b/test/map/transform.test.cpp
index 5bff4ee5ef..b5c92abdcb 100644
--- a/test/map/transform.test.cpp
+++ b/test/map/transform.test.cpp
@@ -44,40 +44,6 @@ TEST(Transform, InvalidScale) {
     ASSERT_DOUBLE_EQ(2, transform.getScale());
 }
 
-TEST(Transform, InvalidLatLng) {
-    Transform transform;
-
-    ASSERT_DOUBLE_EQ(0, transform.getLatLng().latitude);
-    ASSERT_DOUBLE_EQ(0, transform.getLatLng().longitude);
-    ASSERT_DOUBLE_EQ(1, transform.getScale());
-
-    transform.setScale(2 << 0);
-    transform.setLatLng({ 8, 10 });
-
-    ASSERT_DOUBLE_EQ(8, transform.getLatLng().latitude);
-    ASSERT_DOUBLE_EQ(10, transform.getLatLng().longitude);
-    ASSERT_DOUBLE_EQ(2, transform.getScale());
-
-    transform.setLatLngZoom({ 10, 8 }, 2);
-
-    ASSERT_DOUBLE_EQ(10, transform.getLatLng().latitude);
-    ASSERT_DOUBLE_EQ(8, transform.getLatLng().longitude);
-    ASSERT_DOUBLE_EQ(4, transform.getScale());
-
-    const double invalid = std::nan("");
-    transform.setLatLngZoom({ invalid, 8 }, 2);
-
-    ASSERT_DOUBLE_EQ(10, transform.getLatLng().latitude);
-    ASSERT_DOUBLE_EQ(8, transform.getLatLng().longitude);
-    ASSERT_DOUBLE_EQ(4, transform.getScale());
-
-    transform.setLatLngZoom({ 10, invalid }, 2);
-
-    ASSERT_DOUBLE_EQ(10, transform.getLatLng().latitude);
-    ASSERT_DOUBLE_EQ(8, transform.getLatLng().longitude);
-    ASSERT_DOUBLE_EQ(4, transform.getScale());
-}
-
 
 TEST(Transform, InvalidBearing) {
     Transform transform;
@@ -349,17 +315,7 @@ TEST(Transform, Padding) {
         1000.0 / 4.0,
     });
 
-    EdgeInsets padding;
-
-    padding.top = 0;
-    ASSERT_FALSE(bool(padding));
-
-    padding.top = NAN;
-    ASSERT_FALSE(bool(padding));
-
-    padding.top = 1000.0 / 2.0;
-    ASSERT_TRUE(bool(padding));
-
+    EdgeInsets padding(1000.0 / 2.0, 0, 0, 0);
     const LatLng shiftedCenter = transform.getLatLng(padding);
     ASSERT_NE(trueCenter.latitude, shiftedCenter.latitude);
     ASSERT_NEAR(trueCenter.longitude, shiftedCenter.longitude, 1e-9);
diff --git a/test/util/projection.test.cpp b/test/util/projection.test.cpp
index 5efba380b3..260d12edfe 100644
--- a/test/util/projection.test.cpp
+++ b/test/util/projection.test.cpp
@@ -36,25 +36,20 @@ TEST(Projection, MetersPerPixelAtLatitude) {
 }
 
 TEST(Projection, ProjectedMeters) {
-    const auto southWest = LatLng { -util::LATITUDE_MAX, -util::LONGITUDE_MAX };
-    const auto northEast = LatLng { util::LATITUDE_MAX, util::LONGITUDE_MAX };
-
     auto latLng = LatLng {};
     auto projectedMeters = Projection::projectedMetersForLatLng(latLng);
     EXPECT_EQ(projectedMeters.northing, projectedMeters.easting);
     EXPECT_EQ(latLng, Projection::latLngForProjectedMeters(projectedMeters));
 
-    latLng = LatLng { std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest() };
-    projectedMeters = Projection::projectedMetersForLatLng(latLng);
-    EXPECT_EQ(projectedMeters, Projection::projectedMetersForLatLng(southWest));
+    const auto southWest = LatLng { -util::LATITUDE_MAX, -util::LONGITUDE_MAX };
+    projectedMeters = Projection::projectedMetersForLatLng(southWest);
     EXPECT_DOUBLE_EQ(projectedMeters.northing, -20037508.342789274);
     EXPECT_DOUBLE_EQ(projectedMeters.easting, -20037508.342789244);
 
-    latLng = LatLng { std::numeric_limits<double>::max(), std::numeric_limits<double>::max() };
-    projectedMeters = Projection::projectedMetersForLatLng(latLng);
-    EXPECT_EQ(projectedMeters, Projection::projectedMetersForLatLng(northEast));
-    EXPECT_DOUBLE_EQ(projectedMeters.northing, -Projection::projectedMetersForLatLng(southWest).northing);
-    EXPECT_DOUBLE_EQ(projectedMeters.easting, -Projection::projectedMetersForLatLng(southWest).easting);
+    const auto northEast = LatLng { util::LATITUDE_MAX, util::LONGITUDE_MAX };
+    projectedMeters = Projection::projectedMetersForLatLng(northEast);
+    EXPECT_DOUBLE_EQ(projectedMeters.northing, 20037508.342789274);
+    EXPECT_DOUBLE_EQ(projectedMeters.easting, 20037508.342789244);
 
     projectedMeters = ProjectedMeters { std::numeric_limits<double>::lowest(), std::numeric_limits<double>::lowest() };
     latLng = Projection::latLngForProjectedMeters(projectedMeters);