Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[core] Tighten LatLng and other geo.hpp classes
Browse files Browse the repository at this point in the history 
* Remove LatLng::null and enforce invariants
* Remove unnecessary operator bool()
  • Loading branch information
@jfirebaugh
jfirebaugh committed Apr 3, 2017
1 parent da35f64 commit ac78ed1
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 97 deletions.
35 changes: 18 additions & 17 deletions include/mbgl/util/geo.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <mapbox/geometry/box.hpp>

#include <cmath>
#include <stdexcept>

namespace mbgl {

Expand All @@ -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 }; }

Expand All @@ -48,10 +61,6 @@ class LatLng {
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);
Expand All @@ -72,10 +81,6 @@ class ProjectedMeters {

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) {
Expand Down Expand Up @@ -197,10 +202,6 @@ class EdgeInsets {
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;
Expand Down
4 changes: 2 additions & 2 deletions src/mbgl/map/map.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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,
Expand Down
35 changes: 13 additions & 22 deletions src/mbgl/map/transform.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -335,34 +333,27 @@ 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;
easeTo(camera, animation);
}

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;
Expand All @@ -372,15 +363,15 @@ 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();
}
}

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. };
Expand Down Expand Up @@ -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);
}

Expand Down
46 changes: 1 addition & 45 deletions test/map/transform.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
17 changes: 6 additions & 11 deletions test/util/projection.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit ac78ed1

Please sign in to comment.