Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Look the other way when placing icon at start of line #2530

Merged
merged 2 commits into from
Oct 7, 2015

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Oct 5, 2015

This PR fixes a bug exposed by #1840. When an anchor is at the beginning of a line, it is coincident with its segment, resulting in an angle of 0°. In that case, consider the next segment instead.

This change also fixes a rarer bug: for some reason, clipLines() inserts a duplicate point at the beginning of the line for this way, so we must check whether the anchor is coincident with its segment; we can’t simply check whether segment is 0.

before & after

This before and after shows that all the one-way arrows are pointing the right way again. However, collision boxes aren’t being generated for the icons being placed along tile boundaries, so they tend to run right up against icons in adjacent tiles.

Fixes #2474.

/cc @nickidlugash @ansis

Wrong way (CC BY-SA 3.0 by Wikipedia user Coolcaesar)

@friedbunny
Copy link
Contributor

However, collision boxes aren’t being generated for the icons being placed along tile boundaries, so they tend to run right up against icons in adjacent tiles.

What's up with that?

@1ec5
Copy link
Contributor Author

1ec5 commented Oct 6, 2015

The same problem – an anchor coincident with its segment – is tripping up CollisionFeature::bboxifyLabel().

@1ec5 1ec5 added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Oct 6, 2015
@1ec5
Copy link
Contributor Author

1ec5 commented Oct 6, 2015

With d9aaefd3e20065df92e0caa20249137c3a662215, there are bounding boxes, but they don’t keep the anchors from running into anchors in other tiles:

bumper boxes

I don’t see a clear way to give each CollisionFeature enough context about surrounding tiles to keep them from colliding.

@1ec5
Copy link
Contributor Author

1ec5 commented Oct 6, 2015

Also, this line doesn’t achieve the intent of the comment above it when the tile is overscaled: “Offset the first anchor by… half the spacing if the line is continued.”

@1ec5 1ec5 removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Oct 6, 2015
@1ec5
Copy link
Contributor Author

1ec5 commented Oct 6, 2015

Extraneous one-way arrows are now tracked in #2547.

@1ec5 1ec5 added this to the ios-v2.2.0 milestone Oct 7, 2015
@1ec5
Copy link
Contributor Author

1ec5 commented Oct 7, 2015

I reverted the change that added collision boxes because it introduces a latent bug: index should never be less than 0 because it’s used to index into an array. The extra collision boxes did nothing anyways, because cross-tile collision isn’t supported yet.

When an anchor is at the beginning of a line, its segment’s coordinates match its own coordinates, resulting in an angle of 0°. When the segment’s coordinates match the anchor’s coordinates, consider the next segment instead.

Fixes #2474.
@1ec5 1ec5 merged commit 6228888 into master Oct 7, 2015
@1ec5 1ec5 removed the in progress label Oct 7, 2015
@1ec5 1ec5 deleted the 1ec5-wrong-way-2474 branch October 7, 2015 22:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unrotated one-way arrows along tile boundaries
2 participants