From 0b411ca1cd95c38e438df6d4b35678be7fe43a38 Mon Sep 17 00:00:00 2001 From: Aleksandar Stojiljkovic Date: Tue, 23 Jul 2019 17:19:52 +0300 Subject: [PATCH 1/2] [core] Limit pitch based on edge insets. Fix max Z calculation in getProjMatrix. Patch partly fixes #15163 in a way that it doesn't allow loading tens of thousands of tiles and attempt to show area above horizon: Limit pitch based on edge insets. It is not too bad - current limit of 60 degrees stays active until center of perspective is moved towards the bottom, to 84% of screen height. The plan is to split removal of 60 degrees limit to follow up patch. Fix max Z calculation in getProjMatrix. TransformState::getProjMatrix calculation of farZ was complex with possibility to lead to negative z values. Replacing it with simpler, precise calculation: furthestDistance = cameraToCenterDistance / (1 - tanFovAboveCenter * std::tan(getPitch())); TransformState::getProjMatrix calculation of farZ was an aproximation. Replacing it with simpler, but precise calculation. Related to: #15163 --- include/mbgl/util/projection.hpp | 2 +- platform/ios/vendor/mapbox-events-ios | 2 +- src/mbgl/map/transform.cpp | 31 +++++++++++++++++++----- src/mbgl/map/transform.hpp | 3 +++ src/mbgl/map/transform_state.cpp | 20 +++++++-------- test/map/transform.test.cpp | 3 +++ test/util/tile_cover.test.cpp | 35 +++++++++++++++++++++++++++ 7 files changed, 78 insertions(+), 18 deletions(-) diff --git a/include/mbgl/util/projection.hpp b/include/mbgl/util/projection.hpp index 9619a380b42..f0b9115eb07 100644 --- a/include/mbgl/util/projection.hpp +++ b/include/mbgl/util/projection.hpp @@ -98,7 +98,7 @@ class Projection { return Point { 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); } }; diff --git a/platform/ios/vendor/mapbox-events-ios b/platform/ios/vendor/mapbox-events-ios index 2e6bbd9f3c8..9f1be0cd2a8 160000 --- a/platform/ios/vendor/mapbox-events-ios +++ b/platform/ios/vendor/mapbox-events-ios @@ -1 +1 @@ -Subproject commit 2e6bbd9f3c898f33a49e20fb97eca86eeb6351e4 +Subproject commit 9f1be0cd2a8d15312f40343a9d9615b16dcc8a62 diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp index a51d4dd4ff2..e360150cc20 100644 --- a/src/mbgl/map/transform.cpp +++ b/src/mbgl/map/transform.cpp @@ -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 = { @@ -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); } @@ -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 = { @@ -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); + if (pitch != startPitch || maxPitch < startPitch) { + state.pitch = std::min(maxPitch, util::interpolate(startPitch, pitch, k)); + } }, duration); } @@ -576,4 +578,21 @@ LatLng Transform::screenCoordinateToLatLng(const ScreenCoordinate& point, LatLng return state.screenCoordinateToLatLng(flippedPoint, wrapMode); } +double Transform::getMaxPitchForEdgeInsets(const EdgeInsets &insets) const +{ + double centerOffsetY = 0.5 * (insets.top() - insets.bottom()); // See TransformState::getCenterOffset. + + const auto height = state.size.height; + assert(height); + // 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 diff --git a/src/mbgl/map/transform.hpp b/src/mbgl/map/transform.hpp index 0c018a6e487..75dfeff6459 100644 --- a/src/mbgl/map/transform.hpp +++ b/src/mbgl/map/transform.hpp @@ -115,6 +115,9 @@ class Transform : private util::noncopyable { std::function, 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 transitionFrameFn; diff --git a/src/mbgl/map/transform_state.cpp b/src/mbgl/map/transform_state.cpp index 92c02d0bc76..77309a2a55b 100644 --- a/src/mbgl/map/transform_state.cpp +++ b/src/mbgl/map/transform_state.cpp @@ -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; @@ -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()) { diff --git a/test/map/transform.test.cpp b/test/map/transform.test.cpp index 3d37312b170..84fdb06b211 100644 --- a/test/map/transform.test.cpp +++ b/test/map/transform.test.cpp @@ -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()); @@ -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()); @@ -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)); diff --git a/test/util/tile_cover.test.cpp b/test/util/tile_cover.test.cpp index 3fc7681520d..e35e6e2e994 100644 --- a/test/util/tile_cover.test.cpp +++ b/test/util/tile_cover.test.cpp @@ -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{ + { 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 { + { 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 { cover.begin(), cover.begin() + 16}) ); +} + TEST(TileCover, WorldZ1) { EXPECT_EQ((std::vector{ { 1, 0, 0 }, { 1, 0, 1 }, { 1, 1, 0 }, { 1, 1, 1 }, From f37d81d2f1c79d6b99698483c7253920234c9680 Mon Sep 17 00:00:00 2001 From: Aleksandar Stojiljkovic Date: Thu, 1 Aug 2019 08:13:16 +0300 Subject: [PATCH 2/2] [core] Changelog entry for #15195 --- platform/android/CHANGELOG.md | 6 ++++++ platform/ios/CHANGELOG.md | 6 ++++++ platform/macos/CHANGELOG.md | 1 + 3 files changed, 13 insertions(+) diff --git a/platform/android/CHANGELOG.md b/platform/android/CHANGELOG.md index fe41ebd4afb..8e7bd0ed606 100644 --- a/platform/android/CHANGELOG.md +++ b/platform/android/CHANGELOG.md @@ -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): diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index 5f2c0478dde..8fe5ead4452 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -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 diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md index 68b7e243dad..6e32ad5c8be 100644 --- a/platform/macos/CHANGELOG.md +++ b/platform/macos/CHANGELOG.md @@ -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))