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

Complete set of *-rotation-alignment, *-pitch-alignment, and *-pitch-scale properties #4120

Closed
6 of 8 tasks
lucaswoj opened this issue Feb 1, 2017 · 17 comments
Closed
6 of 8 tasks
Assignees
Labels
cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) medium priority

Comments

@lucaswoj
Copy link
Contributor

lucaswoj commented Feb 1, 2017

rotation-pitch-alignment

Full set of properties:


From @lucaswoj on June 9, 2016 15:28

The style spec has the concept of rotation-alignment which specifies how features are placed with respect to map rotation.

  • rotation-alignment: viewport indicates that features should be rotated with respect to the viewport, drawn such that the top of the screen is "up".
  • rotation-alignment: map indicates that features should be rotated with respect to the map, drawn such that north is "up"

I propose that we add the concept of pitch-alignment which specifies how features are placed with respect to map pitch.

  • pitch-alignment: viewport indicates that features should be pitched with respect to the viewport, drawn on plane of the viewport.
  • pitch-alignment: map indicates that features should be pitched with respect to the map, drawn on plane of the map.

cc @yhahn @mourner @tmcw @ansis @jfirebaugh @lbud

Copied from original issue: mapbox/mapbox-gl-style-spec#459

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @yhahn on June 9, 2016 15:40

rotation-pitch-alignment

👍 this makes total sense to me. It's worth noting that atm our renderers don't support this fully but it's something we can work toward.

master gl js/native

rotation pitch text support icon support
map map
map viewport
viewport map
viewport viewport

unskew gl js/native

rotation pitch text support icon support
map map
map viewport
viewport map
viewport viewport

I'll sketch this property out in the unskew branch and PR. @lucaswoj do you feel ok about partial support for this property being out in the wild in our renderers and chipping away at full support iteratively?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

@lucaswoj do you feel ok about partial support for this property being out in the wild in our renderers and chipping away at full support iteratively?

Yup! I'd prefer that to designing our spec to around present partial renderer support.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @1ec5 on June 9, 2016 16:51

I’m implementing something similar for view-backed annotations on iOS in mapbox/mapbox-gl-native#5245. These properties will make it possible for gl-native to eventually support the same effects for GL point annotations too.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @johnnyluu on June 20, 2016 1:0

Dear devs, is there a way for me to implement circle-pitch-alignment at the moment? I need some radius indicator on the map but the circles I added align with the viewport when I add pitch to the map, as shown in the image.

capture

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @yeldarby on August 17, 2016 21:31

@johnnyluu - if you are looking for a workaround, defining a polygon with GeoJSON instead of using the Mapbox-defined Circle works pretty well.

circle

Here is the function I wrote to generate the GeoJSON Polygon

var createGeoJSONCircle = function(center, radiusInKm, points) {
    if(!points) points = 64;

    var coords = {
        latitude: center[1],
        longitude: center[0]
    };

    var km = radiusInKm;

    var ret = [];
    var distanceX = km/(111.320*Math.cos(coords.latitude*Math.PI/180));
    var distanceY = km/110.574;

    var theta, x, y;
    for(var i=0; i<points; i++) {
        theta = (i/points)*(2*Math.PI);
        x = distanceX*Math.cos(theta);
        y = distanceY*Math.sin(theta);

        ret.push([coords.longitude+x, coords.latitude+y]);
    }
    ret.push(ret[0]);

    return {
        "type": "geojson",
        "data": {
            "type": "FeatureCollection",
            "features": [{
                "type": "Feature",
                "geometry": {
                    "type": "Polygon",
                    "coordinates": [ret]
                }
            }]
        }
    };
};

You can use it like this:

map.addSource("polygon", createGeoJSONCircle([-93.6248586, 41.58527859], 0.5));

map.addLayer({
    "id": "polygon",
    "type": "fill",
    "source": "polygon",
    "layout": {},
    "paint": {
        "fill-color": "blue",
        "fill-opacity": 0.6
    }
});

If you need to update the circle you created later you can do it like this (note the need to grab the data property to pass to setData):

map.getSource('polygon').setData(createGeoJSONCircle([-93.6248586, 41.58527859], 1).data);

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @princeofnaxos on October 22, 2016 5:33

+1

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @1ec5 on January 5, 2017 10:36

circle-pitch-scale, icon-pitch-scale, and text-pitch-scale should be renamed circle-scale-alignment, icon-scale-alignment, and text-scale-alignment, respectively: #4165.

@nickidlugash
Copy link

For text-pitch-scale and icon-pitch-scale properties, would it make sense to consider values in a numerical range (rather than map/viewport values), where the range is a proportion of pitch scaling? E.g. 0–1, where 0=no pitch scaling, 1=100% pitch scaling, and 0.5=50% pitch scaling?

Implementing these two properties would be helpful for good labeling of pitched maps (currently many scaled labels are much too small to be legible, and the smaller scale causes undesirable increases in label density), but my instinct is that having no scaling at all would also not be visually ideal in many cases. I think having some scaling is helpful to reinforce the perception of a pitched map, as well as for hierarchy (generally, closer labels are more critical). Also, with no scaling I think too many labels along lines may not get placed, due to label lengths exceeding maximum placement size, and glyph placement angles exceeding text-max-angle on curved lines.

If this sounds like a reasonable suggestion, I can work on testing it out on a branch.

@lucaswoj
Copy link
Contributor Author

@nickidlugash I like that idea! It might be tricky from a style spec perspective because we don't currently have a good way to handle style spec properties that support multiple types. If the prototype is promising, I'm sure we can find a way to make this jive with the spec! Prototype away!

@ChrisLoer
Copy link
Contributor

The hard part of doing this is figuring out how to handle label collisions. The challenge is that when we build the collision tile we don't know ahead of time how far away the tile will be from the viewport (and different labels may respond differently to the distance).

One relatively simple solution would be to make all pan operations on tilted tiles using text-pitch-scale trigger re-placement, allowing us to calculate precise collision boxes for the tile. The downsides of this approach are:

  • It's CPU intensive (although in a navigation app following a route, we're already re-doing placement pretty constantly)
  • The collision tiling will lag a bit behind, so you could get some overlap depending on how CPU-constrained you were
  • At least for now, this approach would break label fading (Fade symbols when colliding on rotation #589)

As an alternative, we could try to do a kind of hacky conservative heuristic collision detection. When calculating the collision box for a tilted tile, we'd treat all symbols as if they had the maximum text-pitch-scale of all the symbols in the tile. When we built the collision boxes, we'd adjust the size of each symbol based on its y-position in the tile (closer symbols = bigger collision box). Then, at render time, when the shaders wanted to see if a label would show, they'd just adjust the "minPlacementZoom" with a factor based on the pitch and y-position of the tile. Because all symbols would behave the same way in response to panning, the relative values in the collision boxes would stay correct.

@nickidlugash Would "labels for a tile are collision detected based on the biggest text-pitch-scale in the tile" be an acceptable constraint for a style designer? On the one hand, it's pretty arcane. On the other hand, it seems pretty odd to want to mix text-pitch-scale: viewport with text-pitch-scale: map, and probably not a disaster if you end up with somewhat lower-than expected label density.

/cc @anandthakker

@nickidlugash
Copy link

@ChrisLoer Re: downsides to making pan operations on tilted tiles using text-pitch-scale trigger re-placement, are these outcomes worse than the outcome already occurring with tilt/rotate operations triggering re-placement? Or more that if we add panning as an additional (more frequently occurring) event that triggers re-placement, we'll compound the existing issues?

Re: heuristic-based approach described above, if all symbol collision boxes get calculated based on the largest text-pitch-scale, wouldn't this make the collision boxes in the lower half of the viewport too small?

Would "labels for a tile are collision detected based on the biggest text-pitch-scale in the tile" be an acceptable constraint for a style designer?

I think in general, some degree of heuristic behavior applied to collision detection is fine from a user perspective. We do a bunch already, between hacks like the current pitch-alignment calculations, to our general strategy of calculating collision boxes once per zoom level (which makes sense but isn't necessarily obvious behavior to users). I think most of our users are either unaware of the concept of collision detection (hence why blog posts like this one are useful), or they are generally aware but don't have detailed understanding of how it works. For most users, I think the ideal experience is to not have to think about collisions at all. While we have style spec properties related to collisions, I think users mainly only use them if they notice something that isn't visually ideal, i.e. the "if it ain't broke don't fix it" approach :P

Others may feel differently, but I think it's fine to just aim for an approach that 1. gives the best overall results by default for as many users as possible and 2. maintains the integrity of the collision-related style spec properties (e.g. increasing -padding values does still clearly create more space around the affected symbols).

Whether this specific heuristic would be too much of a constraint, I think it would be fine. I'm trying to imagine use cases where someone would want to mix very different -pitch-scale values, and they seem like either cases where decreased density would be acceptable, or cases where you may want to use -allow-overlap anyway. The only concern that jumps out to me is if, say, all the symbols in a tile had a value of 0/map, and at the next zoom level, or in the next tile over, there is an additional symbol that has a value 1/viewport, will the visual contrast between these two tiles be jarring?

FWIW, I actually think heuristics along these lines might actually be beneficial for overall label density. One of the reasons -pitch-scale properties are important is that the current behavior (with labels being scaled proportionally to pitch) creates much higher label densities when the map is "further away" from the user – which is the reverse of what actually makes sense from a UX perspective. Features further away from the user are typically less important/less in need of immediate attention, so there should be fewer of them. Using a max -pitch-scale value that more uniformly creates lower density towards the top of the viewport could actually be good default behavior. It's hard to know without seeing prototypes of different options, but that's one theory anyway :P

@andrewharvey
Copy link
Collaborator

andrewharvey commented Apr 9, 2017

Good to see you working on this @ChrisLoer. One use case is to draw a shadow below a symbol marker by using a circle with blur and pitch alignment map, which should work well for 3D maps designed to look like https://www.mapbox.com/blog/3d-leaflet/.

Like the Static API does
600x400

@ChrisLoer
Copy link
Contributor

After experimenting with various text-pitch-scale and icon-pitch-scale settings while working on the pitch-scaling PR (#4547), we've provisionally decided to implement the functionality with a hard-wired value of ".5". Our thinking is roughly:

  • A value of .5 (that is, the effect of distance on label size is reduced by 50%) gives superior results to both 0 and 1 over a range of styles/maps. Where the appearance of existing maps changes, we believe it will generally either (1) not be noticed, or (2) seen as an improvement.
  • Per-layer control of scaling behavior is too much granularity: although it allows style designers to do some interesting tricks, it also puts a lot of burden on the designer.
  • Given the uncertain benefit of exposing these as style properties, and the risk of getting the wrong behavior baked into the style specification, we'll start with a hardwired property. The underlying infrastructure will be in place to expose these properties in the future if we better understand the use-case.

@andrewharvey This is all pretty tangential to the marker-shadow use case you provided (I think circle-pitch-alignment is what we need to support that) -- that's still on our radar.

ChrisLoer added a commit that referenced this issue Jun 22, 2017
Follows model of 'text-pitch-alignment'. Requested in issue #4120.
@mapbox mapbox deleted a comment from lucaswoj Jun 26, 2017
@jfirebaugh jfirebaugh changed the title Expand support for "*-pitch-alignment" and "*-size-alignment" Complete set of *-rotation-alignment, *-pitch-alignment, and *-pitch-scale properties Jun 26, 2017
ChrisLoer added a commit that referenced this issue Jul 3, 2017
Follows model of 'text-pitch-alignment'. Requested in issue #4120.
@ChrisLoer
Copy link
Contributor

These are all finished in GL JS! 🎉 Ports to gl-native are underway.

@irenedelatorre
Copy link

Dear devs, I'm not sure if this is a good place to ask.. but my question is related to pitch values in mapbox. Is there a way to add the pitch of a map into the calculation of the projection of X and Y when that is done externally (for example with D3.js)? If the value is too high (like 60 degrees) x and y are not projected completely correct. Thanks!

@birdage
Copy link

birdage commented Dec 12, 2017

this post saved me!

@thomaszdxsn
Copy link

why dont have a line-pitch-alignment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) medium priority
Projects
None yet
Development

No branches or pull requests

8 participants