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

[WIP] Borders and country name rework #3553

Closed
wants to merge 5 commits into from

Conversation

kocio-pl
Copy link
Collaborator

@kocio-pl kocio-pl commented Dec 8, 2018

Fixes #3489.
Closes #3526.
Related to #621 and #3102.
Related to #3563

Changes proposed in this pull request:

  • make borders much darker (#ac46ac +75% of grey = #6b516b)
  • render country names in bold

This has been discussed and tested before, so now just a short summary: current borders are too bright, especially on low zoom levels and on the water. Other forks have already tried to make them darker using green tint, however it looks bad on the water, looks a bit blurry in general and is too close to natural reserve boundaries. Using pure grey makes some other (however less serious) problems, pointed out in #2695 (comment).

Using dark violet looks good on low zoom, looks good on the water and is different than natural reserve boundaries. It's also similar to the current style, so people should be less surprised. Making country names bold helps to recognize them more easily than currently.

Some examples (I hope @rrzefox could apply it to test such important change on the whole planet):

cjyo 28y

https://www.openstreetmap.org/#map=6/51.849/20.116

1a8c4dr

https://www.openstreetmap.org/#map=16/49.5738/22.6873

birdt_1x

https://www.openstreetmap.org/#map=11/54.3842/19.7208

chtdclj_

https://www.openstreetmap.org/#map=12/52.2356/20.8412

tah3cucw

https://www.openstreetmap.org/#map=19/52.25929/20.99742

1iunm0hv

https://www.openstreetmap.org/#map=18/52.23921/21.03554

_bzqmee9

@matkoniecz
Copy link
Contributor

https://www.openstreetmap.org/#map=19/52.25929/20.99742

It seem to be almost the same as railway=disused.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Dec 8, 2018

Looking at border style, there is some room for making one more pattern for admin_level= between 2 and 8 and just dropping small dots for 9+:

https://wiki.openstreetmap.org/wiki/Tag:boundary%3Dadministrative#Rendering
https://wiki.openstreetmap.org/wiki/LinesTab#Boundaries

One simple idea would be to render level 3 with longer lines, another one could be to render level 5 with 3 dots. I guess the first one is better, because we have not too much of such fat lines, while slim ones are used more often.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Dec 9, 2018

Currently disused railways and smallest admin borders are already similar, because colors of slim lines are not too visible:

https://www.openstreetmap.org/way/461746771#map=19/52.25954/20.98289

current master

aaz9cdmy

z9+ rendered with small dashes (current z7+8 pattern):

tn724nie

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Dec 9, 2018

Rendering level 5 with 3 dots and longer dashes (line-dasharray: 12,3,2,3,2,3,2,3;):

https://www.openstreetmap.org/#map=18/50.17957/18.62354

Before

dxnc2zit

After

hy4ttvfd

@Tomasz-W
Copy link

Tomasz-W commented Dec 9, 2018

I think it looks worst with longer dashes (more similar to railway pattern)

It's related to #2872

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Dec 9, 2018

Which railway pattern does it remind you? I do not see any resemblance, so I think it's safe. It's also unusual pattern (which is good for artificial objects like borders), railways would be more regular:

https://wiki.openstreetmap.org/wiki/LinesTab#Railways

Longer dashes make it more distinct from 1 and 2 dots patterns. Using regular dashes with 3 dots (1+3 also 2+2, 3+2 and 3+3):

https://www.openstreetmap.org/#map=18/50.17897/18.62384

fltcle9n

umw9b_ n

5hzyor q

mg50toyk

@vholten
Copy link
Contributor

vholten commented Dec 10, 2018

@kocio-pl Your test renders show bold country names without text-character-spacing, but this PR does not show that change in the code, the text-character-spacing: 0.5; is still there.

I think we agree that character spacing is no longer required (see also the comment by sommerluk). Could you make that change in the code?

@kocio-pl
Copy link
Collaborator Author

Thanks for checking the code, now it's removed.

@rrzefox
Copy link

rrzefox commented Dec 14, 2018

I hope @rrzefox could apply it to test such important change on the whole planet

This has now been deployed together with #3563, and Z0-9 rerendered.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Dec 14, 2018

Example of different scripts in bold (Chinese font has general issues on this server, so I can't really test it - Korean and Japanese are broken too) - click to see the full size:

screenshot_2018-12-14 rrze openstreetmap server - leaflet demo

screenshot_2018-12-14 rrze openstreetmap server - leaflet demo 1

screenshot_2018-12-14 rrze openstreetmap server - leaflet demo 2

@sommerluk
Copy link
Collaborator

@rrzefox Thanks a lot!

@kocio-pl Maybe medium font could be an option for regions (admin_level 4).

Initially, I thought semi-bold might work well for capitals, but after looking at the rendering live preview, I suppose this would be too invasive. Maybe it’s better to give slightly more emphasis to region names, but semi-bold might be too much. Medium is something between semi-bold and regular. The fallback would be regular.

@kocio-pl
Copy link
Collaborator Author

I finally was able to focus on this important task.

I have added new pattern for z5 as planned, because I see no resemblance with railway and that would help us with highest admin levels to be distinct.

@sommerluk I have added a simple Bold clone and turned it into medium in the code, I hope this is OK. Here is a comparison of medium and regular font (just to show how they are different) - what do you think about it?

Medium

xofv9u

Regular

i8spm 4z

@kocio-pl
Copy link
Collaborator Author

For SemiBold this looks strange, like if using polish diacritics in one line triggers fallback to a font without this style - I did not expect that:

SemiBold

skbk0rkp

This is also how the country name would look in comparison (similar problems with Medium):

SemiBold

xd6dvdyh

Medium

6qdtivwr

@sommerluk
Copy link
Collaborator

@kocio-pl Well, the rendering is indeed strange. And I do not really understand why.

I have added a simple Bold clone and turned it into medium in the code, I hope this is OK.

Theoretically, this should work. Pitfalls in the fallback chain might be

  • "Noto Sans CJK JP Medium" (duplicates all LGC characters, but with different glyphs than Noto Sans)
  • "Noto Naskh Arabic UI Medium" (duplicates some LGC characters)
  • "DejaVu Sans Medium" (doesn’t exist).

But nevertheless, the question is why the fallback chain is triggered. Let’s take województwo śląskie as example. The label renders wrong – for both, medium (fallback to regular for the second word) and semi-bold (fallback for both words). If I understand it correctly, Mapnik chooses the font together with Harfbuzz, and they try to keep at least hole words always in the same font. If a font does not provide all characters for a given word, they fall back to the next font in the chain, and so on. The big question is: Why is the fallback chain triggered at all? I’ve tested the label “województwo śląskie” with Noto Sans in all relevant weight variants using InDesign (which does quite a good job in Unicode-conform rendering and shows missing characters): The result is that there are no missing characters. So everything should be fine. I don’t understand why it doesn’t render correctly. Note that additionally to the fallback, there seems also to be a problem with different line spacing.

(By the way: Where did you get the Noto Sans Medium/Semi-bold font you are using? Github?)

Given that for both – Medium and Semi-bold – there are rendering issues, and we do not understand the reason, I propose not to include Medium/Semi-bold in this PR. Maybe later there can be another PR that focuses exclusively on Medium/Semi-bold.

@sommerluk
Copy link
Collaborator

Maybe also in a later PR, legacy DejaVu support could be dropped. Given that Noto is quite wide-spread today, dropping DejaVu could avoid some side effects in the fallback chain.

@kocio-pl
Copy link
Collaborator Author

(By the way: Where did you get the Noto Sans Medium/Semi-bold font you are using? Github?)

Most probably I was following installation instructions (and looking at files it was year ago):

https://github.com/gravitystorm/openstreetmap-carto/blob/master/INSTALL.md#installation-on-other-operation-systems

I will try to update them and see the results. Could you make independent tests on your system?

@sommerluk
Copy link
Collaborator

Most probably I was following installation instructions

Then you have downloaded from Github, which should be fine.

Could you make independent tests on your system?

Not sure, I’m currently quite busy with other stuff, so I do not have much time…

@kocio-pl
Copy link
Collaborator Author

OK, we have time to test things and find the problem before release. If that fails, I will just drop this part.

BTW: @rrzefox Could you update the code on your system so we could check if this is not just the issue with my setup?

@vholten
Copy link
Contributor

vholten commented Dec 27, 2018

In the Medium examples, some text is shown in Noto Sans CJK JP Medium instead of Noto Sans Medium (the lowercase l has a tail in the CJK font):

lubelskie

noto sans

Fallback is triggered because the CJK font doesn't have Polish diacritics.

@kocio-pl
Copy link
Collaborator Author

Why do you think it was Noto Sans CJK JP Medium and not some other font from the list?

@vholten
Copy link
Contributor

vholten commented Dec 29, 2018

I looked at all the Noto fonts on my system, and the CJK fonts are the only ones where the lowercase l has a tail, so I assume it's that one.

@sommerluk
Copy link
Collaborator

I looked at … so I assume it's that one.

That’s reasonable. Between Noto Sans and Noto Sans CJK, apart from “l”, there are some minor differences also in “s” and “e”, but that’s more difficult to see, especially on the low-resolution rendering we have here.

Fallback is triggered because the CJK font doesn't have Polish diacritics.

Yes and no. Anyway, the LGC font Noto Sans Medium is (correctly) the first choice in the list of @kocio-pl so I would assume that LGC font Noto Sans Medium covers Polish diacritics and there is no further fallback. But apparently my assumption is wrong.

Google’s Noto Sans CJK is actually Adobe’s Source Han Sans – just a copy that has been renamed: Google is commissioning most of it’s Noto fonts to Monotype. One exception are the CJK fonts: For these fonts, they have collaborated with Adobe because it’s really a huge project. So Adobe is producing its CJK font Source Han Sans with a design that match both, Adobe’s own LGC font Source Sans Pro and also Google’s LGC font Noto Sans. After releasing Source Han Sans, they copy the font, rename it to Noto and integrate it in the Noto family.

Now the problem is that Adobe adds the most common LGC characters from Source Sans Pro directly to its Source Han Sans font – as a sort of fallback. Google however follows a different policy with the other Noto fonts: Usually, there are no LGC fallback characters, so when you use for example Noto Sans Hebrew than this font will not have any LGC characters. So Noto Sans CJK is an exception, and additionally its LGC fallback glyphs are not what you would expect, but rather Source Sans Pro glyphs.

*LGC = Latin, Greek & Cyrillic scripts

@kocio-pl
Copy link
Collaborator Author

Could anybody else make such test to check what is the problem? @Adamant36? @jeisenbe?

@jeisenbe
Copy link
Collaborator

jeisenbe commented Dec 31, 2018 via email

@sommerluk
Copy link
Collaborator

I would also rather tend to Semi-bold (but I could live also with the other options).

Anyway, if this gets changed, we would have to make some smaller adaptions on the font list, and also change the install instructions. As far as I know, Noto Sans Medium is not packaged with Ubuntu – is this correct? If so, I could provide some alternative download instructions. And we need to warn the operations team when making a release, so that they are aware of the change. Not sure if they will actually install these fonts, but as we have a fallback, that’s not such a big problem.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Jan 3, 2019

Would you like to prepare separate code with Semi-bold? I can revert Medium changes and leave Italic for now.

@sommerluk
Copy link
Collaborator

Would you like to prepare separate code with Semi-bold?

As I’m currently really, really busy with other stuff, I would prefer not to write a new PR myself. However, I could help you with the font list:

  • Noto Naskh Arabic UI should be removed. (It does neither exist in semi-bold nor in medium, and I think it is unlikely that it will ever be supported in further weights, because the Noto project seems to develop rather on Noto Sans Arabic UI instead. Anyway, we use it as fallback only.)
  • DejaVu Sans should be removed. (It does neither exist in semi-bold nor in medium. We have a sort of legacy support for DejaVu, but this becomes anyway problematic if the number of font weights we are using increases. And as these new weights aren’t available anyway, no point in having this in the list.)
  • The rest of the font list is okay. (The only pitfall could be Noto Sans CJK JP as explained in other comments before. But we have no other choice anyway. I didn’t check if all these scripts are actually available at these weights, but Noto is developing fast.)

About the install instruction: Could you check if semi-bold or medium are available on Ubuntu? (I’m not using Ubuntu, so that’s a little difficult for me. On openSuse all these are available however…) If they are available, which is the package name?

@vholten
Copy link
Contributor

vholten commented Jan 3, 2019

I can revert Medium changes and leave Italic for now.

I would support leaving italic for now and looking at state names in a separate PR, as it seems that more tests are needed.

Copy link
Collaborator

@matthijsmelissen matthijsmelissen left a comment

Choose a reason for hiding this comment

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

Great that you working on this, it's a serious problem that does need addressing.

Could you perhaps propose the change of the label font and the color change in different PRs? They are independent from each other, right?

I also think this color is too close to railways. On low zoom, I think this color is great. However, on high zoom, it might easily be confused with a type of railway, or perhaps even a type of road.

Perhaps we should abandon the idea of having the same border color on all zoom levels?

I think this PR in the current form should not be merged.

@matthijsmelissen matthijsmelissen changed the title Borders and country name rework [WIP] Borders and country name rework Jan 3, 2019
@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Jan 4, 2019

Could you perhaps propose the change of the label font and the color change in different PRs? They are independent from each other, right?

Sure, state labels changes are different thing, so I will remove it soon from this PR.

Perhaps we should abandon the idea of having the same border color on all zoom levels?

Ok, that sounds sane for me.

  • do you think that we should keep the old color for highest admin levels, new color for the lowest and make gradual color change for each admin level between them?
  • what do you think about inserting new level 5 pattern and shifting the rest in such case?

@kocio-pl kocio-pl force-pushed the violet-tint-borders branch from 939bf59 to 4f23825 Compare January 4, 2019 04:33
@matthijsmelissen
Copy link
Collaborator

do you think that we should keep the old color for highest admin levels, new color for the lowest and make gradual color change for each admin level between them?

That sounds reasonable. Or perhaps the new color can be even more gray (or a different color) if it is only for low zoom. Perhaps an abrupt change in color works as well, we'd need to test that.

what do you think about inserting new level 5 pattern and shifting the rest in such case?

I have no opinion on that at the moment.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Jan 4, 2019

Perhaps an abrupt change in color works as well, we'd need to test that.

Well, we already had this once and it was changed... 😄 - see #1749 fixed by #1773.

@matthijsmelissen
Copy link
Collaborator

I think I regret that change now.

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Jan 4, 2019

Since we have 9 levels rendered (l2-l10):

https://wiki.openstreetmap.org/wiki/Tag:boundary%3Dadministrative#Rendering

we need to use 9 steps palette:

https://meyerweb.com/eric/tools/color-blend/#6B516B:AC46AC:7:hex

or if we combine colors with the same pattern (l7+l8, l9+l10) we need just 7 color steps:

https://meyerweb.com/eric/tools/color-blend/#6B516B:AC46AC:5:hex

@vholten
Copy link
Contributor

vholten commented Jan 4, 2019

Test with the 7-color palette suggested by @kocio-pl:
z12, https://www.openstreetmap.org/#map=12/50.4465/16.9049
This area contains many admin_level=8 borders, one admin_level=4 state border and an international border.

7colors

@Tomasz-W
Copy link

Tomasz-W commented Jan 4, 2019

@matthijsmelissen Can you provide some examples of places where gray borders might be a problem?

Btw. See #2872 (comment). I think we should change some railway types rendering, because they cause conflicts with borders even when they are violet, so "palette solution" doesn't work for me anyway.

@matthijsmelissen
Copy link
Collaborator

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Jan 5, 2019

@vholten Could you paste the link to the code branch with the 7 colors palette so I could test it more?

@vholten
Copy link
Contributor

vholten commented Jan 5, 2019

@kocio-pl
Copy link
Collaborator Author

kocio-pl commented Jan 5, 2019

Thanks! Could you also adjust the label colors accordingly?

@vholten
Copy link
Contributor

vholten commented Jan 5, 2019

The color of country labels was already adjusted, state labels may work with that color as well.
I made another branch https://github.com/vholten/openstreetmap-carto/tree/borders-blend-statelabels where the color of state labels is based on the color of state borders.

In both branches I made country names bold.

@kocio-pl
Copy link
Collaborator Author

Sorry for late reply, I am still recovering with my focus on this repo and was unable to get where the problem is.

My idea is to make all the labels correspond to the line color, not only country and state labels. I think it will make it more clear that they are referring to the same admin level.

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