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 incorrect for symbol layers that share the same layout properties #6548

Closed
ChrisLoer opened this issue Apr 20, 2018 · 1 comment
Assignees
Labels
Milestone

Comments

@ChrisLoer
Copy link
Contributor

When we generate symbol buckets for a tile, we use the groupByLayout function to group together all style layers that share a common set of layout properties and a common source layer. Since the two layers share common layout data, they can also share a SymbolBucket, even though they may render differently due to different paint properties.

The shared SymbolBucket breaks the current assumptions of our CrossTileSymbolIndex, because for each layer, we store each symbol's crossTileID in the associated bucket's SymbolInstances. When two symbols from two different layers share the same SymbolInstance, they can overwrite each other's crossTileIDs.

One way this could manifest is:

  • Layer1 and Layer2 share the same bucket, and the layers are allowed to collide with each other.
  • Layer1 gets placed
  • Layer2 gets placed, but matches all the symbols for Layer1, so every SymbolInstance is marked collided
  • At render time, symbols in Layer1 pick up opacities from the collided Layer2, so that no symbols are shown from either layer.

I think the fix here is just to have SymbolInstance store crossTileIDs on a per-layer basis.

/cc @ansis

@ChrisLoer ChrisLoer self-assigned this Apr 20, 2018
ChrisLoer added a commit that referenced this issue Apr 20, 2018
Fixes issue #6548.
Restores behavior from before PR #5150 -- all layers that share the same bucket (and thus same layout properties) share the same placement.
Before this fix, two layers with the same layout properties could collide against each other, and because they shared CrossTileIDs, _both_ layers could end up hidden.
ansis added a commit that referenced this issue Apr 23, 2018
The layer grouping optimizing doesn't ever really help for symbol layers
and removing it fixes problems related to layers sharing cross tile ids.
ChrisLoer added a commit that referenced this issue Apr 23, 2018
Fixes issue #6548.
Restores behavior from before PR #5150 -- all layers that share the same bucket (and thus same layout properties) share the same placement.
Before this fix, two layers with the same layout properties could collide against each other, and because they shared CrossTileIDs, _both_ layers could end up hidden.
ChrisLoer added a commit that referenced this issue Apr 24, 2018
Fixes issue #6548.
Restores behavior from before PR #5150 -- all layers that share the same bucket (and thus same layout properties) share the same placement.
Before this fix, two layers with the same layout properties could collide against each other, and because they shared CrossTileIDs, _both_ layers could end up hidden.
ChrisLoer added a commit that referenced this issue Apr 25, 2018
Fixes issue #6548.
Restores behavior from before PR #5150 -- all layers that share the same bucket (and thus same layout properties) share the same placement.
Before this fix, two layers with the same layout properties could collide against each other, and because they shared CrossTileIDs, _both_ layers could end up hidden.
@ChrisLoer ChrisLoer added this to the 0.45.0 milestone Apr 25, 2018
@ChrisLoer
Copy link
Contributor Author

Fixed in #6558/#6556.

ChrisLoer added a commit to mapbox/mapbox-gl-native that referenced this issue Apr 28, 2018
Native version of mapbox/mapbox-gl-js#6548.
Port of mapbox/mapbox-gl-js#6550.
Prevents symbols that share the same layout properties from colliding against each other.

Bump GL JS pin to get regression test.
Rename "bucketName" -> "bucketLeaderID" to make it clearer what it represents.
ChrisLoer added a commit to mapbox/mapbox-gl-native that referenced this issue May 3, 2018
Native version of mapbox/mapbox-gl-js#6548.
Port of mapbox/mapbox-gl-js#6550.
Prevents symbols that share the same layout properties from colliding against each other.

Bump GL JS pin to get regression test.
Rename "bucketName" -> "bucketLeaderID" to make it clearer what it represents.
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