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

QueryRenderedFeatures and icon-allow-overlap #5172

Closed
ttsirkia opened this issue Aug 19, 2017 · 11 comments
Closed

QueryRenderedFeatures and icon-allow-overlap #5172

ttsirkia opened this issue Aug 19, 2017 · 11 comments
Assignees
Labels

Comments

@ttsirkia
Copy link

ttsirkia commented Aug 19, 2017

mapbox-gl-js version: 0.39.1

Steps to Trigger Behavior

  1. a symbol with an icon assigned to it and icon-allow-overlap is true
  2. another layer which draws a text label for the symbol, set a different min-zoom
  3. when the label becomes visible, the original symbol is not returned anymore by queryRenderedFeatures although still visible (symbol would be hidden in this case without icon-allow-overlap)

kuva

Expected Behavior

queryRenderedFeatures returns always the visible elements

Actual Behavior

queryRenderedFeatures acts as icon-allow-overlap is false and ignores some visible elements

@Erutan409
Copy link

I'm also encountering this EXACT same issue. Is there any headway on looking into this?

@anandthakker
Copy link
Contributor

We should investigate this bug after #5150 lands in master

@ChrisLoer
Copy link
Contributor

@ttsirkia or @Erutan409 can you test this with the latest version of master now that #5150 has landed? Or provide a JSFiddle showing the problem?

@ttsirkia
Copy link
Author

@ChrisLoer I'll try to reproduce the issue soon and report my findings.

@ttsirkia
Copy link
Author

I can still replicate the bug but the behavior is a bit different. Now the rendered icon is sometimes returned and sometimes not depending on the map position. If I drag the map, I get different results although the icon is always visible. I have to see if I can create a minimal JSFiddle.

@ChrisLoer
Copy link
Contributor

@ttsirkia Thanks! I can take a look once you've got the fiddle.

@Erutan409
Copy link

@ChrisLoer I think I "solved" my issue by dynamically adding the icons after loading the style. So, I'm not experiencing it anymore. But, that's not to say the root cause is fixed. And I unfortunately can't create a test case with the data/style I'm using. I'm not at liberty to make public until it's actually approved to merge into production. Sorry :\

@ttsirkia
Copy link
Author

Here https://jsfiddle.net/0b9m5y2e/ is a simple example to reproduce the issue. Zoom in and see how the counter becomes zero.

@ChrisLoer
Copy link
Contributor

Thanks @ttsirkia, that's a really useful fiddle. There's definitely something wrong here -- I quickly put a breakpoint in the fiddle and saw that the text is getting returned from the underlying query but the icon is getting filtered out. I haven't figured out why yet, but I'll look into it and hopefully we can fix this quickly.

@ttsirkia
Copy link
Author

I noticed that using the same data source for the both layers was crucial. If there were two separate data sources, this bug did not appear.

@ChrisLoer
Copy link
Contributor

It looks to me like the bug is in this code:

https://github.com/mapbox/mapbox-gl-js/blob/master/src/symbol/collision_index.js#L271-L275

There are two items in the collision index (one for the icon, one for the text), but since they're both based on the same feature from the same sourceLayer we early exit and only return one of them (I got confused here reading the code at first: although the style layers are different, under the hood the source layers are both '_geojsonTileLayer'). If the ordering works out such that we return the one from 'textlayer', then it'll get filtered out of the results by the 'iconlayer' filter in the query.

I think the right fix here is to include bucketIndex in the early-exit filter (the bucketIndex corresponds to the layers in the tile, so in your example, there's a GeoJSONTile with two layers iconlayer and textlayer, and the buckets for those layers get assigned indices 0 and 1).

It looks like we've had this bug for a long time, and it managed to survive the refactoring of #5150. ;)

@ChrisLoer ChrisLoer self-assigned this Nov 17, 2017
ChrisLoer added a commit that referenced this issue Nov 17, 2017
Fixes issue #5172: items from multiple layers but sharing a common source feature would only show up in queryRenderedSymbols results for one of the layers.
ChrisLoer added a commit that referenced this issue Nov 17, 2017
Creates two layers that both include a feature from the same source. Filter query results to just one of the layers.
ChrisLoer added a commit that referenced this issue Nov 21, 2017
Fixes issue #5172: items from multiple layers but sharing a common source feature would only show up in queryRenderedSymbols results for one of the layers.
ChrisLoer added a commit that referenced this issue Nov 21, 2017
Creates two layers that both include a feature from the same source. Filter query results to just one of the layers.
@jfirebaugh jfirebaugh mentioned this issue Nov 21, 2017
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

5 participants