From cd72818438c8ee22ff1f08b2cb19f3e46dcbb53f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 3 Dec 2022 22:58:37 +0100 Subject: [PATCH 1/2] Remove unused `StructTreeLayerBuilder` constructor parameter Also change the "private" methods into properly private ones. --- web/default_factory.js | 12 ++---------- web/interfaces.js | 8 +------- web/pdf_page_view.js | 2 +- web/pdf_viewer.js | 12 ++---------- web/struct_tree_layer_builder.js | 26 ++++++-------------------- 5 files changed, 12 insertions(+), 48 deletions(-) diff --git a/web/default_factory.js b/web/default_factory.js index 184dfe0738eb0..0688eee589c28 100644 --- a/web/default_factory.js +++ b/web/default_factory.js @@ -144,18 +144,10 @@ class DefaultAnnotationEditorLayerFactory { */ class DefaultStructTreeLayerFactory { /** - * @typedef {Object} CreateStructTreeLayerBuilderParameters - * @property {PDFPageProxy} pdfPage - */ - - /** - * @param {CreateStructTreeLayerBuilderParameters} * @returns {StructTreeLayerBuilder} */ - createStructTreeLayerBuilder({ pdfPage }) { - return new StructTreeLayerBuilder({ - pdfPage, - }); + createStructTreeLayerBuilder() { + return new StructTreeLayerBuilder(); } } diff --git a/web/interfaces.js b/web/interfaces.js index e8b5b13978f0b..e558cb39b8c9a 100644 --- a/web/interfaces.js +++ b/web/interfaces.js @@ -280,15 +280,9 @@ class IPDFXfaLayerFactory { */ class IPDFStructTreeLayerFactory { /** - * @typedef {Object} CreateStructTreeLayerBuilderParameters - * @property {PDFPageProxy} pdfPage - */ - - /** - * @param {CreateStructTreeLayerBuilderParameters} * @returns {StructTreeLayerBuilder} */ - createStructTreeLayerBuilder({ pdfPage }) {} + createStructTreeLayerBuilder() {} } /** diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 3f98b2ea0da3e..80a217434224b 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -873,7 +873,7 @@ class PDFPageView { }; this.eventBus._on("textlayerrendered", this._onTextLayerRendered); this.structTreeLayer = - this.structTreeLayerFactory.createStructTreeLayerBuilder({ pdfPage }); + this.structTreeLayerFactory.createStructTreeLayerBuilder(); } div.setAttribute("data-loaded", true); diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index d0b29997657b3..d6ab1ae3faad5 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -1794,18 +1794,10 @@ class PDFViewer { } /** - * @typedef {Object} CreateStructTreeLayerBuilderParameters - * @property {PDFPageProxy} pdfPage - */ - - /** - * @param {CreateStructTreeLayerBuilderParameters} * @returns {StructTreeLayerBuilder} */ - createStructTreeLayerBuilder({ pdfPage }) { - return new StructTreeLayerBuilder({ - pdfPage, - }); + createStructTreeLayerBuilder() { + return new StructTreeLayerBuilder(); } /** diff --git a/web/struct_tree_layer_builder.js b/web/struct_tree_layer_builder.js index 41306d1c72c2f..463756e9613e9 100644 --- a/web/struct_tree_layer_builder.js +++ b/web/struct_tree_layer_builder.js @@ -13,8 +13,6 @@ * limitations under the License. */ -/** @typedef {import("../src/display/api").PDFPageProxy} PDFPageProxy */ - const PDF_ROLE_TO_HTML_ROLE = { // Document level structure types Document: null, // There's a "document" role, but it doesn't make sense here. @@ -73,24 +71,12 @@ const PDF_ROLE_TO_HTML_ROLE = { const HEADING_PATTERN = /^H(\d+)$/; -/** - * @typedef {Object} StructTreeLayerBuilderOptions - * @property {PDFPageProxy} pdfPage - */ - class StructTreeLayerBuilder { - /** - * @param {StructTreeLayerBuilderOptions} options - */ - constructor({ pdfPage }) { - this.pdfPage = pdfPage; - } - render(structTree) { - return this._walk(structTree); + return this.#walk(structTree); } - _setAttributes(structElement, htmlElement) { + #setAttributes(structElement, htmlElement) { if (structElement.alt !== undefined) { htmlElement.setAttribute("aria-label", structElement.alt); } @@ -102,7 +88,7 @@ class StructTreeLayerBuilder { } } - _walk(node) { + #walk(node) { if (!node) { return null; } @@ -119,16 +105,16 @@ class StructTreeLayerBuilder { } } - this._setAttributes(node, element); + this.#setAttributes(node, element); if (node.children) { if (node.children.length === 1 && "id" in node.children[0]) { // Often there is only one content node so just set the values on the // parent node to avoid creating an extra span. - this._setAttributes(node.children[0], element); + this.#setAttributes(node.children[0], element); } else { for (const kid of node.children) { - element.append(this._walk(kid)); + element.append(this.#walk(kid)); } } } From da0e6bc590b0acf41a855fe6d6f289921c396f9f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 4 Dec 2022 00:27:44 +0100 Subject: [PATCH 2/2] Don't re-create the `structTreeLayer` on zooming and rotation Compared to the recent PR 15722 for the `textLayer` this one should be a (comparatively) much a smaller win overall, since most documents don't have any structTree-data and the required parsing should be cheaper. However, it seems to me that it cannot hurt to improve this nonetheless. Note that by moving the `structTreeLayer` initialization we remove the need for the "textlayerrendered" event listener, which thus simplifies the code a little bit. Also, removes the API-caching of the structTree-data since this was basically done to offset the lack of caching in the viewer. --- src/display/api.js | 6 +-- web/pdf_page_view.js | 66 +++++++++++++++----------------- web/struct_tree_layer_builder.js | 13 ++++++- 3 files changed, 43 insertions(+), 42 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index b14be25d117ac..061c1ddae7294 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -1623,9 +1623,7 @@ class PDFPageProxy { * or `null` when no structure tree is present for the current page. */ getStructTree() { - return (this._structTreePromise ||= this._transport.getStructTree( - this._pageIndex - )); + return this._transport.getStructTree(this._pageIndex); } /** @@ -1659,7 +1657,6 @@ class PDFPageProxy { this._bitmaps.clear(); this._annotationPromises.clear(); this._jsActionsPromise = null; - this._structTreePromise = null; this.pendingCleanup = false; return Promise.all(waitOn); } @@ -1694,7 +1691,6 @@ class PDFPageProxy { this.objs.clear(); this._annotationPromises.clear(); this._jsActionsPromise = null; - this._structTreePromise = null; if (resetStats && this._stats) { this._stats = new StatTimer(); } diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 80a217434224b..5f0d0577b0792 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -318,6 +318,33 @@ class PDFPageView { numTextDivs: textLayer.numTextDivs, error, }); + + if (this.structTreeLayerFactory) { + this.#renderStructTreeLayer(); + } + } + + /** + * The structure tree is currently only supported when the text layer is + * enabled and a canvas is used for rendering. + * + * The structure tree must be generated after the text layer for the + * aria-owns to work. + */ + async #renderStructTreeLayer() { + if (!this.textLayer) { + return; + } + this.structTreeLayer ||= + this.structTreeLayerFactory.createStructTreeLayerBuilder(); + + const tree = await (!this.structTreeLayer.renderingDone + ? this.pdfPage.getStructTree() + : null); + const treeDom = this.structTreeLayer?.render(tree); + if (treeDom) { + this.canvas?.append(treeDom); + } } async _buildXfaTextContentItems(textDivs) { @@ -578,6 +605,9 @@ class PDFPageView { this.textLayer.cancel(); this.textLayer = null; } + if (this.structTreeLayer && !this.textLayer) { + this.structTreeLayer = null; + } if ( this.annotationLayer && (!keepAnnotationLayer || !this.annotationLayer.div) @@ -598,10 +628,6 @@ class PDFPageView { this.xfaLayer = null; this.textHighlighter?.disable(); } - if (this._onTextLayerRendered) { - this.eventBus._off("textlayerrendered", this._onTextLayerRendered); - this._onTextLayerRendered = null; - } } cssTransform({ @@ -844,38 +870,6 @@ class PDFPageView { this._renderXfaLayer(); } - // The structure tree is currently only supported when the text layer is - // enabled and a canvas is used for rendering. - if (this.structTreeLayerFactory && this.textLayer && this.canvas) { - // The structure tree must be generated after the text layer for the - // aria-owns to work. - this._onTextLayerRendered = event => { - if (event.pageNumber !== this.id) { - return; - } - this.eventBus._off("textlayerrendered", this._onTextLayerRendered); - this._onTextLayerRendered = null; - - if (!this.canvas) { - return; // The canvas was removed, prevent errors below. - } - this.pdfPage.getStructTree().then(tree => { - if (!tree) { - return; - } - if (!this.canvas) { - return; // The canvas was removed, prevent errors below. - } - const treeDom = this.structTreeLayer.render(tree); - treeDom.classList.add("structTree"); - this.canvas.append(treeDom); - }); - }; - this.eventBus._on("textlayerrendered", this._onTextLayerRendered); - this.structTreeLayer = - this.structTreeLayerFactory.createStructTreeLayerBuilder(); - } - div.setAttribute("data-loaded", true); this.eventBus.dispatch("pagerender", { diff --git a/web/struct_tree_layer_builder.js b/web/struct_tree_layer_builder.js index 463756e9613e9..5ebc29b1aa27a 100644 --- a/web/struct_tree_layer_builder.js +++ b/web/struct_tree_layer_builder.js @@ -72,8 +72,19 @@ const PDF_ROLE_TO_HTML_ROLE = { const HEADING_PATTERN = /^H(\d+)$/; class StructTreeLayerBuilder { + #treeDom = undefined; + + get renderingDone() { + return this.#treeDom !== undefined; + } + render(structTree) { - return this.#walk(structTree); + if (this.#treeDom !== undefined) { + return this.#treeDom; + } + const treeDom = this.#walk(structTree); + treeDom?.classList.add("structTree"); + return (this.#treeDom = treeDom); } #setAttributes(structElement, htmlElement) {