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

New city labels sometimes overlap #2378

Closed
imagico opened this issue Sep 27, 2016 · 26 comments · Fixed by #2470
Closed

New city labels sometimes overlap #2378

imagico opened this issue Sep 27, 2016 · 26 comments · Fixed by #2470

Comments

@imagico
Copy link
Collaborator

imagico commented Sep 27, 2016

Like here (Freiburg im Breisgau/Basel):

label overlap

Also sometimes labels overlap/touch city dots (like Paris and Frankfurt above) or admin unit labels.

Combined with the large line spacing (#2358) this is quite irritating and difficult to read.

@imagico
Copy link
Collaborator Author

imagico commented Sep 27, 2016

Two more observations regarding the city labels - can open separate issues for them if desirable:

  • the random default is probably a bad idea regarding the metatile edge issues - using a geometry hash would be better.
  • prioritizing N,S or S,N in the label directions leads to frequent label pairs if two cities are close and on a similar latitude like Mary/Bayramaly here . An increased minimum distance might already solve this but alternatively a different order of direction priorities could also be better.

@nebulon42
Copy link
Contributor

I think the overlap problem might have been introduced by e4cb767? There the margin was removed but not replaced with some other distance measure.

The spacing issues was not present when I redesigned the labels, so I cannot really comment on that.

Could you expand on the geometry hash topic?

I didn't realise that this change is already live. Otherwise I would have suggested to delay the update. But I will try to look into a possible fix for the overlap problems. As I'm quite busy currently I cannot promise that it will be fast.

@imagico
Copy link
Collaborator Author

imagico commented Sep 27, 2016

The spacing issues was not present when I redesigned the labels, so I cannot really comment on that.

Yes, it is so obvious it would have been visible in the examples.

Could you expand on the geometry hash topic?

If you derive your dir value from ST_GeoHash() or simply using some minor digit from the coordinates you get the same for the same place in every metatile. This way at least when labels do not compete with others you get consistent rendering at metatile edges.

@nebulon42
Copy link
Contributor

Great idea! That might likely solve the

     on
    x
Lond

issue. Since we cannot/should not use avoid-edges.

@kocio-pl
Copy link
Collaborator

I was thinking about another solution - for small zoom levels with so few elements to render we don't need slicing the world to metatiles, we can check things global - I mean something like z4 or z5. Does this make sense and is technically possible?

@StyXman
Copy link
Contributor

StyXman commented Sep 27, 2016

z5 -> 32x32 tiles, 8192² pixels. It would be interesting to see hoy many labels qualify for rendering in those levels.

@pnorman
Copy link
Collaborator

pnorman commented Sep 28, 2016

the random default is probably a bad idea regarding the metatile edge issues - using a geometry hash would be better.

Yikes - we should have never approved a change that has random rendering.

@nebulon42
Copy link
Contributor

To be fair, with avoid-edges this would not be a big problem. At least I think it would be that way. :)

@matthijsmelissen
Copy link
Collaborator

Sorry, missed that when reviewing'

@kocio-pl
Copy link
Collaborator

Related to #2314.

@nebulon42
Copy link
Contributor

My first research showed that the problems at metatile boundaries also happen without the random logic and also when using ST_GeoHash. This might be due to the fact that Mapnik comes to different conclusions regarding positioning of the shield label in the different metatiles.

I'm not sure how to solve this. Initially, I used avoid-edges but this was dropped as it led to omitting important cities. Any ideas?

@imagico
Copy link
Collaborator Author

imagico commented Oct 7, 2016

Are you saying that a non-competing label (like probably London at z4) still gets messed up? Then i would suspect something is wrong with the code. With competing labels the situation is different of course - there you get an influence of the other labels and that is different for the different metatiles leading to the inconsistencies. But it would be much less frequent than at the moment.

@nebulon42
Copy link
Contributor

I don't know, I have to investigate more. But in a first step I will change the base for randomness so that this can be ruled out.

@simonpoole
Copy link

What is visually actually annoying in the specific case (I was just going to open a separate issue on it) is that the line wrapped "Freiburg im Breisgau" seems to have a proportionally different (and IMHO to large) line spacing compared to higher zoom levels.

@HolgerJeromin
Copy link
Contributor

@simonpoole your issue is probably #2358

@verdy-p
Copy link

verdy-p commented Nov 15, 2016

Anyway the excessive line-height (when linewrap occurs) is easy to correct immediately.
Fonts for each script have a natural line-height (on MediaWiki default stylesheets, it is set to 1.6em, larger than the natural height, but not inheritable when there's a change of font-size, but this is needed to avoid collisions with superscripts and subscripts that won't change the line-height).
But we are not here on a wiki page mixing spans of text with variable size.

This natural line-height is about 1.25em for Latin/Cyrillic/Greek and even Hangul/Hiragana/Katakakana/Bopomofo/Han and also for Arabic (except for the Nasthalik script style of Arabic used in Persian/Urdu, that requires a higher line-height about 1.6em) and Tifinagh, Ethiopic

For Indic scripts this may be more complex, notably scripts in South-East Asia. that require a higher line-height (but names in those scripts are usually smaller and won't wrap).

Note that Many Asian scripts allow wrapping almost anywhere as they don't use whitespaces or hyphens as word separators (there may be some hyphen-like separators used in Japan, but they have semantic purpose and are not just separators, they are considered like regular letters; these CJK words will wrap in the middle as most letters are representing a full syllable, and when this wrap occurs, there's no insertion of hyphen at end of the first wrapped line).

So I suggest setting the line-height to a conservative value of 1.3em, increasing it only if a label contains letters from Asian scripts needing a larger line-height (you need to check the list of fonts supported by your renderer for rendering those scripts, and then define some code for parsing text to find their scripts and minimum line-height: those scripts needing larger heights are in well defined ranges of Unicode, if you cannot identify the language used).

But it seems that for now, the too conservative value exceeding 1.6em was used for all supported scripts, but this is very bad for Latin/Cyrillic/Greek/Arabic/Hebrew/Devanagari/Tamil/Sinhalese, and still excessive for Thai and Lao! If you cannot choose a line-height for multiple scripts, choose 1.6em (rather than any existing larger value) and you'll be fine (as demonstrated in all multilingual wikis), even if we could be more compact to conserve more free space (and margins) for other surrounding labels !

@kocio-pl kocio-pl added the text label Nov 15, 2016
@kocio-pl kocio-pl added this to the Bugs and improvements milestone Nov 15, 2016
@pnorman
Copy link
Collaborator

pnorman commented Nov 15, 2016

increasing it only if a label contains letters from Asian scripts needing a larger line-height

How would you do this?

@verdy-p
Copy link

verdy-p commented Nov 16, 2016

From what I see on the map, the large line-height is there only for Thai.
You would have just to detect the presence of Thai characters on the label to set a "lang(th)" CSS style selector to the label. The stylesheet would then define the line-height: for overriding the smaller default.
To detect Thai, you just need to detect the presence of characters in the range U+0E00..U+E07F assigned to the Thai block (no need to inspect assigned or unassigned positions in that block)

For other scripts, the default line-height should be smaller (1.6 is the default used in MediaWiki for all scripts, including Thai; the Nasthalik style for Eastern Arabic/Persian/Urdu is currently not used as you use the same fonts for all Arabic text, but if you can detect the area of use, you could also set a distinct font for Eastern Arabic needing a larger line-height)

However technically I don't know how this can be insert in the existing code of your renderer.

Note that Thai labels are usually small and don't seem to wrap anywhere on the map.


Note finally that labels may also used mixed scripts (very frequent in South Korea, where labels in Hangul are followed by transcriptions in Latin (Not a good idea in my opinion, as these labels are overlong but this is what is set in the OSM database: may be instead of displaying the full "name=" value in Korean, you could also detect the presence of Hangul characters and then look if there's a "name:ko=" value and use it instead, as it should remove the dual transcription)

I think we should document in OSM a recommendation to avoid dual transcriptionf in default names.

But the renderer can already preprocess the names (for example to find abbreviations of common terms like "Street", "Avenue", "Saint"... and also adapt the style of some letters of abbreaviations using superscripts. This preprocessing step is where you'll detect the scripts used, and then adapt the style to use to render it, select appropriate font families, and select a matching line-height.

You better know than me the list of fonts you have put in your renderer to support multiple scripts. Each font has to be reviewed for their line-height: It is easy to find some text paragraphs in those scripts and create a text HTML page (or wiki page) showing the result for linewraps: put one paragraph for each tested script (insert in the test paragraphs also some Latin text to compare the relative font-sizes used by each selected font for another non-Latin script.

I don't think you actually support a lot of fonts in your stylesheets.

@sommerluk
Copy link
Collaborator

sommerluk commented Nov 16, 2016

I don't think you actually support a lot of fonts in your stylesheets.

Our font list at at https://github.com/gravitystorm/openstreetmap-carto/blob/master/style.mss for the regular style has 68 entries. For bold and for italic style, it has a similar number. It should cover almost all living scripts of the world.

@verdy-p
Copy link

verdy-p commented Nov 16, 2016

OK you use the Noto fonts set, it has coherent metrics across scripts, this makes the choice simpler, and easier to test !
The rest of my comment remains valid: create an HTML using each of these fonts with a small text fragment (long enough to render on 2 lines) for the relevant scripts, that you can tune for defining the relevant line-height values (if a specific larger line-height is needed for some scripts).
You'll see that the current default is too large for most (if not all) scripts.

@verdy-p
Copy link

verdy-p commented Nov 16, 2016

I've started such a test page on https://wiki.openstreetmap.org/wiki/CartoCSS/fonts

It includes all fonts currently in the CartoCSS master stylesheet. The font names are displayed in grey (along with their ISO15924 script code for which they are first built) just above the sample text. All texts in the 3 columns (including the grey lines) use NO paragraphs and no padding (the items are all within divs)

For now the sample text for each script is prefilled with English. Insert the text you want to test there

Then if you want to tune the line-height for a specific script, set it in the switch found in https://wiki.openstreetmap.org/wiki/CartoCSS/fonts/text

@sommerluk
Copy link
Collaborator

CartoCSS has far less possibilities to influence font rendering than the CSS that is used for web pages. Probably really hard would be a script/font file detection.

If this is mostly about line spacing, I would suggest to discuss further at #2358 which seems to me the more appropriate issue. There are also yet some comments about technical possibilities and restrictions.

@pnorman
Copy link
Collaborator

pnorman commented Nov 16, 2016

However technically I don't know how this can be insert in the existing code of your renderer.

This is the hard part, which I was looking for input for. We have to work within the constraints of the rendering tools we use.

But the renderer can already preprocess the names (for example to find abbreviations of common terms like "Street", "Avenue", "Saint"... and also adapt the style of some letters of abbreaviations using superscripts. This preprocessing step is where you'll detect the scripts used, and then adapt the style to use to render it, select appropriate font families, and select a matching line-height.

Preprocessing to change or abbreviate names like this is something we intentionally don't do. If we were to detect language at load time we'd need to implement that in Lua. If this were done, it'd require a reload and probably be a major version bump, and probably wouldn't make it in with the initial merge of Lua.

@verdy-p
Copy link

verdy-p commented Nov 17, 2016

Proprocesssing names is performed routinely on the French renderers producing a map preferably in French, and the one used for the Humanitarian map. Both have worldwide coverage and produce tiles up to level 19.

I don't think there's really a problem in renderer capacities, when you've got enough tile servers using caches. Performance will be an issue if you don't use caches at all for high levels of zoom and need renderers to produce all tiles on demand for these levels. So what is needed is enough storage for the caches at high levels of zoom.

I've not seen any complaints with significant loss of performance by proprocessing names on the French renderers, and no sign at all of storage problems for caching the tiles !

OK there are much less users of the two French maps, so tiles on demand are probably lower and renderers may be less harnessed. But the delay to render again a non-fresh tile on demand (this can be forced with /dirty requests) is short and similar to the delay for getting it on the OSM.org Mapnik renderer...

The French renderer makes many more preprocessing and uses much more advanced CSS styling rules (including detecting some geometries to compute the orientation of icons/symbols: this is even more complex than préprocessing names with same regexps or character scanners). I don't know how these processings are done, but it is probably not in Lua (which is really too slow as it is interpreted after only a precompilation to some internal bytecode)

@pnorman
Copy link
Collaborator

pnorman commented Nov 17, 2016

Anyway the excessive line-height (when linewrap occurs) is easy to correct immediately.

From what I see on the map, the large line-height is there only for Thai.

When replying, I thought this was a different issue. This issue is about overlap on city labels. It is not about vertical spacing in labels (#2358) or preprocessing names. I may clean up the off-topic discussion in a day or so, so if you have comments on something off topic you want to preserve, please move them to an appropriate issue.

@verdy-p
Copy link

verdy-p commented Nov 17, 2016

Overlaps here are clearly related to the case of linewraps with excessive line-heights.
So yes it is related to #2358, but yes it is also related to name preprocessing (that can shorten names and then avoid many overlaps. If you clean this, you will forget that fact.

Note also that there's an overlap of the label with the icon when it wraps on multiple lines (I don't think it is critical to cover this icon, but the positioning of the multiline label may not be what was expected and if may not be centered on the icon partly dovered below the multiline label.

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

Successfully merging a pull request may close this issue.

10 participants