[core] Check for NaNs in EdgeInsets

4 files changed, 53 insertions, 19 deletions

diff --git a/include/mbgl/util/geo.hpp b/include/mbgl/util/geo.hpp
index 9e2d7a2bb1..94fbdc3d09 100644
--- a/include/mbgl/util/geo.hpp
+++ b/include/mbgl/util/geo.hpp
@@ -199,9 +199,10 @@ public:
         : top(t), left(l), bottom(b), right(r) {}
     
     explicit operator bool() const {
-        return top || left || bottom || right;
+        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;
         left += o.left;
diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp
index fe4cbbe673..61635d472a 100644
--- a/src/mbgl/map/transform.cpp
+++ b/src/mbgl/map/transform.cpp
@@ -381,7 +381,7 @@ void Transform::setLatLng(const LatLng& latLng, const ScreenCoordinate& point, c
     padding.left = point.x;
     padding.bottom = state.height - point.y;
     padding.right = state.width - point.x;
-    camera.padding = padding;
+    if (padding) camera.padding = padding;
     easeTo(camera, duration);
 }
 
@@ -436,8 +436,7 @@ void Transform::setZoom(double zoom, const ScreenCoordinate& anchor, const Durat
 }
 
 void Transform::setZoom(double zoom, const EdgeInsets& padding, const Duration& duration) {
-    const ScreenCoordinate center = padding.getCenter(state.width, state.height);
-    setZoom(zoom, center, duration);
+    setScale(state.zoomScale(zoom), padding, duration);
 }
 
 double Transform::getZoom() const {
@@ -455,7 +454,7 @@ void Transform::setScale(double scale, const ScreenCoordinate& anchor, const Dur
     
     CameraOptions camera;
     camera.zoom = state.scaleZoom(scale);
-    camera.anchor = anchor;
+    if (anchor) camera.anchor = anchor;
     easeTo(camera, duration);
 }
 
@@ -501,7 +500,7 @@ void Transform::rotateBy(const ScreenCoordinate& first, const ScreenCoordinate&
 }
 
 void Transform::setAngle(double angle, const Duration& duration) {
-    setAngle(angle, {NAN, NAN}, duration);
+    setAngle(angle, ScreenCoordinate {}, duration);
 }
 
 void Transform::setAngle(double angle, const ScreenCoordinate& anchor, const Duration& duration) {
@@ -526,7 +525,7 @@ double Transform::getAngle() const {
 #pragma mark - Pitch
 
 void Transform::setPitch(double pitch, const Duration& duration) {
-    setPitch(pitch, {NAN, NAN}, duration);
+    setPitch(pitch, ScreenCoordinate {}, duration);
 }
 
 void Transform::setPitch(double pitch, const ScreenCoordinate& anchor, const Duration& duration) {
@@ -536,7 +535,7 @@ void Transform::setPitch(double pitch, const ScreenCoordinate& anchor, const Dur
 
     CameraOptions camera;
     camera.pitch = pitch;
-    camera.anchor = anchor;
+    if (anchor) camera.anchor = anchor;
     easeTo(camera, duration);
 }
 
@@ -580,11 +579,11 @@ void Transform::startTransition(const CameraOptions& camera,
     view.notifyMapChange(isAnimated ? MapChangeRegionWillChangeAnimated : MapChangeRegionWillChange);
     
     // Associate the anchor, if given, with a coordinate.
-    ScreenCoordinate anchor = camera.anchor ? *camera.anchor : ScreenCoordinate(NAN, NAN);
+    optional<ScreenCoordinate> anchor = camera.anchor;
     LatLng anchorLatLng;
-    if (anchor) {
-        anchor.y = state.getHeight() - anchor.y;
-        anchorLatLng = state.screenCoordinateToLatLng(anchor);
+    if (anchor && *anchor) {
+        anchor->y = state.getHeight() - anchor->y;
+        anchorLatLng = state.screenCoordinateToLatLng(*anchor);
     }
 
     transitionStart = Clock::now();
@@ -600,7 +599,7 @@ void Transform::startTransition(const CameraOptions& camera,
             result = frame(ease.solve(t, 0.001));
         }
         
-        if (anchor) state.moveLatLng(anchorLatLng, anchor);
+        if (anchor && *anchor) state.moveLatLng(anchorLatLng, *anchor);
         
         // At t = 1.0, a DidChangeAnimated notification should be sent from finish().
         if (t < 1.0) {
diff --git a/src/mbgl/map/transform.hpp b/src/mbgl/map/transform.hpp
index 6bbfd9c6f7..e904b3956a 100644
--- a/src/mbgl/map/transform.hpp
+++ b/src/mbgl/map/transform.hpp
@@ -56,12 +56,12 @@ public:
         @param ds The difference in scale factors to scale the map by.
         @param anchor A point relative to the top-left corner of the view.
             If unspecified, the center point is fixed within the view. */
-    void scaleBy(double ds, const ScreenCoordinate& anchor = {NAN, NAN}, const Duration& = Duration::zero());
+    void scaleBy(double ds, const ScreenCoordinate& anchor = {}, const Duration& = Duration::zero());
     /** Sets the scale factor, keeping the given point fixed within the view.
         @param scale The new scale factor.
         @param anchor A point relative to the top-left corner of the view.
             If unspecified, the center point is fixed within the view. */
-    void setScale(double scale, const ScreenCoordinate& anchor = {NAN, NAN}, const Duration& = Duration::zero());
+    void setScale(double scale, const ScreenCoordinate& anchor = {}, const Duration& = Duration::zero());
     /** Sets the scale factor, keeping the center point fixed within the inset view.
         @param scale The new scale factor.
         @param padding The viewport padding that affects the fixed center point. */
@@ -70,7 +70,7 @@ public:
         @param zoom The new zoom level.
         @param anchor A point relative to the top-left corner of the view.
             If unspecified, the center point is fixed within the view. */
-    void setZoom(double zoom, const ScreenCoordinate& anchor = {NAN, NAN}, const Duration& = Duration::zero());
+    void setZoom(double zoom, const ScreenCoordinate& anchor = {}, const Duration& = Duration::zero());
     /** Sets the zoom level, keeping the center point fixed within the inset view.
         @param zoom The new zoom level.
         @param padding The viewport padding that affects the fixed center point. */
diff --git a/test/map/transform.cpp b/test/map/transform.cpp
index 4412b78b04..ada20c5872 100644
--- a/test/map/transform.cpp
+++ b/test/map/transform.cpp
@@ -182,7 +182,7 @@ TEST(Transform, ConstrainHeightOnly) {
     transform.setLatLng(LatLngBounds::world().southwest());
     loc = transform.getLatLng();
     ASSERT_NEAR(-util::LATITUDE_MAX, loc.latitude, 0.001);
-    ASSERT_NEAR(-util::LONGITUDE_MAX, loc.longitude, 0.001);
+    ASSERT_NEAR(util::LONGITUDE_MAX, std::abs(loc.longitude), 0.001);
 
     transform.setLatLng(LatLngBounds::world().northeast());
     loc = transform.getLatLng();
@@ -253,8 +253,14 @@ TEST(Transform, Padding) {
     });
     
     EdgeInsets padding;
+
+    padding.top = 0;
+    ASSERT_FALSE(bool(padding));
+
+    padding.top = NAN;
+    ASSERT_FALSE(bool(padding));
+
     padding.top = 1000.0 / 2.0;
-    ASSERT_GT(padding.top, 0);
     ASSERT_TRUE(bool(padding));
     
     const LatLng shiftedCenter = transform.getLatLng(padding);
@@ -263,3 +269,31 @@ TEST(Transform, Padding) {
     ASSERT_DOUBLE_EQ(manualShiftedCenter.latitude, shiftedCenter.latitude);
     ASSERT_DOUBLE_EQ(manualShiftedCenter.longitude, shiftedCenter.longitude);
 }
+
+TEST(Transform, MoveBy) {
+    MockView view;
+    Transform transform(view, ConstrainMode::HeightOnly);
+    transform.resize({{ 1000, 1000 }});
+    transform.setLatLngZoom({ 0, 0 }, 10);
+
+    LatLng trueCenter = transform.getLatLng();
+    ASSERT_DOUBLE_EQ(0, trueCenter.latitude);
+    ASSERT_DOUBLE_EQ(0, trueCenter.longitude);
+    ASSERT_DOUBLE_EQ(10, transform.getZoom());
+
+    for (uint8_t x = 0; x < 20; ++x) {
+        bool odd = x % 2;
+        bool forward = x % 10;
+
+        LatLng coordinate = transform.screenCoordinateToLatLng({ odd ? 400. : 600., forward ? 400. : 600 });
+        transform.moveBy({ odd ? 100. : -100., forward ? 100. : -100 });
+
+        trueCenter = transform.getLatLng();
+        ASSERT_NEAR(coordinate.latitude, trueCenter.latitude, 0.0001);
+        ASSERT_NEAR(coordinate.longitude, trueCenter.longitude, 0.0001);
+    }
+
+    // We have ~1.1 precision loss for each coordinate for 20 rounds of moveBy.
+    ASSERT_NEAR(0, trueCenter.latitude, 1.1);
+    ASSERT_NEAR(0, trueCenter.longitude, 1.1);
+}