search

mapbox / mapbox-gl-native-ios

Interactive, thoroughly customizable maps for iOS powered by vector tiles and OpenGL
https://www.mapbox.com/mobile/
Other
210 stars 122 forks source link

MGLIdeographicFontFamilyName should accept font display or PostScript name #105

Closed chloekraw closed 4 years ago

chloekraw commented 5 years ago

Developers who use localIdeographFontFamily and specify a text-font with "Bold" in the name on some layers are expecting the text of those layers to show up as bolded on iOS, just like they do on Web and Android.

It looks like we did add a CoreText-based LocalGlyphRasterizer implementation on darwin in https://github.com/mapbox/mapbox-gl-native/pull/10572, but for some reason, passing in different font weight values doesn't actually change the font weight: https://github.com/mapbox/mapbox-gl-native/pull/10572#issuecomment-347995568. So, this ticket tracks debugging this issue further.

Note that the existing implementations are slightly different due to limitations on each platform:

To achieve this effect on Android, we use the platform API android.graphics.Typeface in LocalGlyphRasterizer to draw the glyphs locally. Typeface only supports a limited number of formatting options (bold, bold italic, italic, normal) in API 14 (our minimum deployment target) and does not support the specification of a numeric weight value until API 28.

On GL-JS, we check the name of the font on each layer and change the fontWeight if the font's name contains the word "Bold", "Medium", or "Light":

 if (!tinySDF) {
            let fontWeight = '400';
            if (/bold/i.test(stack)) {
                fontWeight = '900';
            } else if (/medium/i.test(stack)) {
                fontWeight = '500';
            } else if (/light/i.test(stack)) {
                fontWeight = '200';
            }
            tinySDF = entry.tinySDF = new GlyphManager.TinySDF(24, 3, 8, .25, family, fontWeight);
        }
alexshalamov commented 4 years ago

@chloekraw I believe it is already implemented in Android https://github.com/mapbox/mapbox-gl-native/blob/e373d8a5924e4f4cf3904ecacbf1d1cf86a5d60f/platform/android/src/text/local_glyph_rasterizer.cpp#L79

Not implemented on iOS / macOS.

Do we need to support only 'Bold' style or other font styles as well? For instance:

Thin
Thin Italic
Light
Light Italic
Regular
Regular Italic
Medium
Medium Italic
Bold
Bold Italic
Black
Black Italic
chloekraw commented 4 years ago

@mapbox/maps-ios there's a request to reach GL JS parity (see OP) with our implementation of font-weight in LocalGlyphRasterizer: https://github.com/mapbox/mapbox-gl-native-ios/blob/master/platform/darwin/src/local_glyph_rasterizer.mm#L29

Apparently Android APIs may prevent us from supporting Medium and potentially other weights, and I'm thinking we should reach parity with Android over JS. Any concerns about the possibility of implementing this?

cc @mapbox/maps-android

chloekraw commented 4 years ago

I poked around the git commit history and it looks like we did add a CoreText-based LocalGlyphRasterizer implementation on iOS in https://github.com/mapbox/mapbox-gl-native/pull/10572, but for some reason, passing in different font weight values doesn't actually change the font weight: https://github.com/mapbox/mapbox-gl-native/pull/10572#issuecomment-347995568

I'm updating the OP of this ticket to match the actual work that would need to happen.

cc @kkaefer

1ec5 commented 4 years ago

Developers who use localIdeographFontFamily and specify a text-font with "Bold" in the name on some layers are expecting the text of those layers to show up as bolded on iOS, just like they do on Web and Android.

As stated in this documentation, MGLIdeographicFontFamilyName expects a font family name. If you instead specify an individual font’s display name or PostScript name, the SDK won’t send any font family name to mbgl due to this validation step in -[MGLRendererConfiguration _localFontFamilyNameWithPropertyDictionary:]:

https://github.com/mapbox/mapbox-gl-native-ios/blob/87fac0bb3447872bb142c57b4d12b4b7cb9b31f7/platform/darwin/src/MGLRendererConfiguration.mm#L110

It would be possible to check against NSFontManager.availableFonts in addition to availableFontFamilies to accept PostScript names. (On iOS, we’d need to call +[UIFont fontNamesForFamilyName:] for each of the family names returned by UIFont.familyNames.) There isn’t a performant way to obtain the display names of every installed font on the system.

A more common approach to validating fonts is to create an NSFontDescriptor/UIFontDescriptor, then attempt to create a font using +[NSFont fontWithDescriptor:size:]/+[UIFont fontWithDescriptor:size:]. As it happens, this is exactly what mbgl::LocalGlyphRasterizer::Impl::getFont() does:

https://github.com/mapbox/mapbox-gl-native/blob/6bed2079e3097336547ac01dda3768caadf0c441/platform/darwin/src/local_glyph_rasterizer.mm#L72-L78

kCTFontFamilyNameAttribute is set to a name that doesn’t match any installed font families. So CTFontCreateWithFontDescriptor() will return the default font, Helvetica, and mbgl::LocalGlyphRasterizer::Impl::drawGlyphBitmap() will use the system’s font cascade list. This code could easily be modified to create a font descriptor from a display name or PostScript name.

I don’t think -[MGLRendererConfiguration _localFontFamilyNameWithPropertyDictionary:] should validate MGLIdeographicFontFamilyName values, since mbgl::LocalGlyphRasterizer::Impl::getFont() is going to do so anyways, just using Core Text instead of AppKit/UIKit. The only reason it seems to do so is that we added support for a cascading list after the fact, so mbgl only stores a single font family name.

A better solution would be to pass all the names into mbgl verbatim. Then, mbgl would create a font descriptor for each name interpreted as a font PostScript name, font display name, and font family name, in that order. Each of these font descriptors would go into a kCTFontCascadeListAttribute, which we’d set instead of or in addition to kCTFontAttributeName. This would also make it possible to perform font substitution on a character-by-character basis rather than globally.

/ref https://github.com/mapbox/mapbox-gl-native/pull/14862#issuecomment-500994073

1ec5 commented 4 years ago

On GL-JS, we check the name of the font on each layer and change the fontWeight if the font's name contains the word "Bold", "Medium", or "Light":

I’m not sure why this is necessary, because the JavaScript implementation of TinySDF relies on the font-family CSS property, which despite its name can contain an individual font’s display name or PostScript name:

https://github.com/mapbox/tiny-sdf/blob/6890bfbdd054efa1e941a2ff4467d2325448c99b/index.js#L12

1ec5 commented 4 years ago

Do we need to support only 'Bold' style or other font styles as well?

This is quite feasible as long as we rely on the system to match each MGLIdeographicFontFamilyName to the available fonts, rather than sniffing the weight out of the display name. For example, here’s Noto Sans CJK JP Black, using the cascade list approach outlined in https://github.com/mapbox/mapbox-gl-native-ios/issues/105#issuecomment-592991225:

black

The Hiragino font families have numeric weights, not keywords, in the display names. Here’s Hiragino Sans W9:

w9

Note that few CJK fonts come with an explicit “italic” style font. Instead, it’s common for a specific, cursive-like style to be distributed as the normal font in a separate font family.

1ec5 commented 4 years ago

I poked around the git commit history and it looks like we did add a CoreText-based LocalGlyphRasterizer implementation on iOS in https://github.com/mapbox/mapbox-gl-native/pull/10572, but for some reason, passing in different font weight values doesn't actually change the font weight: https://github.com/mapbox/mapbox-gl-native/pull/10572#issuecomment-347995568

The assumption during that PR’s development was that there would be an installed font family named “PingFang” that includes a font face with the style “Bold”. In fact, Apple systems come with a font family named “PingFang HK”, “PingFang SC”, or “PingFang TC”, with the styles “Thin”, “Light”, “Ultralight”, “Regular”, “Medium”, and “Semibold”. Nothing seemed to happen, because Core Text couldn’t find a font face matching either criteria. Instead, it fell back to the first CJK font in the cascade list, which happened to be PingFang TC Regular: https://github.com/mapbox/mapbox-gl-native/pull/16253#issuecomment-593133491.

It is possible to create a font based on a font family name and a font weight, as the commented-out, since-removed getFontWeight() method in mapbox/mapbox-gl-native#10572 attempted to do. However, we need to create the font descriptor using the kCTFontTraitsAttribute and kCTFontWeightTrait keys rather than the kCTFontStyleNameAttribute key. kCTFontStyleNameAttribute performs an exact match on the font faces’ declared styles without any interpolation.

1ec5 commented 4 years ago

It’s also possible to create a font based on a font family name and font style name (also known as the font face). As it happens, we can count on the font stack (a symbol layer’s text-font layout property defined in style JSON) to generally contain font display names, since Studio normalizes the values to display names. So this would be the most robust cascade list (in order):

  1. Each of the values in the font stack verbatim as font display names or PostScript names, except for Arial Unicode MS
  2. Each of the MGLIdeographicFontFamilyName values:
    1. as a font display name or font PostScript name
    2. as a font family name with a font style name parsed from the font stack
    3. as a font family name with font traits with a font weight based on the font style name parsed from the font stack

For example, given the following settings:

the font cascade list would be:

The idea is that, if the developer has uploaded a custom font to Studio, they would prefer the SDK to use an identically named font that they’ve bundled with the application or that is already installed on the system. MGLIdeographicFontFamilyName should be thought of as a stylistic fallback, not an override, even though it does override the mechanism for obtaining the font. Matching fonts by style names can be hit-or-miss, because each vendor uses different terminology (Black versus Heavy, etc.), so translating style names to font weights can be useful in some cases.

I think these heuristics would be more intuitive and better respect Studio’s role in the design process, but we’d need to make sure it doesn’t represent a backwards-incompatible change and doesn’t put developers in a catch-22 if they also want the same style JSON to render using the same fonts on other platforms.

1ec5 commented 4 years ago

I still think sniffing traits out of (freeform) font display names is an error-prone approach that will come back to haunt us when we expand local font rendering to scripts beyond CJK – at which point “Italic”, “Condensed”, “Mono”, etc. all become common enough that we’d have to detect these keywords too. (This wouldn’t be an issue if the Fontstack API could return metadata about a font instead of the font itself. Then we’d just plug that metadata into Core Text to get the best local match.)

As a first step, I’m going to add font stack awareness to mapbox/mapbox-gl-native#16253 but leave out the sniffing bit:

  1. Each of the values in the font stack verbatim as a font display name or PostScript name or font family name, except for Arial Unicode MS
  2. Each of the MGLIdeographicFontFamilyName values as a font display name or font PostScript name or font family name

This simpler heuristic will automatically use any installed font that happens to match one that’s available in Studio. If it’s a font that’s unlikely to be installed, the developer can bundle the font with their application, assuming they have distribution rights. This is akin to how fonts normally work in HTML and CSS. The global setting MGLIdeographicFontFamilyName becomes less important, more like the Web browser’s default font settings that hardly anyone touches.