Don't cache server requested glyphs in the local glyph ranges (#8657)

* Use correct value for localIdeographFontFamily in Map#setStyle * Ignore local glyph ranges when caching server requested glyphs * Cache locally generated glyphs in GlyphManager
mapbox · Aug 20, 2019 · 97ba78b · 97ba78b
1 parent 8f0998d
commit 97ba78b
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 8 deletions.
diff --git a/src/render/glyph_manager.js b/src/render/glyph_manager.js
@@ -64,6 +64,7 @@ class GlyphManager {
 
             glyph = this._tinySDF(entry, stack, id);
             if (glyph) {
+                entry.glyphs[id] = glyph;
                 callback(null, {stack, id, glyph});
                 return;
             }
@@ -81,7 +82,9 @@ class GlyphManager {
                     (err, response: ?{[number]: StyleGlyph | null}) => {
                         if (response) {
                             for (const id in response) {
-                                entry.glyphs[+id] = response[+id];
+                                if (!this._doesCharSupportLocalGlyph(+id)) {
+                                    entry.glyphs[+id] = response[+id];
+                                }
                             }
                         }
                         for (const cb of requests) {
@@ -118,17 +121,23 @@ class GlyphManager {
         });
     }
 
+    _doesCharSupportLocalGlyph(id: number): boolean {
+        /* eslint-disable new-cap */
+        return !!this.localIdeographFontFamily &&
+            (isChar['CJK Unified Ideographs'](id) ||
+                isChar['Hangul Syllables'](id) ||
+                isChar['Hiragana'](id) ||
+                isChar['Katakana'](id));
+        /* eslint-enable new-cap */
+    }
+
     _tinySDF(entry: Entry, stack: string, id: number): ?StyleGlyph {
         const family = this.localIdeographFontFamily;
         if (!family) {
             return;
         }
-        /* eslint-disable new-cap */
-        if (!isChar['CJK Unified Ideographs'](id) &&
-            !isChar['Hangul Syllables'](id) &&
-            !isChar['Hiragana'](id) &&
-            !isChar['Katakana'](id)
-        ) { /* eslint-enable new-cap */
+
+        if (!this._doesCharSupportLocalGlyph(id)) {
             return;
         }
 

diff --git a/src/ui/map.js b/src/ui/map.js
@@ -965,7 +965,7 @@ class Map extends Camera {
      * @see [Change a map's style](https://www.mapbox.com/mapbox-gl-js/example/setstyle/)
      */
     setStyle(style: StyleSpecification | string | null, options?: {diff?: boolean} & StyleOptions) {
-        options = extend({}, { localIdeographFontFamily: defaultOptions.localIdeographFontFamily}, options);
+        options = extend({}, { localIdeographFontFamily: this._localIdeographFontFamily}, options);
 
         if ((options.diff !== false && options.localIdeographFontFamily === this._localIdeographFontFamily) && this.style && style) {
             this._diffStyle(style, options);

diff --git a/test/unit/render/glyph_manager.test.js b/test/unit/render/glyph_manager.test.js
@@ -43,6 +43,41 @@ test('GlyphManager requests remote CJK PBF', (t) => {
     });
 });
 
+test('GlyphManager does not cache CJK chars that should be rendered locally', (t) => {
+    t.stub(GlyphManager, 'loadGlyphRange').callsFake((stack, range, urlTemplate, transform, callback) => {
+        const overlappingGlyphs = {};
+        const start = range * 256;
+        const end = start + 256;
+        for (let i = start, j = 0; i < end; i++, j++) {
+            overlappingGlyphs[i] = glyphs[j];
+        }
+        setImmediate(() => callback(null, overlappingGlyphs));
+    });
+    t.stub(GlyphManager, 'TinySDF').value(class {
+        // Return empty 30x30 bitmap (24 fontsize + 3 * 2 buffer)
+        draw() {
+            return new Uint8ClampedArray(900);
+        }
+    });
+    const manager = new GlyphManager((url) => ({url}), 'sans-serif');
+    manager.setURL('https://localhost/fonts/v1/{fontstack}/{range}.pbf');
+
+    //Request char that overlaps Katakana range
+    manager.getGlyphs({'Arial Unicode MS': [0x3005]}, (err, glyphs) => {
+        t.ifError(err);
+        t.notEqual(glyphs['Arial Unicode MS'][0x3005], null);
+        //Request char from Katakana range (te)
+        manager.getGlyphs({'Arial Unicode MS': [0x30C6]}, (err, glyphs) => {
+            t.ifError(err);
+            const glyph = glyphs['Arial Unicode MS'][0x30c6];
+            //Ensure that te is locally generated.
+            t.equal(glyph.bitmap.height, 30);
+            t.equal(glyph.bitmap.width, 30);
+            t.end();
+        });
+    });
+});
+
 test('GlyphManager generates CJK PBF locally', (t) => {
     t.stub(GlyphManager, 'TinySDF').value(class {
         // Return empty 30x30 bitmap (24 fontsize + 3 * 2 buffer)
@@ -98,3 +133,28 @@ test('GlyphManager generates Hiragana PBF locally', (t) => {
         t.end();
     });
 });
+
+test('GlyphManager caches locally generated glyphs', (t) => {
+    let drawCallCount = 0;
+    t.stub(GlyphManager, 'TinySDF').value(class {
+        // Return empty 30x30 bitmap (24 fontsize + 3 * 2 buffer)
+        draw() {
+            drawCallCount++;
+            return new Uint8ClampedArray(900);
+        }
+    });
+
+    const manager = new GlyphManager((url) => ({url}), 'sans-serif');
+    manager.setURL('https://localhost/fonts/v1/{fontstack}/{range}.pbf');
+
+    // Katakana letter te
+    manager.getGlyphs({'Arial Unicode MS': [0x30c6]}, (err, glyphs) => {
+        t.ifError(err);
+        t.equal(glyphs['Arial Unicode MS'][0x30c6].metrics.advance, 24);
+        manager.getGlyphs({'Arial Unicode MS': [0x30c6]}, () => {
+            t.equal(drawCallCount, 1);
+            t.end();
+        });
+    });
+});
+