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

[core] Restore AnnotationSource maximum zoom range to 22 #5517

Merged
brunoabinader merged 3 commits into from
Jul 26, 2016
Merged

[core] Restore AnnotationSource maximum zoom range to 22 #5517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
427773d
[core] Micro-optimizations in geometry code
brunoabinader Jun 30, 2016
26a9213
[build] Upgrade geojson-vt-cpp@6.1.2
brunoabinader Jul 26, 2016
253a3db
[core] Revert annotation source maxZoom to 22
brunoabinader Jul 26, 2016
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 .mason
Open in desktop
Submodule .mason updated 22 files
+1 −1 mason.sh
+15 −0 scripts/afl/2.19b/.travis.yml
+45 −0 scripts/afl/2.19b/script.sh
+11 −6 scripts/clang/3.8.0/.travis.yml
+51 −0 scripts/clang/3.8.0/README.md
+24 −5 scripts/clang/3.8.0/script.sh
+61 −0 scripts/geojson/0.2.0/.travis.yml
+42 −0 scripts/geojson/0.2.0/script.sh
+15 −0 scripts/geojsonvt/6.1.1/.travis.yml
+33 −0 scripts/geojsonvt/6.1.1/script.sh
+15 −0 scripts/geojsonvt/6.1.2/.travis.yml
+33 −0 scripts/geojsonvt/6.1.2/script.sh
+15 −0 scripts/kdbush/0.1.1/.travis.yml
+34 −0 scripts/kdbush/0.1.1/script.sh
+15 −0 scripts/rapidjson/2016-07-20-369de87/.travis.yml
+35 −0 scripts/rapidjson/2016-07-20-369de87/script.sh
+15 −0 scripts/supercluster/0.1.0/.travis.yml
+34 −0 scripts/supercluster/0.1.0/script.sh
+15 −0 scripts/supercluster/0.1.1/.travis.yml
+34 −0 scripts/supercluster/0.1.1/script.sh
+15 −0 scripts/variant/1.1.1/.travis.yml
+36 −0 scripts/variant/1.1.1/script.sh
2 changes: 1 addition & 1 deletion platform/android/scripts/configure.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ NUNICODE_VERSION=1.6
LIBZIP_VERSION=0.11.2
GEOMETRY_VERSION=0.8.0
GEOJSON_VERSION=0.1.4
GEOJSONVT_VERSION=6.1.0
GEOJSONVT_VERSION=6.1.2
VARIANT_VERSION=1.1.0
RAPIDJSON_VERSION=1.0.2
JNI_HPP_VERSION=2.0.0
Expand Down
2 changes: 1 addition & 1 deletion platform/ios/scripts/configure.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ SQLITE_VERSION=system
ZLIB_VERSION=system
GEOMETRY_VERSION=0.8.0
GEOJSON_VERSION=0.1.4
GEOJSONVT_VERSION=6.1.0
GEOJSONVT_VERSION=6.1.2
VARIANT_VERSION=1.1.0
RAPIDJSON_VERSION=1.0.2
GTEST_VERSION=1.7.0
Expand Down
2 changes: 1 addition & 1 deletion platform/linux/scripts/configure.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ ZLIB_VERSION=system
NUNICODE_VERSION=1.6
GEOMETRY_VERSION=0.8.0
GEOJSON_VERSION=0.1.4${CXX11ABI:-}
GEOJSONVT_VERSION=6.1.0
GEOJSONVT_VERSION=6.1.2
VARIANT_VERSION=1.1.0
RAPIDJSON_VERSION=1.0.2
GTEST_VERSION=1.7.0${CXX11ABI:-}
Expand Down
2 changes: 1 addition & 1 deletion platform/macos/scripts/configure.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ ZLIB_VERSION=system
NUNICODE_VERSION=1.6
GEOMETRY_VERSION=0.8.0
GEOJSON_VERSION=0.1.4
GEOJSONVT_VERSION=6.1.0
GEOJSONVT_VERSION=6.1.2
VARIANT_VERSION=1.1.0
RAPIDJSON_VERSION=1.0.2
GTEST_VERSION=1.7.0
Expand Down
2 changes: 1 addition & 1 deletion platform/qt/scripts/configure.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ PROTOZERO_VERSION=1.3.0
BOOST_VERSION=1.60.0
GEOMETRY_VERSION=0.8.0
GEOJSON_VERSION=0.1.4${CXX11ABI:-}
GEOJSONVT_VERSION=6.1.0
GEOJSONVT_VERSION=6.1.2
GTEST_VERSION=1.7.0${CXX11ABI:-}
LIBJPEG_TURBO_VERSION=1.4.2
PIXELMATCH_VERSION=0.9.0
Expand Down
3 changes: 1 addition & 2 deletions src/mbgl/annotation/annotation_source.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ AnnotationSource::Impl::Impl(Source& base_)
}

Range<uint8_t> AnnotationSource::Impl::getZoomRange() {
// Same as default geojson-vt-cpp.
return { 0, 18 };
return { 0, 22 };
}

void AnnotationSource::Impl::load(FileSource&) {
Expand Down
2 changes: 1 addition & 1 deletion src/mbgl/annotation/shape_annotation_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ void ShapeAnnotationImpl::updateTileData(const CanonicalTileID& tileID, Annotati
return Feature { std::move(geom) };
}));
mapbox::geojsonvt::Options options;
options.maxZoom = util::clamp<uint8_t>(maxZoom, 0, 18);
options.maxZoom = maxZoom;
options.buffer = 255u;
options.extent = util::EXTENT;
options.tolerance = baseTolerance;
Expand Down
11 changes: 9 additions & 2 deletions src/mbgl/style/source_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,16 +207,23 @@ static Point<int16_t> coordinateToTilePoint(const UnwrappedTileID& tileID, const
}

std::unordered_map<std::string, std::vector<Feature>> Source::Impl::queryRenderedFeatures(const QueryParameters& parameters) const {
std::unordered_map<std::string, std::vector<Feature>> result;
if (renderTiles.empty()) {
Copy link
Copy Markdown
Contributor

@jfirebaugh jfirebaugh Jun 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The algorithm works fine without checking these boundary conditions right? Why increase the amount of code?

Copy link
Copy Markdown
Contributor Author

@brunoabinader brunoabinader Jun 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an early return - avoids unnecessary creation of queryGeometry.

return result;
}

LineString<double> queryGeometry;

for (const auto& p : parameters.geometry) {
queryGeometry.push_back(TileCoordinate::fromScreenCoordinate(
parameters.transformState, 0, { p.x, parameters.transformState.getHeight() - p.y }).p);
}

mapbox::geometry::box<double> box = mapbox::geometry::envelope(queryGeometry);
if (queryGeometry.empty()) {
return result;
}

std::unordered_map<std::string, std::vector<Feature>> result;
mapbox::geometry::box<double> box = mapbox::geometry::envelope(queryGeometry);

for (const auto& tilePtr : renderTiles) {
const RenderTile& tile = tilePtr.second;
Expand Down
5 changes: 4 additions & 1 deletion src/mbgl/style/style.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,14 +337,17 @@ RenderData Style::getRenderData(MapDebugOptions debugOptions) const {
}

std::vector<Feature> Style::queryRenderedFeatures(const QueryParameters& parameters) const {
std::vector<Feature> result;
std::unordered_map<std::string, std::vector<Feature>> resultsByLayer;

for (const auto& source : sources) {
auto sourceResults = source->baseImpl->queryRenderedFeatures(parameters);
std::move(sourceResults.begin(), sourceResults.end(), std::inserter(resultsByLayer, resultsByLayer.begin()));
}

std::vector<Feature> result;
if (resultsByLayer.empty()) {
Copy link
Copy Markdown
Contributor

@jfirebaugh jfirebaugh Jun 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Copy link
Copy Markdown
Contributor Author

@brunoabinader brunoabinader Jun 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reason as before - this is an early return.

return result;
}

// Combine all results based on the style layer order.
for (const auto& layer : layers) {
Expand Down
17 changes: 10 additions & 7 deletions src/mbgl/util/grid_index.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <mbgl/util/grid_index.hpp>
#include <mbgl/geometry/feature_index.hpp>
#include <mbgl/math/minmax.hpp>

#include <unordered_set>

Expand Down Expand Up @@ -28,9 +29,10 @@ void GridIndex<T>::insert(T&& t, const BBox& bbox) {
auto cx2 = convertToCellCoord(bbox.max.x);
auto cy2 = convertToCellCoord(bbox.max.y);

for (int32_t x = cx1; x <= cx2; x++) {
for (int32_t y = cy1; y <= cy2; y++) {
auto cellIndex = d * y + x;
int32_t x, y, cellIndex;
Copy link
Copy Markdown
Contributor

@jfirebaugh jfirebaugh Jun 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for this change?

Copy link
Copy Markdown
Contributor Author

@brunoabinader brunoabinader Jun 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things:

  • By placing these variables' scope outside of the for-loops we avoid recreating y and cellIndex upon each iteration of x.
  • ++x is faster than x++ because it doesn't create a temporary object.

for (x = cx1; x <= cx2; ++x) {
for (y = cy1; y <= cy2; ++y) {
cellIndex = d * y + x;
cells[cellIndex].push_back(uid);
}
}
Expand All @@ -48,9 +50,10 @@ std::vector<T> GridIndex<T>::query(const BBox& queryBBox) const {
auto cx2 = convertToCellCoord(queryBBox.max.x);
auto cy2 = convertToCellCoord(queryBBox.max.y);

for (int32_t x = cx1; x <= cx2; x++) {
for (int32_t y = cy1; y <= cy2; y++) {
auto cellIndex = d * y + x;
int32_t x, y, cellIndex;
for (x = cx1; x <= cx2; ++x) {
for (y = cy1; y <= cy2; ++y) {
cellIndex = d * y + x;
for (auto uid : cells[cellIndex]) {
if (seenUids.count(uid) == 0) {
seenUids.insert(uid);
Expand All @@ -75,7 +78,7 @@ std::vector<T> GridIndex<T>::query(const BBox& queryBBox) const {

template <class T>
int32_t GridIndex<T>::convertToCellCoord(int32_t x) const {
return std::max(0.0, std::min(d - 1.0, std::floor(x * scale) + padding));
return util::max(0.0, util::min(d - 1.0, std::floor(x * scale) + padding));
}

template class GridIndex<IndexedSubfeature>;
Expand Down