From 8d86e18a32144180f04a96e209f92b7f628018ee Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 7 May 2024 11:37:07 +0200 Subject: [PATCH] Restore the `MAX_TEXT_DIVS_TO_RENDER` limit in the textLayer This limit is currently completely non-functional, since the check happens *after* the entire textLayer has been parsed and appended to the DOM. It seems that this has been *accidentally* broken ever since the introduction of `ReadableStream` support. The reason that this hasn't caused noticeable textLayer-related performance issues in practice is probably because we nowadays manage to coalesce the textLayer into fewer overall DOM elements, whereas years ago many PDF documents ended up with one DOM element *per* glyph. By moving this check, and thus restoring the functionality, we're also able to remove the `render` helper function and simplify the code. --- src/display/text_layer.js | 43 +++++++++++++++------------------------ 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/src/display/text_layer.js b/src/display/text_layer.js index f302ddb7d..f36bf62a4 100644 --- a/src/display/text_layer.js +++ b/src/display/text_layer.js @@ -16,7 +16,7 @@ /** @typedef {import("./display_utils").PageViewport} PageViewport */ /** @typedef {import("./api").TextContent} TextContent */ -import { AbortException, Util } from "../shared/util.js"; +import { AbortException, Util, warn } from "../shared/util.js"; import { setLayerDimensions } from "./display_utils.js"; /** @@ -282,23 +282,6 @@ function layout(params) { } } -function render(task) { - if (task._canceled) { - return; - } - const textDivs = task._textDivs; - const capability = task._capability; - const textDivsLength = textDivs.length; - - // No point in rendering many divs as it would make the browser - // unusable even after the divs are rendered. - if (textDivsLength > MAX_TEXT_DIVS_TO_RENDER) { - capability.resolve(); - return; - } - capability.resolve(); -} - class TextLayerRenderTask { #reader = null; @@ -389,7 +372,19 @@ class TextLayerRenderTask { * @private */ _processItems(items) { + const textDivs = this._textDivs, + textContentItemsStr = this._textContentItemsStr; + for (const item of items) { + // No point in rendering many divs as it would make the browser + // unusable even after the divs are rendered. + if (textDivs.length > MAX_TEXT_DIVS_TO_RENDER) { + warn("Ignoring additional textDivs for performance reasons."); + + this._processItems = () => {}; // Avoid multiple warnings for one page. + return; + } + if (item.str === undefined) { if ( item.type === "beginMarkedContentProps" || @@ -407,7 +402,7 @@ class TextLayerRenderTask { } continue; } - this._textContentItemsStr.push(item.str); + textContentItemsStr.push(item.str); appendText(this, item); } } @@ -435,28 +430,22 @@ class TextLayerRenderTask { * @private */ _render() { - const { promise, resolve, reject } = Promise.withResolvers(); const styleCache = this._styleCache; const pump = () => { this.#reader.read().then(({ value, done }) => { if (done) { - resolve(); + this._capability.resolve(); return; } Object.assign(styleCache, value.styles); this._processItems(value.items); pump(); - }, reject); + }, this._capability.reject); }; - this.#reader = this.#textContentSource.getReader(); pump(); - - promise.then(() => { - render(this); - }, this._capability.reject); } }