Expose ascender/descender metadata properties in protobuf

[tristen]

Pull what's applicable from https://github.com/mapbox/node-fontnik/pull/97 into this commit.

[springmeyer]

The branch is passing now on travis since I fixed this minor issue (https://github.com/mapbox/node-fontnik/pull/161) and merged that fix into this branch. The next steps I see for this work are:

• [x] Ensure that the values are correct and what are needed
  • [x] Would require testing some glyphs inside MBGL and adapting to the new data
• [ ] Based on MBGL testing, write more tests here as needed to assert the right data is being provided and won't regress
  • [ ] Ensure this change is supported in fontstack compositing. Two options:
  • [ ] Option 1: Port https://github.com/mapbox/glyph-pbf-composite to understand these new fields (by updating the duplicated glyphs.proto at https://github.com/mapbox/glyph-pbf-composite/blob/master/proto/glyphs.proto)
    • [ ] Update downstream deps that use glyph-pbf-composite like https://github.com/mapbox/fontmachine
  • [ ] Option 2: Port glyph compositing into node-fontnik to keep all glyph write logic in one place. I've put forth a POC of this at https://github.com/mapbox/node-fontnik/pull/163
    • [ ] Update downstream deps like https://github.com/mapbox/fontmachine to use fontnik compositing instead of glyph-pbf-composite
• [ ] Discuss back compatibly in across service: a) do we update all fonts to have this metadata or do we write opt-in logic in all our renderers to adapt to this information, when it exists in the pbf?

[zmiao]

Some experimenting rendering results after applying the ascender value in MBGL for fixing the dis-alignment of mixed fonts:

After fix	Before fix

Next step for testing would be trying to figure out how to deal with issues in https://github.com/mapbox/mapbox-gl-js/issues/8560

[zmiao]

@tristen @springmeyer in the source code https://github.com/mapbox/node-fontnik/blob/d65c204a3134bfe7b0d4ed020c088861e98744e9/src/glyphs.cpp#L297-L310, each font-face equals to a new fontstack in the pbf. So what does font-face mean? Does it imply a new font type?

I am confused because that I tried to use the executable build-glyphs to generate the pbf data from one .ttc file that containing multiple fonts, but finally the output is still the general ranges pbf, nothing is font related. Did I miss something, how can I generate font-specific pbf data if the *.ttc has multiple fonts inside? For example, the NotoSansCJK.ttc in https://www.google.com/get/noto/help/cjk/ has 36 OTF font files in total.

[springmeyer]

👋 @zmiao thanks for asking. This was also a confusion for me when I first considered .ttc behavior. My understanding is this:

• The node-fontnik code that handles reading faces was originally ported from code from mapnik, which supports .ttc
• But the ttc support in node-fontnik was never tested
• And downstream libraries on top of node-fontnik explicitly do not support ttc
• So, its unclear to what extent the ttc support in node-fontnik is working or not

A little bit of this history is evident in https://github.com/mapbox/node-fontnik/issues/125

So, overall, to support ttc would need someone to comprehensively test and perhaps fix issues in the entire dependency chain: node-fontnik -> fontmachine -> core-fonts -> api-styles -> mapbox-gl. Because several of those repos are private, if you have questions about them it would make most sense to discuss more in tickets at those repos.

[zmiao]

@springmeyer Thanks for the explanation, it is really helpful.

So based on your comments, right now we do not support generating pbf from multi-face ttc, but the weird thing is if I upload the ttc file via MapboxStudio, I could get multiple fonts, how did it work? I assume MapboxStudio uses the same node-fontnik engine, right?

[springmeyer]

Interesting @zmiao - I would have assumed you'd get an error due to this line: https://github.com/mapbox/fontmachine/blob/0e5eaab3fd99153083f08278d7a55792d2b6f3ce/index.js#L32

I assume MapboxStudio uses the same node-fontnik engine, right?

Yes, the backend of Mapbox Studio uses node-fontnik@0.5.3

[zmiao]

@springmeyer It was my bad. Actually for my case, I uploaded one otf file, but I got two fonts without any error prompt, I am wondering why it works?

And also if I use bin/build-glyphs to generate pbf from ttc file, I didn't get any errors neither, just have to wait for a very long while.

[tmpsantos]

Hi all, this has been stalled for a while. Do we have plans to merge it?