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

Commit

Permalink
[core] Harden Transform anchor & padding usage
Browse files Browse the repository at this point in the history 
Use optional values for anchor & padding in Map and Transform functions
instead of NaNs. Added unit tests to stress some edge cases.
  • Loading branch information
@brunoabinader
brunoabinader committed Mar 13, 2016
1 parent a396c6a commit 8e30a4a
Show file tree
Hide file tree
Showing 7 changed files with 223 additions and 144 deletions.
29 changes: 15 additions & 14 deletions include/mbgl/map/map.hpp
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef MBGL_MAP_MAP
#define MBGL_MAP_MAP

#include <mbgl/util/optional.hpp>
#include <mbgl/util/chrono.hpp>
#include <mbgl/util/image.hpp>
#include <mbgl/map/update.hpp>
Expand Down Expand Up @@ -98,23 +99,23 @@ class Map : private util::noncopyable {

// Position
void moveBy(const ScreenCoordinate&, const Duration& = Duration::zero());
void setLatLng(const LatLng&, const ScreenCoordinate&, const Duration& = Duration::zero());
void setLatLng(const LatLng&, const EdgeInsets&, const Duration& = Duration::zero());
void setLatLng(const LatLng&, optional<ScreenCoordinate>, const Duration& = Duration::zero());
void setLatLng(const LatLng&, optional<EdgeInsets>, const Duration& = Duration::zero());
void setLatLng(const LatLng&, const Duration& = Duration::zero());
LatLng getLatLng(const EdgeInsets& = {}) const;
void resetPosition(const EdgeInsets& = {});
LatLng getLatLng(optional<EdgeInsets> = {}) const;
void resetPosition(optional<EdgeInsets> = {});

// Scale
void scaleBy(double ds, const ScreenCoordinate& = { NAN, NAN }, const Duration& = Duration::zero());
void setScale(double scale, const ScreenCoordinate& = { NAN, NAN }, const Duration& = Duration::zero());
void scaleBy(double ds, optional<ScreenCoordinate> = {}, const Duration& = Duration::zero());
void setScale(double scale, optional<ScreenCoordinate> = {}, const Duration& = Duration::zero());
double getScale() const;
void setZoom(double zoom, const Duration& = Duration::zero());
void setZoom(double zoom, const EdgeInsets&, const Duration& = Duration::zero());
void setZoom(double zoom, optional<EdgeInsets>, const Duration& = Duration::zero());
double getZoom() const;
void setLatLngZoom(const LatLng&, double zoom, const Duration& = Duration::zero());
void setLatLngZoom(const LatLng&, double zoom, const EdgeInsets&, const Duration& = Duration::zero());
CameraOptions cameraForLatLngBounds(const LatLngBounds&, const EdgeInsets&);
CameraOptions cameraForLatLngs(const std::vector<LatLng>&, const EdgeInsets&);
void setLatLngZoom(const LatLng&, double zoom, optional<EdgeInsets>, const Duration& = Duration::zero());
CameraOptions cameraForLatLngBounds(const LatLngBounds&, optional<EdgeInsets>);
CameraOptions cameraForLatLngs(const std::vector<LatLng>&, optional<EdgeInsets>);
void resetZoom();
void setMinZoom(const double minZoom);
double getMinZoom() const;
Expand All @@ -124,15 +125,15 @@ class Map : private util::noncopyable {
// Rotation
void rotateBy(const ScreenCoordinate& first, const ScreenCoordinate& second, const Duration& = Duration::zero());
void setBearing(double degrees, const Duration& = Duration::zero());
void setBearing(double degrees, const ScreenCoordinate&, const Duration& = Duration::zero());
void setBearing(double degrees, const EdgeInsets&, const Duration& = Duration::zero());
void setBearing(double degrees, optional<ScreenCoordinate>, const Duration& = Duration::zero());
void setBearing(double degrees, optional<EdgeInsets>, const Duration& = Duration::zero());
double getBearing() const;
void resetNorth(const Duration& = Milliseconds(500));
void resetNorth(const EdgeInsets&, const Duration& = Milliseconds(500));
void resetNorth(optional<EdgeInsets>, const Duration& = Milliseconds(500));

// Pitch
void setPitch(double pitch, const Duration& = Duration::zero());
void setPitch(double pitch, const ScreenCoordinate&, const Duration& = Duration::zero());
void setPitch(double pitch, optional<ScreenCoordinate>, const Duration& = Duration::zero());
double getPitch() const;

// North Orientation
Expand Down
6 changes: 3 additions & 3 deletions platform/default/glfw_view.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ void GLFWView::onScroll(GLFWwindow *window, double /*xOffset*/, double yOffset)
scale = 1.0 / scale;
}

view->map->scaleBy(scale, { view->lastX, view->lastY });
view->map->scaleBy(scale, mbgl::ScreenCoordinate { view->lastX, view->lastY });
}

void GLFWView::onWindowResize(GLFWwindow *window, int width, int height) {
Expand Down Expand Up @@ -363,9 +363,9 @@ void GLFWView::onMouseClick(GLFWwindow *window, int button, int action, int modi
double now = glfwGetTime();
if (now - view->lastClick < 0.4 /* ms */) {
if (modifiers & GLFW_MOD_SHIFT) {
view->map->scaleBy(0.5, { view->lastX, view->lastY }, mbgl::Milliseconds(500));
view->map->scaleBy(0.5, mbgl::ScreenCoordinate { view->lastX, view->lastY }, mbgl::Milliseconds(500));
} else {
view->map->scaleBy(2.0, { view->lastX, view->lastY }, mbgl::Milliseconds(500));
view->map->scaleBy(2.0, mbgl::ScreenCoordinate { view->lastX, view->lastY }, mbgl::Milliseconds(500));
}
}
view->lastClick = now;
Expand Down
14 changes: 7 additions & 7 deletions platform/ios/src/MGLMapView.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,7 @@ - (void)handlePinchGesture:(UIPinchGestureRecognizer *)pinch

if (log2(newScale) < _mbglMap->getMinZoom()) return;

_mbglMap->setScale(newScale, { centerPoint.x, centerPoint.y });
_mbglMap->setScale(newScale, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y });

[self notifyMapChange:mbgl::MapChangeRegionIsChanging];
}
Expand Down Expand Up @@ -1183,7 +1183,7 @@ - (void)handlePinchGesture:(UIPinchGestureRecognizer *)pinch

if (velocity)
{
_mbglMap->setScale(newScale, { centerPoint.x, centerPoint.y }, MGLDurationInSeconds(duration));
_mbglMap->setScale(newScale, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y }, MGLDurationInSeconds(duration));
}

[self notifyGestureDidEndWithDrift:velocity];
Expand Down Expand Up @@ -1229,7 +1229,7 @@ - (void)handleRotateGesture:(UIRotationGestureRecognizer *)rotate
newDegrees = fmaxf(newDegrees, -30);
}

_mbglMap->setBearing(newDegrees, { centerPoint.x, centerPoint.y });
_mbglMap->setBearing(newDegrees, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y });

[self notifyMapChange:mbgl::MapChangeRegionIsChanging];
}
Expand All @@ -1244,7 +1244,7 @@ - (void)handleRotateGesture:(UIRotationGestureRecognizer *)rotate
CGFloat newRadians = radians + velocity * duration * 0.1;
CGFloat newDegrees = MGLDegreesFromRadians(newRadians) * -1;

_mbglMap->setBearing(newDegrees, { centerPoint.x, centerPoint.y }, MGLDurationInSeconds(duration));
_mbglMap->setBearing(newDegrees, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y }, MGLDurationInSeconds(duration));

[self notifyGestureDidEndWithDrift:YES];

Expand Down Expand Up @@ -1395,7 +1395,7 @@ - (void)handleQuickZoomGesture:(UILongPressGestureRecognizer *)quickZoom
centerPoint = self.userLocationAnnotationViewCenter;
}
_mbglMap->scaleBy(powf(2, newZoom) / _mbglMap->getScale(),
{ centerPoint.x, centerPoint.y });
mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y });

[self notifyMapChange:mbgl::MapChangeRegionIsChanging];
}
Expand Down Expand Up @@ -1430,7 +1430,7 @@ - (void)handleTwoFingerDragGesture:(UIPanGestureRecognizer *)twoFingerDrag
{
centerPoint = self.userLocationAnnotationViewCenter;
}
_mbglMap->setPitch(pitchNew, centerPoint);
_mbglMap->setPitch(pitchNew, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y });

[self notifyMapChange:mbgl::MapChangeRegionIsChanging];
}
Expand Down Expand Up @@ -1992,7 +1992,7 @@ - (void)_setDirection:(CLLocationDirection)direction animated:(BOOL)animated
else
{
CGPoint centerPoint = self.userLocationAnnotationViewCenter;
_mbglMap->setBearing(direction, { centerPoint.x, centerPoint.y },
_mbglMap->setBearing(direction, mbgl::ScreenCoordinate { centerPoint.x, centerPoint.y },
MGLDurationInSeconds(duration));
}
}
Expand Down
77 changes: 40 additions & 37 deletions src/mbgl/map/map.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,31 +172,29 @@ void Map::moveBy(const ScreenCoordinate& point, const Duration& duration) {
}

void Map::setLatLng(const LatLng& latLng, const Duration& duration) {
setLatLng(latLng, EdgeInsets(), duration);
setLatLng(latLng, ScreenCoordinate {}, duration);
}

void Map::setLatLng(const LatLng& latLng, const EdgeInsets& padding, const Duration& duration) {
void Map::setLatLng(const LatLng& latLng, optional<EdgeInsets> padding, const Duration& duration) {
transform->setLatLng(latLng, padding, duration);
update(Update::Repaint);
}

void Map::setLatLng(const LatLng& latLng, const ScreenCoordinate& point, const Duration& duration) {
transform->setLatLng(latLng, point, duration);
void Map::setLatLng(const LatLng& latLng, optional<ScreenCoordinate> anchor, const Duration& duration) {
transform->setLatLng(latLng, anchor, duration);
update(Update::Repaint);
}

LatLng Map::getLatLng(const EdgeInsets& padding) const {
LatLng Map::getLatLng(optional<EdgeInsets> padding) const {
return transform->getLatLng(padding);
}

void Map::resetPosition(const EdgeInsets& padding) {
void Map::resetPosition(optional<EdgeInsets> padding) {
CameraOptions camera;
camera.angle = 0;
camera.pitch = 0;
camera.center = LatLng(0, 0);
if (padding) {
camera.padding = padding;
}
camera.padding = padding;
camera.zoom = 0;
transform->jumpTo(camera);
update(Update::Zoom);
Expand All @@ -205,13 +203,13 @@ void Map::resetPosition(const EdgeInsets& padding) {

#pragma mark - Scale

void Map::scaleBy(double ds, const ScreenCoordinate& point, const Duration& duration) {
transform->scaleBy(ds, point, duration);
void Map::scaleBy(double ds, optional<ScreenCoordinate> anchor, const Duration& duration) {
transform->scaleBy(ds, anchor, duration);
update(Update::Zoom);
}

void Map::setScale(double scale, const ScreenCoordinate& point, const Duration& duration) {
transform->setScale(scale, point, duration);
void Map::setScale(double scale, optional<ScreenCoordinate> anchor, const Duration& duration) {
transform->setScale(scale, anchor, duration);
update(Update::Zoom);
}

Expand All @@ -223,7 +221,7 @@ void Map::setZoom(double zoom, const Duration& duration) {
setZoom(zoom, {}, duration);
}

void Map::setZoom(double zoom, const EdgeInsets& padding, const Duration& duration) {
void Map::setZoom(double zoom, optional<EdgeInsets> padding, const Duration& duration) {
transform->setZoom(zoom, padding, duration);
update(Update::Zoom);
}
Expand All @@ -236,12 +234,12 @@ void Map::setLatLngZoom(const LatLng& latLng, double zoom, const Duration& durat
setLatLngZoom(latLng, zoom, {}, duration);
}

void Map::setLatLngZoom(const LatLng& latLng, double zoom, const EdgeInsets& padding, const Duration& duration) {
void Map::setLatLngZoom(const LatLng& latLng, double zoom, optional<EdgeInsets> padding, const Duration& duration) {
transform->setLatLngZoom(latLng, zoom, padding, duration);
update(Update::Zoom);
}

CameraOptions Map::cameraForLatLngBounds(const LatLngBounds& bounds, const EdgeInsets& padding) {
CameraOptions Map::cameraForLatLngBounds(const LatLngBounds& bounds, optional<EdgeInsets> padding) {
AnnotationSegment segment = {
bounds.northwest(),
bounds.southwest(),
Expand All @@ -251,7 +249,7 @@ CameraOptions Map::cameraForLatLngBounds(const LatLngBounds& bounds, const EdgeI
return cameraForLatLngs(segment, padding);
}

CameraOptions Map::cameraForLatLngs(const std::vector<LatLng>& latLngs, const EdgeInsets& padding) {
CameraOptions Map::cameraForLatLngs(const std::vector<LatLng>& latLngs, optional<EdgeInsets> padding) {
CameraOptions options;
if (latLngs.empty()) {
return options;
Expand All @@ -272,26 +270,31 @@ CameraOptions Map::cameraForLatLngs(const std::vector<LatLng>& latLngs, const Ed
double height = nePixel.y - swPixel.y;

// Calculate the zoom level.
double scaleX = (getWidth() - padding.left - padding.right) / width;
double scaleY = (getHeight() - padding.top - padding.bottom) / height;
double scaleX = getWidth() / width;
double scaleY = getHeight() / height;
if (padding && *padding) {
scaleX -= (padding->left + padding->right) / width;
scaleY -= (padding->top + padding->bottom) / height;
}
double minScale = ::fmin(scaleX, scaleY);
double zoom = ::log2(getScale() * minScale);
zoom = util::clamp(zoom, getMinZoom(), getMaxZoom());

// Calculate the center point of a virtual bounds that is extended in all directions by padding.
ScreenCoordinate paddedNEPixel = {
nePixel.x + padding.right / minScale,
nePixel.y + padding.top / minScale,
};
ScreenCoordinate paddedSWPixel = {
swPixel.x - padding.left / minScale,
swPixel.y - padding.bottom / minScale,
};
ScreenCoordinate centerPixel = {
(paddedNEPixel.x + paddedSWPixel.x) / 2,
(paddedNEPixel.y + paddedSWPixel.y) / 2,
};

ScreenCoordinate centerPixel = nePixel + swPixel;
if (padding && *padding) {
ScreenCoordinate paddedNEPixel = {
padding->right / minScale,
padding->top / minScale,
};
ScreenCoordinate paddedSWPixel = {
padding->left / minScale,
padding->bottom / minScale,
};
centerPixel = centerPixel - paddedNEPixel - paddedSWPixel;
}
centerPixel /= 2;

// CameraOptions origin is at the top-left corner.
centerPixel.y = viewportHeight - centerPixel.y;

Expand Down Expand Up @@ -349,12 +352,12 @@ void Map::setBearing(double degrees, const Duration& duration) {
setBearing(degrees, EdgeInsets(), duration);
}

void Map::setBearing(double degrees, const ScreenCoordinate& center, const Duration& duration) {
transform->setAngle(-degrees * util::DEG2RAD, center, duration);
void Map::setBearing(double degrees, optional<ScreenCoordinate> anchor, const Duration& duration) {
transform->setAngle(-degrees * util::DEG2RAD, anchor, duration);
update(Update::Repaint);
}

void Map::setBearing(double degrees, const EdgeInsets& padding, const Duration& duration) {
void Map::setBearing(double degrees, optional<EdgeInsets> padding, const Duration& duration) {
transform->setAngle(-degrees * util::DEG2RAD, padding, duration);
update(Update::Repaint);
}
Expand All @@ -372,10 +375,10 @@ void Map::resetNorth(const Duration& duration) {
#pragma mark - Pitch

void Map::setPitch(double pitch, const Duration& duration) {
setPitch(pitch, {NAN, NAN}, duration);
setPitch(pitch, {}, duration);
}

void Map::setPitch(double pitch, const ScreenCoordinate& anchor, const Duration& duration) {
void Map::setPitch(double pitch, optional<ScreenCoordinate> anchor, const Duration& duration) {
transform->setPitch(pitch * util::DEG2RAD, anchor, duration);
update(Update::Repaint);
}
Expand Down
Loading

0 comments on commit 8e30a4a

Please sign in to comment.