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.

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

fix fill-extrusion querying #7499

Merged
merged 18 commits into from
Jan 9, 2019
Merged
85 changes: 85 additions & 0 deletions debug/extrusion-query.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<!DOCTYPE html>
<html>
<head>
<title>Mapbox GL JS debug page</title>
<meta charset='utf-8'>
<meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=no">
<link rel='stylesheet' href='/dist/mapbox-gl.css' />
<style>
body { margin: 0; padding: 0; }
html, body, #map { height: 100%; }
</style>
</head>

<body>
<div id='map'></div>

<script src='/dist/mapbox-gl-dev.js'></script>
<script src='/debug/access_token_generated.js'></script>
<script>

var map = window.map = new mapboxgl.Map({
container: 'map',
zoom: 16.9,
center: [-73.989816, 40.76263],
pitch: 60,
style: 'mapbox://styles/mapbox/streets-v10',
hash: true
});

const r = 255 * 0.65;
map.on('style.load', function() {
map.addLayer({
'id': '3d-buildings',
//'source': 'composite',
'source': {
'type': 'vector',
'url': 'mapbox://mapbox.3d-buildings'
},
'source-layer': 'building',
'filter': ['==', 'extrude', 'true'],
'type': 'fill-extrusion',
'minzoom': 15,
'paint': {
'fill-extrusion-color': ['rgb', ['number', ['feature-state', 'hover-r'], r], ['number', ['feature-state', 'hover-g'], r], ['number', ['feature-state', 'hover-b'], r]],
'fill-extrusion-height': ["get", "height"],
'fill-extrusion-opacity': 0.6

}
});

let hovered = [];
window.addEventListener('mousemove', function(e) {
e.point = new mapboxgl.Point(e.clientX, e.clientY);
console.time('query');
const features = map.queryRenderedFeatures(e.point, { layers: ['3d-buildings'] });
console.timeEnd('query');

for (const feature of hovered) {
map.setFeatureState(feature, {
'hover-r': r,
'hover-g': r,
'hover-b': r
});
}

const seen = {};
hovered = features;
let i = 0;
for (const feature of hovered) {
if (seen[feature.id]) continue;

seen[feature.id] = true;
map.setFeatureState(feature, {
'hover-r': i === 0 ? 255 : r,
'hover-g': i === 1 ? 255 : r,
'hover-b': i >= 2 ? 255 : r
});
i++;
}
});
});

</script>
</body>
</html>
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"geojson-rewind": "^0.3.0",
"geojson-vt": "^3.2.1",
"gl-matrix": "^2.6.1",
"grid-index": "^1.0.0",
"grid-index": "^1.1.0",
"minimist": "0.0.8",
"murmurhash-js": "^1.0.0",
"pbf": "^3.0.5",
Expand Down
2 changes: 1 addition & 1 deletion src/data/bucket/fill_extrusion_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class FillExtrusionBucket implements Bucket {
this.addFeature(patternFeature, geometry, index, {});
}

options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index);
options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index, true);
}
}

Expand Down
64 changes: 42 additions & 22 deletions src/data/feature_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { OverscaledTileID } from '../source/tile_id';
import { register } from '../util/web_worker_transfer';
import EvaluationParameters from '../style/evaluation_parameters';
import SourceFeatureState from '../source/source_state';
import {polygonIntersectsBox} from '../util/intersection_tests';

import type StyleLayer from '../style/style_layer';
import type {FeatureFilter} from '../style-spec/feature_filter';
Expand All @@ -25,10 +26,11 @@ import { FeatureIndexArray } from './array_types';

type QueryParameters = {
scale: number,
posMatrix: Float32Array,
pixelPosMatrix: Float32Array,
transform: Transform,
tileSize: number,
queryGeometry: Array<Array<Point>>,
queryGeometry: Array<Point>,
cameraQueryGeometry: Array<Point>,
queryPadding: number,
params: {
filter: FilterSpecification,
Expand All @@ -42,6 +44,7 @@ class FeatureIndex {
y: number;
z: number;
grid: Grid;
grid3D: Grid;
featureIndexArray: FeatureIndexArray;

rawTileData: ArrayBuffer;
Expand All @@ -58,13 +61,16 @@ class FeatureIndex {
this.y = tileID.canonical.y;
this.z = tileID.canonical.z;
this.grid = grid || new Grid(EXTENT, 16, 0);
this.grid3D = new Grid(EXTENT, 16, 0);
this.featureIndexArray = featureIndexArray || new FeatureIndexArray();
}

insert(feature: VectorTileFeature, geometry: Array<Array<Point>>, featureIndex: number, sourceLayerIndex: number, bucketIndex: number) {
insert(feature: VectorTileFeature, geometry: Array<Array<Point>>, featureIndex: number, sourceLayerIndex: number, bucketIndex: number, is3D?: boolean) {
const key = this.featureIndexArray.length;
this.featureIndexArray.emplaceBack(featureIndex, sourceLayerIndex, bucketIndex);

const grid = is3D ? this.grid3D : this.grid;

for (let r = 0; r < geometry.length; r++) {
const ring = geometry[r];

Expand All @@ -81,7 +87,7 @@ class FeatureIndex {
bbox[1] < EXTENT &&
bbox[2] >= 0 &&
bbox[3] >= 0) {
this.grid.insert(key, bbox[0], bbox[1], bbox[2], bbox[3]);
grid.insert(key, bbox[0], bbox[1], bbox[2], bbox[3]);
}
}
}
Expand All @@ -105,23 +111,22 @@ class FeatureIndex {
const queryGeometry = args.queryGeometry;
const queryPadding = args.queryPadding * pixelsToTileUnits;

let minX = Infinity;
let minY = Infinity;
let maxX = -Infinity;
let maxY = -Infinity;
for (let i = 0; i < queryGeometry.length; i++) {
const ring = queryGeometry[i];
for (let k = 0; k < ring.length; k++) {
const p = ring[k];
minX = Math.min(minX, p.x);
minY = Math.min(minY, p.y);
maxX = Math.max(maxX, p.x);
maxY = Math.max(maxY, p.y);
}
const bounds = getBounds(queryGeometry);
const matching = this.grid.query(bounds.minX - queryPadding, bounds.minY - queryPadding, bounds.maxX + queryPadding, bounds.maxY + queryPadding);

const cameraBounds = getBounds(args.cameraQueryGeometry);
const matching3D = this.grid3D.query(
cameraBounds.minX - queryPadding, cameraBounds.minY - queryPadding, cameraBounds.maxX + queryPadding, cameraBounds.maxY + queryPadding,
(bx1, by1, bx2, by2) => {
return polygonIntersectsBox(args.cameraQueryGeometry, bx1 - queryPadding, by1 - queryPadding, bx2 + queryPadding, by2 + queryPadding);
});

for (const key of matching3D) {
ansis marked this conversation as resolved.
Show resolved Hide resolved
matching.push(key);
}

const matching = this.grid.query(minX - queryPadding, minY - queryPadding, maxX + queryPadding, maxY + queryPadding);
matching.sort(topDownFeatureComparator);

const result = {};
let previousIndex;
for (let k = 0; k < matching.length; k++) {
Expand Down Expand Up @@ -150,7 +155,7 @@ class FeatureIndex {
// `feature-state` expression evaluation requires feature state to be available
featureState = sourceFeatureState.getState(styleLayer.sourceLayer || '_geojsonTileLayer', feature.id);
}
return styleLayer.queryIntersectsFeature(queryGeometry, feature, featureState, featureGeometry, this.z, args.transform, pixelsToTileUnits, args.posMatrix);
return styleLayer.queryIntersectsFeature(queryGeometry, feature, featureState, featureGeometry, this.z, args.transform, pixelsToTileUnits, args.pixelPosMatrix);
}
);
}
Expand All @@ -166,7 +171,7 @@ class FeatureIndex {
filter: FeatureFilter,
filterLayerIDs: Array<string>,
styleLayers: {[string]: StyleLayer},
intersectionTest?: (feature: VectorTileFeature, styleLayer: StyleLayer) => boolean) {
intersectionTest?: (feature: VectorTileFeature, styleLayer: StyleLayer) => boolean | number) {

const layerIDs = this.bucketLayerIDs[bucketIndex];
if (filterLayerIDs && !arraysIntersect(filterLayerIDs, layerIDs))
Expand All @@ -189,7 +194,8 @@ class FeatureIndex {
const styleLayer = styleLayers[layerID];
if (!styleLayer) continue;

if (intersectionTest && !intersectionTest(feature, styleLayer)) {
const intersectionZ = !intersectionTest || intersectionTest(feature, styleLayer);
if (!intersectionZ) {
// Only applied for non-symbol features
continue;
}
Expand All @@ -200,7 +206,7 @@ class FeatureIndex {
if (layerResult === undefined) {
layerResult = result[layerID] = [];
}
layerResult.push({ featureIndex, feature: geojsonFeature });
layerResult.push({ featureIndex, feature: geojsonFeature, intersectionZ });
}
}

Expand Down Expand Up @@ -251,6 +257,20 @@ register(

export default FeatureIndex;

function getBounds(geometry: Array<Point>) {
let minX = Infinity;
let minY = Infinity;
let maxX = -Infinity;
let maxY = -Infinity;
for (const p of geometry) {
minX = Math.min(minX, p.x);
minY = Math.min(minY, p.y);
maxX = Math.max(maxX, p.x);
maxY = Math.max(maxY, p.y);
}
return { minX, minY, maxX, maxY };
}

function topDownFeatureComparator(a, b) {
return b - a;
}
53 changes: 53 additions & 0 deletions src/geo/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,59 @@ class Transform {
const topPoint = vec4.transformMat4(p, p, this.pixelMatrix);
return topPoint[3] / this.cameraToCenterDistance;
}

/*
* The camera looks at the map from a 3D (lng, lat, altitude) location. Let's use `cameraLocation`
* as the name for the location under the camera and on the surface of the earth (lng, lat, 0).
* `cameraPoint` is the projected position of the `cameraLocation`.
*
* This point is useful to us because only fill-extrusions that are between `cameraPoint` and
* the query point on the surface of the earth can extend and intersect the query.
*
* When the map is not pitched the `cameraPoint` is equivalent to the center of the map because
* the camera is right above the center of the map.
*/
getCameraPoint() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I struggled figuring out just from the name and the math, what "camera point" meant. I think we should comment (and maybe rename?). Here's the intuition I worked out for how to think of it:

    // For a given query point, this represents an upper bound on how far
    // away the base of something rendered at that point could be.
    // With pitch=0, this is essentially dividing the screen into quadrants
    // around the center point of the screen, since perspective just pushes
    // the top of extrusions "away" from the center.
    // As pitch increases, the potential base for a building shifts "down"
    // towards the bottom of the screen, and potentially below the bottom of
    // the screen

Although I think that's probably not the most accurate description. Am I right that this calculation is basically just super conservative -- always include half the screen horizontally, and for the y-coordinate shift use Math.tan(pitch) * this.cameraToCenterDistance as a rough approximation of how far down the "tallest building we could render" would go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 0b38e35

const pitch = this._pitch;
const yOffset = Math.tan(pitch) * (this.cameraToCenterDistance || 1);
return this.centerPoint.add(new Point(0, yOffset));
}

/*
* When the map is pitched, some of the 3D features that intersect a query will not intersect
* the query at the surface of the earth. Instead the feature may be closer and only intersect
* the query because it extrudes into the air.
*
* This returns a geometry that includes all of the original query as well as all possible ares of the
* screen where the *base* of a visible extrusion could be.
* - For point queries, the line from the query point to the "camera point"
* - For other geometries, the envelope of the query geometry and the "camera point"
*/
getCameraQueryGeometry(queryGeometry: Array<Point>): Array<Point> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Notes I made for myself trying to figure out how this worked:

    // Given a query geometry in screen coordinates, return a geometry that includes
    // all of the original query as well as all possible areas of the screen where the
    // *base* of a visible extrusion could be.
    //  - For point queries, the line from the query point to the "camera point"
    //  - For other geometries, the envelope of the query geometry and the "camera point"

I think this should be commented to explain what it means... I'm still kind of working that out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

improved in 92d59b7, thanks for the wording!

const c = this.getCameraPoint();

if (queryGeometry.length === 1) {
return [queryGeometry[0], c];
} else {
let minX = c.x;
let minY = c.y;
let maxX = c.x;
let maxY = c.y;
for (const p of queryGeometry) {
minX = Math.min(minX, p.x);
minY = Math.min(minY, p.y);
maxX = Math.max(maxX, p.x);
maxY = Math.max(maxY, p.y);
}
return [
new Point(minX, minY),
new Point(maxX, minY),
new Point(maxX, maxY),
new Point(minX, maxY),
new Point(minX, minY)
ChrisLoer marked this conversation as resolved.
Show resolved Hide resolved
];
}
}
}

export default Transform;
Loading