-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Render oneway markers inline in dashed lines #10849
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
Conversation
requires slight tweaking of colors and dash lengths and dash spacings
e7835f9
to
aa3a72b
Compare
_defsSelection | ||
.append('marker') | ||
.attr('id', `ideditor-oneway-marker-${name}`) | ||
.attr('viewBox', '0 0 10 5') | ||
.attr('refX', 2.5) | ||
.attr('refX', 4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the new version does look a bit squished… I needed to shrink the length of the double-sided arrows a bit though, in order to fit them into the existing dash spacings of the different line styles.
I experimented a bit with the shape of the arrows, and found two that seem to work reasonably well even in the more condensed form for the bidirectional arrows:
- original
M 5,3 L 0,3 L 0,2 L 5,2 L 5,0 L 10,2.5 L 5,5 z
(withrefX=2.5
) - first version of this PR: same shape as above, but
refX=4
- arrow alternative 1
M 6,3 L 0,3 L 0,2 L 6,2 L 5,0 L 10,2.5 L 5,5 z
(refX=4
) - arrow alternative 2
M 6.5,3 L 0,3 L 0,2 L 6.5,2 L 5.5,0 L 10,2.5 L 5.5,5 z
(refX=4
)
Here's the test suite from above again with alternatives 1 and 2:
PS: I also bumped up the opacity to 100% for all arrows in the last two screenshots, as I think the slight transparency of the arrows is not necessary and only caused a minor issue where the two arrows overlap in for bidirectional lines. But maybe it would be worth toning down the color for the black
arrows to something like #333
to tone down the contrast a little bit, and bring it back in line with how the arrows previously appeared on typical lines.
otherwise they would be rendered out of sync of the line's dashes when the feature extends outside of the viewport
see #10849 (comment) (this is "arrow alternative 1")
This fixes the awkward overlaps of oneway arrows in dashed lines with inconsistent spacing, and hard to choose well contrasting colors for the arrows (see #9143). Now, for lines with dashed rendering style (e.g. railway lines, some footways/paths, runways) the oneway arrows are rendered in place of one of the line's dashes. This allows the oneway arrows to be rendered in the same color as the line itself, leading to a more elegant, less noisy rendering for these lines.
Below are rendering examples of various lines with different combinations of
oneway
tags (none,yes
,-1
,alternating
), from left to right: regular roads, railways, railway bridges, pedestrian bridge, pedestrian road, runways, road under construction, footpaths/cycleway, steps and ladders, unmarked pedestrian/cycleway crossing.after:

before:

(this PR replaces #10846)