[core] Fix tileCover / TileCoordinate fromScreenCoordinate(#15442)

Remove unecessary tile coordinate unproject -> project  step.
Unproject+project isn't always a noop and this was the issue leading to wrong cover calculation in high pitch values.

Fixes: https://github.com/mapbox/mapbox-gl-native/issues/15442
This commit is contained in:
Aleksandar Stojiljkovic
2019-08-23 12:04:32 +03:00
parent 13d2667035
commit 9eff849c74
6 changed files with 41 additions and 16 deletions

View File

@@ -3,6 +3,7 @@
#include <mbgl/util/constants.hpp>
#include <mbgl/util/interpolate.hpp>
#include <mbgl/util/projection.hpp>
#include <mbgl/util/tile_coordinate.hpp>
#include <mbgl/math/log2.hpp>
#include <mbgl/math/clamp.hpp>
@@ -289,9 +290,9 @@ ScreenCoordinate TransformState::latLngToScreenCoordinate(const LatLng& latLng)
return { p[0] / p[3], size.height - p[1] / p[3] };
}
LatLng TransformState::screenCoordinateToLatLng(const ScreenCoordinate& point, LatLng::WrapMode wrapMode) const {
TileCoordinate TransformState::screenCoordinateToTileCoordinate(const ScreenCoordinate& point, uint8_t atZoom) const {
if (size.isEmpty()) {
return {};
return { {}, 0 };
}
float targetZ = 0;
@@ -325,7 +326,13 @@ LatLng TransformState::screenCoordinateToLatLng(const ScreenCoordinate& point, L
double z1 = coord1[2] / w1;
double t = z0 == z1 ? 0 : (targetZ - z0) / (z1 - z0);
return Projection::unproject(util::interpolate(p0, p1, t), scale / util::tileSize, wrapMode);
Point<double> p = util::interpolate(p0, p1, t) / scale * static_cast<double>(1 << atZoom);
return { { p.x, p.y }, static_cast<double>(atZoom) };
}
LatLng TransformState::screenCoordinateToLatLng(const ScreenCoordinate& point, LatLng::WrapMode wrapMode) const {
auto coord = screenCoordinateToTileCoordinate(point, 0);
return Projection::unproject(coord.p, 1 / util::tileSize, wrapMode);
}
mat4 TransformState::coordinatePointMatrix() const {

View File

@@ -17,6 +17,7 @@
namespace mbgl {
class UnwrappedTileID;
class TileCoordinate;
class TransformState {
friend class Transform;
@@ -81,6 +82,8 @@ public:
// Conversion
ScreenCoordinate latLngToScreenCoordinate(const LatLng&) const;
LatLng screenCoordinateToLatLng(const ScreenCoordinate&, LatLng::WrapMode = LatLng::Unwrapped) const;
// Implements mapbox-gl-js pointCoordinate() : MercatorCoordinate.
TileCoordinate screenCoordinateToTileCoordinate(const ScreenCoordinate&, uint8_t atZoom) const;
double zoomScale(double zoom) const;
double scaleZoom(double scale) const;

View File

@@ -13,6 +13,7 @@ using TileCoordinatePoint = Point<double>;
// Has floating point x/y coordinates.
// Used for computing the tiles that need to be visible in the viewport.
// In mapbox-gl-js, this is named MercatorCoordinate.
class TileCoordinate {
public:
TileCoordinatePoint p;
@@ -23,8 +24,8 @@ public:
return { Projection::project(latLng, scale) / util::tileSize, zoom };
}
static TileCoordinate fromScreenCoordinate(const TransformState& state, double zoom, const ScreenCoordinate& screenCoordinate) {
return fromLatLng(zoom, state.screenCoordinateToLatLng(screenCoordinate));
static TileCoordinate fromScreenCoordinate(const TransformState& state, uint8_t zoom, const ScreenCoordinate& screenCoordinate) {
return state.screenCoordinateToTileCoordinate(screenCoordinate, zoom);
}
TileCoordinate zoomTo(double zoom) const {

View File

@@ -84,7 +84,7 @@ std::vector<UnwrappedTileID> tileCover(const Point<double>& tl,
const Point<double>& br,
const Point<double>& bl,
const Point<double>& c,
int32_t z) {
uint8_t z) {
const int32_t tiles = 1 << z;
struct ID {
@@ -139,7 +139,7 @@ int32_t coveringZoomLevel(double zoom, style::SourceType type, uint16_t size) {
}
}
std::vector<UnwrappedTileID> tileCover(const LatLngBounds& bounds_, int32_t z) {
std::vector<UnwrappedTileID> tileCover(const LatLngBounds& bounds_, uint8_t z) {
if (bounds_.isEmpty() ||
bounds_.south() > util::LATITUDE_MAX ||
bounds_.north() < -util::LATITUDE_MAX) {
@@ -159,7 +159,7 @@ std::vector<UnwrappedTileID> tileCover(const LatLngBounds& bounds_, int32_t z) {
z);
}
std::vector<UnwrappedTileID> tileCover(const TransformState& state, int32_t z) {
std::vector<UnwrappedTileID> tileCover(const TransformState& state, uint8_t z) {
assert(state.valid());
const double w = state.getSize().width;
@@ -173,7 +173,7 @@ std::vector<UnwrappedTileID> tileCover(const TransformState& state, int32_t z) {
z);
}
std::vector<UnwrappedTileID> tileCover(const Geometry<double>& geometry, int32_t z) {
std::vector<UnwrappedTileID> tileCover(const Geometry<double>& geometry, uint8_t z) {
std::vector<UnwrappedTileID> result;
TileCover tc(geometry, z, true);
while (tc.hasNext()) {
@@ -213,7 +213,7 @@ uint64_t tileCount(const Geometry<double>& geometry, uint8_t z) {
return tileCount;
}
TileCover::TileCover(const LatLngBounds&bounds_, int32_t z) {
TileCover::TileCover(const LatLngBounds&bounds_, uint8_t z) {
LatLngBounds bounds = LatLngBounds::hull(
{ std::max(bounds_.south(), -util::LATITUDE_MAX), bounds_.west() },
{ std::min(bounds_.north(), util::LATITUDE_MAX), bounds_.east() });
@@ -233,7 +233,7 @@ TileCover::TileCover(const LatLngBounds&bounds_, int32_t z) {
impl = std::make_unique<TileCover::Impl>(z, p, false);
}
TileCover::TileCover(const Geometry<double>& geom, int32_t z, bool project/* = true*/)
TileCover::TileCover(const Geometry<double>& geom, uint8_t z, bool project/* = true*/)
: impl( std::make_unique<TileCover::Impl>(z, geom, project)) {
}

View File

@@ -18,9 +18,9 @@ namespace util {
// Helper class to stream tile-cover results per row
class TileCover {
public:
TileCover(const LatLngBounds&, int32_t z);
TileCover(const LatLngBounds&, uint8_t z);
// When project == true, projects the geometry points to tile coordinates
TileCover(const Geometry<double>&, int32_t z, bool project = true);
TileCover(const Geometry<double>&, uint8_t z, bool project = true);
~TileCover();
optional<UnwrappedTileID> next();
@@ -33,9 +33,9 @@ private:
int32_t coveringZoomLevel(double z, style::SourceType type, uint16_t tileSize);
std::vector<UnwrappedTileID> tileCover(const TransformState&, int32_t z);
std::vector<UnwrappedTileID> tileCover(const LatLngBounds&, int32_t z);
std::vector<UnwrappedTileID> tileCover(const Geometry<double>&, int32_t z);
std::vector<UnwrappedTileID> tileCover(const TransformState&, uint8_t z);
std::vector<UnwrappedTileID> tileCover(const LatLngBounds&, uint8_t z);
std::vector<UnwrappedTileID> tileCover(const Geometry<double>&, uint8_t z);
// Compute only the count of tiles needed for tileCover
uint64_t tileCount(const LatLngBounds&, uint8_t z);

View File

@@ -43,6 +43,20 @@ TEST(TileCover, Pitch) {
util::tileCover(transform.getState(), 2));
}
TEST(TileCover, PitchIssue15442) {
Transform transform;
transform.resize({ 412, 691 });
transform.jumpTo(CameraOptions().withCenter(LatLng { 59.116898740996106, 91.565660781803615, })
.withZoom(2.0551126748417214).withBearing(0.74963938256567264 * util::RAD2DEG)
.withPitch(1.0471975511965976 * util::RAD2DEG));
EXPECT_EQ((std::vector<UnwrappedTileID>{
{ 2, 3, 1 }, { 2, 2, 1 }, { 2, 3, 0 }, { 2, 2, 0 }, { 1, { 2, 0, 0 } }, { 1, { 2, 1, 0 } }
}),
util::tileCover(transform.getState(), 2));
}
TEST(TileCover, PitchOverAllowedByContentInsets) {
Transform transform;
transform.resize({ 512, 512 });