mapbox / mapbox-gl-style-spec

76 stars 38 forks source link

text-padding property that affects only that layer #269

Closed ansis closed 9 years ago

ansis commented 9 years ago

@nickidlugash and @peterqliu said that sometimes you want to thin out labels within a certain layer without blocking labels in other layers. We could do this by having a second padding property that only affects other labels in the current layer.

What is this property called in carto?

peterqliu commented 9 years ago

I see text-repeat-distance in Studio:

screen shot 2015-03-24 at 4 58 52 pm

cc/ @ajashton

peterqliu commented 9 years ago

I would actually really like this layer-specific padding, as an antidote to the super-packed freeway shields introduced by the new denser labeling code.

peterqliu commented 9 years ago

@ansis would this be a big lift?

ansis commented 9 years ago

@peterqliu no

nickidlugash commented 9 years ago

text-repeat-distance in carto is specifically for labels that have the same text (this works for either line or point placement, so it works for things like road labels as well as things like big parks that have multiple points). This might be a useful too or instead of layer-specific.

ansis commented 9 years ago

This might be a useful too or instead of layer-specific.

It would be great to implement only one if it can cover the needs. Would only text-repeat-distance be enough?

peterqliu commented 9 years ago

Would only text-repeat-distance be enough?

I think so. in fact, I'd rather not thin out the layer where it's two different highway shields next to each other-- that's still informative.

thin only in cases where they're the exact same shield for the same road

nickidlugash commented 9 years ago

@ansis To me this is a bit of a toss up, but from the last map design scrum, I think the consensus was that we should try implementing a padding that affects a whole layer, rather than one that affects only the same name. @peterqliu @samanpwbb @ajashton?

peterqliu commented 9 years ago

I think the consensus was that we should try implementing a padding that affects a whole layer, rather than one that affects only the same name

I initially felt that, too, til I thought about what would happen:

screen shot 2015-04-07 at 12 08 53 pm

Here, I'd definitely want to thin out the I78 from each other, and 1•9 shields from each other, as they are undoubtedly redundant.

However, the I95 and I78 shields on the lower right are labeling two different freeways. Not only are they not redundant, but actually more critical in distinguishing freeways that are close together.

The difference here is subtle, but I'm leaning toward same-name padding. If i'm setting 500px, or even 1000px padding to thin out shields on the same freeway, I don't want to nuke the shields on another freeway if it happens to intersect with the first one.

samanpwbb commented 9 years ago

Either solution would be okay with me.

Same-name padding is ideal for road labelling like @peterqliu described above.

Per-layer padding is more generally useful, but would not deal with complex road labelling problems as well as same-name padding.

peterqliu commented 9 years ago

oh yes, I had sort of co-opted this discussion for spacing out labels along a single line. I'm game for have a padding property that spaces out everything within a layer, as long as we have another solution for thinning out labels along a single road.

nickidlugash commented 9 years ago

@ansis would it be crazy to implement both? They both seem useful, and cover different use cases.

If we just do one, I think I'm leaning towards same-name padding. All other types of padding (including our current padding) have to be used more carefully since you don't have control over which features within the layer will be visible and which will not. If a layer is way too dense then the optimal solution is probably to have appropriate fields in the data by which to filter (or auto filtering).

ajashton commented 9 years ago

@ansis would it be crazy to implement both? They both seem useful, and cover different use cases.

If we just do one, I think I'm leaning towards same-name padding.

+1 from me. Prefer both, same-name if we can only have one.

Reasoning: overly dense but unique POIs looks less bad than overly dense repeating shields/labels.

nickidlugash commented 9 years ago

@ansis do you have bandwidth to look into implementing same-name padding? Similar to carto's text-repeat-distance?

peterqliu commented 9 years ago

bump @ansis

this would be great to have before launch, if possible

ansis commented 9 years ago

If we just do one, I think I'm leaning towards same-name padding.

How would this work specifically?

Would it have an affect on labels with the same name that are in different layers? If it works across layers, would text-repeat-distance only affect the placement of labels in it's layer, or would the distance apply to labels placed after it too?

For example

peterqliu commented 9 years ago

@ansis same-name is a subset of same-layer, so this property would only apply to labels in the same layer AND same text content.

so,

would "Dundas Street" in B need to be at least 8px from "Dundas Street" in A? would "Dundas Street" in C need to be at least 8px from "Dundas Street" in B?

no to both

nickidlugash commented 9 years ago

Pulling in @ajashton for the question above ^ Would it possibly be useful to have same-name repeat distance work across all layers? Assuming that we are implementing this property for points as well as lines, this could be useful. E.g. Sometimes you see multiple pois with the same name, but different scaleranks – if you separate pois into separate style layers by scalerank, then text-repeat-distance would still work to avoid this repetition. Could this even be helpful for some of the repetition in our data, like names in both the poi and place label layers?

ajashton commented 9 years ago

I think the simplest and easiest-to-understand approach would be to only have it affect other labels of the same name in the same style layer.

The main case I can see for making it work across layers is if we are talking about multiple layers in the style but still a single layer in the vector tiles. Eg "Dundas Street" in road_label_main could affect another "Dundas Street" in road_label_other, but not a "Dundas Street" in poi_label_rail_stations or something.

nickidlugash commented 9 years ago

@ansis is helping me to implement this. Getting started on it now.

nickidlugash commented 9 years ago

Per chat with @ansis, this is what I'm going to move forward with implementing (subject to change if it doesn't seem to be solving the density issue well enough):

text-repeat-distance: Minimum distance between anchors of text with the same text-field value. This will be only within the same GL layer.

icon-repeat-distance: Minimum distance between anchors of icons with the same icon-image value. This will be only within the same GL layer.

If text-repeat-distance and icon-repeat-distance are both assigned, they will be applied separately (as opposed to applying to features with same text-field value AND same icon-image value).

Both properties will thin out the label layer by removing labels that are 1. within the -repeat-distance of a label and 2. have the same text or icon. This should happen before collision checks, so that labels that are removed don't get added to the collision tree.

This applies the same way to line and point placements.

I'm currently trying to figure out how/where to implement this so that it affects labels beyond the tile boundaries.

/cc @mourner

peterqliu commented 9 years ago

@nickidlugash great that you're pushing on this!

You might've answered this above, but what would happen if a shielded highway label had different repeat distances for icon and text? Would one appear without the other, in some cases?

nickidlugash commented 9 years ago

@peterqliu no, these properties would hide entire labels.

mourner commented 9 years ago

@nickidlugash how would the new properties play with symbol-min-distance which currently defines distance between labels interpolated along a line?

nickidlugash commented 9 years ago

@mourner symbol-min-distance will be used first to calculate the distance between labels interpolated along a line, and repeat-distance will be used afterwards to remove labels within a certain distance to others (this is a straight line distance, not along the line, and so would work the same for line and point placement). Does this sound ok to you?

mourner commented 9 years ago

Yes, sounds good. I'm now trying to understand what this means for cross-tile boundaries...

nickidlugash commented 9 years ago

Per chat, going to not worry about tile boundaries for now. After implementing and playing around with different values for the property, we'll look at the boundary issue. Possible solution would be adjusting label offset.

nickidlugash commented 9 years ago

@ansis notes for our scrum today:

This is happening in this branch of the style spec and js implementation (including offset updates) are happening in this branch of js.

So far I've created a text-repeat-distance property and an icon-repeat-distance property with default values of 0. This value is used to check the distance of a potential anchor to all the previous anchors with the same text name or icon file name, and if that anchor is within the specified distance from another anchor, it doesn't get rendered/placed in the collision tree.

I've also fed the repeat values into the getAnchors function to adjust the first anchor offsets. If a line has either a text or icon repeat distance value and it's first anchor falls on a tile boundary, I made the offset equal to the repeat distance (initially I tried making it half the repeat distance, but the offsets seemed to small).

I also adjusted the labelLength in get_anchors to take into account icon width.

Outstanding questions/to-dos:

nickidlugash commented 9 years ago

@ansis notes for our scrum today:

These are all the changes I've made to anchor placements so far:

In getAnchors.js:

In addFeature function of symbol_bucket:

Next steps:

peterqliu commented 9 years ago

Nice work!

and is a long label and is not a long label

are these a specific, defined length, or variable?

nickidlugash commented 9 years ago

@peterqliu what I'm defining as a "long label" is relative to the symbol-spacing value (the spacing sets the distance between label anchors).

nickidlugash commented 9 years ago

Implemented here in js and here in native. These PRs included some changes to line label anchor placement (offsets and spacing), in addition to the repeat-distance filter. Currently, the filter is only for text labels along a line.

Since we decided not to implement this as a style spec property, no changes were actually made to this repo.