Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Re-implement "avoid edges" for static map tile generation #12461

Closed
ChrisLoer opened this issue Jul 23, 2018 · 4 comments
Closed

Re-implement "avoid edges" for static map tile generation #12461

ChrisLoer opened this issue Jul 23, 2018 · 4 comments
Labels
Node.js node-mapbox-gl-native

Comments

@ChrisLoer
Copy link
Contributor

When we implemented global collision detection (#10436), we removed the per-tile implementation of the symbol-avoid-edges style property because it was no longer necessary -- global collision detection handled the tile boundaries. For static tile generation, we relied on running "global" collision detection on the tile including the symbols in its buffers, in order to handle collisions at the boundary.

That approach works for point labels, but it can break down with line labels because the anchors can get placed at different locations in neighboring tiles. I don't know how common this will actually be in practice, but in theory it will cause more bad label clipping at tile boundaries in api-gl.

I think to implement this we'd project the tile boundaries into viewport space and then do a check against that bounding geometry before we tried testing the line label against the rest of the collision index.

/cc @ansis @jfirebaugh @ian29

@ChrisLoer
Copy link
Contributor Author

I spent some time looking at one of our core navigation styles using an updated version of api-gl. My impressions were:

  • With tile boundary debug lines turned on, it was pretty easy to find streets that brushed up against a tile boundary and were thus partially clipped.
  • With the boundary lines off, it took a little bit of searching to find cases of clipping, it usually wasn't very visually obvious. The one situation where it gets pretty noticeable is when you have a north-south aligned street grid and a major street/highway lines up right along the tile boundary.
  • I noticed other tile-clipping issues for point labels that are presumably unrelated to this regression (my guess these are cases where a point symbol is not included in the buffers of a neighbor for some reason).

screenshot 2018-07-27 15 49 21

screenshot 2018-07-27 16 07 22

I think to implement this we'd project the tile boundaries into viewport space and then do a check against that bounding geometry...

It occurs to me that since this only matters for api-gl, we could even get away with skipping most of the logic necessary for handling off-axis (e.g pitched) tile boundaries.

My gut right now is that this is small-medium fix difficulty and small-medium priority -- i'm going to drop it off my immediate todo list, but anyone with more perspective feel free to bump it back up.

cc @lilykaiser

@lilykaiser
Copy link

Misread this 🤦‍♀️ I will unassign you

@ChrisLoer
Copy link
Contributor Author

I looked through a bunch of mapbox core styles, and we barely use symbol-avoid-edges in them -- and not at all in for instance streets-v10. Also, as far as I can tell from looking at the old implementation (pre-global collision detection), we never defaulted line labels to avoid edges (See old behavior at calls to placeFeature in https://github.com/mapbox/mapbox-gl-native/blob/18d735ccc20e1e7d4616b3d3b5b1a527a01d1b91/src/mbgl/layout/symbol_layout.cpp). So we would have had these problems before too.

We should still re-implement support for symbol-avoid-edges in the tiled map case just so we support styles that do use it, but unless I'm missing something we've already been living with the line-clipping issues.

As part of re-implementing symbol-avoid-edges, maybe it makes sense to make the default for line layers be true? The overall effect would be something like:

  • Decrease line label density overall by conservatively removing them even when they might cross a tile boundary
  • Greatly reduce (eliminate?) line labels getting clipped at tile boundaries
  • Increase point symbol density at tile boundaries (because the line labels got thinned out)
  • Maybe (?) decrease point label clipping at tile boundaries? It would still be the case that a point label could get collided in one tile and not the other, but point labels near boundaries would have relatively fewer unpredictable line labels nearby and thus be more likely to get the same result between tiles.

ChrisLoer added a commit that referenced this issue Jul 31, 2018
 - Fixes issue #12461.
 - Only implement "avoid edges" in MapMode::Tile since it's no longer relevant in Static or Continuous mode.
 - New: Force "avoid edges" to "true" for line labels, since in tile mode they'll always clip poorly at tile boundaries.
 - Remove unused "withinPlus0/inside" logic.
ChrisLoer added a commit that referenced this issue Aug 17, 2018
 - Fixes issue #12461.
 - Only implement "avoid edges" in MapMode::Tile since it's no longer relevant in Static or Continuous mode.
 - New: Force "avoid edges" to "true" for line labels, since in tile mode they'll always clip poorly at tile boundaries.
 - Remove unused "withinPlus0/inside" logic.
ChrisLoer added a commit that referenced this issue Aug 20, 2018
 - Fixes issue #12461.
 - Only implement "avoid edges" in MapMode::Tile since it's no longer relevant in Static or Continuous mode.
 - New: Force "avoid edges" to "true" for line labels, since in tile mode they'll always clip poorly at tile boundaries.
 - Remove unused "withinPlus0/inside" logic.
ChrisLoer added a commit that referenced this issue Aug 20, 2018
 - Fixes issue #12461.
 - Only implement "avoid edges" in MapMode::Tile since it's no longer relevant in Static or Continuous mode.
 - New: Force "avoid edges" to "true" for line labels, since in tile mode they'll always clip poorly at tile boundaries.
 - Remove unused "withinPlus0/inside" logic.
@ChrisLoer
Copy link
Contributor Author

Fixed in #12520.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Node.js node-mapbox-gl-native
Projects
None yet
Development

No branches or pull requests

2 participants