[core] Tighten geo.hpp ctors

12 files changed, 170 insertions, 74 deletions

diff --git a/include/mbgl/util/geo.hpp b/include/mbgl/util/geo.hpp
index edd40e73ca..5f5f8bb33b 100644
--- a/include/mbgl/util/geo.hpp
+++ b/include/mbgl/util/geo.hpp
@@ -70,27 +70,42 @@ public:
     LatLng(const CanonicalTileID& id);
     LatLng(const UnwrappedTileID& id);
 
-    friend constexpr bool operator==(const LatLng& a, const LatLng& b) {
+    friend bool operator==(const LatLng& a, const LatLng& b) {
         return a.lat == b.lat && a.lon == b.lon;
     }
 
-    friend constexpr bool operator!=(const LatLng& a, const LatLng& b) {
+    friend bool operator!=(const LatLng& a, const LatLng& b) {
         return !(a == b);
     }
 };
 
 class ProjectedMeters {
+private:
+    double _northing; // Distance measured northwards.
+    double _easting;  // Distance measured eastwards.
+
 public:
-    double northing;
-    double easting;
+    ProjectedMeters(double n_ = 0, double e_ = 0)
+        : _northing(n_), _easting(e_) {
+        if (std::isnan(_northing)) {
+            throw std::domain_error("northing must not be NaN");
+        }
+        if (std::isnan(_easting)) {
+            throw std::domain_error("easting must not be NaN");
+        }
+    }
 
-    ProjectedMeters(double n = 0, double e = 0)
-        : northing(n), easting(e) {}
-};
+    double northing() const { return _northing; }
+    double easting() const { return _easting; }
 
-constexpr bool operator==(const ProjectedMeters& a, const ProjectedMeters& b) {
-    return a.northing == b.northing && a.easting == b.easting;
-}
+    friend bool operator==(const ProjectedMeters& a, const ProjectedMeters& b) {
+        return a._northing == b._northing && a._easting == b._easting;
+    }
+
+    friend bool operator!=(const ProjectedMeters& a, const ProjectedMeters& b) {
+        return !(a == b);
+    }
+};
 
 class LatLngBounds {
 public:
@@ -188,17 +203,14 @@ private:
     LatLngBounds(LatLng sw_, LatLng ne_)
         : sw(std::move(sw_)), ne(std::move(ne_)) {}
 
-    friend constexpr bool operator==(const LatLngBounds&, const LatLngBounds&);
-    friend constexpr bool operator!=(const LatLngBounds&, const LatLngBounds&);
-};
-
-constexpr bool operator==(const LatLngBounds& a, const LatLngBounds& b) {
-    return a.sw == b.sw && a.ne == b.ne;
-}
+    friend bool operator==(const LatLngBounds& a, const LatLngBounds& b) {
+        return a.sw == b.sw && a.ne == b.ne;
+    }
 
-constexpr bool operator!=(const LatLngBounds& a, const LatLngBounds& b) {
-    return !(a == b);
-}
+    friend bool operator!=(const LatLngBounds& a, const LatLngBounds& b) {
+        return !(a == b);
+    }
+};
 
 // Determines the orientation of the map.
 enum class NorthOrientation : uint8_t {
@@ -210,43 +222,60 @@ enum class NorthOrientation : uint8_t {
 
 /// The distance on each side between a rectangle and a rectangle within.
 class EdgeInsets {
-public:
-    double top = 0;    // Number of pixels inset from the top edge.
-    double left = 0;   // Number of pixels inset from the left edge.
-    double bottom = 0; // Number of pixels inset from the bottom edge.
-    double right = 0;  // Number of pixels inset from the right edge.
+private:
+    double _top;    // Number of pixels inset from the top edge.
+    double _left;   // Number of pixels inset from the left edge.
+    double _bottom; // Number of pixels inset from the bottom edge.
+    double _right;  // Number of pixels inset from the right edge.
 
-    EdgeInsets() {}
+public:
+    EdgeInsets(double t_ = 0, double l_ = 0, double b_ = 0, double r_ = 0)
+        : _top(t_), _left(l_), _bottom(b_), _right(r_) {
+        if (std::isnan(_top)) {
+            throw std::domain_error("top must not be NaN");
+        }
+        if (std::isnan(_left)) {
+            throw std::domain_error("left must not be NaN");
+        }
+        if (std::isnan(_bottom)) {
+            throw std::domain_error("bottom must not be NaN");
+        }
+        if (std::isnan(_right)) {
+            throw std::domain_error("right must not be NaN");
+        }
+    }
 
-    EdgeInsets(const double t, const double l, const double b, const double r)
-        : top(t), left(l), bottom(b), right(r) {}
+    double top() const { return _top; }
+    double left() const { return _left; }
+    double bottom() const { return _bottom; }
+    double right() const { return _right; }
 
     bool isFlush() const {
-        return top == 0 && left == 0 && bottom == 0 && right == 0;
+        return _top == 0 && _left == 0 && _bottom == 0 && _right == 0;
     }
 
     void operator+=(const EdgeInsets& o) {
-        top += o.top;
-        left += o.left;
-        bottom += o.bottom;
-        right += o.right;
+        _top += o._top;
+        _left += o._left;
+        _bottom += o._bottom;
+        _right += o._right;
     }
 
     EdgeInsets operator+(const EdgeInsets& o) const {
         return {
-            top + o.top, left + o.left, bottom + o.bottom, right + o.right,
+            _top + o._top, _left + o._left, _bottom + o._bottom, _right + o._right,
         };
     }
 
     ScreenCoordinate getCenter(uint16_t width, uint16_t height) const;
-};
 
-constexpr bool operator==(const EdgeInsets& a, const EdgeInsets& b) {
-    return a.top == b.top && a.left == b.left && a.bottom == b.bottom && a.right == b.right;
-}
+    friend bool operator==(const EdgeInsets& a, const EdgeInsets& b) {
+        return a._top == b._top && a._left == b._left && a._bottom == b._bottom && a._right == b._right;
+    }
 
-constexpr bool operator!=(const EdgeInsets& a, const EdgeInsets& b) {
-    return !(a == b);
-}
+    friend bool operator!=(const EdgeInsets& a, const EdgeInsets& b) {
+        return !(a == b);
+    }
+};
 
 } // namespace mbgl
diff --git a/include/mbgl/util/projection.hpp b/include/mbgl/util/projection.hpp
index e50e29e22b..ed34261b18 100644
--- a/include/mbgl/util/projection.hpp
+++ b/include/mbgl/util/projection.hpp
@@ -38,8 +38,8 @@ public:
     }
 
     static LatLng latLngForProjectedMeters(const ProjectedMeters& projectedMeters) {
-        double latitude = (2 * std::atan(std::exp(projectedMeters.northing / util::EARTH_RADIUS_M)) - (M_PI / 2.0)) * util::RAD2DEG;
-        double longitude = projectedMeters.easting * util::RAD2DEG / util::EARTH_RADIUS_M;
+        double latitude = (2 * std::atan(std::exp(projectedMeters.northing() / util::EARTH_RADIUS_M)) - (M_PI / 2.0)) * util::RAD2DEG;
+        double longitude = projectedMeters.easting() * util::RAD2DEG / util::EARTH_RADIUS_M;
 
         latitude = util::clamp(latitude, -util::LATITUDE_MAX, util::LATITUDE_MAX);
         longitude = util::clamp(longitude, -util::LONGITUDE_MAX, util::LONGITUDE_MAX);
diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp
index 78e021c321..e5edc1c30f 100755
--- a/platform/android/src/native_map_view.cpp
+++ b/platform/android/src/native_map_view.cpp
@@ -643,7 +643,7 @@ jni::jdouble NativeMapView::getMetersPerPixelAtLatitude(JNIEnv&, jni::jdouble la
 
 jni::Object<ProjectedMeters> NativeMapView::projectedMetersForLatLng(JNIEnv& env, jni::jdouble latitude, jni::jdouble longitude) {
     mbgl::ProjectedMeters projectedMeters = map->projectedMetersForLatLng(mbgl::LatLng(latitude, longitude));
-    return ProjectedMeters::New(env, projectedMeters.northing, projectedMeters.easting);
+    return ProjectedMeters::New(env, projectedMeters.northing(), projectedMeters.easting());
 }
 
 jni::Object<PointF> NativeMapView::pixelForLatLng(JNIEnv& env, jdouble latitude, jdouble longitude) {
diff --git a/platform/darwin/src/MGLMapCamera.mm b/platform/darwin/src/MGLMapCamera.mm
index 49c7e70900..1611dbf4a3 100644
--- a/platform/darwin/src/MGLMapCamera.mm
+++ b/platform/darwin/src/MGLMapCamera.mm
@@ -32,11 +32,11 @@ BOOL MGLEqualFloatWithAccuracy(CGFloat left, CGFloat right, CGFloat accuracy)
         
         mbgl::ProjectedMeters centerMeters = mbgl::Projection::projectedMetersForLatLng(centerLatLng);
         mbgl::ProjectedMeters eyeMeters = mbgl::Projection::projectedMetersForLatLng(eyeLatLng);
-        heading = std::atan((centerMeters.northing - eyeMeters.northing) /
-                            (centerMeters.easting - eyeMeters.easting));
+        heading = std::atan((centerMeters.northing() - eyeMeters.northing()) /
+                            (centerMeters.easting() - eyeMeters.easting()));
         
-        double groundDistance = std::hypot(centerMeters.northing - eyeMeters.northing,
-                                           centerMeters.easting - eyeMeters.easting);
+        double groundDistance = std::hypot(centerMeters.northing() - eyeMeters.northing(),
+                                           centerMeters.easting() - eyeMeters.easting());
         pitch = std::atan(eyeAltitude / groundDistance);
     }
 
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index 5ae588d619..6badb1d857 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -4559,8 +4559,8 @@ public:
             mbgl::LatLng targetLatLng = MGLLatLngFromLocationCoordinate2D(self.targetCoordinate);
             mbgl::ProjectedMeters userMeters = mbgl::Projection::projectedMetersForLatLng(userLatLng);
             mbgl::ProjectedMeters targetMeters = mbgl::Projection::projectedMetersForLatLng(targetLatLng);
-            double angle = atan2(targetMeters.easting - userMeters.easting,
-                                 targetMeters.northing - userMeters.northing);
+            double angle = atan2(targetMeters.easting() - userMeters.easting(),
+                                 targetMeters.northing() - userMeters.northing());
             direction = mbgl::util::wrap(MGLDegreesFromRadians(angle), 0., 360.);
         }
         else
diff --git a/platform/qt/src/qmapboxgl.cpp b/platform/qt/src/qmapboxgl.cpp
index 2f5b3d84db..143af2ed16 100644
--- a/platform/qt/src/qmapboxgl.cpp
+++ b/platform/qt/src/qmapboxgl.cpp
@@ -1130,7 +1130,7 @@ double QMapboxGL::metersPerPixelAtLatitude(double latitude, double zoom) const
 QMapbox::ProjectedMeters QMapboxGL::projectedMetersForCoordinate(const QMapbox::Coordinate &coordinate_) const
 {
     auto projectedMeters = d_ptr->mapObj->projectedMetersForLatLng(mbgl::LatLng { coordinate_.first, coordinate_.second });
-    return QMapbox::ProjectedMeters(projectedMeters.northing, projectedMeters.easting);
+    return QMapbox::ProjectedMeters(projectedMeters.northing(), projectedMeters.easting());
 }
 
 /*!
@@ -1229,10 +1229,10 @@ void QMapboxGL::setMargins(const QMargins &margins_)
 QMargins QMapboxGL::margins() const
 {
     return QMargins(
-        d_ptr->margins.left,
-        d_ptr->margins.top,
-        d_ptr->margins.right,
-        d_ptr->margins.bottom
+        d_ptr->margins.left(),
+        d_ptr->margins.top(),
+        d_ptr->margins.right(),
+        d_ptr->margins.bottom()
     );
 }
 
diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
index 19b1222f0e..fde466054c 100644
--- a/src/mbgl/map/map.cpp
+++ b/src/mbgl/map/map.cpp
@@ -600,8 +600,8 @@ CameraOptions Map::cameraForLatLngs(const std::vector<LatLng>& latLngs, const Ed
     if (width > 0 || height > 0) {
         double scaleX = double(getSize().width) / width;
         double scaleY = double(getSize().height) / height;
-        scaleX -= (padding.left + padding.right) / width;
-        scaleY -= (padding.top + padding.bottom) / height;
+        scaleX -= (padding.left() + padding.right()) / width;
+        scaleY -= (padding.top() + padding.bottom()) / height;
         minScale = util::min(scaleX, scaleY);
     }
     double zoom = getZoom() + util::log2(minScale);
@@ -610,12 +610,12 @@ CameraOptions Map::cameraForLatLngs(const std::vector<LatLng>& latLngs, const Ed
     // Calculate the center point of a virtual bounds that is extended in all directions by padding.
     ScreenCoordinate centerPixel = nePixel + swPixel;
     ScreenCoordinate paddedNEPixel = {
-        padding.right / minScale,
-        padding.top / minScale,
+        padding.right() / minScale,
+        padding.top() / minScale,
     };
     ScreenCoordinate paddedSWPixel = {
-        padding.left / minScale,
-        padding.bottom / minScale,
+        padding.left() / minScale,
+        padding.bottom() / minScale,
     };
     centerPixel = centerPixel + paddedNEPixel - paddedSWPixel;
     centerPixel /= 2.0;
diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp
index 67729f7fe2..f9a687480c 100644
--- a/src/mbgl/map/transform.cpp
+++ b/src/mbgl/map/transform.cpp
@@ -200,8 +200,8 @@ 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 = std::max(state.size.width - padding.left - padding.right,
-                         state.size.height - padding.top - padding.bottom);
+    double w0 = std::max(state.size.width - padding.left() - padding.right(),
+                         state.size.height - padding.top() - padding.bottom());
     /// w₁: Final visible span, measured in pixels with respect to the initial
     /// scale.
     double w1 = w0 / state.zoomScale(zoom - startZoom);
diff --git a/src/mbgl/util/geo.cpp b/src/mbgl/util/geo.cpp
index 1244a975a4..f38aba20c4 100644
--- a/src/mbgl/util/geo.cpp
+++ b/src/mbgl/util/geo.cpp
@@ -34,8 +34,8 @@ LatLngBounds::LatLngBounds(const CanonicalTileID& id)
 
 ScreenCoordinate EdgeInsets::getCenter(uint16_t width, uint16_t height) const {
     return {
-        (width - left - right) / 2.0f + left,
-        (height - top - bottom) / 2.0f + top,
+        (width - left() - right()) / 2.0 + left(),
+        (height - top() - bottom()) / 2.0 + top(),
     };
 }
 
diff --git a/test/map/transform.test.cpp b/test/map/transform.test.cpp
index 8a86598a1d..accf7d293c 100644
--- a/test/map/transform.test.cpp
+++ b/test/map/transform.test.cpp
@@ -471,14 +471,12 @@ TEST(Transform, DefaultTransform) {
     // Cannot assign invalid sizes.
     std::vector<Size> invalidSizes = { {}, { min, max }, { max, min } };
     for (const Size& size : invalidSizes) {
-        ASSERT_TRUE(size.isEmpty());
-        bool pass = false;
         try {
             transform.resize(size);
+            ASSERT_TRUE(false) << "Should throw";
         } catch (...) {
-            pass = true;
+            ASSERT_TRUE(size.isEmpty());
         }
-        ASSERT_TRUE(pass) << "Expected to throw";
     }
 
     Size validSize { max, max };
diff --git a/test/util/geo.test.cpp b/test/util/geo.test.cpp
index 3f625f21fa..3dfa8e1cca 100644
--- a/test/util/geo.test.cpp
+++ b/test/util/geo.test.cpp
@@ -6,6 +6,75 @@
 
 using namespace mbgl;
 
+TEST(LatLng, InvalidLatLng) {
+    try {
+        LatLng { NAN };
+        ASSERT_TRUE(false) << "should throw";
+    } catch (const std::domain_error& error) {
+        ASSERT_EQ(std::string(error.what()), "latitude must not be NaN");
+    }
+    try {
+        LatLng { 0, NAN };
+        ASSERT_TRUE(false) << "should throw";
+    } catch (const std::domain_error& error) {
+        ASSERT_EQ(std::string(error.what()), "longitude must not be NaN");
+    }
+    try {
+        LatLng { 91.0 };
+        ASSERT_TRUE(false) << "should throw";
+    } catch (const std::domain_error& error) {
+        ASSERT_EQ(std::string(error.what()), "latitude must be between -90 and 90");
+    }
+    try {
+        LatLng { 0, std::numeric_limits<double>::infinity() };
+        ASSERT_TRUE(false) << "should throw";
+    } catch (const std::domain_error& error) {
+        ASSERT_EQ(std::string(error.what()), "longitude must not be infinite");
+    }
+}
+
+TEST(ProjectedMeters, InvalidProjectedMeters) {
+    try {
+        ProjectedMeters { NAN };
+        ASSERT_TRUE(false) << "should throw";
+    } catch (const std::domain_error& error) {
+        ASSERT_EQ(std::string(error.what()), "northing must not be NaN");
+    }
+    try {
+        ProjectedMeters { 0, NAN };
+        ASSERT_TRUE(false) << "should throw";
+    } catch (const std::domain_error& error) {
+        ASSERT_EQ(std::string(error.what()), "easting must not be NaN");
+    }
+}
+
+TEST(EdgeInsets, InvalidEdgeInsets) {
+    try {
+        EdgeInsets { NAN };
+        ASSERT_TRUE(false) << "should throw";
+    } catch (const std::domain_error& error) {
+        ASSERT_EQ(std::string(error.what()), "top must not be NaN");
+    }
+    try {
+        EdgeInsets { 0, NAN };
+        ASSERT_TRUE(false) << "should throw";
+    } catch (const std::domain_error& error) {
+        ASSERT_EQ(std::string(error.what()), "left must not be NaN");
+    }
+    try {
+        EdgeInsets { 0, 0, NAN };
+        ASSERT_TRUE(false) << "should throw";
+    } catch (const std::domain_error& error) {
+        ASSERT_EQ(std::string(error.what()), "bottom must not be NaN");
+    }
+    try {
+        EdgeInsets { 0, 0, 0, NAN };
+        ASSERT_TRUE(false) << "should throw";
+    } catch (const std::domain_error& error) {
+        ASSERT_EQ(std::string(error.what()), "right must not be NaN");
+    }
+}
+
 TEST(LatLngBounds, World) {
     auto result = LatLngBounds::world();
     ASSERT_DOUBLE_EQ(-90,  result.south());
diff --git a/test/util/projection.test.cpp b/test/util/projection.test.cpp
index b0fa0bd060..9f27fe3d14 100644
--- a/test/util/projection.test.cpp
+++ b/test/util/projection.test.cpp
@@ -38,18 +38,18 @@ TEST(Projection, MetersPerPixelAtLatitude) {
 TEST(Projection, ProjectedMeters) {
     auto latLng = LatLng {};
     auto projectedMeters = Projection::projectedMetersForLatLng(latLng);
-    EXPECT_EQ(projectedMeters.northing, projectedMeters.easting);
+    EXPECT_EQ(projectedMeters.northing(), projectedMeters.easting());
     EXPECT_EQ(latLng, Projection::latLngForProjectedMeters(projectedMeters));
 
     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);
+    EXPECT_DOUBLE_EQ(projectedMeters.northing(), -20037508.342789274);
+    EXPECT_DOUBLE_EQ(projectedMeters.easting(), -20037508.342789244);
 
     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);
+    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);