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

Label multipoint #409

Merged
merged 27 commits into from
Oct 25, 2016
Merged

Label multipoint #409

merged 27 commits into from
Oct 25, 2016

Conversation

dmvaldman
Copy link
Contributor

WIP for different strategies to place point-like features along line geometry. Previously point features on lines would just place the points at the endpoints of the line segments. However for use-cases like highway shields and street arrows you may want different behavior. Street arrows should be "evenly spaced" along a line, for instance.

Here's an example for this scene file snippet

...
arrows:
    filter: { $zoom: {min: 17} }
    draw:
        arrow:
            size: 8px
            spacing: 40px     // space arrows 40px apart
            angle: auto       // inherit line geometry angle

screen shot 2016-09-09 at 12 08 59 pm

Here arrow is a user-defined texture.

We may want to consider a better terminology than spacing, as the similarly to repeat_distance is a source for confusion. Similarly for auto angles. We may also want to include a placement_strategy option such as endpoints, midpoints, spaced or something like that.

In the code, I call this new class label_multipoint, but this is pretty unclear, so we can also revisit the naming of label_line (which is becoming highly specialized for text labels) and label_multipoint, etc.

@dmvaldman
Copy link
Contributor Author

Would love some feedback :-) @nvkelso @bcamper

@nvkelso
Copy link
Member

nvkelso commented Sep 12, 2016

@dmvaldman Can you please add screenshots showing highway shield spacing, too?

@bcamper
Copy link
Member

bcamper commented Sep 12, 2016

@nvkelso spacing is configurable so we should be able to tune in the style. That said, I suspect this feature will be more useful for oneway arrows than for shields, because I assume shields will be less frequently spaced (in fact right now we are placing more than necessary and then the repeat logic culls them out, this PR could exacerbate that!), combined with tile discontinuities between lines makes it hard to get good continuous spacing.

But, even if it's not placing very many shields, this PR will make it place them in better locations, away from the line vertices/intersections.

@dmvaldman
Copy link
Contributor Author

Here's a comparison between using the midpoints of lines versus endpoints (as was previously used) for highway shields. The road segments are also colored differently so you can see their endpoints.

www gifcreator me_b91y19 1

@bcamper
Copy link
Member

bcamper commented Sep 13, 2016

I think we're close here, just a few questions to resolve:

1. Do we want to continue to support the existing behavior that places of points on line or polygon vertices?

My feeling is that while this probably isn't the typical scenario, it's useful in some situations (like visualizing the geometry below) and would be nice to preserve -- though we could consider making it no longer the default.

screen shot 2016-09-13 at 10 32 01 am

2. If the answer to the above is yes, then we need an option for specifying the placement strategy, as @dmvaldman suggests.

Mapbox calls their similar placement options point (places points at line vertices) and line (places points along a line with user-specified spacing). We also have an option to place the label at a polygon centroid, currently set with centroid: true (this property can be deprecated and folded into a single placement property).

A couple possibilities:
placement: point | line | polygon
placement: vertex | spaced | centroid

I would avoid endpoints because that implies to me only the start and end vertices of the line rather than all of them (and I don't know of a current need for actual start/end-only behavior).

I'm also unsure if we currently have a need for a "midpoints" placement strategy, though it is worth considering for shields (where I think keeping them away from vertices/intersections is more important than actually spacing them out, given that they don't need to repeat too often). Terminology like vertex | spaced | centroid might also be more extensible for adding a midpoints option.

3. Ambiguity with repeat_distance
We could consider placement_spacing to make the relationship with a placement property more explicit.

Could also consider renaming repeat_distance to something like repeat_tolerance, with backward compatible-syntax / deprecation path.

4. Angle of point/sprite
It's valid to put a specific numeric angle in angle, so I do think a string value indicating a different placement type makes sense.

I'm OK with either angle: auto as you have it, or maybe angle: line?

5. Polygon handling
For consistency with other behavior, when a spaced placement strategy is specified, the placement logic for polygons/mutlipolygons should follow that of lines, placing points along each polygon ring as if it was a line (here).

@nvkelso @blair1618 thoughts on all of the above please?

@nvkelso
Copy link
Member

nvkelso commented Sep 14, 2016

💯 @dmvaldman thanks for the extra shield animated GIF! Ship it!

For terminology:

  • repeat_spacing makes slightly more sense to me than spacing alone (which could refer to character, line spacing, etc). Looks like Mapnik uses spacing alone for this. I'm fine with spacing.
  • The existing repeat_distance could also be thought of as a culling distance? It already matches Mapnik's repeat-distance. No change recommended.

@bcamper
Copy link
Member

bcamper commented Sep 14, 2016

What about placement_spacing so the link to the placement property is clearer? I'm OK with either that or spacing though.

@bcamper bcamper added this to the v0.11.0 milestone Sep 14, 2016
@nvkelso
Copy link
Member

nvkelso commented Sep 14, 2016

@bcamper I missed your earlier comment, responses to all below:

  1. Do we want to continue to support the existing behavior that places of points on line or polygon vertices?

Yes (agree). Especially to allow data editing like the OSM discussion.

  1. If the answer to the above is yes, then we need an option for specifying the placement strategy, as @dmvaldman suggests.

These sound slightly different to me. One is about general label alignment and implied angle changes (snapping to point(s), lines, or polygon centroid), versus specific placement strategy (start, stop, all vertexes, midpoint of line, poor man's centroid, centroid on surface, the more interesting "best" placement Mapbox just blogged about). The repeat / placement strategy extensions would include (midpoint between vertexes, centroids in all parts of multi-polygon).

  1. Ambiguity with repeat_distance

Agree with placement_spacing to make the relationship with a placement property more explicit.

Agree with renaming repeat_distance to something like repeat_tolerance, with backward compatible-syntax / deprecation path.

  1. Angle of point/sprite

If I've chosen to "align" my label to the line, then I think the default angle value of auto (not line) would change from 0° to whatever the line's curve is. I shouldn't need to modify the angle property to get line labels to show up.

I've wanted to do angles for points for a while. Viewpoints from OSM include this (expressed often in cardinal directions but I'd convert that to 0-360° server side in tiles).

  1. Polygon handling

+1

@bcamper
Copy link
Member

bcamper commented Oct 17, 2016

@dmvaldman follow-up from IRL discussion: let's make sure that at least one point is placed on each geometry (maybe only if that one point will fit along the line length?).

@@ -11,7 +11,7 @@ import Geo from '../../geo';
import Vector from '../../vector';
import Collision from '../../labels/collision';
import LabelPoint from '../../labels/label_point';
import placePointsOnLine from '../../labels/label_multipoint';
import placePointsOnLine from '../../labels/point_placement';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@bcamper
Copy link
Member

bcamper commented Oct 24, 2016

I made some updates re: the issues discussed above:

  • Exposed other placement strategies via the placement parameter:
    • placement: vertex: previous default behavior, place points at line/polygon ring vertices
    • placement: midpoint: new option, places points at line/polygon ring segment midpoints (better for shields where you want them away from ambiguous intersections)
    • placement: spaced: new option, places points along a line/polygon ring at fixed intervals defined in pixels with placement_spacing; useful for symbols like one-way street arrows where consistent spacing is desirable
      • placement_spacing defaults to 80px and also accepts zoom stops or functions
    • placement: centroid: only applicable for polygons, migrated from the old centroid: true parameter, places points at polygon outer ring centroid; of limited use due to issues with polygons split across tiles, but may continue to be of some use
  • Add "tile edge" handling to point placement logic, with same logic as with lines. Useful for avoiding point placement on tile borders, which is usually undesirable (tile_edges defaults to false, as with lines):
    output_athq2b
  • Clarified repeat group behavior between points and attached text, so that different repeat_distance values can apply to each, e.g. one can configure highway shields generally to be allowed to be within 80px of one another, while ensuring that specific highway shield text will not repeat within 200px

Not yet implemented: changing repeat_distance to repeat_tolerance, with backwards compatibility and plan to deprecate. I still think this makes sense for clarity though.

I think this is in good pretty good shape and can be merged, with syntax/naming changes made before final release if needed. Comments @dmvaldman @blair1618 @nvkelso?

I have some additional comments around point/shield placement that I will follow up with.

@bcamper
Copy link
Member

bcamper commented Oct 25, 2016

Going to merge this as-is to coordinate with other features, but feel free to comment on above and we can make adjustments in master (pre-release).

@bcamper bcamper merged commit 2e80127 into master Oct 25, 2016
@bcamper bcamper deleted the label-multipoint branch October 27, 2016 14:36
meetar added a commit to tangrams/tangram-docs that referenced this pull request Nov 7, 2016
Based on tangrams/tangram#409@bcamper to review, with the further expectation of `repeat_distance` being renamed `repeat_tolerance` at some point tbd.
let positions = [];
let angles = [];

let distance = 0.5 * remainder;
Copy link
Member

@tallytalwar tallytalwar Dec 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmvaldman can you explain the heuristic of halving the remainder "placing space" here? I was thinking of starting the line interpolation from remainder/num_labels... Basically to distribute the actual placing between n labels. Will be good know if I am totally in the wrong direction here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

img_20161230_165653

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect. Picture speaks more than words :). Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants