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

Port global symbol query from GL JS #11742

Merged
merged 3 commits into from
Apr 25, 2018
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 mapbox-gl-js
Submodule mapbox-gl-js updated 155 files
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public static PropertyValue<Expression> fillTranslateAnchor(Expression expressio
}

/**
* Name of image in sprite to use for drawing image fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512).
* Name of image in sprite to use for drawing image fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.
*
* @param value a String value
* @return property wrapper around String
Expand All @@ -176,7 +176,7 @@ public static PropertyValue<String> fillPattern(String value) {
}

/**
* Name of image in sprite to use for drawing image fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512).
* Name of image in sprite to use for drawing image fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.
*
* @param expression an expression statement
* @return property wrapper around an expression statement
Expand Down Expand Up @@ -356,7 +356,7 @@ public static PropertyValue<Expression> lineBlur(Expression expression) {
}

/**
* Specifies the lengths of the alternating dashes and gaps that form the dash pattern. The lengths are later scaled by the line width. To convert a dash length to density-independent pixels, multiply the length by the current line width.
* Specifies the lengths of the alternating dashes and gaps that form the dash pattern. The lengths are later scaled by the line width. To convert a dash length to density-independent pixels, multiply the length by the current line width. Note that GeoJSON sources with `lineMetrics: true` specified won't render dashed lines to the expected scale. Also note that zoom-dependent expressions will be evaluated only at integer zoom levels.
*
* @param value a Float[] value
* @return property wrapper around Float[]
Expand All @@ -366,7 +366,7 @@ public static PropertyValue<Float[]> lineDasharray(Float[] value) {
}

/**
* Specifies the lengths of the alternating dashes and gaps that form the dash pattern. The lengths are later scaled by the line width. To convert a dash length to density-independent pixels, multiply the length by the current line width.
* Specifies the lengths of the alternating dashes and gaps that form the dash pattern. The lengths are later scaled by the line width. To convert a dash length to density-independent pixels, multiply the length by the current line width. Note that GeoJSON sources with `lineMetrics: true` specified won't render dashed lines to the expected scale. Also note that zoom-dependent expressions will be evaluated only at integer zoom levels.
*
* @param expression an expression statement
* @return property wrapper around an expression statement
Expand All @@ -376,7 +376,7 @@ public static PropertyValue<Expression> lineDasharray(Expression expression) {
}

/**
* Name of image in sprite to use for drawing image lines. For seamless patterns, image width must be a factor of two (2, 4, 8, ..., 512).
* Name of image in sprite to use for drawing image lines. For seamless patterns, image width must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.
*
* @param value a String value
* @return property wrapper around String
Expand All @@ -386,7 +386,7 @@ public static PropertyValue<String> linePattern(String value) {
}

/**
* Name of image in sprite to use for drawing image lines. For seamless patterns, image width must be a factor of two (2, 4, 8, ..., 512).
* Name of image in sprite to use for drawing image lines. For seamless patterns, image width must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.
*
* @param expression an expression statement
* @return property wrapper around an expression statement
Expand Down Expand Up @@ -1156,7 +1156,7 @@ public static PropertyValue<Expression> fillExtrusionTranslateAnchor(Expression
}

/**
* Name of image in sprite to use for drawing images on extruded fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512).
* Name of image in sprite to use for drawing images on extruded fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.
*
* @param value a String value
* @return property wrapper around String
Expand All @@ -1166,7 +1166,7 @@ public static PropertyValue<String> fillExtrusionPattern(String value) {
}

/**
* Name of image in sprite to use for drawing images on extruded fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512).
* Name of image in sprite to use for drawing images on extruded fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.
*
* @param expression an expression statement
* @return property wrapper around an expression statement
Expand Down Expand Up @@ -1536,7 +1536,7 @@ public static PropertyValue<Expression> backgroundColor(Expression expression) {
}

/**
* Name of image in sprite to use for drawing an image background. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512).
* Name of image in sprite to use for drawing an image background. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.
*
* @param value a String value
* @return property wrapper around String
Expand All @@ -1546,7 +1546,7 @@ public static PropertyValue<String> backgroundPattern(String value) {
}

/**
* Name of image in sprite to use for drawing an image background. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512).
* Name of image in sprite to use for drawing an image background. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.
*
* @param expression an expression statement
* @return property wrapper around an expression statement
Expand Down
3 changes: 2 additions & 1 deletion platform/darwin/src/MGLFillStyleLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ MGL_EXPORT

/**
Name of image in sprite to use for drawing image fills. For seamless patterns,
image width and height must be a factor of two (2, 4, 8, ..., 512).
image width and height must be a factor of two (2, 4, 8, ..., 512). Note that
zoom-dependent expressions will be evaluated only at integer zoom levels.

You can set this property to an expression containing any of the following:

Expand Down
5 changes: 4 additions & 1 deletion platform/darwin/src/MGLLineStyleLayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,10 @@ MGL_EXPORT
/**
Specifies the lengths of the alternating dashes and gaps that form the dash
pattern. The lengths are later scaled by the line width. To convert a dash
length to points, multiply the length by the current line width.
length to points, multiply the length by the current line width. Note that
GeoJSON sources with `lineMetrics: true` specified won't render dashed lines to
the expected scale. Also note that zoom-dependent expressions will be evaluated
only at integer zoom levels.

This property is measured in line widths.

Expand Down
24 changes: 22 additions & 2 deletions platform/node/test/ignores.json
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
{
"expression-tests/collator/accent-equals-de": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/accent-lt-en": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/accent-not-equals-en": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/base-default-locale": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/base-equals-en": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/base-gt-en": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/case-lteq-en": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/case-not-equals-en": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/case-omitted-en": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/comparison-number-error": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/diacritic-omitted-en": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/equals-non-string-error": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/non-object-error": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/variant-equals-en": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/collator/variant-gteq-en": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/is-supported-script/default": "https://github.com/mapbox/mapbox-gl-native/issues/11693",
"expression-tests/resolved-locale/basic": "https://github.com/mapbox/mapbox-gl-native/issues/11692",
"expression-tests/to-string/basic": "https://github.com/mapbox/mapbox-gl-native/issues/11719",
"query-tests/circle-pitch-scale/viewport-inside-align-map": "https://github.com/mapbox/mapbox-gl-native/issues/10615",
"query-tests/circle-pitch-scale/viewport-inside-align-viewport": "https://github.com/mapbox/mapbox-gl-native/issues/10615",
"query-tests/edge-cases/box-cutting-antimeridian-z0": "https://github.com/mapbox/mapbox-gl-native/issues/11607",
"query-tests/edge-cases/null-island": "https://github.com/mapbox/mapbox-gl-native/issues/11607",
"query-tests/geometry/multilinestring": "needs investigation",
"query-tests/geometry/multipolygon": "needs investigation",
"query-tests/geometry/polygon": "needs investigation",
"query-tests/symbol/panned-after-insert": "https://github.com/mapbox/mapbox-gl-native/issues/10408",
"query-tests/symbol/rotated-after-insert": "https://github.com/mapbox/mapbox-gl-native/issues/10408",
"query-tests/world-wrapping/box": "skip - needs issue",
"query-tests/world-wrapping/point": "skip - needs issue",
"render-tests/background-color/transition": "https://github.com/mapbox/mapbox-gl-native/issues/10619",
Expand All @@ -27,6 +43,10 @@
"render-tests/fill-extrusion-pattern/opacity": "https://github.com/mapbox/mapbox-gl-js/issues/3327",
"render-tests/geojson/inline-linestring-fill": "current behavior is arbitrary",
"render-tests/geojson/inline-polygon-symbol": "behavior needs reconciliation with gl-js",
"render-tests/is-supported-script/filter": "https://github.com/mapbox/mapbox-gl-native/issues/11693",
"render-tests/is-supported-script/layout": "https://github.com/mapbox/mapbox-gl-native/issues/11693",
"render-tests/line-gradient/gradient": "https://github.com/mapbox/mapbox-gl-native/issues/11718",
"render-tests/line-gradient/translucent": "https://github.com/mapbox/mapbox-gl-native/issues/11718",
"render-tests/mixed-zoom/z10-z11": "https://github.com/mapbox/mapbox-gl-native/issues/10397",
"render-tests/raster-masking/overlapping-zoom": "https://github.com/mapbox/mapbox-gl-native/issues/10195",
"render-tests/real-world/bangkok": "https://github.com/mapbox/mapbox-gl-native/issues/10412",
Expand Down
2 changes: 2 additions & 0 deletions scripts/generate-shaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ const outputPath = 'src/mbgl/shaders';

var shaders = require('../mapbox-gl-js/src/shaders');

delete shaders.lineGradient;

require('./style-code');

writeIfModified(path.join(outputPath, 'preludes.hpp'), `// NOTE: DO NOT CHANGE THIS FILE. IT IS AUTOMATICALLY GENERATED.
Expand Down
1 change: 1 addition & 0 deletions scripts/style-spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var spec = module.exports = require('../mapbox-gl-js/src/style-spec/reference/v8');

// Make temporary modifications here when Native doesn't have all features that JS has.
delete spec.paint_line['line-gradient'];
5 changes: 2 additions & 3 deletions src/mbgl/annotation/render_annotation_source.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,8 @@ std::unordered_map<std::string, std::vector<Feature>>
RenderAnnotationSource::queryRenderedFeatures(const ScreenLineString& geometry,
const TransformState& transformState,
const std::vector<const RenderLayer*>& layers,
const RenderedQueryOptions& options,
const CollisionIndex& collisionIndex) const {
return tilePyramid.queryRenderedFeatures(geometry, transformState, layers, options, collisionIndex);
const RenderedQueryOptions& options) const {
return tilePyramid.queryRenderedFeatures(geometry, transformState, layers, options);
}

std::vector<Feature> RenderAnnotationSource::querySourceFeatures(const SourceQueryOptions&) const {
Expand Down
3 changes: 1 addition & 2 deletions src/mbgl/annotation/render_annotation_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ class RenderAnnotationSource : public RenderSource {
queryRenderedFeatures(const ScreenLineString& geometry,
const TransformState& transformState,
const std::vector<const RenderLayer*>& layers,
const RenderedQueryOptions& options,
const CollisionIndex& collisionIndex) const final;
const RenderedQueryOptions& options) const final;

std::vector<Feature>
querySourceFeatures(const SourceQueryOptions&) const final;
Expand Down
58 changes: 41 additions & 17 deletions src/mbgl/geometry/feature_index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,6 @@ void FeatureIndex::insert(const GeometryCollection& geometries,
}
}

static bool topDown(const IndexedSubfeature& a, const IndexedSubfeature& b) {
return a.sortIndex > b.sortIndex;
}

static bool topDownSymbols(const IndexedSubfeature& a, const IndexedSubfeature& b) {
return a.sortIndex < b.sortIndex;
}

void FeatureIndex::query(
std::unordered_map<std::string, std::vector<Feature>>& result,
const GeometryCoordinates& queryGeometry,
Expand All @@ -49,9 +41,7 @@ void FeatureIndex::query(
const double scale,
const RenderedQueryOptions& queryOptions,
const UnwrappedTileID& tileID,
const std::string& sourceID,
const std::vector<const RenderLayer*>& layers,
const CollisionIndex& collisionIndex,
const float additionalQueryRadius) const {

if (!tileData) {
Expand All @@ -68,31 +58,65 @@ void FeatureIndex::query(
convertPoint<float>(box.max + additionalRadius) });


std::sort(features.begin(), features.end(), topDown);
std::sort(features.begin(), features.end(), [](const IndexedSubfeature& a, const IndexedSubfeature& b) {
return a.sortIndex > b.sortIndex;
});
size_t previousSortIndex = std::numeric_limits<size_t>::max();
for (const auto& indexedFeature : features) {

// If this feature is the same as the previous feature, skip it.
if (indexedFeature.sortIndex == previousSortIndex) continue;
previousSortIndex = indexedFeature.sortIndex;

addFeature(result, indexedFeature, queryGeometry, queryOptions, tileID.canonical, layers, bearing, pixelsToTileUnits);
addFeature(result, indexedFeature, queryOptions, tileID.canonical, layers, queryGeometry, bearing, pixelsToTileUnits);
}
}

std::unordered_map<std::string, std::vector<Feature>> FeatureIndex::lookupSymbolFeatures(const std::vector<IndexedSubfeature>& symbolFeatures,
const RenderedQueryOptions& queryOptions,
const std::vector<const RenderLayer*>& layers,
const OverscaledTileID& tileID,
const std::shared_ptr<std::vector<size_t>>& featureSortOrder) const {
std::unordered_map<std::string, std::vector<Feature>> result;
if (!tileData) {
return result;
}
std::vector<IndexedSubfeature> sortedFeatures(symbolFeatures.begin(), symbolFeatures.end());

std::sort(sortedFeatures.begin(), sortedFeatures.end(), [featureSortOrder](const IndexedSubfeature& a, const IndexedSubfeature& b) {
// Same idea as the non-symbol sort order, but symbol features may have changed their sort order
// since their corresponding IndexedSubfeature was added to the CollisionIndex
// The 'featureSortOrder' is relatively inefficient for querying but cheap to build on every bucket sort
if (featureSortOrder) {
// queryRenderedSymbols documentation says we'll return features in
// "top-to-bottom" rendering order (aka last-to-first).
// Actually there can be multiple symbol instances per feature, so
// we sort each feature based on the first matching symbol instance.
auto sortedA = std::find(featureSortOrder->begin(), featureSortOrder->end(), a.index);
auto sortedB = std::find(featureSortOrder->begin(), featureSortOrder->end(), b.index);
assert(sortedA != featureSortOrder->end());
assert(sortedB != featureSortOrder->end());
return sortedA > sortedB;
} else {
// Bucket hasn't been re-sorted based on angle, so use same "reverse of appearance in source data"
// logic as non-symboles
return a.sortIndex > b.sortIndex;
}
});

std::vector<IndexedSubfeature> symbolFeatures = collisionIndex.queryRenderedSymbols(queryGeometry, tileID, sourceID);
std::sort(symbolFeatures.begin(), symbolFeatures.end(), topDownSymbols);
for (const auto& symbolFeature : symbolFeatures) {
addFeature(result, symbolFeature, queryGeometry, queryOptions, tileID.canonical, layers, bearing, pixelsToTileUnits);
for (const auto& symbolFeature : sortedFeatures) {
addFeature(result, symbolFeature, queryOptions, tileID.canonical, layers, GeometryCoordinates(), 0, 0);
}
return result;
}

void FeatureIndex::addFeature(
std::unordered_map<std::string, std::vector<Feature>>& result,
const IndexedSubfeature& indexedFeature,
const GeometryCoordinates& queryGeometry,
const RenderedQueryOptions& options,
const CanonicalTileID& tileID,
const std::vector<const RenderLayer*>& layers,
const GeometryCoordinates& queryGeometry,
const float bearing,
const float pixelsToTileUnits) const {

Expand Down
Loading