From 6323f8532ae3dcd891b8de88636f0d1fa65283dc Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 1 Nov 2021 11:52:45 +0100 Subject: [PATCH] Let `getVisibleElements` return a Set containing the visible element `id`s Note how in `PDFPageViewBuffer.resize` we're manually iterating through the visible pages in order to build a Set of the visible page `id`s. By instead moving the building of this Set into the `getVisibleElements` helper function, as part of the existing parsing, this code becomes *ever so slightly* more efficient. Furthermore, more direct access to the visible page `id`s also come in handy in other parts of the viewer as well. In the `BaseViewer.isPageVisible` method we no longer need to loop through the visible pages, but can instead directly check if the pageNumber is visible. In the `PDFRenderingQueue.getHighestPriority` method, when checking for "holes" in the page layout, we can also avoid some unnecessary look-ups this way. --- test/unit/ui_utils_spec.js | 8 ++++++-- web/base_viewer.js | 36 +++++++++++++++--------------------- web/pdf_rendering_queue.js | 8 ++++++-- web/ui_utils.js | 4 +++- 4 files changed, 30 insertions(+), 26 deletions(-) diff --git a/test/unit/ui_utils_spec.js b/test/unit/ui_utils_spec.js index b0441d36e..5eb21330c 100644 --- a/test/unit/ui_utils_spec.js +++ b/test/unit/ui_utils_spec.js @@ -497,7 +497,8 @@ describe("ui_utils", function () { // This is a reimplementation of getVisibleElements without the // optimizations. function slowGetVisibleElements(scroll, pages) { - const views = []; + const views = [], + ids = new Set(); const { scrollLeft, scrollTop } = scroll; const scrollRight = scrollLeft + scroll.clientWidth; const scrollBottom = scrollTop + scroll.clientHeight; @@ -535,9 +536,10 @@ describe("ui_utils", function () { percent, widthPercent: (fractionWidth * 100) | 0, }); + ids.add(view.id); } } - return { first: views[0], last: views[views.length - 1], views }; + return { first: views[0], last: views[views.length - 1], views, ids }; } // This function takes a fixed layout of pages and compares the system under @@ -699,6 +701,7 @@ describe("ui_utils", function () { first: undefined, last: undefined, views: [], + ids: new Set(), }); }); @@ -715,6 +718,7 @@ describe("ui_utils", function () { first: undefined, last: undefined, views: [], + ids: new Set(), }); }); diff --git a/web/base_viewer.js b/web/base_viewer.js index 7fac4d7c1..ed7be475e 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -104,22 +104,19 @@ function PDFPageViewBuffer(size) { data.shift().destroy(); } }; + /** - * After calling resize, the size of the buffer will be newSize. The optional - * parameter pagesToKeep is, if present, an array of pages to push to the back - * of the buffer, delaying their destruction. The size of pagesToKeep has no - * impact on the final size of the buffer; if pagesToKeep has length larger - * than newSize, some of those pages will be destroyed anyway. + * After calling resize, the size of the buffer will be `newSize`. + * The optional parameter `idsToKeep` is, if present, a Set of page-ids to + * push to the back of the buffer, delaying their destruction. The size of + * `idsToKeep` has no impact on the final size of the buffer; if `idsToKeep` + * is larger than `newSize`, some of those pages will be destroyed anyway. */ - this.resize = function (newSize, pagesToKeep) { + this.resize = function (newSize, idsToKeep = null) { size = newSize; - if (pagesToKeep) { - const pageIdsToKeep = new Set(); - for (let i = 0, iMax = pagesToKeep.length; i < iMax; ++i) { - pageIdsToKeep.add(pagesToKeep[i].id); - } + if (idsToKeep) { moveToEndOfArray(data, function (page) { - return pageIdsToKeep.has(page.id); + return idsToKeep.has(page.id); }); } while (data.length > size) { @@ -1145,7 +1142,7 @@ class BaseViewer { return; } const newCacheSize = Math.max(DEFAULT_CACHE_SIZE, 2 * numVisiblePages + 1); - this._buffer.resize(newCacheSize, visiblePages); + this._buffer.resize(newCacheSize, visible.ids); this.renderingQueue.renderHighestPriority(visible); @@ -1231,7 +1228,9 @@ class BaseViewer { y: element.offsetTop + element.clientTop, view: pageView, }; - return { first: view, last: view, views: [view] }; + const ids = new Set([pageView.id]); + + return { first: view, last: view, views: [view], ids }; } _getVisiblePages() { @@ -1273,16 +1272,14 @@ class BaseViewer { console.error(`isPageVisible: "${pageNumber}" is not a valid page.`); return false; } - return this._getVisiblePages().views.some(function (view) { - return view.id === pageNumber; - }); + return this._getVisiblePages().ids.has(pageNumber); } /** * @param {number} pageNumber */ isPageCached(pageNumber) { - if (!this.pdfDocument || !this._buffer) { + if (!this.pdfDocument) { return false; } if ( @@ -1296,9 +1293,6 @@ class BaseViewer { return false; } const pageView = this._pages[pageNumber - 1]; - if (!pageView) { - return false; - } return this._buffer.has(pageView); } diff --git a/web/pdf_rendering_queue.js b/web/pdf_rendering_queue.js index 1f80b5809..9c28bd149 100644 --- a/web/pdf_rendering_queue.js +++ b/web/pdf_rendering_queue.js @@ -133,9 +133,13 @@ class PDFRenderingQueue { // All the visible views have rendered; try to handle any "holes" in the // page layout (can happen e.g. with spreadModes at higher zoom levels). if (lastId - firstId + 1 > numVisible) { + const visibleIds = visible.ids; for (let i = 1, ii = lastId - firstId; i < ii; i++) { - const holeId = scrolledDown ? firstId + i : lastId - i, - holeView = views[holeId - 1]; + const holeId = scrolledDown ? firstId + i : lastId - i; + if (visibleIds.has(holeId)) { + continue; + } + const holeView = views[holeId - 1]; if (!this.isViewFinished(holeView)) { return holeView; } diff --git a/web/ui_utils.js b/web/ui_utils.js index e8f61436b..c769bf3f0 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -473,6 +473,7 @@ function getVisibleElements({ } const visible = [], + ids = new Set(), numViews = views.length; let firstVisibleElementInd = binarySearchFirstItem( views, @@ -558,6 +559,7 @@ function getVisibleElements({ percent, widthPercent: (fractionWidth * 100) | 0, }); + ids.add(view.id); } const first = visible[0], @@ -572,7 +574,7 @@ function getVisibleElements({ return a.id - b.id; // ensure stability }); } - return { first, last, views: visible }; + return { first, last, views: visible, ids }; } /**