Rename 'cjkGlyphFont' -> 'localIdeographFontFamily'

Add comments
Add GlyphSource unit test

debug/debug.html

@@ -38,7 +38,7 @@
     center: [-77.01866, 38.888],
     style: '/debug/streets-v10-viewport.json',
     hash: true,
-    cjkGlyphFont: '"Noto Sans", "Noto Sans CJK SC", sans-serif'
+    localIdeographFontFamily: '"Noto Sans", "Noto Sans CJK SC", sans-serif'
 });
 
 map.addControl(new mapboxgl.NavigationControl());

debug/tinysdf.html

@@ -64,7 +64,7 @@
     zoom: 8.8,
     center: [121.574, 31.1489],
     style: '/debug/streets-zh-global-v1.json',
-    cjkGlyphFont: '"Noto Sans", "Noto Sans CJK SC", sans-serif',
+    localIdeographFontFamily: '"Noto Sans", "Noto Sans CJK SC", sans-serif',
     hash: true
 });
 

src/style/style.js

@@ -106,7 +106,7 @@ class Style extends Evented {
                 this.sprite = new ImageSprite(stylesheet.sprite, this);
             }
 
-            this.glyphSource = new GlyphSource(stylesheet.glyphs, options.cjkGlyphFont);
+            this.glyphSource = new GlyphSource(stylesheet.glyphs, options.localIdeographFontFamily);
             this._resolve();
             this.fire('data', {dataType: 'style'});
             this.fire('style.load');

src/symbol/glyph_source.js

@@ -31,12 +31,12 @@ class GlyphSource {
     /**
      * @param {string} url glyph template url
      */
-    constructor(url, cjkGlyphFont) {
+    constructor(url, localIdeographFontFamily) {
         this.url = url && normalizeURL(url);
         this.atlases = {};
         this.stacks = {};
         this.loading = {};
-        this.cjkGlyphFont = cjkGlyphFont;
+        this.localIdeographFontFamily = localIdeographFontFamily;
         this.tinySDFs = {};
     }
 
@@ -60,7 +60,7 @@ class GlyphSource {
 
         const getGlyph = (glyphID) => {
             const range = Math.floor(glyphID / 256);
-            if (this.cjkGlyphFont &&
+            if (this.localIdeographFontFamily &&
                 (isChar['CJK Unified Ideographs'](glyphID) ||
                  isChar['Hangul Syllables'](glyphID))) {
                 if (!stack.cjkGlyphs[glyphID]) {
@@ -115,13 +115,13 @@ class GlyphSource {
         }
     }
 
+    createTinySDF(fontFamily, fontWeight) {
+        return  new TinySDF(24, 3, 8, .25, fontFamily, fontWeight);
+    }
+
     loadCJKGlyph(fontstack, glyphID) {
         let tinySDF = this.tinySDFs[fontstack];
         if (!tinySDF) {
-            const fontSize = 24;
-            const buffer = 3;
-            const radius = fontSize / 3;
-            const cutoff = .25;
             let fontWeight = '400';
             if (/bold/i.test(fontstack)) {
                 fontWeight = '900';
@@ -130,7 +130,7 @@ class GlyphSource {
             } else if (/light/i.test(fontstack)) {
                 fontWeight = '200';
             }
-            tinySDF = this.tinySDFs[fontstack] = new TinySDF(fontSize, buffer, radius, cutoff, this.cjkGlyphFont, fontWeight);
+            tinySDF = this.tinySDFs[fontstack] = this.createTinySDF(this.localIdeographFontFamily, fontWeight);
         }
 
         return {
@@ -144,6 +144,10 @@ class GlyphSource {
         };
     }
 
+    loadPBF(url, callback) {
+        ajax.getArrayBuffer(url, callback);
+    }
+
     loadRange(fontstack, range, callback) {
         if (range * 256 > 65535) return callback('glyphs > 65535 not supported');
 
@@ -160,7 +164,7 @@ class GlyphSource {
             const rangeName = `${range * 256}-${range * 256 + 255}`;
             const url = glyphUrl(fontstack, rangeName, this.url);
 
-            ajax.getArrayBuffer(url, (err, response) => {
+            this.loadPBF(url, (err, response) => {
                 const glyphs = !err && new Glyphs(new Protobuf(response.data));
                 for (let i = 0; i < loading[range].length; i++) {
                     loading[range][i](err, range, glyphs);

src/ui/map.js

@@ -191,7 +191,11 @@ const defaultOptions = {
  * @param {number} [options.bearing=0] The initial bearing (rotation) of the map, measured in degrees counter-clockwise from north. If `bearing` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `0`.
  * @param {number} [options.pitch=0] The initial pitch (tilt) of the map, measured in degrees away from the plane of the screen (0-60). If `pitch` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `0`.
  * @param {boolean} [options.renderWorldCopies=true]  If `true`, multiple copies of the world will be rendered, when zoomed out.
-  * @param {number} [options.maxTileCacheSize=null]  The maxiumum number of tiles stored in the tile cache for a given source. If omitted, the cache will be dynamically sized based on the current viewport.
+ * @param {number} [options.maxTileCacheSize=null]  The maxiumum number of tiles stored in the tile cache for a given source. If omitted, the cache will be dynamically sized based on the current viewport.
+ * @param {string} [options.localIdeographFontFamily=null] If specified, defines a css font-family
+ *   for locally overriding generation of glyphs in the 'CJK Unified Ideographs' and 'Hangul Syllables' ranges.
+ *   In these ranges, font settings from the map's style will be mostly ignored, except for font-weight keywords (light/regular/medium/bold).
+ *   The purpose of this option is to avoid bandwidth-intensive glyph server requests.
  * @example
  * var map = new mapboxgl.Map({
  *   container: 'map',
@@ -293,7 +297,7 @@ class Map extends Camera {
         this.resize();
 
         if (options.classes) this.setClasses(options.classes);
-        if (options.style) this.setStyle(options.style, options.cjkGlyphFont ? { cjkGlyphFont: options.cjkGlyphFont } : null);
+        if (options.style) this.setStyle(options.style, { localIdeographFontFamily: options.localIdeographFontFamily });
 
         if (options.attributionControl) this.addControl(new AttributionControl());
         this.addControl(new LogoControl(), options.logoPosition);
@@ -928,11 +932,14 @@ class Map extends Camera {
      * @param {Object} [options]
      * @param {boolean} [options.diff=true] If false, force a 'full' update, removing the current style
      *   and adding building the given one instead of attempting a diff-based update.
+     * @param {string} [options.localIdeographFontFamily=null] If non-null, defines a css font-family
+     *   for locally overriding generation of glyphs in the 'CJK Unified Ideographs' and 'Hangul Syllables'
+     *   ranges. Forces a full update.
      * @returns {Map} `this`
      * @see [Change a map's style](https://www.mapbox.com/mapbox-gl-js/example/setstyle/)
      */
     setStyle(style: any, options?: {diff: boolean}) {
-        const shouldTryDiff = (!options || options.diff !== false) && this.style && style &&
+        const shouldTryDiff = (!options || (options.diff !== false && !options.localIdeographFontFamily)) && this.style && style &&
             !(style instanceof Style) && typeof style !== 'string';
         if (shouldTryDiff) {
             try {

test/unit/symbol/glyph_source.test.js

@@ -0,0 +1,59 @@
+'use strict';
+
+const test = require('mapbox-gl-js-test').test;
+const GlyphSource = require('../../../src/symbol/glyph_source');
+const fs = require('fs');
+
+const mockTinySDF = {
+    // Return empty 30x30 bitmap (24 fontsize + 3 * 2 buffer)
+    draw: function () { return new Uint8ClampedArray(900); }
+};
+
+function createSource(t, localIdeographFontFamily) {
+    const aPBF = fs.readFileSync('./test/fixtures/0-255.pbf');
+    const source = new GlyphSource("https://localhost/fonts/v1{fontstack}/{range}.pbf", localIdeographFontFamily);
+    t.stub(source, 'createTinySDF').returns(mockTinySDF);
+    // It would be better to mock with FakeXMLHttpRequest, but the binary encoding
+    // doesn't survive the mocking
+    source.loadPBF = function(url, callback) {
+        callback(null, { data: aPBF });
+    };
+
+    return source;
+}
+
+
+test('GlyphSource', (t) => {
+    t.test('requests 0-255 PBF', (t) => {
+        const source = createSource(t);
+        source.getSimpleGlyphs("Arial Unicode MS", [55], 0, (err, glyphs, fontName) => {
+            t.notOk(err);
+            t.equal(fontName, "Arial Unicode MS");
+            t.equal(glyphs['55'].advance, 12);
+            t.end();
+        });
+    });
+
+    t.test('requests remote CJK PBF', (t) => {
+        const source = createSource(t);
+        source.getSimpleGlyphs("Arial Unicode MS", [0x5e73], 0, (err, glyphs, fontName) => {
+            t.notOk(err);
+            t.equal(fontName, "Arial Unicode MS");
+            t.notOk(Object.keys(glyphs).length); // The fixture returns a PBF without the glyph we requested
+            t.end();
+        });
+
+    });
+
+    t.test('locally generates CJK PBF', (t) => {
+        const source = createSource(t, 'sans-serif');
+        source.getSimpleGlyphs("Arial Unicode MS", [0x5e73], 0, (err, glyphs, fontName) => {
+            t.notOk(err);
+            t.equal(fontName, "Arial Unicode MS");
+            t.equal(glyphs['24179'].advance, 24);
+            t.end();
+        });
+    });
+
+    t.end();
+});