Introduce descender and ascender for font metrics + fix several text rendering issues (mapbox#8781)

* Introduce descender+ascender, fix fonts dis-alignment

* Fix failing tests

* Add glyph baseine checker

* Update render test

* Update font baseline, make vertical mode applying with font baseline

* Fix failing tests

* Update render test case source position

* Move ascender/descender to font level attributes, remove non-necessary pbf files

* Add new glyph pbfs

Remove duplicates render tests

Fix confilicts

* rename the inner callback parameter naming

* gl-native parity: Extend Ideographs rasterization range to include CJK symbols and punctuations

update comments

* Adjust text shaping

fix flow and lint errors

code clean

* Fix shaping.test contents parsing

Fix error

* Update render tests expectation

update expectations

Update test ignore list

* Fix vertical text rendering with 'text-offset' property

* Fix text shaping for characters that have very big part below baselines, such as p, g, y, etc.
Fix vertical text shaping with ZWSP and punctuations.

additional fix

* Update test expectations

* Fix text rendering when both 'text-rotate' and 'text-offset' is set

* Update render tests

* Fix text rendering for line labels

* Enable ignored render tests regarding text rendering

Update render tests

* Make ascender and descender optional

* Update render tests

* Update shaping.js logic

* Update render test expectations

* Update shaping.test

Fix unit tests

* Fix review comments

* fix typo

Author: zmiao

src/render/glyph_atlas.js

@@ -5,7 +5,7 @@ import {AlphaImage} from '../util/image.js';
 import {register} from '../util/web_worker_transfer.js';
 import potpack from 'potpack';
 
-import type {GlyphMetrics, StyleGlyph} from '../style/style_glyph.js';
+import type {StyleGlyph} from '../style/style_glyph.js';
 
 const glyphPadding = 1;
 /*
@@ -17,34 +17,31 @@ const glyphPadding = 1;
 */
 const localGlyphPadding = glyphPadding * SDF_SCALE;
 
-export type Rect = {
+export type GlyphRect = {
     x: number,
     y: number,
     w: number,
     h: number
 };
+// {glyphID: glyphRect}
+export type GlyphPositionMap = { [_: number]: GlyphRect };
 
-export type GlyphPosition = {
-    rect: Rect,
-    metrics: GlyphMetrics
-};
-
-export type GlyphPositions = {[_: string]: {[_: number]: GlyphPosition } }
+// {fontStack: glyphPoistionMap}
+export type GlyphPositions = { [_: string]: GlyphPositionMap };
 
 export default class GlyphAtlas {
     image: AlphaImage;
     positions: GlyphPositions;
-
-    constructor(stacks: {[_: string]: {[_: number]: ?StyleGlyph } }) {
+    constructor(stacks: {[_: string]: {glyphs: {[_: number]: ?StyleGlyph }, ascender?: number, descender?: number }}) {
         const positions = {};
         const bins = [];
 
         for (const stack in stacks) {
-            const glyphs = stacks[stack];
-            const stackPositions = positions[stack] = {};
+            const glyphData = stacks[stack];
+            const glyphPositionMap = positions[stack] = {};
 
-            for (const id in glyphs) {
-                const src = glyphs[+id];
+            for (const id in glyphData.glyphs) {
+                const src = glyphData.glyphs[+id];
                 if (!src || src.bitmap.width === 0 || src.bitmap.height === 0) continue;
 
                 const padding = src.metrics.localGlyph ? localGlyphPadding : glyphPadding;
@@ -55,20 +52,20 @@ export default class GlyphAtlas {
                     h: src.bitmap.height + 2 * padding
                 };
                 bins.push(bin);
-                stackPositions[id] = {rect: bin, metrics: src.metrics};
+                glyphPositionMap[id] = bin;
             }
         }
 
         const {w, h} = potpack(bins);
         const image = new AlphaImage({width: w || 1, height: h || 1});
 
         for (const stack in stacks) {
-            const glyphs = stacks[stack];
+            const glyphData = stacks[stack];
 
-            for (const id in glyphs) {
-                const src = glyphs[+id];
+            for (const id in glyphData.glyphs) {
+                const src = glyphData.glyphs[+id];
                 if (!src || src.bitmap.width === 0 || src.bitmap.height === 0) continue;
-                const bin = positions[stack][id].rect;
+                const bin = positions[stack][id];
                 const padding = src.metrics.localGlyph ? localGlyphPadding : glyphPadding;
                 AlphaImage.copy(src.bitmap, image, {x: 0, y: 0}, {x: bin.x + padding, y: bin.y + padding}, src.bitmap);
             }

src/render/glyph_manager.js

@@ -37,9 +37,11 @@ export const SDF_SCALE = 2;
 type Entry = {
     // null means we've requested the range, but the glyph wasn't included in the result.
     glyphs: {[id: number]: StyleGlyph | null},
-    requests: {[range: number]: Array<Callback<{[_: number]: StyleGlyph | null}>>},
+    requests: {[range: number]: Array<Callback<{glyphs: {[number]: StyleGlyph | null}, ascender?: number, descender?: number}>>},
     ranges: {[range: number]: boolean | null},
-    tinySDF?: TinySDF
+    tinySDF?: TinySDF,
+    ascender?: number,
+    descender?: number
 };
 
 export const LocalGlyphMode = {
@@ -55,7 +57,7 @@ class GlyphManager {
     entries: {[_: string]: Entry};
     // Multiple fontstacks may share the same local glyphs, so keep an index
     // into the glyphs based soley on font weight
-    localGlyphs: {[_: string]: {[id: number]: StyleGlyph | null}};
+    localGlyphs: {[_: string]: {glyphs: {[id: number]: StyleGlyph | null}, ascender: ?number, descender: ?number}};
     url: ?string;
 
     // exposed as statics to enable stubbing in unit tests
@@ -80,7 +82,7 @@ class GlyphManager {
         this.url = url;
     }
 
-    getGlyphs(glyphs: {[stack: string]: Array<number>}, callback: Callback<{[stack: string]: {[id: number]: ?StyleGlyph}}>) {
+    getGlyphs(glyphs: {[stack: string]: Array<number>}, callback: Callback<{[stack: string]: {glyphs: {[_: number]: ?StyleGlyph}, ascender?: number, descender?: number}}>) {
         const all = [];
 
         for (const stack in glyphs) {
@@ -89,49 +91,53 @@ class GlyphManager {
             }
         }
 
-        asyncAll(all, ({stack, id}, callback: Callback<{stack: string, id: number, glyph: ?StyleGlyph}>) => {
+        asyncAll(all, ({stack, id}, fnCallback: Callback<{stack: string, id: number, glyph: ?StyleGlyph}>) => {
             let entry = this.entries[stack];
             if (!entry) {
                 entry = this.entries[stack] = {
                     glyphs: {},
                     requests: {},
-                    ranges: {}
+                    ranges: {},
+                    ascender: undefined,
+                    descender: undefined
                 };
             }
 
             let glyph = entry.glyphs[id];
             if (glyph !== undefined) {
-                callback(null, {stack, id, glyph});
+                fnCallback(null, {stack, id, glyph});
                 return;
             }
 
             glyph = this._tinySDF(entry, stack, id);
             if (glyph) {
                 entry.glyphs[id] = glyph;
-                callback(null, {stack, id, glyph});
+                fnCallback(null, {stack, id, glyph});
                 return;
             }
 
             const range = Math.floor(id / 256);
             if (range * 256 > 65535) {
-                callback(new Error('glyphs > 65535 not supported'));
+                fnCallback(new Error('glyphs > 65535 not supported'));
                 return;
             }
 
             if (entry.ranges[range]) {
-                callback(null, {stack, id, glyph});
+                fnCallback(null, {stack, id, glyph});
                 return;
             }
 
             let requests = entry.requests[range];
             if (!requests) {
                 requests = entry.requests[range] = [];
                 GlyphManager.loadGlyphRange(stack, range, (this.url: any), this.requestManager,
-                    (err, response: ?{[_: number]: StyleGlyph | null}) => {
+                    (err, response: ?{glyphs: {[_: number]: StyleGlyph | null}, ascender?: number, descender?: number}) => {
                         if (response) {
-                            for (const id in response) {
+                            entry.ascender = response.ascender;
+                            entry.descender = response.descender;
+                            for (const id in response.glyphs) {
                                 if (!this._doesCharSupportLocalGlyph(+id)) {
-                                    entry.glyphs[+id] = response[+id];
+                                    entry.glyphs[+id] = response.glyphs[+id];
                                 }
                             }
                             entry.ranges[range] = true;
@@ -143,11 +149,11 @@ class GlyphManager {
                     });
             }
 
-            requests.push((err, result: ?{[_: number]: StyleGlyph | null}) => {
+            requests.push((err, result: ?{glyphs: {[_: number]: StyleGlyph | null}, ascender?: number, descender?: number}) => {
                 if (err) {
-                    callback(err);
+                    fnCallback(err);
                 } else if (result) {
-                    callback(null, {stack, id, glyph: result[id] || null});
+                    fnCallback(null, {stack, id, glyph: result.glyphs[id] || null});
                 }
             });
         }, (err, glyphs: ?Array<{stack: string, id: number, glyph: ?StyleGlyph}>) => {
@@ -158,11 +164,15 @@ class GlyphManager {
 
                 for (const {stack, id, glyph} of glyphs) {
                     // Clone the glyph so that our own copy of its ArrayBuffer doesn't get transferred.
-                    (result[stack] || (result[stack] = {}))[id] = glyph && {
+                    if (result[stack] === undefined) result[stack] = {};
+                    if (result[stack].glyphs === undefined) result[stack].glyphs = {};
+                    result[stack].glyphs[id] = glyph && {
                         id: glyph.id,
                         bitmap: glyph.bitmap.clone(),
                         metrics: glyph.metrics
                     };
+                    result[stack].ascender = this.entries[stack].ascender;
+                    result[stack].descender = this.entries[stack].descender;
                 }
 
                 callback(null, result);
@@ -181,7 +191,9 @@ class GlyphManager {
             (isChar['CJK Unified Ideographs'](id) ||
                 isChar['Hangul Syllables'](id) ||
                 isChar['Hiragana'](id) ||
-                isChar['Katakana'](id));
+                isChar['Katakana'](id)) ||
+                // gl-native parity: Extend Ideographs rasterization range to include CJK symbols and punctuations
+                isChar['CJK Symbols and Punctuation'](id);
             /* eslint-enable new-cap */
         }
     }

src/source/worker_source.js

@@ -59,7 +59,7 @@ export type WorkerTileResult = {
     rawTileData?: ArrayBuffer,
     resourceTiming?: Array<PerformanceResourceTiming>,
     // Only used for benchmarking:
-    glyphMap?: {[_: string]: {[_: number]: ?StyleGlyph}} | null,
+    glyphMap?: {[_: string]: {glyphs: {[_: number]: ?StyleGlyph}, ascender?: number, descender?: number}} | null,
     iconMap?: {[_: string]: StyleImage} | null,
     glyphPositions?: GlyphPositions | null
 };

src/source/worker_tile.js

@@ -163,7 +163,7 @@ class WorkerTile {
         lineAtlas.trim();
 
         let error: ?Error;
-        let glyphMap: ?{[_: string]: {[_: number]: ?StyleGlyph}};
+        let glyphMap: ?{[_: string]: {glyphs: {[_: number]: ?StyleGlyph}, ascender?: number, descender?: number}};
         let iconMap: ?{[_: string]: StyleImage};
         let patternMap: ?{[_: string]: StyleImage};
         const taskMetadata = {type: 'maybePrepare', isSymbolTile: this.isSymbolTile, zoom: this.zoom};

src/style/load_glyph_range.js

@@ -12,7 +12,7 @@ export default function (fontstack: string,
                            range: number,
                            urlTemplate: string,
                            requestManager: RequestManager,
-                           callback: Callback<{[_: number]: StyleGlyph | null}>) {
+                           callback: Callback<{glyphs: {[number]: StyleGlyph | null}, ascender?: number, descender?: number}>) {
     const begin = range * 256;
     const end = begin + 255;
 
@@ -27,12 +27,11 @@ export default function (fontstack: string,
             callback(err);
         } else if (data) {
             const glyphs = {};
-
-            for (const glyph of parseGlyphPBF(data)) {
+            const glyphData = parseGlyphPBF(data);
+            for (const glyph of glyphData.glyphs) {
                 glyphs[glyph.id] = glyph;
             }
-
-            callback(null, glyphs);
+            callback(null, {glyphs, ascender: glyphData.ascender, descender: glyphData.descender});
         }
     });
 }

src/style/parse_glyph_pbf.js

@@ -7,23 +7,28 @@ const border = 3;
 
 import type {StyleGlyph} from './style_glyph.js';
 
-function readFontstacks(tag: number, glyphs: Array<StyleGlyph>, pbf: Protobuf) {
+function readFontstacks(tag: number, glyphData: {glyphs: Array<StyleGlyph>, ascender?: number, descender?: number}, pbf: Protobuf) {
+    glyphData.glyphs = [];
     if (tag === 1) {
-        pbf.readMessage(readFontstack, glyphs);
+        pbf.readMessage(readFontstack, glyphData);
     }
 }
 
-function readFontstack(tag: number, glyphs: Array<StyleGlyph>, pbf: Protobuf) {
+function readFontstack(tag: number,  glyphData: {glyphs: Array<StyleGlyph>, ascender?: number, descender?: number}, pbf: Protobuf) {
     if (tag === 3) {
         const {id, bitmap, width, height, left, top, advance} = pbf.readMessage(readGlyph, {});
-        glyphs.push({
+        glyphData.glyphs.push({
             id,
             bitmap: new AlphaImage({
                 width: width + 2 * border,
                 height: height + 2 * border
             }, bitmap),
             metrics: {width, height, left, top, advance}
         });
+    } else if (tag === 4) {
+        glyphData.ascender = pbf.readSVarint();
+    } else if (tag === 5) {
+        glyphData.descender = pbf.readSVarint();
     }
 }
 
@@ -37,8 +42,8 @@ function readGlyph(tag: number, glyph: Object, pbf: Protobuf) {
     else if (tag === 7) glyph.advance = pbf.readVarint();
 }
 
-export default function (data: ArrayBuffer | Uint8Array): Array<StyleGlyph> {
-    return new Protobuf(data).readFields(readFontstacks, []);
+export default function (data: ArrayBuffer | Uint8Array): {glyphs: Array<StyleGlyph>, ascender?: number, descender?: number} {
+    return new Protobuf(data).readFields(readFontstacks, {});
 }
 
 export const GLYPH_PBF_BORDER = border;

src/style/style.js

@@ -1723,7 +1723,7 @@ class Style extends Evented {
         setDependencies(this._symbolSourceCaches[params.source]);
     }
 
-    getGlyphs(mapId: string, params: {stacks: {[_: string]: Array<number>}}, callback: Callback<{[_: string]: {[_: number]: ?StyleGlyph}}>) {
+    getGlyphs(mapId: string, params: {stacks: {[_: string]: Array<number>}}, callback: Callback<{[_: string]: {glyphs: {[_: number]: ?StyleGlyph}, ascender?: number, descender?: number}}>) {
         this.glyphManager.getGlyphs(params.stacks, callback);
     }