Skip to content

Commit

Permalink
Don't load FeatureIndex#vtLayers from outside FeatureIndex code. (#6559)
Browse files Browse the repository at this point in the history
* Don't load FeatureIndex#vtLayers from outside FeatureIndex code.
Fixes issue #6555 (a possible crash when calling queryRenderedFeatures after querySourceFeatures)
The actual contents of the regression test are not that important -- before this fix, the test would crash.

* Fix Tile unit test by deserializing the FeatureIndex before using it.
  • Loading branch information
ChrisLoer authored and jfirebaugh committed Apr 30, 2018
1 parent 7419c60 commit 3df26a8
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 18 deletions.
14 changes: 8 additions & 6 deletions src/data/feature_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,17 @@ class FeatureIndex {
}
}

// Finds non-symbol features in this tile at a particular position.
query(args: QueryParameters, styleLayers: {[string]: StyleLayer}): {[string]: Array<{ featureIndex: number, feature: GeoJSONFeature }>} {
loadVTLayers(): {[string]: VectorTileLayer} {
if (!this.vtLayers) {
this.vtLayers = new vt.VectorTile(new Protobuf(this.rawTileData)).layers;
this.sourceLayerCoder = new DictionaryCoder(this.vtLayers ? Object.keys(this.vtLayers).sort() : ['_geojsonTileLayer']);
}
return this.vtLayers;
}

// Finds non-symbol features in this tile at a particular position.
query(args: QueryParameters, styleLayers: {[string]: StyleLayer}): {[string]: Array<{ featureIndex: number, feature: GeoJSONFeature }>} {
this.loadVTLayers();

const params = args.params || {},
pixelsToTileUnits = EXTENT / args.tileSize / args.scale,
Expand Down Expand Up @@ -201,10 +206,7 @@ class FeatureIndex {
filterLayerIDs: Array<string>,
styleLayers: {[string]: StyleLayer}) {
const result = {};
if (!this.vtLayers) {
this.vtLayers = new vt.VectorTile(new Protobuf(this.rawTileData)).layers;
this.sourceLayerCoder = new DictionaryCoder(this.vtLayers ? Object.keys(this.vtLayers).sort() : ['_geojsonTileLayer']);
}
this.loadVTLayers();

const filter = featureFilter(filterSpec);

Expand Down
8 changes: 1 addition & 7 deletions src/source/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import { uniqueId, deepEqual, parseCacheControl } from '../util/util';
import { deserialize as deserializeBucket } from '../data/bucket';
import FeatureIndex from '../data/feature_index';
import vt from '@mapbox/vector-tile';
import Protobuf from 'pbf';
import GeoJSONFeature from '../util/vectortile_to_geojson';
import featureFilter from '../style-spec/feature_filter';
import SymbolBucket from '../data/bucket/symbol_bucket';
Expand Down Expand Up @@ -266,11 +264,7 @@ class Tile {
querySourceFeatures(result: Array<GeoJSONFeature>, params: any) {
if (!this.latestFeatureIndex || !this.latestFeatureIndex.rawTileData) return;

if (!this.latestFeatureIndex.vtLayers) {
this.latestFeatureIndex.vtLayers =
new vt.VectorTile(new Protobuf(this.latestFeatureIndex.rawTileData)).layers;
}
const vtLayers = this.latestFeatureIndex.vtLayers;
const vtLayers = this.latestFeatureIndex.loadVTLayers();

const sourceLayer = params ? params.sourceLayer : '';
const layer = vtLayers._geojsonTileLayer || vtLayers[sourceLayer];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[
{
"geometry": {
"type": "Point",
"coordinates": [
0,
0
]
},
"type": "Feature",
"properties": {}
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
{
"version": 8,
"metadata": {
"test": {
"width": 128,
"height": 128,
"operations": [
[
"querySourceFeatures",
[
"source"
]
]
],
"queryGeometry": [
79,
64
],
"queryOptions": {
}
}
},
"zoom": 0,
"glyphs": "local://glyphs/{fontstack}/{range}.pbf",
"sprite": "local://sprites/sprite",
"sources": {
"source": {
"type": "geojson",
"data": {
"type": "FeatureCollection",
"features": [{
"type": "Feature",
"geometry": {
"type": "Point",
"coordinates": [0, 0]
}
}]
}
}
},
"layers": [
{
"id": "background",
"type": "background",
"paint": {
"background-color": "white"
}
},
{
"id": "iconlayer",
"type": "symbol",
"source": "source",
"layout": {
"icon-image": "building-12",
"icon-offset": [15, 0]
}
}
]
}
12 changes: 7 additions & 5 deletions test/unit/source/tile.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import FeatureIndex from '../../../src/data/feature_index';
import { CollisionBoxArray } from '../../../src/data/array_types';
import { extend } from '../../../src/util/util';
import Context from '../../../src/gl/context';
import { serialize } from '../../../src/util/web_worker_transfer';
import { serialize, deserialize } from '../../../src/util/web_worker_transfer';

test('querySourceFeatures', (t) => {
const features = [{
Expand All @@ -21,7 +21,6 @@ test('querySourceFeatures', (t) => {

t.test('geojson tile', (t) => {
const tile = new Tile(new OverscaledTileID(1, 0, 1, 1, 1));
tile.latestFeatureIndex = new FeatureIndex(tile.tileID);
let result;

result = [];
Expand All @@ -30,7 +29,10 @@ test('querySourceFeatures', (t) => {

const geojsonWrapper = new GeoJSONWrapper(features);
geojsonWrapper.name = '_geojsonTileLayer';
tile.latestFeatureIndex.rawTileData = vtpbf({ layers: { '_geojsonTileLayer': geojsonWrapper }});
tile.loadVectorData(
createVectorData({rawTileData: vtpbf({ layers: { '_geojsonTileLayer': geojsonWrapper }})}),
createPainter()
);

result = [];
tile.querySourceFeatures(result);
Expand Down Expand Up @@ -309,8 +311,8 @@ function createRawTileData() {
function createVectorData(options) {
const collisionBoxArray = new CollisionBoxArray();
return extend({
collisionBoxArray: serialize(collisionBoxArray),
featureIndex: serialize(new FeatureIndex(new OverscaledTileID(1, 0, 1, 1, 1))),
collisionBoxArray: deserialize(serialize(collisionBoxArray)),
featureIndex: deserialize(serialize(new FeatureIndex(new OverscaledTileID(1, 0, 1, 1, 1)))),
buckets: []
}, options);
}
Expand Down

0 comments on commit 3df26a8

Please sign in to comment.