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

[core] Limit pitch based on edge insets. Fix max Z calculation in getProjMatrix. #15195

Merged
merged 2 commits into from
Aug 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/mbgl/util/projection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class Projection {
return Point<double> {
util::LONGITUDE_MAX + latLng.longitude(),
util::LONGITUDE_MAX - util::RAD2DEG * std::log(std::tan(M_PI / 4 + latitude * M_PI / util::DEGREES_MAX))
} * worldSize / util::DEGREES_MAX;
} * (worldSize / util::DEGREES_MAX);
}
};

Expand Down
6 changes: 6 additions & 0 deletions platform/android/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

Mapbox welcomes participation and contributions from everyone. If you'd like to do so please see the [`Contributing Guide`](https://github.com/mapbox/mapbox-gl-native/blob/master/CONTRIBUTING.md) first to get started.

## master

### Bug fixes

- Fixed an issue where it was possible to set the map’s content insets then tilt the map enough to see the horizon, causing performance issues [#15195](https://github.com/mapbox/mapbox-gl-native/pull/15195)

## 8.0.2 - July 31, 2019
[Changes](https://github.com/mapbox/mapbox-gl-native/compare/android-v8.0.1...android-v8.0.2) since [Mapbox Maps SDK for Android v8.0.1](https://github.com/mapbox/mapbox-gl-native/releases/tag/android-v8.0.1):

Expand Down
6 changes: 6 additions & 0 deletions platform/ios/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

Mapbox welcomes participation and contributions from everyone. Please read [CONTRIBUTING.md](../../CONTRIBUTING.md) to get started.

## master

### Styles and rendering

* Fixed an issue where it was possible to set the map’s content insets then tilt the map enough to see the horizon, causing performance issues. ([#15195](https://github.com/mapbox/mapbox-gl-native/pull/15195))

## 5.3.0

### Styles and rendering
Expand Down
1 change: 1 addition & 0 deletions platform/macos/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## master

* Fixed an issue where it was possible to set the map’s content insets then tilt the map enough to see the horizon, causing performance issues. ([#15195](https://github.com/mapbox/mapbox-gl-native/pull/15195))
* Added an `MGLMapView.prefetchesTiles` property to configure lower-resolution tile prefetching behavior. ([#14816](https://github.com/mapbox/mapbox-gl-native/pull/14816))
* Fixed queryRenderedFeatues bug caused by incorrect sort feature index calculation. ([#14884](https://github.com/mapbox/mapbox-gl-native/pull/14884))
* Ideographic glyphs from Chinese, Japanese, and Korean are no longer downloaded by default as part of offline packs; they are instead rendered on-device, saving bandwidth and storage while improving performance. ([#14176](https://github.com/mapbox/mapbox-gl-native/pull/14176))
Expand Down
31 changes: 25 additions & 6 deletions src/mbgl/map/transform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,6 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim
if (bearing != startBearing) {
state.bearing = util::wrap(util::interpolate(startBearing, bearing, t), -M_PI, M_PI);
}
if (pitch != startPitch) {
state.pitch = util::interpolate(startPitch, pitch, t);
}
if (padding != startEdgeInsets) {
// Interpolate edge insets
state.edgeInsets = {
Expand All @@ -150,6 +147,10 @@ void Transform::easeTo(const CameraOptions& camera, const AnimationOptions& anim
util::interpolate(startEdgeInsets.right(), padding.right(), t)
};
}
auto maxPitch = getMaxPitchForEdgeInsets(state.edgeInsets);
if (pitch != startPitch || maxPitch < startPitch) {
state.pitch = std::min(maxPitch, util::interpolate(startPitch, pitch, t));
}
}, duration);
}

Expand Down Expand Up @@ -302,9 +303,6 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima
if (bearing != startBearing) {
state.bearing = util::wrap(util::interpolate(startBearing, bearing, k), -M_PI, M_PI);
}
if (pitch != startPitch) {
state.pitch = util::interpolate(startPitch, pitch, k);
}
if (padding != startEdgeInsets) {
// Interpolate edge insets
state.edgeInsets = {
Expand All @@ -314,6 +312,10 @@ void Transform::flyTo(const CameraOptions &camera, const AnimationOptions &anima
util::interpolate(startEdgeInsets.right(), padding.right(), us)
};
}
auto maxPitch = getMaxPitchForEdgeInsets(state.edgeInsets);
astojilj marked this conversation as resolved.
Show resolved Hide resolved
if (pitch != startPitch || maxPitch < startPitch) {
state.pitch = std::min(maxPitch, util::interpolate(startPitch, pitch, k));
}
}, duration);
}

Expand Down Expand Up @@ -576,4 +578,21 @@ LatLng Transform::screenCoordinateToLatLng(const ScreenCoordinate& point, LatLng
return state.screenCoordinateToLatLng(flippedPoint, wrapMode);
}

double Transform::getMaxPitchForEdgeInsets(const EdgeInsets &insets) const
astojilj marked this conversation as resolved.
Show resolved Hide resolved
{
double centerOffsetY = 0.5 * (insets.top() - insets.bottom()); // See TransformState::getCenterOffset.

const auto height = state.size.height;
assert(height);
astojilj marked this conversation as resolved.
Show resolved Hide resolved
// For details, see description at https://github.com/mapbox/mapbox-gl-native/pull/15195
// The definition of half of TransformState::fov with no inset, is: fov = arctan((height / 2) / (height * 1.5)).
// We use half of fov, as it is field of view above perspective center.
// With inset, this angle changes and tangentOfFovAboveCenterAngle = (h/2 + centerOffsetY) / (height * 1.5).
// 1.03 is a bit extra added to prevent parallel ground to viewport clipping plane.
const double tangentOfFovAboveCenterAngle = 1.03 * (height / 2.0 + centerOffsetY) / (1.5 * height);
const double fovAboveCenter = std::atan(tangentOfFovAboveCenterAngle);
return M_PI * 0.5 - fovAboveCenter;
// e.g. Maximum pitch of 60 degrees is when perspective center's offset from the top is 84% of screen height.
}

} // namespace mbgl
3 changes: 3 additions & 0 deletions src/mbgl/map/transform.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ class Transform : private util::noncopyable {
std::function<void(double)>,
const Duration&);

// We don't want to show horizon: limit max pitch based on edge insets.
double getMaxPitchForEdgeInsets(const EdgeInsets &insets) const;

TimePoint transitionStart;
Duration transitionDuration;
std::function<bool(const TimePoint)> transitionFrameFn;
Expand Down
20 changes: 10 additions & 10 deletions src/mbgl/map/transform_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,18 @@ void TransformState::getProjMatrix(mat4& projMatrix, uint16_t nearZ, bool aligne
const double cameraToCenterDistance = getCameraToCenterDistance();
auto offset = getCenterOffset();

// Find the distance from the viewport center point
// [width/2 + offset.x, height/2 + offset.y] to the top edge, to point
// [width/2 + offset.x, 0] in Z units, using the law of sines.
// Find the Z distance from the viewport center point
// [width/2 + offset.x, height/2 + offset.y] to the top edge; to point
// [width/2 + offset.x, 0] in Z units.
// 1 Z unit is equivalent to 1 horizontal px at the center of the map
// (the distance between[width/2, height/2] and [width/2 + 1, height/2])
const double fovAboveCenter = getFieldOfView() * (0.5 + offset.y / size.height);
const double groundAngle = M_PI / 2.0 + getPitch();
const double aboveCenterSurfaceDistance = std::sin(fovAboveCenter) * cameraToCenterDistance / std::sin(M_PI - groundAngle - fovAboveCenter);


// See https://github.com/mapbox/mapbox-gl-native/pull/15195 for details.
// See TransformState::fov description: fov = 2 * arctan((height / 2) / (height * 1.5)).
const double tanFovAboveCenter = (size.height * 0.5 + offset.y) / (size.height * 1.5);
const double tanMultiple = tanFovAboveCenter * std::tan(getPitch());
assert(tanMultiple < 1);
// Calculate z distance of the farthest fragment that should be rendered.
const double furthestDistance = std::cos(M_PI / 2 - getPitch()) * aboveCenterSurfaceDistance + cameraToCenterDistance;
const double furthestDistance = cameraToCenterDistance / (1 - tanMultiple);
// Add a bit extra to avoid precision problems when a fragment's distance is exactly `furthestDistance`
const double farZ = furthestDistance * 1.01;

Expand All @@ -64,7 +64,7 @@ void TransformState::getProjMatrix(mat4& projMatrix, uint16_t nearZ, bool aligne
const bool flippedY = viewportMode == ViewportMode::FlippedY;
matrix::scale(projMatrix, projMatrix, 1.0, flippedY ? 1 : -1, 1);

matrix::translate(projMatrix, projMatrix, 0, 0, -getCameraToCenterDistance());
matrix::translate(projMatrix, projMatrix, 0, 0, -cameraToCenterDistance);

using NO = NorthOrientation;
switch (getNorthOrientation()) {
Expand Down
3 changes: 3 additions & 0 deletions test/map/transform.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ using namespace mbgl;

TEST(Transform, InvalidZoom) {
Transform transform;
transform.resize({1, 1});

ASSERT_DOUBLE_EQ(0, transform.getLatLng().latitude());
ASSERT_DOUBLE_EQ(0, transform.getLatLng().longitude());
Expand Down Expand Up @@ -56,6 +57,7 @@ TEST(Transform, InvalidZoom) {

TEST(Transform, InvalidBearing) {
Transform transform;
transform.resize({1, 1});

ASSERT_DOUBLE_EQ(0, transform.getLatLng().latitude());
ASSERT_DOUBLE_EQ(0, transform.getLatLng().longitude());
Expand All @@ -78,6 +80,7 @@ TEST(Transform, InvalidBearing) {

TEST(Transform, IntegerZoom) {
Transform transform;
transform.resize({1, 1});

auto checkIntegerZoom = [&transform](uint8_t zoomInt, double zoom) {
transform.jumpTo(CameraOptions().withZoom(zoom));
Expand Down
35 changes: 35 additions & 0 deletions test/util/tile_cover.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,41 @@ TEST(TileCover, Pitch) {
util::tileCover(transform.getState(), 2));
}

TEST(TileCover, PitchOverAllowedByContentInsets) {
Transform transform;
transform.resize({ 512, 512 });

transform.jumpTo(CameraOptions().withCenter(LatLng { 0.1, -0.1 }).withPadding(EdgeInsets { 376, 0, 0, 0 })
.withZoom(8.0).withBearing(45.0).withPitch(60.0));
// Top padding of 376 leads to capped pitch. See Transform::getMaxPitchForEdgeInsets.
EXPECT_LE(transform.getPitch() + 0.001, util::DEG2RAD * 60);

EXPECT_EQ((std::vector<UnwrappedTileID>{
{ 3, 4, 3 }, { 3, 3, 3 }, { 3, 4, 4 }, { 3, 3, 4 }, { 3, 4, 2 }, { 3, 5, 3 }, { 3, 5, 2 }
}),
util::tileCover(transform.getState(), 3));
}

TEST(TileCover, PitchWithLargerResultSet) {
Transform transform;
transform.resize({ 1024, 768 });

// The values used here triggered the regression with left and right edge
// selection in tile_cover.cpp scanSpans.
transform.jumpTo(CameraOptions().withCenter(LatLng { 0.1, -0.1 }).withPadding(EdgeInsets { 400, 0, 0, 0 })
.withZoom(5).withBearing(-142.2630000003529176).withPitch(60.0));

auto cover = util::tileCover(transform.getState(), 5);
// Returned vector has above 100 elements, we check first 16 as there is a
// plan to return lower LOD for distant tiles.
EXPECT_EQ((std::vector<UnwrappedTileID> {
{ 5, 15, 16 }, { 5, 15, 17 }, { 5, 14, 16 }, { 5, 14, 17 },
{ 5, 16, 16 }, { 5, 16, 17 }, { 5, 15, 15 }, { 5, 14, 15 },
{ 5, 15, 18 }, { 5, 14, 18 }, { 5, 16, 15 }, { 5, 13, 16 },
{ 5, 13, 17 }, { 5, 16, 18 }, { 5, 13, 18 }, { 5, 15, 19 }
}), (std::vector<UnwrappedTileID> { cover.begin(), cover.begin() + 16}) );
}

TEST(TileCover, WorldZ1) {
EXPECT_EQ((std::vector<UnwrappedTileID>{
{ 1, 0, 0 }, { 1, 0, 1 }, { 1, 1, 0 }, { 1, 1, 1 },
Expand Down