[core] Take the simple approach

7 files changed, 62 insertions, 151 deletions

diff --git a/benchmark/api/query.benchmark.cpp b/benchmark/api/query.benchmark.cpp
index 5e7fd823e0..12a28b95e7 100644
--- a/benchmark/api/query.benchmark.cpp
+++ b/benchmark/api/query.benchmark.cpp
@@ -38,6 +38,30 @@ public:
 
 } // end namespace
 
+static void API_queryPixelsForLatLngs(::benchmark::State& state) {
+    std::vector<LatLng> points;
+    int count = 10000;
+    for (int i = 0; i < count; ++i) {
+        points.emplace_back(1, 1);
+    }
+    QueryBenchmark bench;
+    while (state.KeepRunning()) {
+        (void)bench.map.pixelsForLatLngs(points);
+    }
+}
+
+static void API_queryLatLngsForPixels(::benchmark::State& state) {
+    std::vector<ScreenCoordinate> points;
+    int count = 10000;
+    for (int i = 0; i < count; ++i) {
+        points.emplace_back(1, 1);
+    }
+    QueryBenchmark bench;
+    while (state.KeepRunning()) {
+        (void)bench.map.latLngsForPixels(points);
+    }
+}
+
 static void API_queryRenderedFeaturesAll(::benchmark::State& state) {
     QueryBenchmark bench;
 
@@ -61,7 +85,8 @@ static void API_queryRenderedFeaturesLayerFromHighDensity(::benchmark::State& st
         bench.frontend.getRenderer()->queryRenderedFeatures(bench.box, {{{"road-street" }}, {}});
     }
 }
-
+BENCHMARK(API_queryPixelsForLatLngs);
+BENCHMARK(API_queryLatLngsForPixels);
 BENCHMARK(API_queryRenderedFeaturesAll);
 BENCHMARK(API_queryRenderedFeaturesLayerFromLowDensity);
 BENCHMARK(API_queryRenderedFeaturesLayerFromHighDensity);
diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
index 7714227e4d..3ea2fc43c6 100644
--- a/src/mbgl/map/map.cpp
+++ b/src/mbgl/map/map.cpp
@@ -183,8 +183,8 @@ CameraOptions cameraForLatLngs(const std::vector<LatLng>& latLngs, const Transfo
     ScreenCoordinate nePixel = {-INFINITY, -INFINITY};
     ScreenCoordinate swPixel = {INFINITY, INFINITY};
     double viewportHeight = size.height;
-    const std::vector<ScreenCoordinate> pixels = transform.latLngsToScreenCoordinates(latLngs);
-    for (const auto& pixel : pixels) {
+    for (LatLng latLng : latLngs) {
+        ScreenCoordinate pixel = transform.latLngToScreenCoordinate(latLng);
         swPixel.x = std::min(swPixel.x, pixel.x);
         nePixel.x = std::max(nePixel.x, pixel.x);
         swPixel.y = std::min(swPixel.y, viewportHeight - pixel.y);
@@ -365,18 +365,21 @@ LatLng Map::latLngForPixel(const ScreenCoordinate& pixel) const {
 }
 
 std::vector<ScreenCoordinate> Map::pixelsForLatLngs(const std::vector<LatLng>& latLngs) const {
-    std::vector<LatLng> unwrappedLatLngs;
-    unwrappedLatLngs.reserve(latLngs.size());
+    std::vector<ScreenCoordinate> ret;
+    ret.reserve(latLngs.size());
     for (const auto& latLng : latLngs) {
-        LatLng unwrappedLatLng = latLng.wrapped();
-        unwrappedLatLng.unwrapForShortestPath(impl->transform.getLatLng());
-        unwrappedLatLngs.emplace_back(unwrappedLatLng);
+        ret.emplace_back(pixelForLatLng(latLng));
     }
-    return impl->transform.latLngsToScreenCoordinates(unwrappedLatLngs);
+    return ret;
 }
 
 std::vector<LatLng> Map::latLngsForPixels(const std::vector<ScreenCoordinate>& screenCoords) const {
-    return impl->transform.screenCoordinatesToLatLngs(screenCoords);
+    std::vector<LatLng> ret;
+    ret.reserve(screenCoords.size());
+    for (const auto& point : screenCoords) {
+        ret.emplace_back(latLngForPixel(point));
+    }
+    return ret;
 }
 
 #pragma mark - Annotations
diff --git a/src/mbgl/map/transform.cpp b/src/mbgl/map/transform.cpp
index 66edf0bba4..e88bb5465c 100644
--- a/src/mbgl/map/transform.cpp
+++ b/src/mbgl/map/transform.cpp
@@ -590,23 +590,6 @@ LatLng Transform::screenCoordinateToLatLng(const ScreenCoordinate& point, LatLng
     return state.screenCoordinateToLatLng(flippedPoint, wrapMode);
 }
 
-std::vector<ScreenCoordinate> Transform::latLngsToScreenCoordinates(const std::vector<LatLng>& latLngs) const {
-    std::vector<ScreenCoordinate> points = state.latLngsToScreenCoordinates(latLngs);
-    for (auto& point : points) {
-        point.y = state.size.height - point.y;
-    }
-    return points;
-}
-
-std::vector<LatLng> Transform::screenCoordinatesToLatLngs(const std::vector<ScreenCoordinate>& points,
-                                                          LatLng::WrapMode wrapMode) const {
-    auto flippedPoints = points;
-    for (auto& flippedPoint : flippedPoints) {
-        flippedPoint.y = state.size.height - flippedPoint.y;
-    }
-    return state.screenCoordinatesToLatLngs(flippedPoints, wrapMode);
-}
-
 double Transform::getMaxPitchForEdgeInsets(const EdgeInsets &insets) const
 {
     double centerOffsetY = 0.5 * (insets.top() - insets.bottom()); // See TransformState::getCenterOffset.
diff --git a/src/mbgl/map/transform.hpp b/src/mbgl/map/transform.hpp
index 8b2928ee2e..30ce8a37a4 100644
--- a/src/mbgl/map/transform.hpp
+++ b/src/mbgl/map/transform.hpp
@@ -109,9 +109,6 @@ public:
     // Conversion and projection
     ScreenCoordinate latLngToScreenCoordinate(const LatLng&) const;
     LatLng screenCoordinateToLatLng(const ScreenCoordinate&, LatLng::WrapMode = LatLng::Wrapped) const;
-    std::vector<ScreenCoordinate> latLngsToScreenCoordinates(const std::vector<LatLng>&) const;
-    std::vector<LatLng> screenCoordinatesToLatLngs(const std::vector<ScreenCoordinate>&,
-                                                   LatLng::WrapMode = LatLng::Wrapped) const;
 
 private:
     MapObserver& observer;
diff --git a/src/mbgl/map/transform_state.cpp b/src/mbgl/map/transform_state.cpp
index 04f227cd77..61007422cb 100644
--- a/src/mbgl/map/transform_state.cpp
+++ b/src/mbgl/map/transform_state.cpp
@@ -276,40 +276,6 @@ double TransformState::zoomScale(double zoom) const {
 double TransformState::scaleZoom(double s) const {
     return util::log2(s);
 }
-namespace {
-mbgl::TileCoordinate getTileCoordinate(const ScreenCoordinate& point,
-                                       const mbgl::Size& size,
-                                       const mbgl::mat4& inverted,
-                                       const uint8_t atZoom,
-                                       const double scale) {
-    float targetZ = 0;
-    double flippedY = size.height - point.y;
-
-    // since we don't know the correct projected z value for the point,
-    // unproject two points to get a line and then find the point on that
-    // line with z=0
-
-    vec4 coord0;
-    vec4 coord1;
-    vec4 point0 = {{point.x, flippedY, 0, 1}};
-    vec4 point1 = {{point.x, flippedY, 1, 1}};
-    matrix::transformMat4(coord0, point0, inverted);
-    matrix::transformMat4(coord1, point1, inverted);
-
-    double w0 = coord0[3];
-    double w1 = coord1[3];
-
-    Point<double> p0 = Point<double>(coord0[0], coord0[1]) / w0;
-    Point<double> p1 = Point<double>(coord1[0], coord1[1]) / w1;
-
-    double z0 = coord0[2] / w0;
-    double z1 = coord1[2] / w1;
-    double t = z0 == z1 ? 0 : (targetZ - z0) / (z1 - z0);
-
-    Point<double> p = util::interpolate(p0, p1, t) / scale * static_cast<double>(1 << atZoom);
-    return {{p.x, p.y}, static_cast<double>(atZoom)};
-}
-} // namespace
 
 ScreenCoordinate TransformState::latLngToScreenCoordinate(const LatLng& latLng) const {
     if (size.isEmpty()) {
@@ -324,54 +290,44 @@ ScreenCoordinate TransformState::latLngToScreenCoordinate(const LatLng& latLng)
     return { p[0] / p[3], size.height - p[1] / p[3] };
 }
 
-std::vector<ScreenCoordinate> TransformState::latLngsToScreenCoordinates(const std::vector<LatLng>& latLngs) const {
-    if (size.isEmpty()) {
-        return {};
-    }
-    std::vector<ScreenCoordinate> points;
-    points.reserve(latLngs.size());
-    mat4 mat = coordinatePointMatrix();
-    vec4 p;
-    for (const auto& latLng : latLngs) {
-        Point<double> pt = Projection::project(latLng, scale) / util::tileSize;
-        vec4 c = {{pt.x, pt.y, 0, 1}};
-        matrix::transformMat4(p, c, mat);
-        points.emplace_back(p[0] / p[3], size.height - p[1] / p[3]);
-    }
-    return points;
-}
-
 TileCoordinate TransformState::screenCoordinateToTileCoordinate(const ScreenCoordinate& point, uint8_t atZoom) const {
     if (size.isEmpty()) {
         return { {}, 0 };
     }
+
+    float targetZ = 0;
     mat4 mat = coordinatePointMatrix();
 
     mat4 inverted;
     bool err = matrix::invert(inverted, mat);
 
     if (err) throw std::runtime_error("failed to invert coordinatePointMatrix");
-    return getTileCoordinate(point, size, inverted, atZoom, scale);
-}
 
-std::vector<TileCoordinate> TransformState::screenCoordinatesToTileCoordinates(
-    const std::vector<ScreenCoordinate>& points, uint8_t atZoom) const {
-    if (size.isEmpty()) {
-        return {{{}, 0}};
-    }
-    mat4 mat = coordinatePointMatrix();
+    double flippedY = size.height - point.y;
 
-    mat4 inverted;
-    bool err = matrix::invert(inverted, mat);
+    // since we don't know the correct projected z value for the point,
+    // unproject two points to get a line and then find the point on that
+    // line with z=0
 
-    if (err) throw std::runtime_error("failed to invert coordinatePointMatrix");
+    vec4 coord0;
+    vec4 coord1;
+    vec4 point0 = {{ point.x, flippedY, 0, 1 }};
+    vec4 point1 = {{ point.x, flippedY, 1, 1 }};
+    matrix::transformMat4(coord0, point0, inverted);
+    matrix::transformMat4(coord1, point1, inverted);
 
-    std::vector<TileCoordinate> coords;
-    coords.reserve(points.size());
-    for (const auto& point : points) {
-        coords.emplace_back(getTileCoordinate(point, size, inverted, atZoom, scale));
-    }
-    return coords;
+    double w0 = coord0[3];
+    double w1 = coord1[3];
+
+    Point<double> p0 = Point<double>(coord0[0], coord0[1]) / w0;
+    Point<double> p1 = Point<double>(coord1[0], coord1[1]) / w1;
+
+    double z0 = coord0[2] / w0;
+    double z1 = coord1[2] / w1;
+    double t = z0 == z1 ? 0 : (targetZ - z0) / (z1 - z0);
+
+    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 {
@@ -379,17 +335,6 @@ LatLng TransformState::screenCoordinateToLatLng(const ScreenCoordinate& point, L
     return Projection::unproject(coord.p, 1 / util::tileSize, wrapMode);
 }
 
-std::vector<LatLng> TransformState::screenCoordinatesToLatLngs(const std::vector<ScreenCoordinate>& points,
-                                                               LatLng::WrapMode wrapMode) const {
-    const auto coords = screenCoordinatesToTileCoordinates(points, 0);
-    std::vector<LatLng> latLngs;
-    latLngs.reserve(coords.size());
-    for (const auto& coord : coords) {
-        latLngs.emplace_back(Projection::unproject(coord.p, 1 / util::tileSize, wrapMode));
-    }
-    return latLngs;
-}
-
 mat4 TransformState::coordinatePointMatrix() const {
     mat4 proj;
     getProjMatrix(proj);
diff --git a/src/mbgl/map/transform_state.hpp b/src/mbgl/map/transform_state.hpp
index deab91dc78..10a92187d5 100644
--- a/src/mbgl/map/transform_state.hpp
+++ b/src/mbgl/map/transform_state.hpp
@@ -82,13 +82,8 @@ public:
     // Conversion
     ScreenCoordinate latLngToScreenCoordinate(const LatLng&) const;
     LatLng screenCoordinateToLatLng(const ScreenCoordinate&, LatLng::WrapMode = LatLng::Unwrapped) const;
-    std::vector<ScreenCoordinate> latLngsToScreenCoordinates(const std::vector<LatLng>&) const;
-    std::vector<LatLng> screenCoordinatesToLatLngs(const std::vector<ScreenCoordinate>&,
-                                                   LatLng::WrapMode = LatLng::Unwrapped) const;
     // Implements mapbox-gl-js pointCoordinate() : MercatorCoordinate.
     TileCoordinate screenCoordinateToTileCoordinate(const ScreenCoordinate&, uint8_t atZoom) const;
-    std::vector<TileCoordinate> screenCoordinatesToTileCoordinates(const std::vector<ScreenCoordinate>&,
-                                                                   uint8_t atZoom) const;
 
     double zoomScale(double zoom) const;
     double scaleZoom(double scale) const;
diff --git a/test/map/transform.test.cpp b/test/map/transform.test.cpp
index ec2abe9a84..84fdb06b21 100644
--- a/test/map/transform.test.cpp
+++ b/test/map/transform.test.cpp
@@ -123,14 +123,6 @@ TEST(Transform, PerspectiveProjection) {
     ASSERT_NEAR(-76.75823239205641, loc.longitude(), 1e-6);
     ASSERT_NEAR(37.692872969426375, loc.latitude(), 1e-6);
 
-    // Using batch conversion
-    const auto locs = transform.getState().screenCoordinatesToLatLngs({{0, 1000}, {1000, 0}});
-    ASSERT_EQ(2, locs.size());
-    ASSERT_NEAR(-77.59198961199148, locs.front().longitude(), 1e-6);
-    ASSERT_NEAR(38.74661326302018, locs.front().latitude(), 1e-6);
-    ASSERT_NEAR(-76.75823239205641, locs.back().longitude(), 1e-6);
-    ASSERT_NEAR(37.692872969426375, locs.back().latitude(), 1e-6);
-
     ScreenCoordinate point = transform.getState().latLngToScreenCoordinate({38.74661326302018, -77.59198961199148});
     ASSERT_NEAR(point.x, 0.0, 1e-5);
     ASSERT_NEAR(point.y, 1000.0, 1e-4);
@@ -138,15 +130,6 @@ TEST(Transform, PerspectiveProjection) {
     point = transform.getState().latLngToScreenCoordinate({37.692872969426375, -76.75823239205641});
     ASSERT_NEAR(point.x, 1000.0, 1e-5);
     ASSERT_NEAR(point.y, 0.0, 1e-4);
-
-    // Using batch conversion
-    const auto points = transform.getState().latLngsToScreenCoordinates(
-        {{38.74661326302018, -77.59198961199148}, {37.692872969426375, -76.75823239205641}});
-    ASSERT_EQ(2, points.size());
-    ASSERT_NEAR(points.front().x, 0.0, 1e-5);
-    ASSERT_NEAR(points.front().y, 1000.0, 1e-4);
-    ASSERT_NEAR(points.back().x, 1000.0, 1e-5);
-    ASSERT_NEAR(points.back().y, 0.0, 1e-4);
 }
 
 TEST(Transform, UnwrappedLatLng) {
@@ -177,26 +160,6 @@ TEST(Transform, UnwrappedLatLng) {
     ASSERT_NEAR(wrappedLeftwards.longitude(), -437.0, 1e-8);
     wrappedLeftwards.wrap();
     ASSERT_NEAR(wrappedLeftwards.longitude(), -77.0, 1e-8);
-
-    // Using batch conversion
-    std::vector<ScreenCoordinate> coords{{500, 500}};
-    auto tempCoords = state.latLngsToScreenCoordinates({{38, 283}, {38, -437}});
-    coords.insert(coords.end(), tempCoords.begin(), tempCoords.end());
-    auto latLngs = state.screenCoordinatesToLatLngs(coords);
-    ASSERT_EQ(3, latLngs.size());
-
-    ASSERT_NEAR(38.0, latLngs[0].latitude(), 1e-8);
-    ASSERT_NEAR(-77.0, latLngs[0].longitude(), 1e-8);
-
-    ASSERT_NEAR(38.0, latLngs[1].latitude(), 1e-8);
-    ASSERT_NEAR(283.0, latLngs[1].longitude(), 1e-8);
-    latLngs[1].wrap();
-    ASSERT_NEAR(-77.0, latLngs[1].longitude(), 1e-8);
-
-    ASSERT_DOUBLE_EQ(latLngs[1].latitude(), latLngs[2].latitude());
-    ASSERT_NEAR(-437.0, latLngs[2].longitude(), 1e-8);
-    latLngs[2].wrap();
-    ASSERT_NEAR(-77.0, latLngs[2].longitude(), 1e-8);
 }
 
 TEST(Transform, ConstrainHeightOnly) {