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

Create new layer for "ref" of minor roads and paths #3709

Merged
merged 6 commits into from
Aug 30, 2019

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Mar 8, 2019

Partially replaces PR #3663
Follow-up to PR #3654

Changes proposed in this pull request:

  1. Move unclassified, residential and track highways, and aeroways "ref" to a later layer, after name labels are rendered

  2. Change text-halo to standard size for highway=unclassified/residential refs and aeroway refs

  3. Change residential / unclassified road "ref" size to 11 point at z17, to be the same size as the name text labels at this zoom level and higher.

Explanation

1) The minor roads, aeroways, and tracks with text-based ref labels currently render in the same layer as the shields for major roads.

  • The ref text is rendered before the name label, and can block the name from rendering when spaces is limited.
  • Major road shields render much sooner than the ref text labels of minor roads, which are not shown until z15 or z16.
  • Major road shields are usually more important than the name; the number of a motorway is used more often, but for minor roads, such as residential streets, the name is more useful and should be shown with higher priority
  • Changing residential, unclassified and tracks to a later layer will fix this rendering. Aeroways "ref" text labels are also moved to this new layer because they are also rendered only at z15 and higher, though this does not change the cartography because they do not have a rendering for names

2) The "ref" labels rendering for residential/unclassified roads needs minor adjustments
At z17, the "name" text label increases to 11pt font size, but the ref does not change size till z18 currently. This should be adjusted so that the "ref" label is the same size as the name label, as at z15 and z16.

3) The "ref" halo needs to be changed to standard size.
Right now it is 2 pixels wide, which would overlap with the casing at certain zoom levels.

Test rendering with links to the example places:

highway=unclassified with name and ref - Basilicata, Italy
https://www.openstreetmap.org/#map=17/40.3776321/15.7281581
Before z17
via-chiusululle-unclassified-master
z18
z18-via-chiusululle-master
After z17
chiusululle-z17-after
z18
chiusululle-z18-after

highway=residential with name and ref - Basilicata, Italy
https://www.openstreetmap.org/#map=17/40.1412792/16.0893142
Before z17
via-vittorio-res-z17-master
z18
via-vitorio-z18-master
After z17
z17-via-vittorio-after
z18
via-vittori-z18-after

jeisenbe added 4 commits March 8, 2019 21:42
This makes the ref lables at z17 and up consistent between track roads, paths, residential and unclassified highways

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@jeisenbe
Copy link
Collaborator Author

Does anyone have time to look at this PR?

@kocio-pl
Copy link
Collaborator

I don't have much time lately, but looking at the rendering this is a good idea.

@imagico
Copy link
Collaborator

imagico commented Mar 30, 2019

I am sorry for the long wait - I looked at this and am concerned about two things:

  • by moving aeroway refs to the new layer you make them lower priority than road names. I am not sure this would work so well - in particular for service roads and tunnels on airports
  • rendering refs with labels indistinguishable from names does not seem a good idea.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Mar 31, 2019

  • by moving aeroway refs to the new layer you make them lower priority than road names. I am not sure this would work so well - in particular for service roads and tunnels on airports

The "ref" for runways (and taxiways) is only rendered at z15 and greater, and it's rendered one time at the center of the way. So this is only an issue for airports where a named road crosses the center point a runway or taxiway. I can think of several examples of paths and roads that cross under or level with runways (e.g.: Gibraltar, LAX, some airstrips in my area) but I don't know of any where the road is close enough to the runway center for this to cause interference with the label, so I doubt this is an issue in practice.

However, It would be easy enough to keep the aeroways in the main roads-text-ref layer. I only moved them because they render at z15 and higher only, so I thought it would slightly improve rendering performance for z13 and z14, which are included in roads-text-ref but not in the new layer.

  • rendering refs with labels indistinguishable from names does not seem a good idea.

This is the current situation with "ref" on residential, unclassified and track roads. The PR only makes 2 tiny adjustments for consistency:

-      [zoom >= 18] {
-        text-size: 10;
+      [zoom >= 17] {
+        text-size: 11;
-      text-halo-radius: 2;
+      text-halo-radius: @standard-halo-radius;

If we want the 'ref' to be clearly distinct from the road name label on this minor roads, there would need to be a more significant distinction than the halo radius or a 1 point font size difference at z18 and z19.

I could try rendering the ref with italic font, perhaps? Or should this be a separate PR?

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Apr 5, 2019

Does anyone else have comments about this? Should I move the aeroways back to the earlier layer? Does the "ref" label need to be more clearly distinguished from the "name" label?

@jeisenbe
Copy link
Collaborator Author

  1. Do we need to keep aeroways in the earlier layer?
  2. Should the "ref" rendering be changed in this PR, or can it wait till later?
    See comments above Create new layer for "ref" of minor roads and paths  #3709 (comment)

@imagico
Copy link
Collaborator

imagico commented Apr 15, 2019

Would be good to have another opinion on this, maybe from @matkoniecz, @kocio-pl or @sommerluk. I mentioned two points of concern, but these are not essential for me. Another look at this from someone else would be useful.

@sommerluk
Copy link
Collaborator

Okay. I didn’t look to the code. But the idea to unify the ref label rendering with the name label rendering seems good to me. The current situation is that we have some differences in font size between name and ref, and these differences are not big enough within the same zoom level (and not systematic enough across different zoom levels) to allow a clear distinction anyway. Personally I also think different font sizes on the same zoom level for the same object in the same rendering (here: along the line) make the map harder to read (less intuitive).

Using the italic font instead of the regluar font for ref might also work. (Side note: We have currently no systematic approach about when using italic fonts and when not.)

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Apr 17, 2019 via email

jeisenbe added 2 commits May 20, 2019 21:30

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@jeisenbe
Copy link
Collaborator Author

I've moved aeroway ref rendering back to the original layer (roads-text-ref)
I also changed the rendering of minor roads and aeroways to use an oblique (italic) font, as suggested above

@jeisenbe
Copy link
Collaborator Author

Test renderings with oblique font for ref. It's a subtle change:

highway=unclassified with name and ref - Basilicata, Italy
https://www.openstreetmap.org/#map=17/40.3776321/15.7281581
z18-ref-oblique-via-chiusululle

highway=residential with name and ref - Basilicata, Italy
https://www.openstreetmap.org/#map=17/40.1412792/16.0893142
z18-ref-oblique-via-vittorio
z17-ref-oblique-via-vittorio

Runway with ref in new oblique font:
z15-wamena-runway-15-33

(Compare to current rendering:)
z15-current-runway-rendering

@jeisenbe
Copy link
Collaborator Author

Any comments on the new commit?

@jeisenbe
Copy link
Collaborator Author

@imagico, your original 2 points of concern were addressed by the most recent commits.

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

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

Seems to work fine - and while i am still not quite sure if this is ultimately a good way to render road refs in the long term and also if refs, in particular on tracks, are actually used in a consistent way it is an improvement from the current situation and does not seem to cause specific new issues.

@jeisenbe
Copy link
Collaborator Author

@imagico has approved this now, and it looks like @kocio-pl and @sommerluk were in favor, so I'm going to merge this PR tomorrow, once I figure out when the changelog should be updated.

@jeisenbe jeisenbe merged commit 68144f5 into gravitystorm:master Aug 30, 2019
@jeisenbe jeisenbe deleted the ref-layer branch August 30, 2019 23:05
@jeisenbe jeisenbe added the text label Aug 30, 2019
@pnorman
Copy link
Collaborator

pnorman commented Aug 31, 2019

once I figure out when the changelog should be updated

It's not necessary, but a big help when releasing. Just edit the changelog directly on master.

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.

None yet

5 participants