From ac70ada2d3e9b842a1fd3f46361d323ddb20d232 Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Thu, 10 Mar 2016 19:49:09 +0200 Subject: [core] Check for NaNs in mbgl::util::{min,max,clamp} --- include/mbgl/util/math.hpp | 33 +++++++++++---------- include/mbgl/util/projection.hpp | 9 +++--- src/mbgl/annotation/shape_annotation_impl.cpp | 5 ++-- src/mbgl/map/transform.cpp | 1 + src/mbgl/util/geo.cpp | 5 ++-- test/test.gypi | 1 + test/util/math.cpp | 42 +++++++++++++++++++++++++++ 7 files changed, 72 insertions(+), 24 deletions(-) create mode 100644 test/util/math.cpp diff --git a/include/mbgl/util/math.hpp b/include/mbgl/util/math.hpp index 08625ffc0c..87f9caf478 100644 --- a/include/mbgl/util/math.hpp +++ b/include/mbgl/util/math.hpp @@ -3,6 +3,7 @@ #include #include +#include #include @@ -11,33 +12,33 @@ namespace util { template -inline T max(T a, T b) { - return b > a ? b : a; +typename std::enable_if_t::value, T> max(T a, T b) { + return std::max(a, b); } template -inline T max(T a, T b, T c) { - return max(max(a, b), c); +typename std::enable_if_t::value, T> max(T a, T b) { + return std::fmax(a, b); } -template -inline T max(T a, T b, T c, T d) { - return max(max(a, b), max(c, d)); +template +typename std::enable_if_t::value, T> max(T a, T b, Ts... args) { + return max(a, max(b, args...)); } template -inline T min(T a, T b) { - return b < a ? b : a; +typename std::enable_if_t::value, T> min(T a, T b) { + return std::min(a, b); } template -inline T min(T a, T b, T c) { - return min(min(a, b), c); +typename std::enable_if_t::value, T> min(T a, T b) { + return std::fmin(a, b); } -template -inline T min(T a, T b, T c, T d) { - return min(min(a, b), min(c, d)); +template +typename std::enable_if_t::value, T> min(T a, T b, Ts... args) { + return min(a, min(b, args...)); } // Find the angle of the two vectors, solving the formula for the cross product @@ -113,8 +114,8 @@ inline S unit(const S& a) { } template -T clamp(T value, T min, T max) { - return value < min ? min : (value > max ? max : value); +T clamp(T value, T min_, T max_) { + return max(min_, min(max_, value)); } // Constrains n to the given range (including min, excluding max) via modular diff --git a/include/mbgl/util/projection.hpp b/include/mbgl/util/projection.hpp index bee5eac8ae..09c8ce74ab 100644 --- a/include/mbgl/util/projection.hpp +++ b/include/mbgl/util/projection.hpp @@ -3,6 +3,7 @@ #include #include +#include #include @@ -13,16 +14,16 @@ class Projection { public: static inline double getMetersPerPixelAtLatitude(double lat, double zoom) { const double mapPixelWidthAtZoom = std::pow(2.0, zoom) * util::tileSize; - const double constrainedLatitude = ::fmin(::fmax(lat, -util::LATITUDE_MAX), util::LATITUDE_MAX); + const double constrainedLatitude = util::clamp(lat, -util::LATITUDE_MAX, util::LATITUDE_MAX); return std::cos(constrainedLatitude * util::DEG2RAD) * util::M2PI * util::EARTH_RADIUS_M / mapPixelWidthAtZoom; } static inline ProjectedMeters projectedMetersForLatLng(const LatLng& latLng) { - const double constrainedLatitude = ::fmin(::fmax(latLng.latitude, -util::LATITUDE_MAX), util::LATITUDE_MAX); + const double constrainedLatitude = util::clamp(latLng.latitude, -util::LATITUDE_MAX, util::LATITUDE_MAX); const double m = 1 - 1e-15; - const double f = ::fmin(::fmax(std::sin(util::DEG2RAD * constrainedLatitude), -m), m); + const double f = util::clamp(std::sin(util::DEG2RAD * constrainedLatitude), -m, m); const double easting = util::EARTH_RADIUS_M * latLng.longitude * util::DEG2RAD; const double northing = 0.5 * util::EARTH_RADIUS_M * std::log((1 + f) / (1 - f)); @@ -34,7 +35,7 @@ public: double latitude = (2 * std::atan(std::exp(projectedMeters.northing / util::EARTH_RADIUS_M)) - (M_PI / 2)) * util::RAD2DEG; double longitude = projectedMeters.easting * util::RAD2DEG / util::EARTH_RADIUS_M; - latitude = ::fmin(::fmax(latitude, -util::LATITUDE_MAX), util::LATITUDE_MAX); + latitude = util::clamp(latitude, -util::LATITUDE_MAX, util::LATITUDE_MAX); return LatLng(latitude, longitude); } diff --git a/src/mbgl/annotation/shape_annotation_impl.cpp b/src/mbgl/annotation/shape_annotation_impl.cpp index a9277ee0a7..9adebf177b 100644 --- a/src/mbgl/annotation/shape_annotation_impl.cpp +++ b/src/mbgl/annotation/shape_annotation_impl.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -90,8 +91,8 @@ void ShapeAnnotationImpl::updateTile(const TileID& tileID, AnnotationTile& tile) std::vector points; for (size_t i = 0; i < shape.segments[0].size(); ++i) { // first segment for now (no holes) - const double constraintedLatitude = ::fmin(::fmax(shape.segments[0][i].latitude, -util::LATITUDE_MAX), util::LATITUDE_MAX); - points.push_back(geojsonvt::LonLat(shape.segments[0][i].longitude, constraintedLatitude)); + const double constrainedLatitude = util::clamp(shape.segments[0][i].latitude, -util::LATITUDE_MAX, util::LATITUDE_MAX); + points.push_back(geojsonvt::LonLat(shape.segments[0][i].longitude, constrainedLatitude)); } if (type == geojsonvt::ProjectedFeatureType::Polygon && diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp index da085ac750..0c9f7bf1e6 100644 --- a/src/mbgl/map/transform.cpp +++ b/src/mbgl/map/transform.cpp @@ -410,6 +410,7 @@ void Transform::scaleBy(double ds, const Duration& duration) { } void Transform::scaleBy(double ds, optional anchor, const Duration& duration) { + if (std::isnan(ds)) return; double scale = util::clamp(state.scale * ds, state.min_scale, state.max_scale); setScale(scale, anchor, duration); } diff --git a/src/mbgl/util/geo.cpp b/src/mbgl/util/geo.cpp index bf7cf38117..7d9db65c4c 100644 --- a/src/mbgl/util/geo.cpp +++ b/src/mbgl/util/geo.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -13,8 +14,8 @@ LatLng::LatLng(const TileID& id) { } ScreenCoordinate LatLng::project() const { - // Clamp to the latitude limits of Mercator. - const double constrainedLatitude = ::fmin(::fmax(latitude, -util::LATITUDE_MAX), util::LATITUDE_MAX); + // Clamp to the latitude limits of Web Mercator. + const double constrainedLatitude = util::clamp(latitude, -util::LATITUDE_MAX, util::LATITUDE_MAX); // Project a coordinate into unit space in a square map. const double sine = std::sin(constrainedLatitude * util::DEG2RAD); diff --git a/test/test.gypi b/test/test.gypi index d85faf2f5a..b9ba820e72 100644 --- a/test/test.gypi +++ b/test/test.gypi @@ -42,6 +42,7 @@ 'util/geo.cpp', 'util/image.cpp', 'util/mapbox.cpp', + 'util/math.cpp', 'util/merge_lines.cpp', 'util/run_loop.cpp', 'util/text_conversions.cpp', diff --git a/test/util/math.cpp b/test/util/math.cpp new file mode 100644 index 0000000000..0074b43509 --- /dev/null +++ b/test/util/math.cpp @@ -0,0 +1,42 @@ +#include "../fixtures/util.hpp" + +#include +#include + +using namespace mbgl; + +TEST(Math, MinMaxFloatingPoint) { + double minimumValue = std::numeric_limits::min(); + double lowestValue = std::numeric_limits::lowest(); + double maximumValue = std::numeric_limits::max(); + double quietNAN = std::numeric_limits::quiet_NaN(); + + ASSERT_DOUBLE_EQ(lowestValue, util::min(minimumValue, lowestValue)); + ASSERT_DOUBLE_EQ(lowestValue, util::min(double(NAN), lowestValue)); + ASSERT_DOUBLE_EQ(lowestValue, util::min(quietNAN, lowestValue)); + ASSERT_DOUBLE_EQ(0., util::min(2., 1., 0.)); + + ASSERT_DOUBLE_EQ(maximumValue, util::max(double(NAN), maximumValue)); + ASSERT_DOUBLE_EQ(maximumValue, util::max(quietNAN, maximumValue)); + ASSERT_DOUBLE_EQ(10., util::max(8., 9., 10.)); + + ASSERT_DOUBLE_EQ(0., util::clamp(0., quietNAN, quietNAN)); + ASSERT_DOUBLE_EQ(0., util::clamp(0., lowestValue, maximumValue)); +} + +TEST(Math, MinMaxIntegral) { + int32_t minimumValue = std::numeric_limits::min(); + int32_t lowestValue = std::numeric_limits::lowest(); + int32_t maximumValue = std::numeric_limits::max(); + int32_t quietNAN = std::numeric_limits::quiet_NaN(); + + ASSERT_EQ(lowestValue, util::min(minimumValue, lowestValue)); + ASSERT_EQ(lowestValue, util::min(quietNAN, lowestValue)); + ASSERT_EQ(0, util::min(2, 1, 0)); + + ASSERT_EQ(maximumValue, util::max(quietNAN, maximumValue)); + ASSERT_EQ(10, util::max(8, 9, 10)); + + ASSERT_EQ(0, util::clamp(0, quietNAN, quietNAN)); + ASSERT_EQ(0, util::clamp(0, lowestValue, maximumValue)); +} -- cgit v1.2.1