Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include Hiragana and Katakana code blocks for local glyph generation #8365

Merged
merged 1 commit into from
Jun 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/render/glyph_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,12 @@ class GlyphManager {
if (!family) {
return;
}

if (!isChar['CJK Unified Ideographs'](id) && !isChar['Hangul Syllables'](id)) { // eslint-disable-line new-cap
/* eslint-disable new-cap */
if (!isChar['CJK Unified Ideographs'](id) &&
!isChar['Hangul Syllables'](id) &&
!isChar['Hiragana'](id) &&
!isChar['Katakana'](id)
) { /* eslint-enable new-cap */
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ const defaultOptions = {
* @param {boolean} [options.renderWorldCopies=true] If `true`, multiple copies of the world will be rendered, when zoomed out.
* @param {number} [options.maxTileCacheSize=null] The maximum 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='sans-serif'] Defines a CSS
* font-family for locally overriding generation of glyphs in the 'CJK Unified Ideographs' and 'Hangul Syllables' ranges.
* font-family for locally overriding generation of glyphs in the 'CJK Unified Ideographs', 'Hiragana', 'Katakana' and 'Hangul Syllables' ranges.
* In these ranges, font settings from the map's style will be ignored, except for font-weight keywords (light/regular/medium/bold).
* Set to `false`, to enable font settings from the map's style for these glyph ranges.
* The purpose of this option is to avoid bandwidth-intensive glyph server requests. (see [Use locally generated ideographs](https://www.mapbox.com/mapbox-gl-js/example/local-ideographs))
Expand Down Expand Up @@ -950,7 +950,7 @@ class Map extends Camera {
* @param {boolean} [options.diff=true] If false, force a 'full' update, removing the current style
* and building the given one instead of attempting a diff-based update.
* @param {string} [options.localIdeographFontFamily='sans-serif'] Defines a CSS
* font-family for locally overriding generation of glyphs in the 'CJK Unified Ideographs' and 'Hangul Syllables' ranges.
* font-family for locally overriding generation of glyphs in the 'CJK Unified Ideographs', 'Hiragana', 'Katakana' and 'Hangul Syllables' ranges.
* In these ranges, font settings from the map's style will be ignored, except for font-weight keywords (light/regular/medium/bold).
* Set to `false`, to enable font settings from the map's style for these glyph ranges.
* Forces a full update.
Expand Down
38 changes: 38 additions & 0 deletions test/unit/render/glyph_manager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,41 @@ test('GlyphManager generates CJK PBF locally', (t) => {
t.end();
});
});

test('GlyphManager generates Katakana PBF locally', (t) => {
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');

// Katakana letter te
manager.getGlyphs({'Arial Unicode MS': [0x30c6]}, (err, glyphs) => {
t.ifError(err);
t.equal(glyphs['Arial Unicode MS'][0x30c6].metrics.advance, 24);
t.end();
});
});

test('GlyphManager generates Hiragana PBF locally', (t) => {
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');

//Hiragana letter te
manager.getGlyphs({'Arial Unicode MS': [0x3066]}, (err, glyphs) => {
t.ifError(err);
t.equal(glyphs['Arial Unicode MS'][0x3066].metrics.advance, 24);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I fully understand what the tests do. I get that you're stubbing the draw method of the static TinySDF object. But how does this test ensure that TinySDF.draw was called as opposed to going through the regular pathway? In either case, wouldn't we expect the glyph to be 24px wide?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GlyphManager#loadGlyphRange would be the other code path, and that is not stubbed, so the test will error out if it tries to go down that path. The metrics test is to ensure that the code path called _tinySDF successfully and returned a StyleGlyph successfully.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, that makes sense.

t.end();
});
});