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

Collision detection perspective ratios are incorrect #5911

Closed
ChrisLoer opened this issue Dec 21, 2017 · 1 comment
Closed

Collision detection perspective ratios are incorrect #5911

ChrisLoer opened this issue Dec 21, 2017 · 1 comment
Assignees
Labels

Comments

@ChrisLoer
Copy link
Contributor

If you look at a dense map in the distant field, you can find places where the collision boxes for two labels clearly overlap, but both labels are still showing. Look at "Kenneth Hahn State Recreational Area" and "VIEW PARK-WINDSOR HILLS":

screenshot 2017-12-21 14 42 21

The collision boxes themselves look right, and if you look at their rendering code, you'll see they adjust their size by:

highp float collision_perspective_ratio = 0.5 + 0.5 * (u_camera_to_center_distance / camera_to_anchor_distance);

On the other hand, the code in CollisionIndex is using the opposite ratio (p[3] is the same as camera_to_anchor_distance):

perspectiveRatio: 0.5 + 0.5 * (p[3] / this.transform.cameraToCenterDistance)

Why is this not completely broken? Well, we end up dividing by the perspective ratio instead of multiplying by it:

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;

But 1/(.5 + .5 * y/x) is not the same as (.5 + .5 * x/y)! It is however a close enough approximation when x and y are relatively near each other that we didn't notice! 😅 😬

Dividing by tileToViewport always should have felt fishy -- it turned out to mostly work because we inverted the textPixelRatio as well. Inversions all the way down!

I've got a fix that un-inverts everything and makes "collision detection" + "collision debug boxes" + "actually rendered labels" all be consistent (and I believe correct against the expected "attentuates 50% of the effect of perspective scaling" behavior). I'm working through the associated test changes.

Same problem exists in gl-native.

/cc @ansis @jfirebaugh

@ChrisLoer
Copy link
Contributor Author

Fixed with #5913.

ChrisLoer added a commit that referenced this issue Jan 3, 2018
Perspective ratio used for circles wasn't corrected correctly.
Add circle-specific regression test.
ChrisLoer added a commit that referenced this issue Jan 4, 2018
Perspective ratio used for circles wasn't corrected correctly.
Add circle-specific regression test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant