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

Crossroad names #813

Merged
merged 2 commits into from
Sep 5, 2014
Merged

Conversation

matthijsmelissen
Copy link
Collaborator

Render names on nodes with junction=yes or highway=traffic_signals.

This resolves #374.

@matthijsmelissen
Copy link
Collaborator Author

Example rendering:

screenshot from 2014-08-02 10 03 42

This renders names of junction=yes and junction=roundabout.
Rendering is similar to highway=motorway_junction.
* Also render name for traffic_signals without junction tag
* Don't render junction names on roundabouts
* Render junctions in black and in a smaller font
* Offset junction names only when the traffic signal icon shows
* Start rendering junctions from z14
* Wrap lines
@matthijsmelissen
Copy link
Collaborator Author

Rebased.

@matthijsmelissen
Copy link
Collaborator Author

I think we can speed up the merging of pull requests if more people assist in reviewing them. Is anyone interesting in reviewing this pull request?

@sommerluk
Copy link
Collaborator

While I cannot do a code review or testing (due to a lack of skills in cartocss), nevertheless here some feedback about what I can see:

  • The rendering example looks very good. Using the same font color as for residential roads is reasonable. It does not look overloaded like Google Maps in Japan.
  • Concerning traffic signal names: It seems that the name will be rendered starting with zoom level 14, but the icon will be rendered starting with zoom level 17, right? I’m not sure if this is good idea. Would prefer this:
    – if highway=traffic_signals and name=* are both present, than the name and the icon are both rendered starting from zoom level 14.
    – if only highway=traffic_signals is present (without a name=*), than the icon is rendered starting from zoom level 17.
    Otherwise it would be confusing to have the traffic signal names sometimes with and sometimes without their corresponding icon.

@matthijsmelissen
Copy link
Collaborator Author

Concerning traffic signal names: It seems that the name will be rendered starting with zoom level 14, but the icon will be rendered starting with zoom level 17, right? I

I see your point. However, if we only rendered named traffic lights, then some traffic lights would be visible while others wouldn't. I think rendering all traffic lights from z14 is too early.

@sommerluk
Copy link
Collaborator

if we only rendered named traffic lights, then some traffic lights would be visible while others wouldn't.

Would this be a big disadvantage or breaking with some styling conventions? For me, this would be exactly what is expected.

I think rendering all traffic lights from z14 is too early.

Agreed.

@sommerluk
Copy link
Collaborator

We could even refine this, to catch also the combination of junction=yes + highway=traffic_signals correctly:

  • junction=yes + name=* (without highway=traffic_signals): This is a named junction. Starting from z14 render the name.
  • highway=traffic_signals + name=* (without junction=yes): This is named traffic signal. Starting from z14 render both: name and icon.
  • highway=traffic_signals (without name=* and without junction=yes): This is a traffic signal without a name. Starting from z17 render the icon.
  • highway=traffic_signals + name=* + junction=yes: This is a named junction (that has additionally a traffic signal without a name). Like for named junctions, the name is rendered starting with z14, but like for traffic signals without a name, the icon starts with z17.

gravitystorm added a commit that referenced this pull request Sep 5, 2014
@gravitystorm gravitystorm merged commit 738a46b into gravitystorm:master Sep 5, 2014
@matthijsmelissen matthijsmelissen deleted the crossroad-names branch September 25, 2014 01:43
@matthijsmelissen matthijsmelissen restored the crossroad-names branch September 25, 2014 16:03
@matthijsmelissen matthijsmelissen deleted the crossroad-names branch November 15, 2014 17:22
@matthijsmelissen matthijsmelissen restored the crossroad-names branch November 15, 2014 17:24
@matthijsmelissen matthijsmelissen deleted the crossroad-names branch November 15, 2014 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

render the name of crossroads
3 participants