Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix dynamically added props not appearing in queryRenderedFeatures results #10102

Closed
wants to merge 2 commits into from
Closed

Fix dynamically added props not appearing in queryRenderedFeatures results #10102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94dbbff
Fix dynamically added props not appearing in queryRenderedFeatures
osvodef Nov 13, 2020
529eec5
Add query regression test
osvodef Nov 13, 2020
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
21 changes: 14 additions & 7 deletions src/data/feature_index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import DictionaryCoder from '../util/dictionary_coder';
import vt from '@mapbox/vector-tile';
import Protobuf from 'pbf';
import GeoJSONFeature from '../util/vectortile_to_geojson';
import {arraysIntersect, mapObject, extend} from '../util/util';
import {arraysIntersect, extend} from '../util/util';
import {OverscaledTileID} from '../source/tile_id';
import {register} from '../util/web_worker_transfer';
import EvaluationParameters from '../style/evaluation_parameters';
Expand Down Expand Up @@ -216,8 +216,8 @@ class FeatureIndex {

const serializedLayer = extend({}, serializedLayers[layerID]);

serializedLayer.paint = evaluateProperties(serializedLayer.paint, styleLayer.paint, feature, featureState, availableImages);
serializedLayer.layout = evaluateProperties(serializedLayer.layout, styleLayer.layout, feature, featureState, availableImages);
serializedLayer.paint = evaluateProperties(styleLayer._transitionablePaint, styleLayer.paint, feature, featureState, availableImages);
serializedLayer.layout = evaluateProperties(styleLayer._unevaluatedLayout, styleLayer.layout, feature, featureState, availableImages);
Comment on lines +219 to +220
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's where we can pass in the CanonicalTileID by doing (styleLayer._transitionablePaint, styleLayer.paint, feature, featureState, this.tileID.canonical, availableImages).


const intersectionZ = !intersectionTest || intersectionTest(feature, styleLayer, featureState);
if (!intersectionZ) {
Expand Down Expand Up @@ -296,11 +296,18 @@ register(

export default FeatureIndex;

function evaluateProperties(serializedProperties, styleLayerProperties, feature, featureState, availableImages) {
return mapObject(serializedProperties, (property, key) => {
function evaluateProperties(paintOrLayout, styleLayerProperties, feature, featureState, availableImages) {
const keys = paintOrLayout ? Object.keys(paintOrLayout._values) : [];
const evaluated = {};

for (const key of keys) {
const prop = styleLayerProperties instanceof PossiblyEvaluated ? styleLayerProperties.get(key) : null;
return prop && prop.evaluate ? prop.evaluate(feature, featureState, availableImages) : prop;
});
const value = prop && prop.evaluate ? prop.evaluate(feature, featureState, availableImages) : prop;
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be updated or it won't evaluate properly down the line because the property evaluation method takes the arguments (feature, featureState, canonical, availableImage). You can pass the CanonicalTileID into this function above.

mapbox-gl-js/src/style/properties.js

Lines 456 to 458 in 15f0983

evaluate(feature: Feature, featureState: FeatureState, canonical?: CanonicalTileID, availableImages?: Array<string>): T {
return this.property.evaluate(this.value, this.parameters, feature, featureState, canonical, availableImages);
}


evaluated[key] = value;
}

return evaluated;
}

function getBounds(geometry: Array<Point>) {
Expand Down
37 changes: 37 additions & 0 deletions test/integration/query-tests/regressions/mapbox-gl-js#10102/expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
[
{
"geometry": {
"type": "LineString",
"coordinates": [
[
0,
0
],
[
5.009765625,
14.987239525774243
]
]
},
"type": "Feature",
"properties": {},
"id": 1,
"layer": {
"id": "line",
"type": "line",
"source": "mapbox",
"paint": {
"line-width": 20,
"line-color": {
"r": 0,
"g": 0,
"b": 0,
"a": 1
}
},
"layout": {}
},
"source": "mapbox",
"state": {}
}
]
48 changes: 48 additions & 0 deletions test/integration/query-tests/regressions/mapbox-gl-js#10102/style.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
{
"version": 8,
"metadata": {
"skipLayerDelete": true,
"test": {
"width": 64,
"height": 64,
"operations": [
["setPaintProperty", "line", "line-color", "#000000"]
],
"queryGeometry": [
32,
16
]
}
},
"sources": {
"mapbox": {
"type": "geojson",
"data": {
"type": "FeatureCollection",
"features": [
{
"id": 1,
"type": "Feature",
"geometry": {
"type": "LineString",
"coordinates": [
[ 0, 0],
[ 5, 15]
]
}
}
]
}
}
},
"layers": [
{
"id": "line",
"type": "line",
"source": "mapbox",
"paint": {
"line-width": 20
}
}
]
}