Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

allowsFixedWidthGlyphGeneration filtering is grossly overaggressive #15658

Closed
dstrebe-tableau opened this issue Sep 18, 2019 · 5 comments
Closed
Labels
archived Archived because of inactivity Core The cross-platform C++ core, aka mbgl

Comments

@dstrebe-tableau
Copy link

Platform:
Mapbox SDK version:

This is a whitebox bug report. The conditions to reproduce the bug are complicated and require particular font configurations on the host desktop. The bug expresses as placename text being rendered in an incorrect font.

Affected functions:
glyph_manager.js::_tinySDF (head)
il8n.cpp::allowsFixedWidthGlyphGeneration (head)

These filters only pass normal ideographic glyphs and Hangul syllables. This results in all Japanese kana (for example) rendering in some fallback font instead of what is specified for LocalGlyphRasterizer. It is possible, in some fonts, for kana to be proportional, but not for the fonts we specify. (This points to a much deeper problem throughout the Mapbox architecture of basing writing locale and other judgments on UTF-16 code points, when correct judgment should be on external factors such as a designated writing system or should be font-specific, and also should not expect a single UTF-16 code point to be the complete character in play.)

@dstrebe-tableau
Copy link
Author

See Tableau branch for il8n.cpp fix.

@asheemmamoowala
Copy link
Contributor

These filters only pass normal ideographic glyphs and Hangul syllables. This results in all Japanese kana (for example) rendering in some fallback font instead of what is specified for LocalGlyphRasterizer. It is possible, in some fonts, for kana to be proportional, but not for the fonts we specify.

@dstrebe-tableau Thank you for this report. As you have discovered the local glyph generation is not enabled for all Japanese character ranges. Given that kana are not always fixed width, it does not make sense to enable this in the core library. The local glyph generation is a fallback that allows using locally installed and available fonts to reduce the number of font glyphs requested from the server and to decrease time to first render.

when correct judgment should be on external factors such as a designated writing system or should be font-specific, and also should not expect a single UTF-16 code point to be the complete character in play

We are considering alternative approaches, like locally rendering all text as described in #7862 and #7528. We'll update those as we make progress in this area.

@dstrebe-tableau
Copy link
Author

dstrebe-tableau commented Sep 30, 2019

Given that kana are not always fixed width, it does not make sense to enable this in the core library.

Leaving aside kana for the moment, the filtering in play that I point out excludes every range of ideographic glyphs except a single range. There is no reason the filtered ideographic ranges should be filtered more aggressively than the one ideographic range that passes the filter. They are all ideographic. This is a bug no matter how one parses it.

Meanwhile, any glyph could be variable-width, including ideographic and Korean syllables. Korean syllables, which pass the filter, are just as likely not to be fixed-width as kana is. On the other hand, it is quite uncommon for kana to be variable-width, and that would never happen in the fonts we use and care about anyway because we specify what those fonts are. Nor are they variable-width in Arial Unicode MS, which seems to be Mapbox’s favored font for rendering worldwide.

@chloekraw chloekraw added the Core The cross-platform C++ core, aka mbgl label Dec 6, 2019
@1ec5
Copy link
Contributor

1ec5 commented Mar 2, 2020

#15009 added Hiragana and Katakana to the set of Unicode blocks that participate in local font rendering. mapbox/mapbox-gl-js#9362 would generalize this behavior to all blocks that the Unicode standard considers to be intrinsically fixed-width.

@stale stale bot added the archived Archived because of inactivity label Aug 29, 2020
@stale
Copy link

stale bot commented Aug 30, 2020

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Aug 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived Archived because of inactivity Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

No branches or pull requests

4 participants