Skip to content

Commit

Permalink
Fall back to local glyph rendering if glyph PBF is unavailable
Browse files Browse the repository at this point in the history
  • Loading branch information
1ec5 committed Aug 15, 2024
1 parent ae25dd5 commit a56d0f8
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 64 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
- Fix 3D map freezing when camera is adjusted against map bounds. ([#4537](https://github.com/maplibre/maplibre-gl-js/issues/4537))
- Fix `getStyle()` to return a clone so the object cannot be internally changed ([#4488](https://github.com/maplibre/maplibre-gl-js/issues/4488))
- Prefer local glyph rendering for all CJKV characters, not just those in the CJK Unified Ideographs, Hiragana, Katakana, and Hangul Syllables blocks. ([#4560](https://github.com/maplibre/maplibre-gl-js/pull/4560)))
- Fix crash on missing glyph PBF.
- If a glyph PBF is unavailable or lacks a glyph for a character in a `text-field`, try to render it locally instead of crashing. ([#4564](https://github.com/maplibre/maplibre-gl-js/pull/4564))
- - _...Add new stuff here..._

## 4.5.2
Expand Down
7 changes: 7 additions & 0 deletions src/render/glyph_manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ describe('GlyphManager', () => {
expect(manager._doesCharSupportLocalGlyph(0xC544)).toBe(true);
});

test('GlyphManager generates missing PBF locally', async () => {
const manager = createGlyphManager('sans-serif');

const returnedGlyphs = await manager.getGlyphs({'Arial Unicode MS': [0x10e1]});
expect(returnedGlyphs['Arial Unicode MS'][0x10e1].metrics.advance).toBe(12);
});

test('GlyphManager caches locally generated glyphs', async () => {

const manager = createGlyphManager('sans-serif');
Expand Down
24 changes: 20 additions & 4 deletions src/render/glyph_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {loadGlyphRange} from '../style/load_glyph_range';

import TinySDF from '@mapbox/tiny-sdf';
import {AlphaImage} from '../util/image';
import {warnOnce} from '../util/util';

import type {StyleGlyph} from '../style/style_glyph';
import type {RequestManager} from '../util/request_manager';
Expand Down Expand Up @@ -84,7 +85,7 @@ export class GlyphManager {
return {stack, id, glyph};
}

glyph = this._tinySDF(entry, stack, id);
glyph = this._tinySDF(entry, stack, id, false);
if (glyph) {
entry.glyphs[id] = glyph;
return {stack, id, glyph};
Expand All @@ -108,7 +109,22 @@ export class GlyphManager {
entry.requests[range] = promise;
}

const response = await entry.requests[range];
let response;
try {
response = await entry.requests[range];
} catch (e) {
glyph = this._tinySDF(entry, stack, id, true);
const begin = range * 256;
const end = begin + 255;
const codePoint = id.toString(16).toUpperCase();
if (glyph) {
warnOnce(`Unable to load glyph range ${range}, ${begin}-${end}. Rendering codepoint U+${codePoint} locally instead. ${e}`);
entry.glyphs[id] = glyph;
return {stack, id, glyph};
} else {
warnOnce(`Unable to load glyph range ${range}, ${begin}-${end}, or render codepoint U+${codePoint} locally. ${e}`);
}

Check warning on line 126 in src/render/glyph_manager.ts

View check run for this annotation

Codecov / codecov/patch

src/render/glyph_manager.ts#L125-L126

Added lines #L125 - L126 were not covered by tests
}
for (const id in response) {
if (!this._doesCharSupportLocalGlyph(+id)) {
entry.glyphs[+id] = response[+id];
Expand All @@ -129,13 +145,13 @@ export class GlyphManager {
/\p{Ideo}|\p{sc=Hang}|\p{sc=Hira}|\p{sc=Kana}/u.test(String.fromCodePoint(id));
}

_tinySDF(entry: Entry, stack: string, id: number): StyleGlyph {
_tinySDF(entry: Entry, stack: string, id: number, force: boolean): StyleGlyph {
const fontFamily = this.localIdeographFontFamily;
if (!fontFamily) {
return;
}

if (!this._doesCharSupportLocalGlyph(id)) {
if (!force && !this._doesCharSupportLocalGlyph(id)) {
return;
}

Expand Down
73 changes: 24 additions & 49 deletions src/style/load_glyph_range.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {loadGlyphRange} from './load_glyph_range';
import {fakeServer} from 'nise';
import {bufferToArrayBuffer} from '../util/test/util';

describe('loadGlyphRange', () => {
test('loadGlyphRange', async () => {
global.fetch = null;

const transform = jest.fn().mockImplementation((url) => {
Expand All @@ -14,52 +14,27 @@ describe('loadGlyphRange', () => {

const manager = new RequestManager(transform);

afterEach(() => {
jest.clearAllMocks();
});

test('requests and receives a glyph range', async () => {
const server = fakeServer.create();
server.respondWith(bufferToArrayBuffer(fs.readFileSync(path.join(__dirname, '../../test/unit/assets/0-255.pbf'))));

const promise = loadGlyphRange('Arial Unicode MS', 0, 'https://localhost/fonts/v1/{fontstack}/{range}.pbf', manager);
server.respond();
const result = await promise;

expect(transform).toHaveBeenCalledTimes(1);
expect(transform).toHaveBeenCalledWith('https://localhost/fonts/v1/Arial Unicode MS/0-255.pbf', 'Glyphs');

expect(Object.keys(result)).toHaveLength(223);
for (const key in result) {
const id = Number(key);
const glyph = result[id];

expect(glyph.id).toBe(Number(id));
expect(glyph.metrics).toBeTruthy();
expect(typeof glyph.metrics.width).toBe('number');
expect(typeof glyph.metrics.height).toBe('number');
expect(typeof glyph.metrics.top).toBe('number');
expect(typeof glyph.metrics.advance).toBe('number');
}
expect(server.requests[0].url).toBe('https://localhost/fonts/v1/Arial Unicode MS/0-255.pbf');
});

test('warns on missing glyph range', async () => {
jest.spyOn(console, 'warn').mockImplementation(() => { });

const server = fakeServer.create();

const promise = loadGlyphRange('Arial Unicode MS', 2, 'https://localhost/fonts/v1/{fontstack}/{range}.pbf', manager);
server.respond();
expect(async () => {
const result = await promise;
expect(console.warn).toHaveBeenCalledTimes(1);
expect(Object.keys(result)).toHaveLength(0);
}).not.toThrow();

expect(transform).toHaveBeenCalledTimes(1);
expect(transform).toHaveBeenCalledWith('https://localhost/fonts/v1/Arial Unicode MS/512-767.pbf', 'Glyphs');

expect(server.requests[0].url).toBe('https://localhost/fonts/v1/Arial Unicode MS/512-767.pbf');
});
const server = fakeServer.create();
server.respondWith(bufferToArrayBuffer(fs.readFileSync(path.join(__dirname, '../../test/unit/assets/0-255.pbf'))));

const promise = loadGlyphRange('Arial Unicode MS', 0, 'https://localhost/fonts/v1/{fontstack}/{range}.pbf', manager);
server.respond();
const result = await promise;

expect(transform).toHaveBeenCalledTimes(1);
expect(transform).toHaveBeenCalledWith('https://localhost/fonts/v1/Arial Unicode MS/0-255.pbf', 'Glyphs');

expect(Object.keys(result)).toHaveLength(223);
for (const key in result) {
const id = Number(key);
const glyph = result[id];

expect(glyph.id).toBe(Number(id));
expect(glyph.metrics).toBeTruthy();
expect(typeof glyph.metrics.width).toBe('number');
expect(typeof glyph.metrics.height).toBe('number');
expect(typeof glyph.metrics.top).toBe('number');
expect(typeof glyph.metrics.advance).toBe('number');
}
expect(server.requests[0].url).toBe('https://localhost/fonts/v1/Arial Unicode MS/0-255.pbf');
});
16 changes: 6 additions & 10 deletions src/style/load_glyph_range.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,14 @@ export async function loadGlyphRange(fontstack: string,
ResourceType.Glyphs
);

const response = await getArrayBuffer(request, new AbortController());
if (!response || !response.data) {
throw new Error(`Could not load glyph range. range: ${range}, ${begin}-${end}`);
}
const glyphs = {};
try {
const response = await getArrayBuffer(request, new AbortController());
if (!response || !response.data) {
throw new Error(`Could not load glyph range. range: ${range}, ${begin}-${end}`);
}

for (const glyph of parseGlyphPbf(response.data)) {
glyphs[glyph.id] = glyph;
}
} catch (e) {
console.warn(`Could not load glyph range. range: ${range}, ${begin}-${end}`, e);
for (const glyph of parseGlyphPbf(response.data)) {
glyphs[glyph.id] = glyph;
}

return glyphs;
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit a56d0f8

Please sign in to comment.