Skip to content

Commit

Permalink
fix querying features that extend across tile boundaries
Browse files Browse the repository at this point in the history
fix #5756

Some rendered features (circles, symbols, lines) can extend outside of
the boundaries of the tile they are stored in. When querying we have to
check all tiles within `additionalRadius` of the query.
  • Loading branch information
ansis committed Jan 22, 2018
1 parent 19d233e commit 06d2dd3
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 227 deletions.
19 changes: 12 additions & 7 deletions src/data/feature_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,6 @@ class FeatureIndex {

// Finds features in this tile at a particular position.
query(args: QueryParameters, styleLayers: {[string]: StyleLayer}) {
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']);
}

const result = {};

const params = args.params || {},
pixelsToTileUnits = EXTENT / args.tileSize / args.scale,
filter = featureFilter(params.filter);
Expand All @@ -117,6 +110,18 @@ class FeatureIndex {
}
}

const result = {};

if (maxX + additionalRadius < 0 || maxY + additionalRadius < 0 || minX - additionalRadius >= EXTENT || minY - additionalRadius >= EXTENT) {
// query does not intersect tile, return early
return 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']);
}

const matching = this.grid.query(minX - additionalRadius, minY - additionalRadius, maxX + additionalRadius, maxY + additionalRadius);
matching.sort(topDownFeatureComparator);
this.filterMatching(result, matching, this.featureIndexArray, queryGeometry, filter, params.layers, styleLayers, args.bearing, pixelsToTileUnits);
Expand Down
51 changes: 43 additions & 8 deletions src/source/query_features.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
// @flow

const Point = require('@mapbox/point-geometry');
const EXTENT = require('../data/extent');

import type SourceCache from './source_cache';
import type StyleLayer from '../style/style_layer';
import type Coordinate from '../geo/coordinate';
import type CollisionIndex from '../symbol/collision_index';
import type {OverscaledTileID} from './tile_id';

exports.rendered = function(sourceCache: SourceCache,
styleLayers: {[string]: StyleLayer},
Expand All @@ -12,18 +16,37 @@ exports.rendered = function(sourceCache: SourceCache,
zoom: number,
bearing: number,
collisionIndex: CollisionIndex) {
const tilesIn = sourceCache.tilesIn(queryGeometry);

tilesIn.sort(sortTilesIn);
const tiles = [];
const ids = sourceCache.getIds();

for (const id of ids) {
const tile = sourceCache.getTileByID(id);
const tileID = tile.tileID;

const tileSpaceQueryGeometry = [];
for (let j = 0; j < queryGeometry.length; j++) {
tileSpaceQueryGeometry.push(coordinateToTilePoint(tileID, queryGeometry[j]));
}

tiles.push({
tile: tile,
tileID: tileID,
queryGeometry: [tileSpaceQueryGeometry],
scale: Math.pow(2, zoom - tile.tileID.overscaledZ)
});
}

tiles.sort(sortTiles);

const renderedFeatureLayers = [];
for (const tileIn of tilesIn) {
for (const tile of tiles) {
renderedFeatureLayers.push({
wrappedTileID: tileIn.tileID.wrapped().key,
queryResults: tileIn.tile.queryRenderedFeatures(
wrappedTileID: tile.tileID.wrapped().key,
queryResults: tile.tile.queryRenderedFeatures(
styleLayers,
tileIn.queryGeometry,
tileIn.scale,
tile.queryGeometry,
tile.scale,
params,
bearing,
sourceCache.id,
Expand Down Expand Up @@ -54,7 +77,19 @@ exports.source = function(sourceCache: SourceCache, params: any) {
return result;
};

function sortTilesIn(a, b) {
/**
* * Convert a coordinate to a point in a tile's coordinate space.
* * @private
* */
function coordinateToTilePoint(tileID: OverscaledTileID, coord: Coordinate): Point {
const zoomedCoord = coord.zoomTo(tileID.canonical.z);
return new Point(
(zoomedCoord.column - (tileID.canonical.x + tileID.wrap * Math.pow(2, tileID.canonical.z))) * EXTENT,
(zoomedCoord.row - tileID.canonical.y) * EXTENT
);
}

function sortTiles(a, b) {
const idA = a.tileID;
const idB = b.tileID;
return (idA.overscaledZ - idB.overscaledZ) || (idA.canonical.y - idB.canonical.y) || (idA.wrap - idB.wrap) || (idA.canonical.x - idB.canonical.x);
Expand Down
68 changes: 0 additions & 68 deletions src/source/source_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ const createSource = require('./source').create;
const Tile = require('./tile');
const Evented = require('../util/evented');
const Cache = require('../util/lru_cache');
const Coordinate = require('../geo/coordinate');
const util = require('../util/util');
const EXTENT = require('../data/extent');
const Context = require('../gl/context');
const Point = require('@mapbox/point-geometry');
const browser = require('../util/browser');
Expand Down Expand Up @@ -676,60 +674,6 @@ class SourceCache extends Evented {
this._cache.reset();
}

/**
* Search through our current tiles and attempt to find the tiles that
* cover the given bounds.
* @param queryGeometry coordinates of the corners of bounding rectangle
* @returns {Array<Object>} result items have {tile, minX, maxX, minY, maxY}, where min/max bounding values are the given bounds transformed in into the coordinate space of this tile.
*/
tilesIn(queryGeometry: Array<Coordinate>) {
const tileResults = [];
const ids = this.getIds();

let minX = Infinity;
let minY = Infinity;
let maxX = -Infinity;
let maxY = -Infinity;
const z = queryGeometry[0].zoom;

for (let k = 0; k < queryGeometry.length; k++) {
const p = queryGeometry[k];
minX = Math.min(minX, p.column);
minY = Math.min(minY, p.row);
maxX = Math.max(maxX, p.column);
maxY = Math.max(maxY, p.row);
}


for (let i = 0; i < ids.length; i++) {
const tile = this._tiles[ids[i]];
const tileID = tile.tileID;

const tileSpaceBounds = [
coordinateToTilePoint(tileID, new Coordinate(minX, minY, z)),
coordinateToTilePoint(tileID, new Coordinate(maxX, maxY, z))
];

if (tileSpaceBounds[0].x < EXTENT && tileSpaceBounds[0].y < EXTENT &&
tileSpaceBounds[1].x >= 0 && tileSpaceBounds[1].y >= 0) {

const tileSpaceQueryGeometry = [];
for (let j = 0; j < queryGeometry.length; j++) {
tileSpaceQueryGeometry.push(coordinateToTilePoint(tileID, queryGeometry[j]));
}

tileResults.push({
tile: tile,
tileID: tileID,
queryGeometry: [tileSpaceQueryGeometry],
scale: Math.pow(2, this.transform.zoom - tile.tileID.overscaledZ)
});
}
}

return tileResults;
}

getVisibleCoordinates() {
const coords = this.getRenderableIds().map((id) => this._tiles[id].tileID);
for (const coord of coords) {
Expand Down Expand Up @@ -759,18 +703,6 @@ class SourceCache extends Evented {
SourceCache.maxOverzooming = 10;
SourceCache.maxUnderzooming = 3;

/**
* Convert a coordinate to a point in a tile's coordinate space.
* @private
*/
function coordinateToTilePoint(tileID: OverscaledTileID, coord: Coordinate): Point {
const zoomedCoord = coord.zoomTo(tileID.canonical.z);
return new Point(
(zoomedCoord.column - (tileID.canonical.x + tileID.wrap * Math.pow(2, tileID.canonical.z))) * EXTENT,
(zoomedCoord.row - tileID.canonical.y) * EXTENT
);
}

function isRasterType(type) {
return type === 'raster' || type === 'image' || type === 'video';
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[
{
"geometry": {
"type": "Point",
"coordinates": [
-84.3310546875,
33.92512970007199
]
},
"type": "Feature",
"properties": {}
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{
"version": 8,
"metadata": {
"test": {
"debug": true,
"height": 256,
"queryGeometry": [
240,
125
]
}
},
"center": [
-92.3780691249957,
35.94102132783915
],
"zoom": 2,
"sources": {
"geojson": {
"type": "geojson",
"data": {
"type": "FeatureCollection",
"features": [{
"type": "Feature",
"geometry": {
"type": "Point",
"coordinates": [-84.333565, 33.925575]
}
}]
}
}
},
"layers": [
{
"id": "road",
"type": "circle",
"source": "geojson",
"paint": {
"circle-radius": 100
}
}
]
}
11 changes: 0 additions & 11 deletions test/unit/source/query_features.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,6 @@ const test = require('mapbox-gl-js-test').test;
const QueryFeatures = require('../../../src/source/query_features.js');
const SourceCache = require('../../../src/source/source_cache.js');

test('QueryFeatures#rendered', (t) => {
t.test('returns empty object if source returns no tiles', (t) => {
const mockSourceCache = { tilesIn: function () { return []; } };
const result = QueryFeatures.rendered(mockSourceCache);
t.deepEqual(result, []);
t.end();
});

t.end();
});

test('QueryFeatures#source', (t) => {
t.test('returns empty result when source has no features', (t) => {
const sourceCache = new SourceCache('test', {
Expand Down
Loading

0 comments on commit 06d2dd3

Please sign in to comment.