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

Ignore "holes" in query*Features collision detection #2293

Closed
ansis opened this issue Mar 18, 2016 · 4 comments
Closed

Ignore "holes" in query*Features collision detection #2293

ansis opened this issue Mar 18, 2016 · 4 comments

Comments

@ansis
Copy link
Contributor

ansis commented Mar 18, 2016

Polygon holes don't need to be indexed:

// TODO: skip holes when we start using vector tile spec 2.0

@lucaswoj lucaswoj changed the title update feature indexing for vectortile spec 2 Ignore "holes" in query*Features collision detection Jul 29, 2016
@mourner
Copy link
Member

mourner commented Jul 25, 2018

@ansis Can you clarify this — why would we ignore holes in polygons?

@ansis
Copy link
Contributor Author

ansis commented Jul 26, 2018

We don't need to index holes because the ring containing the hole is indexed. After looking up features in the index we use the actual geometry (including holes) to make sure the intersection is real.

But we should remove this comment. We can't (with v1 tiles) know whether something is a hole right now, and even then I don't think it would be significant.

@mourner
Copy link
Member

mourner commented Jul 26, 2018

We can't (with v1 tiles) know whether something is a hole right now

We can because we rely on v2 tiles since switching to Earcut. E.g. here https://github.com/mapbox/mapbox-gl-js/blob/master/src/util/classify_rings.js

@ansis
Copy link
Contributor Author

ansis commented Jul 26, 2018

Right, of course! I'd guess that skipping the hole is still not really worth it

@mourner mourner closed this as completed Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants