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

"Map#queryRenderedFeatures" fails on features whose size is determined by a property function #3604

Closed
jfirebaugh opened this issue Nov 12, 2016 · 8 comments
Assignees
Labels

Comments

@jfirebaugh
Copy link
Contributor

queryRenderedFeatures fails to return results for circles that use a data-driven circle-radius property, due to the use of paint['circle-radius'] here.

This is a special case of #3044. Possibly a hard one to fix. @ansis did we ever discuss how to solve this?

jfirebaugh added a commit to mapbox/mapbox-gl-test-suite that referenced this issue Nov 12, 2016
@ansis
Copy link
Contributor

ansis commented Nov 22, 2016

Feature querying works in two parts:

First it does a grid lookup to get all the features that might intersect with the query. Then it checks all the candidates for whether they actually intersect the query.

For the first part you need to know how big to make the box for the grid query. Features are indexed by the source geometry so you need to know how far the drawn representation from the actual geometry. This is calculated here by looping over all the style layers. This doesn't account for data-driven styling. To fix this we either need to:

  • write a way to calculate the maximum possible value of a data driven function at the current zoom
  • or, hardcode it to the maximum spec values (which might have perf implications, but might be ok?)

For the second part, all the the paint[prop] accesses need to be replaced with getPaintValue to calculate the value for that individual feature.

@jfirebaugh
Copy link
Contributor Author

jfirebaugh commented Nov 23, 2016

A similar issue exists for data-driven line-width, line-gap-width, and line-offset.

@anandthakker
Copy link
Contributor

write a way to calculate the maximum possible value of a data driven function at the current zoom

@ansis @jfirebaugh This is surely a naive question, but: when we're doing this querying, have the tile's buckets/buffers already been populated w/ the calculated DDS values? If so, could we read the max value for the tile directly from the buffer data instead of trying to determine it analytically or using a global max?

@jfirebaugh
Copy link
Contributor Author

If you had access to the buffer data, yes, you could do that in theory. However, we discard the JS-side buffer data once it's uploaded to the GPU, since we don't otherwise need it and it consumes memory.

@lucaswoj lucaswoj changed the title queryRenderedFeatures fails on data-driven circle-radius "Map#queryRenderedFeatures" fails on features whose size is determined by a property function Jan 26, 2017
@stevage
Copy link
Contributor

stevage commented Feb 2, 2017

In case anyone needs a live repro case:

  1. https://city-of-melbourne.github.io/anything-map/#gh7s-qda8
  2. Hover over a circle
  3. Click 'resi_dwellings' in the table on the right
  4. Hover over more circles, watch when the mouse cursor changes.

In this use case, the end result is not severe - small circles are just a bit too easy to select.

@anandthakker
Copy link
Contributor

If you had access to the buffer data, yes, you could do that in theory. However, we discard the JS-side buffer data once it's uploaded to the GPU, since we don't otherwise need it and it consumes memory.

@jfirebaugh I am looking into a strategy where, when we deserialize bucket data on the main thread, we scan the paint attribute arraybuffers and save the maximum value of each paint attribute (as metadata on buffers.programConfugration.layerData[id]). I had originally dismissed this idea as too cumbersome/complicated, but it actually doesn't seem too bad.

@jfirebaugh
Copy link
Contributor Author

@anandthakker How about tracking the maximum while populating the buffers, and transferring it as auxiliary data to the buffer itself?

@anandthakker
Copy link
Contributor

@jfirebaugh hah yeah, just came back to say almost the same thing. One (implementation detail) difference: it may be cleaner to collate the maximums from each layer(/array) and send them back bundled into the serialized FeatureIndex.

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

6 participants