From 3346dc6b398e9ae83a93f73ee1ba2034511b6837 Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Tue, 6 Mar 2018 16:00:14 -0500 Subject: [PATCH] fix querying circles with -pitch-scaling=viewport `"circle-pitch-scaling": "viewport"` adjsuts circle-radius based on the distance from the center of the viewport. This incorporates this scaling into the calculations used by feature querying. --- src/data/feature_index.js | 12 +++-- src/geo/transform.js | 7 +++ src/source/query_features.js | 6 ++- src/source/source_cache.js | 4 +- src/source/tile.js | 7 ++- src/style/style_layer.js | 4 +- src/style/style_layer/circle_style_layer.js | 24 ++++++++-- src/util/intersection_tests.js | 21 +++++---- .../map-inside-align-map/expected.json | 13 ++++++ .../map-inside-align-map/style.json | 45 +++++++++++++++++++ .../map-outside-align-map/expected.json | 1 + .../map-outside-align-map/style.json | 45 +++++++++++++++++++ .../viewport-inside-align-map/expected.json | 13 ++++++ .../viewport-inside-align-map/style.json | 45 +++++++++++++++++++ .../viewport-outside-align-map/expected.json | 1 + .../viewport-outside-align-map/style.json | 45 +++++++++++++++++++ 16 files changed, 273 insertions(+), 20 deletions(-) create mode 100644 test/integration/query-tests/circle-pitch-scale/map-inside-align-map/expected.json create mode 100644 test/integration/query-tests/circle-pitch-scale/map-inside-align-map/style.json create mode 100644 test/integration/query-tests/circle-pitch-scale/map-outside-align-map/expected.json create mode 100644 test/integration/query-tests/circle-pitch-scale/map-outside-align-map/style.json create mode 100644 test/integration/query-tests/circle-pitch-scale/viewport-inside-align-map/expected.json create mode 100644 test/integration/query-tests/circle-pitch-scale/viewport-inside-align-map/style.json create mode 100644 test/integration/query-tests/circle-pitch-scale/viewport-outside-align-map/expected.json create mode 100644 test/integration/query-tests/circle-pitch-scale/viewport-outside-align-map/style.json diff --git a/src/data/feature_index.js b/src/data/feature_index.js index 1e064f5a02d..15638915840 100644 --- a/src/data/feature_index.js +++ b/src/data/feature_index.js @@ -23,6 +23,8 @@ const {FeatureIndexArray} = require('./array_types'); type QueryParameters = { scale: number, bearing: number, + cameraToCenterDistance: number, + posMatrix: Float32Array, tileSize: number, queryGeometry: Array>, queryPadding: number, @@ -117,13 +119,13 @@ class FeatureIndex { const matching = this.grid.query(minX - queryPadding, minY - queryPadding, maxX + queryPadding, maxY + queryPadding); matching.sort(topDownFeatureComparator); - this.filterMatching(result, matching, this.featureIndexArray, queryGeometry, filter, params.layers, styleLayers, args.bearing, pixelsToTileUnits); + this.filterMatching(result, matching, this.featureIndexArray, queryGeometry, filter, params.layers, styleLayers, args.bearing, pixelsToTileUnits, args.cameraToCenterDistance, args.posMatrix); const matchingSymbols = args.collisionIndex ? args.collisionIndex.queryRenderedSymbols(queryGeometry, this.tileID, args.tileSize / EXTENT, args.collisionBoxArray, args.sourceID, args.bucketInstanceIds) : []; matchingSymbols.sort(); - this.filterMatching(result, matchingSymbols, args.collisionBoxArray, queryGeometry, filter, params.layers, styleLayers, args.bearing, pixelsToTileUnits); + this.filterMatching(result, matchingSymbols, args.collisionBoxArray, queryGeometry, filter, params.layers, styleLayers, args.bearing, pixelsToTileUnits, args.cameraToCenterDistance, args.posMatrix); return result; } @@ -137,7 +139,9 @@ class FeatureIndex { filterLayerIDs: Array, styleLayers: {[string]: StyleLayer}, bearing: number, - pixelsToTileUnits: number + pixelsToTileUnits: number, + cameraToCenterDistance: number, + posMatrix: Float32Array ) { let previousIndex; for (let k = 0; k < matching.length; k++) { @@ -175,7 +179,7 @@ class FeatureIndex { if (!geometry) { geometry = loadGeometry(feature); } - if (!styleLayer.queryIntersectsFeature(queryGeometry, feature, geometry, this.z, bearing, pixelsToTileUnits)) { + if (!styleLayer.queryIntersectsFeature(queryGeometry, feature, geometry, this.z, bearing, pixelsToTileUnits, cameraToCenterDistance, posMatrix)) { continue; } } diff --git a/src/geo/transform.js b/src/geo/transform.js index b3e3e62563a..e75b422b5cc 100644 --- a/src/geo/transform.js +++ b/src/geo/transform.js @@ -559,6 +559,13 @@ class Transform { this._posMatrixCache = {}; this._alignedPosMatrixCache = {}; } + + getPitchScaleFactor() { + const coord = this.pointCoordinate(new Point(0, 0)).zoomTo(this.zoom); + const p = [coord.column * this.tileSize, coord.row * this.tileSize, 0, 1]; + const topPoint = vec4.transformMat4(p, p, this.pixelMatrix); + return topPoint[3] / this.cameraToCenterDistance; + } } module.exports = Transform; diff --git a/src/source/query_features.js b/src/source/query_features.js index 9744d9f6bf8..e3e372bb792 100644 --- a/src/source/query_features.js +++ b/src/source/query_features.js @@ -12,7 +12,8 @@ exports.rendered = function(sourceCache: SourceCache, zoom: number, bearing: number, collisionIndex: ?CollisionIndex) { - const tilesIn = sourceCache.tilesIn(queryGeometry); + const pitchScaleFactor = sourceCache.transform.getPitchScaleFactor(); + const tilesIn = sourceCache.tilesIn(queryGeometry, pitchScaleFactor); tilesIn.sort(sortTilesIn); @@ -26,6 +27,9 @@ exports.rendered = function(sourceCache: SourceCache, tileIn.scale, params, bearing, + sourceCache.transform.cameraToCenterDistance, + pitchScaleFactor, + sourceCache.transform.calculatePosMatrix(tileIn.tileID.toUnwrapped()), sourceCache.id, collisionIndex) }); diff --git a/src/source/source_cache.js b/src/source/source_cache.js index a3e5173abc9..e4ef2e49c1d 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -682,7 +682,7 @@ class SourceCache extends Evented { * @param queryGeometry coordinates of the corners of bounding rectangle * @returns {Array} 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) { + tilesIn(queryGeometry: Array, pitchScaleFactor: number) { const tileResults = []; const ids = this.getIds(); @@ -705,7 +705,7 @@ class SourceCache extends Evented { const tile = this._tiles[ids[i]]; const tileID = tile.tileID; const scale = Math.pow(2, this.transform.zoom - tile.tileID.overscaledZ); - const queryPadding = tile.queryPadding * EXTENT / tile.tileSize / scale; + const queryPadding = pitchScaleFactor * tile.queryPadding * EXTENT / tile.tileSize / scale; const tileSpaceBounds = [ coordinateToTilePoint(tileID, new Coordinate(minX, minY, z)), diff --git a/src/source/tile.js b/src/source/tile.js index b46f9e932f1..7a21ade4dc8 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -242,6 +242,9 @@ class Tile { scale: number, params: { filter: FilterSpecification, layers: Array }, bearing: number, + cameraToCenterDistance: number, + pitchScaleFactor: number, + posMatrix: Float32Array, sourceID: string, collisionIndex: ?CollisionIndex): {[string]: Array<{ featureIndex: number, feature: GeoJSONFeature }>} { if (!this.featureIndex || !this.collisionBoxArray) @@ -265,8 +268,10 @@ class Tile { scale: scale, tileSize: this.tileSize, bearing: bearing, + cameraToCenterDistance: cameraToCenterDistance, + posMatrix: posMatrix, params: params, - queryPadding: this.queryPadding, + queryPadding: this.queryPadding * pitchScaleFactor, collisionBoxArray: this.collisionBoxArray, sourceID: sourceID, collisionIndex: collisionIndex, diff --git a/src/style/style_layer.js b/src/style/style_layer.js index 313079ad00b..16b9160f173 100644 --- a/src/style/style_layer.js +++ b/src/style/style_layer.js @@ -49,7 +49,9 @@ class StyleLayer extends Evented { geometry: Array>, zoom: number, bearing: number, - pixelsToTileUnits: number) => boolean; + pixelsToTileUnits: number, + cameraToCenterDistance: number, + posMatrix: Float32Array) => boolean; constructor(layer: LayerSpecification, properties: {layout?: Properties<*>, paint: Properties<*>}) { super(); diff --git a/src/style/style_layer/circle_style_layer.js b/src/style/style_layer/circle_style_layer.js index 6ffa8865e32..6fbf4cc1632 100644 --- a/src/style/style_layer/circle_style_layer.js +++ b/src/style/style_layer/circle_style_layer.js @@ -2,9 +2,10 @@ const StyleLayer = require('../style_layer'); const CircleBucket = require('../../data/bucket/circle_bucket'); -const {multiPolygonIntersectsBufferedMultiPoint} = require('../../util/intersection_tests'); +const {multiPolygonIntersectsBufferedPoint} = require('../../util/intersection_tests'); const {getMaximumPaintValue, translateDistance, translate} = require('../query_utils'); const properties = require('./circle_style_layer_properties'); +const {vec4} = require('@mapbox/gl-matrix'); const { Transitionable, @@ -41,14 +42,31 @@ class CircleStyleLayer extends StyleLayer { geometry: Array>, zoom: number, bearing: number, - pixelsToTileUnits: number): boolean { + pixelsToTileUnits: number, + cameraToCenterDistance: number, + posMatrix: Float32Array): boolean { const translatedPolygon = translate(queryGeometry, this.paint.get('circle-translate'), this.paint.get('circle-translate-anchor'), bearing, pixelsToTileUnits); const radius = this.paint.get('circle-radius').evaluate(feature) * pixelsToTileUnits; const stroke = this.paint.get('circle-stroke-width').evaluate(feature) * pixelsToTileUnits; - return multiPolygonIntersectsBufferedMultiPoint(translatedPolygon, geometry, radius + stroke); + const size = radius + stroke; + + for (const ring of geometry) { + for (const point of ring) { + let adjustedSize = size; + + if (this.paint.get('circle-pitch-scale') === 'viewport') { + const projectedCenter = vec4.transformMat4([], [point.x, point.y, 0, 1], posMatrix); + adjustedSize *= projectedCenter[3] / cameraToCenterDistance; + } + + if (multiPolygonIntersectsBufferedPoint(translatedPolygon, point, adjustedSize)) return true; + } + } + + return false; } } diff --git a/src/util/intersection_tests.js b/src/util/intersection_tests.js index 7f680d2b9f7..1782ae5e30c 100644 --- a/src/util/intersection_tests.js +++ b/src/util/intersection_tests.js @@ -5,6 +5,7 @@ const {isCounterClockwise} = require('./util'); import type Point from '@mapbox/point-geometry'; module.exports = { + multiPolygonIntersectsBufferedPoint, multiPolygonIntersectsBufferedMultiPoint, multiPolygonIntersectsMultiPolygon, multiPolygonIntersectsBufferedMultiLine, @@ -32,16 +33,20 @@ function polygonIntersectsPolygon(polygonA: Polygon, polygonB: Polygon) { return false; } -function multiPolygonIntersectsBufferedMultiPoint(multiPolygon: MultiPolygon, rings: Array, radius: number) { +function multiPolygonIntersectsBufferedPoint(multiPolygon: MultiPolygon, point: Point, radius: number) { for (let j = 0; j < multiPolygon.length; j++) { const polygon = multiPolygon[j]; - for (let i = 0; i < rings.length; i++) { - const ring = rings[i]; - for (let k = 0; k < ring.length; k++) { - const point = ring[k]; - if (polygonContainsPoint(polygon, point)) return true; - if (pointIntersectsBufferedLine(point, polygon, radius)) return true; - } + if (polygonContainsPoint(polygon, point)) return true; + if (pointIntersectsBufferedLine(point, polygon, radius)) return true; + } + return false; +} + +function multiPolygonIntersectsBufferedMultiPoint(multiPolygon: MultiPolygon, rings: Array, radius: number) { + for (let i = 0; i < rings.length; i++) { + const ring = rings[i]; + for (let k = 0; k < ring.length; k++) { + if (multiPolygonIntersectsBufferedPoint(multiPolygon, ring[k], radius)) return true; } } return false; diff --git a/test/integration/query-tests/circle-pitch-scale/map-inside-align-map/expected.json b/test/integration/query-tests/circle-pitch-scale/map-inside-align-map/expected.json new file mode 100644 index 00000000000..51c0a144bad --- /dev/null +++ b/test/integration/query-tests/circle-pitch-scale/map-inside-align-map/expected.json @@ -0,0 +1,13 @@ +[ + { + "geometry": { + "type": "Point", + "coordinates": [ + -84.3310546875, + 33.92512970007199 + ] + }, + "type": "Feature", + "properties": {} + } +] \ No newline at end of file diff --git a/test/integration/query-tests/circle-pitch-scale/map-inside-align-map/style.json b/test/integration/query-tests/circle-pitch-scale/map-inside-align-map/style.json new file mode 100644 index 00000000000..91e488103ff --- /dev/null +++ b/test/integration/query-tests/circle-pitch-scale/map-inside-align-map/style.json @@ -0,0 +1,45 @@ +{ + "version": 8, + "metadata": { + "test": { + "height": 256, + "queryGeometry": [ + 292, + 35 + ] + } + }, + "center": [ + -92.3780691249957, + -20 + ], + "zoom": 2, + "pitch": 60, + "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-pitch-alignment": "map", + "circle-pitch-scale": "map", + "circle-radius": 20 + } + } + ] +} diff --git a/test/integration/query-tests/circle-pitch-scale/map-outside-align-map/expected.json b/test/integration/query-tests/circle-pitch-scale/map-outside-align-map/expected.json new file mode 100644 index 00000000000..fe51488c706 --- /dev/null +++ b/test/integration/query-tests/circle-pitch-scale/map-outside-align-map/expected.json @@ -0,0 +1 @@ +[] diff --git a/test/integration/query-tests/circle-pitch-scale/map-outside-align-map/style.json b/test/integration/query-tests/circle-pitch-scale/map-outside-align-map/style.json new file mode 100644 index 00000000000..9908102df6b --- /dev/null +++ b/test/integration/query-tests/circle-pitch-scale/map-outside-align-map/style.json @@ -0,0 +1,45 @@ +{ + "version": 8, + "metadata": { + "test": { + "height": 256, + "queryGeometry": [ + 295, + 35 + ] + } + }, + "center": [ + -92.3780691249957, + -20 + ], + "zoom": 2, + "pitch": 60, + "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-pitch-alignment": "map", + "circle-pitch-scale": "map", + "circle-radius": 20 + } + } + ] +} diff --git a/test/integration/query-tests/circle-pitch-scale/viewport-inside-align-map/expected.json b/test/integration/query-tests/circle-pitch-scale/viewport-inside-align-map/expected.json new file mode 100644 index 00000000000..51c0a144bad --- /dev/null +++ b/test/integration/query-tests/circle-pitch-scale/viewport-inside-align-map/expected.json @@ -0,0 +1,13 @@ +[ + { + "geometry": { + "type": "Point", + "coordinates": [ + -84.3310546875, + 33.92512970007199 + ] + }, + "type": "Feature", + "properties": {} + } +] \ No newline at end of file diff --git a/test/integration/query-tests/circle-pitch-scale/viewport-inside-align-map/style.json b/test/integration/query-tests/circle-pitch-scale/viewport-inside-align-map/style.json new file mode 100644 index 00000000000..96a880be0ba --- /dev/null +++ b/test/integration/query-tests/circle-pitch-scale/viewport-inside-align-map/style.json @@ -0,0 +1,45 @@ +{ + "version": 8, + "metadata": { + "test": { + "height": 256, + "queryGeometry": [ + 300, + 35 + ] + } + }, + "center": [ + -92.3780691249957, + -20 + ], + "zoom": 2, + "pitch": 60, + "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-pitch-alignment": "map", + "circle-pitch-scale": "viewport", + "circle-radius": 20 + } + } + ] +} diff --git a/test/integration/query-tests/circle-pitch-scale/viewport-outside-align-map/expected.json b/test/integration/query-tests/circle-pitch-scale/viewport-outside-align-map/expected.json new file mode 100644 index 00000000000..fe51488c706 --- /dev/null +++ b/test/integration/query-tests/circle-pitch-scale/viewport-outside-align-map/expected.json @@ -0,0 +1 @@ +[] diff --git a/test/integration/query-tests/circle-pitch-scale/viewport-outside-align-map/style.json b/test/integration/query-tests/circle-pitch-scale/viewport-outside-align-map/style.json new file mode 100644 index 00000000000..8d3897db2c3 --- /dev/null +++ b/test/integration/query-tests/circle-pitch-scale/viewport-outside-align-map/style.json @@ -0,0 +1,45 @@ +{ + "version": 8, + "metadata": { + "test": { + "height": 256, + "queryGeometry": [ + 303, + 35 + ] + } + }, + "center": [ + -92.3780691249957, + -20 + ], + "zoom": 2, + "pitch": 60, + "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-pitch-alignment": "map", + "circle-pitch-scale": "viewport", + "circle-radius": 20 + } + } + ] +}