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

better and faster labeling #1079

Merged
merged 62 commits into from
Mar 13, 2015
Merged

better and faster labeling #1079

merged 62 commits into from
Mar 13, 2015

Conversation

ansis
Copy link
Contributor

@ansis ansis commented Mar 11, 2015

Labeling is now faster and denser. Especially for rotated map views.

before / after for rotated view

screen shot 2015-03-11 at 12 54 18 pm screen shot 2015-03-11 at 12 53 51 pm

We made it faster by using a different representation of labels for collisions than for rendering. For collisions we now represent a label as a series of boxes spacing along the line. Unlike the rendered glyphs, they do not slide along the line and a single box does not represent a single glyph. It's less precise, but way faster. Separating the collision representation from the rendering representation also made the code a lot cleaner.

Making it faster means we can redo placement frequently. We redo placement whenever you rotate the map, which fixes density issues related to rotation.

The new collision debug view, enabled with map.collisionDebug = true, has been very useful:

screen shot 2015-03-11 at 12 42 43 pm

Other improvements:

  • label placement works with perspective view (see this commit)
  • text-max-angle now checks the total anchor within a window the size of a glyph, instead of at individual angles
  • lines are clipped to tile edges before being interpolated to fix density issues near edges
  • line label interpolation has a special case for really short lines to improve labeling for those

I also started using types like SymbolQuad and PositionedGlyph instead of just { ... }, and renaming things that had multiple meanings (like "glyph").

  • @jfirebaugh can you review the changes in /source/* for redoing placement on rotation?
  • @mourner can you review everything else (as much as you want, tons of changes)

test-suite pr: mapbox/mapbox-gl-test-suite#18

ansis and others added 30 commits February 17, 2015 16:49
The general idea is to generate fewer, separate boxes used for placement
and only generate glyphs for rendering after features have been placed.

This commit breaks lots of things.
Conflicts:
	js/data/create_bucket.js
	js/source/vector_tile_source.js
	js/source/worker.js
	js/source/worker_tile.js
	js/symbol/collision.js
For overscaled tiles, line label interpolation now places labels in the
same places it placed them in the parent tile, so that when you zoom the
labels don't shift.
ansis added 21 commits March 9, 2015 14:54
Start another redoPlacement request only when the previous one has
finished.
Conflicts:
	js/data/symbol_bucket.js
	js/render/painter.js
	js/source/vector_tile_source.js
	js/source/worker.js
	js/source/worker_tile.js
	js/style/style.js
	js/symbol/glyph_source.js
	shaders/icon.vertex.glsl
	shaders/sdf.vertex.glsl
See the comment in the commit for more details.
Labels should mostly not overlap in perspective view.

The problem is that the angle from the camera to the surface is
different at the lower edge compared to the upper edge. This means that
labels near the upper edge cover a larger area than at the bottom edge,
even after they have been scaled for size. The only way to really fix
this is to redo placement when panning in the y direction.

The current implementation seems like it might be ok, so let's try this
for a while and see how it works.
Otherwise minScale is sometimes NaN and then GlyphVertexBuffer.add can't
be optimized ("optimized too many times" warning).
@mourner
Copy link
Member

mourner commented Mar 11, 2015

I've been following all your commits here so nothing left to review now. AMAZING. Can't wait to see this merged.

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.

4 participants