From af3fcca88d172fa43ef22d4840537587d85459ea Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 10 Feb 2019 14:01:49 +0100 Subject: [PATCH 1/3] Convert `BaseFontLoader.bind` to be async, and only utilize `BaseFontLoader._queueLoadingCallback` when actually necessary Currently all fonts are using the `_queueLoadingCallback` method to determine when they have been loaded[1]. However in most cases this is just adding unnecessary overhead, especially with `BaseFontLoader.bind` now being asynchronous, given how fonts are loaded: - For fonts loaded using the Font Loading API, it's already possible to easily tell when a font has been loaded simply by checking the `loaded` promise on the FontFace object itself. - For browsers, e.g. Firefox, which support synchronous font loading it's already assumed that fonts are immediately available. Hence the `_queueLoadingCallback` method is moved into the `GenericFontLoader`, such that it's only utilized for fonts which are loaded using CSS. --- [1] In the "fonts loaded using CSS" case, this is already a hack anyway as outlined in the comments. --- src/display/api.js | 7 ++-- src/display/font_loader.js | 65 ++++++++++++++++++++------------------ 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 811a2fe61..f6a4a5cfa 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -1944,11 +1944,10 @@ class WorkerTransport { onUnsupportedFeature: this._onUnsupportedFeature.bind(this), fontRegistry, }); - const fontReady = (fontObjs) => { - this.commonObjs.resolve(id, font); - }; - this.fontLoader.bind([font], fontReady); + this.fontLoader.bind([font]).then(() => { + this.commonObjs.resolve(id, font); + }); break; case 'FontPath': this.commonObjs.resolve(id, exportedData); diff --git a/src/display/font_loader.js b/src/display/font_loader.js index d35ba6adf..f0db58ed4 100644 --- a/src/display/font_loader.js +++ b/src/display/font_loader.js @@ -27,10 +27,6 @@ class BaseFontLoader { this.nativeFontFaces = []; this.styleElement = null; - this.loadingContext = { - requests: [], - nextRequestId: 0, - }; } addNativeFontFace(nativeFontFace) { @@ -64,7 +60,7 @@ class BaseFontLoader { } } - bind(fonts, callback) { + async bind(fonts) { const rules = []; const fontsToLoad = []; const fontLoadPromises = []; @@ -99,37 +95,19 @@ class BaseFontLoader { } } - const request = this._queueLoadingCallback(callback); if (this.isFontLoadingAPISupported) { - Promise.all(fontLoadPromises).then(request.complete); + return Promise.all(fontLoadPromises); } else if (rules.length > 0 && !this.isSyncFontLoadingSupported) { - this._prepareFontLoadEvent(rules, fontsToLoad, request); - } else { - request.complete(); + return new Promise((resolve) => { + const request = this._queueLoadingCallback(resolve); + this._prepareFontLoadEvent(rules, fontsToLoad, request); + }); } + return; // Synchronous font loading supported. } _queueLoadingCallback(callback) { - function completeRequest() { - assert(!request.done, 'completeRequest() cannot be called twice.'); - request.done = true; - - // Sending all completed requests in order of how they were queued. - while (context.requests.length > 0 && context.requests[0].done) { - const otherRequest = context.requests.shift(); - setTimeout(otherRequest.callback, 0); - } - } - - const context = this.loadingContext; - const request = { - id: `pdfjs-font-loading-${context.nextRequestId++}`, - done: false, - complete: completeRequest, - callback, - }; - context.requests.push(request); - return request; + unreachable('Abstract method `_queueLoadingCallback`.'); } get isFontLoadingAPISupported() { @@ -168,6 +146,10 @@ FontLoader = class MozcentralFontLoader extends BaseFontLoader { FontLoader = class GenericFontLoader extends BaseFontLoader { constructor(docId) { super(docId); + this.loadingContext = { + requests: [], + nextRequestId: 0, + }; this.loadTestFontId = 0; } @@ -205,6 +187,29 @@ FontLoader = class GenericFontLoader extends BaseFontLoader { return shadow(this, 'isSyncFontLoadingSupported', supported); } + _queueLoadingCallback(callback) { + function completeRequest() { + assert(!request.done, 'completeRequest() cannot be called twice.'); + request.done = true; + + // Sending all completed requests in order of how they were queued. + while (context.requests.length > 0 && context.requests[0].done) { + const otherRequest = context.requests.shift(); + setTimeout(otherRequest.callback, 0); + } + } + + const context = this.loadingContext; + const request = { + id: `pdfjs-font-loading-${context.nextRequestId++}`, + done: false, + complete: completeRequest, + callback, + }; + context.requests.push(request); + return request; + } + get _loadTestFont() { const getLoadTestFont = function() { // This is a CFF font with 1 glyph for '.' that fills its entire width and From 13230a1123c3c540527cef46c3727bd0d63f6294 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 10 Feb 2019 14:11:23 +0100 Subject: [PATCH 2/3] Remove the ability to pass in more than one font to `BaseFontLoader.bind` - The only existing call-site, of this method, is never passing more than *one* font at a time anyway. - As far as I can remember, this functionality has never actually been used (caveat: I didn't check the git history). - This allows simplification of the method, especially by making use of the fact that it's now asynchronous. - It should be just as easy to call `BaseFontLoader.bind` from within a loop, rather than having the loop in the method itself. --- src/display/api.js | 2 +- src/display/font_loader.js | 65 ++++++++++++++++---------------------- 2 files changed, 29 insertions(+), 38 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index f6a4a5cfa..8c2371fff 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -1945,7 +1945,7 @@ class WorkerTransport { fontRegistry, }); - this.fontLoader.bind([font]).then(() => { + this.fontLoader.bind(font).then(() => { this.commonObjs.resolve(id, font); }); break; diff --git a/src/display/font_loader.js b/src/display/font_loader.js index f0db58ed4..19e592ad3 100644 --- a/src/display/font_loader.js +++ b/src/display/font_loader.js @@ -60,50 +60,41 @@ class BaseFontLoader { } } - async bind(fonts) { - const rules = []; - const fontsToLoad = []; - const fontLoadPromises = []; - const getNativeFontPromise = function(nativeFontFace) { - // Return a promise that is always fulfilled, even when the font fails to - // load. - return nativeFontFace.loaded.catch(function(reason) { - warn(`Failed to load font "${nativeFontFace.family}": ${reason}`); - }); - }; - - for (const font of fonts) { - // Add the font to the DOM only once; skip if the font is already loaded. - if (font.attached || font.missingFile) { - continue; - } - font.attached = true; - - if (this.isFontLoadingAPISupported) { - const nativeFontFace = font.createNativeFontFace(); - if (nativeFontFace) { - this.addNativeFontFace(nativeFontFace); - fontLoadPromises.push(getNativeFontPromise(nativeFontFace)); - } - } else { - const rule = font.createFontFaceRule(); - if (rule) { - this.insertRule(rule); - rules.push(rule); - fontsToLoad.push(font); - } - } + async bind(font) { + // Add the font to the DOM only once; skip if the font is already loaded. + if (font.attached || font.missingFile) { + return; } + font.attached = true; if (this.isFontLoadingAPISupported) { - return Promise.all(fontLoadPromises); - } else if (rules.length > 0 && !this.isSyncFontLoadingSupported) { + const nativeFontFace = font.createNativeFontFace(); + if (nativeFontFace) { + this.addNativeFontFace(nativeFontFace); + try { + await nativeFontFace.loaded; + } catch (ex) { + // Return a promise that is always fulfilled, even when the font + // failed to load. + warn(`Failed to load font '${nativeFontFace.family}': '${ex}'.`); + } + } + return; // The font was, asynchronously, loaded. + } + + // !this.isFontLoadingAPISupported + const rule = font.createFontFaceRule(); + if (rule) { + this.insertRule(rule); + + if (this.isSyncFontLoadingSupported) { + return; // The font was, synchronously, loaded. + } return new Promise((resolve) => { const request = this._queueLoadingCallback(resolve); - this._prepareFontLoadEvent(rules, fontsToLoad, request); + this._prepareFontLoadEvent([rule], [font], request); }); } - return; // Synchronous font loading supported. } _queueLoadingCallback(callback) { From b6d090cc147b21a569baeec0b8bedb11e97b4602 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 11 Feb 2019 00:47:56 +0100 Subject: [PATCH 3/3] Fallback to the built-in font renderer when font loading fails After PR 9340 all glyphs are now re-mapped to a Private Use Area (PUA) which means that if a font fails to load, for whatever reason[1], all glyphs in the font will now render as Unicode glyph outlines. This obviously doesn't look good, to say the least, and might be seen as a "regression" since previously many glyphs were left in their original positions which provided a slightly better fallback[2]. Hence this patch, which implements a *general* fallback to the PDF.js built-in font renderer for fonts that fail to load (i.e. are rejected by the sanitizer). One caveat here is that this only works for the Font Loading API, since it's easy to handle errors in that case[3]. The solution implemented in this patch does *not* in any way delay the loading of valid fonts, which was the problem with my previous attempt at a solution, and will only require a bit of extra work/waiting for those fonts that actually fail to load. *Please note:* This patch doesn't fix any of the underlying PDF.js font conversion bugs that's responsible for creating corrupt font files, however it does *improve* rendering in a number of cases; refer to this possibly incomplete list: [Bug 1524888](https://bugzilla.mozilla.org/show_bug.cgi?id=1524888) Issue 10175 Issue 10232 --- [1] Usually because the PDF.js font conversion code wasn't able to parse the font file correctly. [2] Glyphs fell back to some default font, which while not accurate was more useful than the current state. [3] Furthermore I'm not sure how to implement this generally, assuming that's even possible, and don't really have time/interest to look into it either. --- src/core/document.js | 4 ++ src/core/evaluator.js | 84 +++++++++++++++++++++++-------------- src/core/fonts.js | 6 +++ src/core/obj.js | 16 +++++++ src/core/pdf_manager.js | 4 ++ src/core/worker.js | 4 ++ src/display/api.js | 11 ++++- src/display/font_loader.js | 10 +++-- src/shared/compatibility.js | 12 ++---- 9 files changed, 107 insertions(+), 44 deletions(-) diff --git a/src/core/document.js b/src/core/document.js index 115c8011c..3e0a20299 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -666,6 +666,10 @@ class PDFDocument { }); } + fontFallback(id, handler) { + return this.catalog.fontFallback(id, handler); + } + cleanup() { return this.catalog.cleanup(); } diff --git a/src/core/evaluator.js b/src/core/evaluator.js index df87c6ba6..cd2841155 100644 --- a/src/core/evaluator.js +++ b/src/core/evaluator.js @@ -610,37 +610,18 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { }); }, - handleText: function PartialEvaluator_handleText(chars, state) { - var font = state.font; - var glyphs = font.charsToGlyphs(chars); - var isAddToPathSet = !!(state.textRenderingMode & - TextRenderingMode.ADD_TO_PATH_FLAG); - if (font.data && (isAddToPathSet || this.options.disableFontFace || - state.fillColorSpace.name === 'Pattern')) { - var buildPath = (fontChar) => { - if (!font.renderer.hasBuiltPath(fontChar)) { - var path = font.renderer.getPathJs(fontChar); - this.handler.send('commonobj', [ - font.loadedName + '_path_' + fontChar, - 'FontPath', - path - ]); - } - }; + handleText(chars, state) { + const font = state.font; + const glyphs = font.charsToGlyphs(chars); - for (var i = 0, ii = glyphs.length; i < ii; i++) { - var glyph = glyphs[i]; - buildPath(glyph.fontChar); - - // If the glyph has an accent we need to build a path for its - // fontChar too, otherwise CanvasGraphics_paintChar will fail. - var accent = glyph.accent; - if (accent && accent.fontChar) { - buildPath(accent.fontChar); - } + if (font.data) { + const isAddToPathSet = !!(state.textRenderingMode & + TextRenderingMode.ADD_TO_PATH_FLAG); + if (isAddToPathSet || state.fillColorSpace.name === 'Pattern' || + font.disableFontFace || this.options.disableFontFace) { + PartialEvaluator.buildFontPaths(font, glyphs, this.handler); } } - return glyphs; }, @@ -2623,6 +2604,30 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { }, }; + PartialEvaluator.buildFontPaths = function(font, glyphs, handler) { + function buildPath(fontChar) { + if (font.renderer.hasBuiltPath(fontChar)) { + return; + } + handler.send('commonobj', [ + `${font.loadedName}_path_${fontChar}`, + 'FontPath', + font.renderer.getPathJs(fontChar), + ]); + } + + for (const glyph of glyphs) { + buildPath(glyph.fontChar); + + // If the glyph has an accent we need to build a path for its + // fontChar too, otherwise CanvasGraphics_paintChar will fail. + const accent = glyph.accent; + if (accent && accent.fontChar) { + buildPath(accent.fontChar); + } + } + }; + return PartialEvaluator; })(); @@ -2639,14 +2644,31 @@ var TranslatedFont = (function TranslatedFontClosure() { if (this.sent) { return; } - var fontData = this.font.exportData(); + this.sent = true; + handler.send('commonobj', [ this.loadedName, 'Font', - fontData + this.font.exportData(), ]); - this.sent = true; }, + + fallback(handler) { + if (!this.font.data) { + return; + } + // When font loading failed, fall back to the built-in font renderer. + this.font.disableFontFace = true; + // An arbitrary number of text rendering operators could have been + // encountered between the point in time when the 'Font' message was sent + // to the main-thread, and the point in time when the 'FontFallback' + // message was received on the worker-thread. + // To ensure that all 'FontPath's are available on the main-thread, when + // font loading failed, attempt to resend *all* previously parsed glyphs. + const glyphs = this.font.glyphCacheValues; + PartialEvaluator.buildFontPaths(this.font, glyphs, handler); + }, + loadType3Data(evaluator, resources, parentOperatorList, task) { if (!this.font.isType3Font) { throw new Error('Must be a Type3 font.'); diff --git a/src/core/fonts.js b/src/core/fonts.js index 8d618a8c9..a52525127 100644 --- a/src/core/fonts.js +++ b/src/core/fonts.js @@ -1159,6 +1159,8 @@ var Font = (function FontClosure() { font: null, mimetype: null, encoding: null, + disableFontFace: false, + get renderer() { var renderer = FontRendererFactory.create(this, SEAC_ANALYSIS_ENABLED); return shadow(this, 'renderer', renderer); @@ -2944,6 +2946,10 @@ var Font = (function FontClosure() { // Enter the translated string into the cache return (charsCache[charsCacheKey] = glyphs); }, + + get glyphCacheValues() { + return Object.values(this.glyphCache); + }, }; return Font; diff --git a/src/core/obj.js b/src/core/obj.js index b71a503d0..484460f8d 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -490,6 +490,22 @@ class Catalog { return shadow(this, 'javaScript', javaScript); } + fontFallback(id, handler) { + const promises = []; + this.fontCache.forEach(function(promise) { + promises.push(promise); + }); + + return Promise.all(promises).then((translatedFonts) => { + for (const translatedFont of translatedFonts) { + if (translatedFont.loadedName === id) { + translatedFont.fallback(handler); + return; + } + } + }); + } + cleanup() { this.pageKidsCountCache.clear(); diff --git a/src/core/pdf_manager.js b/src/core/pdf_manager.js index cdbbc3a58..abe2b7df1 100644 --- a/src/core/pdf_manager.js +++ b/src/core/pdf_manager.js @@ -68,6 +68,10 @@ class BasePdfManager { return this.pdfDocument.getPage(pageIndex); } + fontFallback(id, handler) { + return this.pdfDocument.fontFallback(id, handler); + } + cleanup() { return this.pdfDocument.cleanup(); } diff --git a/src/core/worker.js b/src/core/worker.js index 910d2692f..46daf0a27 100644 --- a/src/core/worker.js +++ b/src/core/worker.js @@ -667,6 +667,10 @@ var WorkerMessageHandler = { }); }); + handler.on('FontFallback', function(data) { + return pdfManager.fontFallback(data.id, handler); + }); + handler.on('Cleanup', function wphCleanup(data) { return pdfManager.cleanup(); }); diff --git a/src/display/api.js b/src/display/api.js index 8c2371fff..271aaead3 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -1676,7 +1676,10 @@ class WorkerTransport { this.messageHandler = messageHandler; this.loadingTask = loadingTask; this.commonObjs = new PDFObjects(); - this.fontLoader = new FontLoader(loadingTask.docId); + this.fontLoader = new FontLoader({ + docId: loadingTask.docId, + onUnsupportedFeature: this._onUnsupportedFeature.bind(this), + }); this._params = params; this.CMapReaderFactory = new params.CMapReaderFactory({ baseUrl: params.cMapUrl, @@ -1947,6 +1950,12 @@ class WorkerTransport { this.fontLoader.bind(font).then(() => { this.commonObjs.resolve(id, font); + }, (reason) => { + messageHandler.sendWithPromise('FontFallback', { + id, + }).finally(() => { + this.commonObjs.resolve(id, font); + }); }); break; case 'FontPath': diff --git a/src/display/font_loader.js b/src/display/font_loader.js index 19e592ad3..8aa8cda55 100644 --- a/src/display/font_loader.js +++ b/src/display/font_loader.js @@ -19,11 +19,12 @@ import { } from '../shared/util'; class BaseFontLoader { - constructor(docId) { + constructor({ docId, onUnsupportedFeature, }) { if (this.constructor === BaseFontLoader) { unreachable('Cannot initialize BaseFontLoader.'); } this.docId = docId; + this._onUnsupportedFeature = onUnsupportedFeature; this.nativeFontFaces = []; this.styleElement = null; @@ -74,9 +75,12 @@ class BaseFontLoader { try { await nativeFontFace.loaded; } catch (ex) { - // Return a promise that is always fulfilled, even when the font - // failed to load. + this._onUnsupportedFeature({ featureId: UNSUPPORTED_FEATURES.font, }); warn(`Failed to load font '${nativeFontFace.family}': '${ex}'.`); + + // When font loading failed, fall back to the built-in font renderer. + font.disableFontFace = true; + throw ex; } } return; // The font was, asynchronously, loaded. diff --git a/src/shared/compatibility.js b/src/shared/compatibility.js index b858ee3fc..c67de876d 100644 --- a/src/shared/compatibility.js +++ b/src/shared/compatibility.js @@ -23,11 +23,6 @@ if ((typeof PDFJSDev === 'undefined' || globalScope._pdfjsCompatibilityChecked = true; -// In the Chrome extension, most of the polyfills are unnecessary. -// We support down to Chrome 49, because it's still commonly used by Windows XP -// users - https://github.com/mozilla/pdf.js/issues/9397 -if (typeof PDFJSDev === 'undefined' || !PDFJSDev.test('CHROME')) { - const isNodeJS = require('./is_node'); const hasDOM = typeof window === 'object' && typeof document === 'object'; @@ -199,14 +194,15 @@ const hasDOM = typeof window === 'object' && typeof document === 'object'; Number.isInteger = require('core-js/fn/number/is-integer'); })(); -// Support: IE, Safari<8, Chrome<32 +// Support: IE, Safari<11, Chrome<63 (function checkPromise() { if (typeof PDFJSDev !== 'undefined' && PDFJSDev.test('IMAGE_DECODERS')) { // The current image decoders are synchronous, hence `Promise` shouldn't // need to be polyfilled for the IMAGE_DECODERS build target. return; } - if (globalScope.Promise) { + if (globalScope.Promise && (globalScope.Promise.prototype && + globalScope.Promise.prototype.finally)) { return; } globalScope.Promise = require('core-js/fn/promise'); @@ -254,8 +250,6 @@ const hasDOM = typeof window === 'object' && typeof document === 'object'; require('core-js/es6/symbol'); })(); -} // End of !PDFJSDev.test('CHROME') - // Provides support for String.prototype.padStart in legacy browsers. // Support: IE, Chrome<57 (function checkStringPadStart() {