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 collision detection perspective ratios #5913

Merged
merged 2 commits into from
Jan 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions src/shaders/collision_circle.vertex.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ varying vec2 v_extrude_scale;
void main() {
vec4 projectedPoint = u_matrix * vec4(a_anchor_pos, 0, 1);
highp float camera_to_anchor_distance = projectedPoint.w;
highp float collision_perspective_ratio = 0.5 + 0.5 * (camera_to_anchor_distance / u_camera_to_center_distance);
highp float collision_perspective_ratio = 0.5 + 0.5 * (u_camera_to_center_distance / camera_to_anchor_distance);

gl_Position = u_matrix * vec4(a_pos, 0.0, 1.0);

highp float padding_factor = 1.2; // Pad the vertices slightly to make room for anti-alias blur
gl_Position.xy += a_extrude * u_extrude_scale * padding_factor * gl_Position.w / collision_perspective_ratio;
gl_Position.xy += a_extrude * u_extrude_scale * padding_factor * gl_Position.w * collision_perspective_ratio;

v_placed = a_placed.x;
v_notUsed = a_placed.y;
v_radius = abs(a_extrude.y); // We don't pitch the circles, so both units of the extrusion vector are equal in magnitude to the radius

v_extrude = a_extrude * padding_factor;
v_extrude_scale = u_extrude_scale * u_camera_to_center_distance / collision_perspective_ratio;
v_extrude_scale = u_extrude_scale * u_camera_to_center_distance * collision_perspective_ratio;
}
2 changes: 1 addition & 1 deletion src/source/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ class Tile {
const posMatrix = collisionIndex.transform.calculatePosMatrix(this.tileID.toUnwrapped());

const pitchWithMap = bucket.layers[0].layout.get('text-pitch-alignment') === 'map';
const textPixelRatio = EXTENT / this.tileSize; // text size is not meant to be affected by scale
const textPixelRatio = this.tileSize / EXTENT; // text size is not meant to be affected by scale
const pixelRatio = pixelsToTileUnits(this, 1, collisionIndex.transform.zoom);

const labelPlaneMatrix = projection.getLabelPlaneMatrix(posMatrix, pitchWithMap, true, collisionIndex.transform, pixelRatio);
Expand Down
27 changes: 16 additions & 11 deletions src/symbol/collision_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ class CollisionIndex {
placeCollisionBox(collisionBox: SingleCollisionBox, allowOverlap: boolean, textPixelRatio: number, posMatrix: mat4): Array<number> {
const projectedPoint = this.projectAndGetPerspectiveRatio(posMatrix, collisionBox.anchorPointX, collisionBox.anchorPointY);
const tileToViewport = textPixelRatio * projectedPoint.perspectiveRatio;
const tlX = collisionBox.x1 / tileToViewport + projectedPoint.point.x;
const tlY = collisionBox.y1 / tileToViewport + projectedPoint.point.y;
const brX = collisionBox.x2 / tileToViewport + projectedPoint.point.x;
const brY = collisionBox.y2 / tileToViewport + projectedPoint.point.y;
const tlX = collisionBox.x1 * tileToViewport + projectedPoint.point.x;
const tlY = collisionBox.y1 * tileToViewport + projectedPoint.point.y;
const brX = collisionBox.x2 * tileToViewport + projectedPoint.point.x;
const brY = collisionBox.y2 * tileToViewport + projectedPoint.point.y;

if (!allowOverlap) {
if (this.grid.hitTest(tlX, tlY, brX, brY)) {
Expand Down Expand Up @@ -138,8 +138,10 @@ class CollisionIndex {
let collisionDetected = false;

const tileToViewport = projectedAnchor.perspectiveRatio * textPixelRatio;
// pixelsToTileUnits is used for translating line geometry to tile units
// ... so we care about 'scale' but not 'perspectiveRatio'
// equivalent to pixel_to_tile_units
const pixelsToTileUnits = tileToViewport / scale;
const pixelsToTileUnits = 1 / (textPixelRatio * scale);

let firstTileDistance = 0, lastTileDistance = 0;
if (firstAndLastGlyph) {
Expand All @@ -163,7 +165,7 @@ class CollisionIndex {
}

const projectedPoint = this.projectPoint(posMatrix, anchorPointX, anchorPointY);
const radius = tileUnitRadius / tileToViewport;
const radius = tileUnitRadius * tileToViewport;

const atLeastOneCirclePlaced = placedCollisionCircles.length > 0;
if (atLeastOneCirclePlaced) {
Expand Down Expand Up @@ -292,10 +294,10 @@ class CollisionIndex {
// to work with.
const projectedPoint = this.projectAndGetPerspectiveRatio(posMatrix, blocking.anchorPointX, blocking.anchorPointY);
const tileToViewport = textPixelRatio * projectedPoint.perspectiveRatio;
const x1 = blocking.x1 / tileToViewport + projectedPoint.point.x;
const y1 = blocking.y1 / tileToViewport + projectedPoint.point.y;
const x2 = blocking.x2 / tileToViewport + projectedPoint.point.x;
const y2 = blocking.y2 / tileToViewport + projectedPoint.point.y;
const x1 = blocking.x1 * tileToViewport + projectedPoint.point.x;
const y1 = blocking.y1 * tileToViewport + projectedPoint.point.y;
const x2 = blocking.x2 * tileToViewport + projectedPoint.point.x;
const y2 = blocking.y2 * tileToViewport + projectedPoint.point.y;
const bbox = [
new Point(x1, y1),
new Point(x2, y1),
Expand Down Expand Up @@ -356,7 +358,10 @@ class CollisionIndex {
);
return {
point: a,
perspectiveRatio: 0.5 + 0.5 * (p[3] / this.transform.cameraToCenterDistance)
// See perspective ratio comment in symbol_sdf.vertex
// We're doing collision detection in viewport space so we need
// to scale down boxes in the distance
perspectiveRatio: 0.5 + 0.5 * (this.transform.cameraToCenterDistance / p[3])
};
}

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
{
"version": 8,
"metadata": {
"test": {
"collisionDebug": true,
"width": 256,
"height": 2048,
"description": "Issue #5911 caused items in the distance on a pitched map to have collision boxes that were too small, although the debug box drew correctly. Before the fix, this test would show both labels, with the boxes visibly overlapping."
}
},
"center": [
0,
70
],
"zoom": 5,
"pitch": 60,
"bearing": 0,
"sources": {
"geojson": {
"type": "geojson",
"data": {
"type": "MultiPoint",
"coordinates": [
[
-0.8,
81.25
],
[
1,
82
]
]
}
}
},
"glyphs": "local://glyphs/{fontstack}/{range}.pbf",
"sprite": "local://sprites/sprite",
"layers": [
{
"id": "background",
"type": "background",
"paint": {
"background-color": "white"
}
},
{
"id": "symbol",
"type": "symbol",
"source": "geojson",
"layout": {
"symbol-placement": "point",
"text-field": "test\ntest test\ntest",
"text-pitch-alignment": "viewport",
"text-font": [
"Open Sans Semibold",
"Arial Unicode MS Bold"
]
}
}
]
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.