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

WIP: Pitched label legibility #4682

Closed
wants to merge 41 commits into from
Closed

Conversation

ChrisLoer
Copy link
Contributor

Splitting changes to improve legibility of curved labels on tilted maps (#3057) out of #4547.

The motivation for these changes is that we want to improve our rendering support for the "navigation" use case, in which:

  • Tilting the map is important, because in turn by turn directions you want to be able to see ahead

  • Legibility becomes more important relative to information density or aesthetics, because users of the map need to be able to read labels with a quick glance.

  • For curved labels with pitch-alignment: viewport: replace "foreshortening" behavior with "legibility expansion" that spaces vertical/tilted glyphs further apart in tile space so that they're still legible in projected space.

  • Make "legibility expansion" use improved approximation math from https://gist.github.com/ChrisLoer/fb8c22845b6c3216afff0f19dc53f6d6

  • Make collision box approximation take account of "legibility expansion"

  • Pack attributes into as little space as possible

  • Explore max-curvature property (or default) to prevent labels in the distance from ending up with silly geometry (e.g. "horseshoe labels")

  • Verify text-max-angle logic (see check_max_angle.js) isn't broken

ChrisLoer added 30 commits May 1, 2017 12:22
 symbols scale relative to the rest of the map as tiles are tilted and
 panned.
 - '1' = previous behavior (symbols have same scale as the surrounding
   features)
 - '0' = symbols maintain the same size no matter what distance they're
   at
 - fraction values = split the difference
- Curved labels currently use a broken approximation to determine glyph
  position.
- This approach makes collision detection much more expensive because we
  pre-calculate collisions over a much greater scale range to account
  for pan operations changing the effective scale of tiles.
 instead of glyph anchors in order to keep all glyphs in sync.
No glyph should be able to show if any one of the glyphs in the label
can't show.
 When doing collision detection, assume all labels are scaled the same
 as the least scaled label in the tile.
Update collision tiles whenever the map moves because tile distance changes.
 and boxes for collision scale calculation.
 (and accessible to color blind folk)
 labels now, but make sure the position of the central boxes doesn't
 change.
Necessary so that we can make collision detection conservative in the
near field as well as the far field.
 Must do pitch scaling based on label anchor point, not box anchor point.
…to buffers because their placementScale is above the maxScale for the collision tile.
 - Don't show boxes over maxScale (it was getting too confusing)
 - Use green background for test so white boxes are visible.
ChrisLoer added 11 commits May 1, 2017 21:41
inserted before first CollisionFeature is added to a CollisionBoxArray.
line labels. This could lead to label collisions in the distant field.
… in:

#2096
Change some variable names from camel-case to underscore-separated.
perspective to make a significant difference in collision detection.
 - Lay out glyphs in tile space so we can use existing curve following
    logic
 - Space glyphs out based on the incidence angle of the middle of the
    label and the orientation of the middle of the label
   (So that a vertical line in the distance gets spaced out a lot)
 - Apply glyph rotation in projected space.

Significant problems:
 - "We're out of space on this line" logic is mixed with collision
   detection logic, and they no longer stay in sync perfectly, so it's
   possible to have individual glyphs clipped before the whole label is
   clipped.
 - The spacing approximation doesn't work for highly curved labels, and
   you end up with silly looking results.
@ChrisLoer
Copy link
Contributor Author

From #4547 (comment):

I got an initial implementation of the fancy trigonometry for label expansion... kind of working. It looks like it's doing a better job of keeping spacing even on curved labels, but there are two big problems:

  • There's lots of glyph flicker/duplication around segment junctions. This might be caused by the placement-time calculations working with higher precision angles than the part of the calculation done in the shader (the positioning calculations only have to disagree by a tiny amount between two segments to cause artifacts). It looks bad enough it might just be a matter of me messing up the math.
  • This approach ties the minZoom and maxZoom for a glyph to the "legibility expansion" calculated for that glyph... which breaks the labelMinScale trick we use at https://github.com/mapbox/mapbox-gl-js/blob/master/src/symbol/quads.js#L218 to make all the glyphs disappear if one of the glyphs has to disappear. To fix this, we'd need to add even more vertex attributes. 😞

Overall, it's feeling like too complicated a solution, but I'll keep poking at it a bit longer to see if I can come up with anything compelling.

Here's a shot showing the algorithm actually doing its thing. This is the kind of label that does poorly with the simple algorithm: the center of the label is at about 45 degrees, while the left side is near vertical and the right side is near horizontal. As a result, the simple algorithm (on the left) squishes the left side and stretches out the right side. The fancy algorithm appears to do a reasonable job of spacing across the length of the label.

vistagrande improvements

@ChrisLoer
Copy link
Contributor Author

/cc @nickidlugash

@ansis ansis mentioned this pull request May 11, 2017
@ChrisLoer
Copy link
Contributor Author

This work was superseded #4781.

@ChrisLoer ChrisLoer closed this Jun 19, 2017
@ChrisLoer ChrisLoer deleted the cloer_pitched_legibility branch June 19, 2017 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant