text-pitch-alignment #2668
text-pitch-alignment #2668
Conversation
| addVertex(vertexArray, anchorPoint.x, anchorPoint.y, bl.x, bl.y, tex.x, tex.y + tex.h, minZoom, maxZoom, placementZoom); | ||
| addVertex(vertexArray, anchorPoint.x, anchorPoint.y, br.x, br.y, tex.x + tex.w, tex.y + tex.h, minZoom, maxZoom, placementZoom); | ||
| // Encode angle of line together with symbol.alongLine | ||
| var placementAngleLine = symbol.curved ? 0 : 1 + Math.round(((angle % Math.PI) / Math.PI) * 255); |
There was a problem hiding this comment.
angleLine name implies a line rather than angle, perhaps lineAngle would be better?
|
I'm not sold on this PR from a map-design-asthetic standpoint. All the labels should be "billboards" or none of them should be.
For reference, here is a screenshot that demonstrates how Apple Maps draws labels.
(some wordage and screenshots from @1ec5) |
|
@lucaswoj Can you justify why it's inherently better to be consistent? I'm not sure what the objection is if unskewing the text increases readability. Roads like beachmont are much easier to resolve when they're unskewed when compared to their skewed version. There also isn't really a clear way to unskew linearly independent glyphs using this strategy, so until we discover a better solution I agree with not attempting to unskew them. |
|
I agree with @lucaswoj on this one. The mixture of skewed and not skewed labels really messes with perspective and makes the map look distorted. I think it adds to the cognitive load required to orient yourself in 3D when looking at the map. |
|
Per @lucaswoj + @1ec5's feedback I took another pass at unskewing curved labels. Overall the new approach is much cleaner:
But there is one major caveat -- the way quads are positioned for curved labels isn't forgiving to this approach beyond around 45 degrees of pitching (and our general strategy for placing glyphs along curves has some 😖 conflicts with placement on a pitched map that I can capture in a separate ticket). Also worth noting that most other map apps/renderers don't allow extreme pitching even beyond 30 degrees or so. That said I definitely think this is the approach to go with right now and we can circle back around to a larger refactor around curved labels if needed in the future.
Churning through the rest of the backlog above and native port next before we land on the style spec question. |
|
This is awesome @yhahn! Even with the caveats, I feel 😍 about merging this from map-design-asthetic standpoint. Let me know if I can help with the remaining technical backlog. |
|
That example is pretty compelling :). You can definitely see the difference in glyph rotation for curved roads but it's quite minor. My only concern is that without some sort of curvature heuristic there might be cases where the difference between non-linearly placed glyphs is more severe. |
|
@yhahn great work! One question though — placement/collision detection are not affected by unskewing here, so in some cases, labels can appear slightly colliding or protruding beyond the end of a line, right? Do we want to account for that, or is that a subject to follow-up PRs? |
@mourner yes, especially if the pitch angle is very high. Designers can adjust for this pretty well right now as a stopgap. I think this is something we can address in a further iteration/concepting -- OTOH I think to account for pitch when calculating collision it'll require doing redoplacement on map x/y movement in addition to just rotation. |
yhahn
force-pushed
the
unskew
branch
2 times, most recently
from
9fb7000
to
6f5df92
Compare
yhahn
changed the title
|
OP updated as this PR has evolved. Will hop next to updating native and then getting a full set of the rotate/pitch permutations in place in the render tests. |
|
@lucaswoj I think this ready for your review! I've also reviewed the current benchmarks and manually tested the debug page without seeing any noticeable difference in user experience. Let me know if you think we should take on adding some more systematic benchmarks for this work specifically. |
|
Amazing work @yhahn! |
* First pass at port of mapbox/mapbox-gl-js#2668 * RotationAlignmentType => AlignmentType * Handle undefined default value for text-pitch-alignment and implement inheritance for this value from text-rotation-alignment * Update dependencies * Move handling fo undefined default value out of camelize functions
* First pass at unskewed line labels on pitched maps * Unskew curved labels as well * Inherit default value of text-pitch-alignment from text-rotation-alignment. * Update dependencies
* First pass at unskewed line labels on pitched maps * Unskew curved labels as well * Inherit default value of text-pitch-alignment from text-rotation-alignment. * Update dependencies
* First pass at unskewed line labels on pitched maps * Unskew curved labels as well * Inherit default value of text-pitch-alignment from text-rotation-alignment. * Update dependencies
* First pass at unskewed line labels on pitched maps * Unskew curved labels as well * Inherit default value of text-pitch-alignment from text-rotation-alignment. * Update dependencies
Implements full support for the proposed
text-pitch-alignmentproperty.How it works
Backlog
cc @lucaswoj @mollymerp @ansis