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

Better line spacing #2730

Merged
merged 1 commit into from
Aug 15, 2017
Merged

Conversation

sommerluk
Copy link
Collaborator

Resolves #2358 (last part is solved by this PR)

Before:
screenshot 1

After:
screenshot 2

@kocio-pl
Copy link
Collaborator

kocio-pl commented Aug 7, 2017

Could you also show examples how would other scripts look like with this change?

@sommerluk
Copy link
Collaborator Author

screenshot 3
screenshot 4
screenshot 5
screenshot 6

Note that the used line spacing is simply the same that we have also applied in the rest of openstreetmap-carto (−0.15 em for line wrap ≤ 3 em).

This is the last one of various PR that implemented the fine-tuned line spacing.

Copy link
Collaborator

@pnorman pnorman left a comment

Choose a reason for hiding this comment

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

Screenshots look good to me, but I haven't tested the code.

@matthijsmelissen
Copy link
Collaborator

Thanks! By the way, it would also make sense to keep the wrap-width a constant em amount.

@matthijsmelissen matthijsmelissen merged commit cc1e3a5 into gravitystorm:master Aug 15, 2017
@verdy-p
Copy link

verdy-p commented Aug 15, 2017

Closed without testing with Myanmar ?

@verdy-p
Copy link

verdy-p commented Aug 15, 2017

And still no test for Arabic : (high Khufi font, suitable for real trafic signs where they are isolated and have cumfortable line-height) or UI font (better suited for multilingual text on a dense map) ?
I summary the tests above are only for alphabetic (Latin/Greek/Cyrillic) script, Indic script, CJK scripts, it will probably work for Hebrew and Tifinagh
But I still have doubts for Arabic, Myanmar, Khmer, Lao, and Tibetan, and possibly even with Thai.

@kocio-pl
Copy link
Collaborator

Could you provide test rendering for missing scripts?

@verdy-p
Copy link

verdy-p commented Aug 16, 2017

All these scripts were in the initial #2358
When closing this bug with limited tests, someone marked #2358 as resolved even if you tested much less things here.

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

Successfully merging this pull request may close these issues.

5 participants