diff --git a/CHANGELOG.md b/CHANGELOG.md index 84fa02adca..c37b1fa21b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/render/glyph_manager.test.ts b/src/render/glyph_manager.test.ts index 05d07f2fed..b2aac6aee8 100644 --- a/src/render/glyph_manager.test.ts +++ b/src/render/glyph_manager.test.ts @@ -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'); diff --git a/src/render/glyph_manager.ts b/src/render/glyph_manager.ts index 0efe9f066f..db0f39e5f7 100644 --- a/src/render/glyph_manager.ts +++ b/src/render/glyph_manager.ts @@ -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'; @@ -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}; @@ -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}`); + } + } for (const id in response) { if (!this._doesCharSupportLocalGlyph(+id)) { entry.glyphs[+id] = response[+id]; @@ -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; } diff --git a/src/style/load_glyph_range.test.ts b/src/style/load_glyph_range.test.ts index e0c1bc6244..f79334a301 100644 --- a/src/style/load_glyph_range.test.ts +++ b/src/style/load_glyph_range.test.ts @@ -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) => { @@ -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'); }); diff --git a/src/style/load_glyph_range.ts b/src/style/load_glyph_range.ts index 32203c3533..ba63c10a4b 100644 --- a/src/style/load_glyph_range.ts +++ b/src/style/load_glyph_range.ts @@ -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; diff --git a/test/integration/render/tests/text-local-glyphs/missing/expected-mac.png b/test/integration/render/tests/text-local-glyphs/missing/expected-mac.png index 42cb9d520d..6a6bbfc88d 100644 Binary files a/test/integration/render/tests/text-local-glyphs/missing/expected-mac.png and b/test/integration/render/tests/text-local-glyphs/missing/expected-mac.png differ diff --git a/test/integration/render/tests/text-local-glyphs/missing/expected-ubuntu.png b/test/integration/render/tests/text-local-glyphs/missing/expected-ubuntu.png index 42cb9d520d..a59e6a550c 100644 Binary files a/test/integration/render/tests/text-local-glyphs/missing/expected-ubuntu.png and b/test/integration/render/tests/text-local-glyphs/missing/expected-ubuntu.png differ diff --git a/test/integration/render/tests/text-local-glyphs/missing/expected-windows.png b/test/integration/render/tests/text-local-glyphs/missing/expected-windows.png index 42cb9d520d..7b82bd12ae 100644 Binary files a/test/integration/render/tests/text-local-glyphs/missing/expected-windows.png and b/test/integration/render/tests/text-local-glyphs/missing/expected-windows.png differ