Skip to content

Commit

Permalink
Avoid degrading scroll performance due to the detail view
Browse files Browse the repository at this point in the history
When scrolling quickly, the constant re-rendering of the detail view
significantly affects rendering performance, causing Firefox to
not render even the _background canvas_, which is just a static canvas
not being re-drawn by JavaScript.

This commit changes the viewer to only render the detail view while
scrolling if its rendering hasn't just been cancelled. This means that:
- when the user is scrolling slowly, we have enough time to render the
  detail view before that we need to change its area, so the user always
  sees the full screen as high resolution.
- when the user is scrolling quickly, as soon as we have to cancel a
  rendering we just give up, and the user will see the lower resolution
  canvas. When then the user stops scrolling, we render the detail view
  for the new visible area.
  • Loading branch information
nicolo-ribaudo committed Feb 21, 2025
1 parent 458b2ee commit dc5d6aa
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 8 deletions.
22 changes: 22 additions & 0 deletions web/pdf_page_detail_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ import { RenderingStates } from "./ui_utils.js";
class PDFPageDetailView extends BasePDFPageView {
#detailArea = null;

/**
* @type {boolean} True when the last rendering attempt of the view was
* cancelled due to a `.reset()` call. This will happen when
* the visible area changes so much during the rendering that
* we need to cancel the rendering and start over.
*/
renderingCancelled = false;

constructor({ pageView }) {
super(pageView);

Expand All @@ -41,9 +49,23 @@ class PDFPageDetailView extends BasePDFPageView {
return this.pageView.pdfPage;
}

get renderingState() {
return super.renderingState;
}

set renderingState(value) {
this.renderingCancelled = false;
super.renderingState = value;
}

reset({ keepCanvas = false } = {}) {
const renderingCancelled =
this.renderingCancelled ||
this.renderingState === RenderingStates.RUNNING ||
this.renderingState === RenderingStates.PAUSED;
this.cancelRendering();
this.renderingState = RenderingStates.INITIAL;
this.renderingCancelled = renderingCancelled;

if (!keepCanvas) {
this._resetCanvas();
Expand Down
21 changes: 15 additions & 6 deletions web/pdf_rendering_queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,22 @@ class PDFRenderingQueue {
* @param {Array} views
* @param {boolean} scrolledDown
* @param {boolean} [preRenderExtra]
* @param {boolean} [ignoreDetailViews]
*/
getHighestPriority(visible, views, scrolledDown, preRenderExtra = false) {
getHighestPriority(
visible,
views,
scrolledDown,
preRenderExtra = false,
ignoreDetailViews = false
) {
/**
* The state has changed. Figure out which page has the highest priority to
* render next (if any).
*
* Priority:
* 1. visible pages
* 2. zoomed-in partial views of visible pages
* 2. zoomed-in partial views of visible pages, unless `ignoreDetailViews`
* 3. if last scrolled down, the page after the visible pages, or
* if last scrolled up, the page before the visible pages
*/
Expand All @@ -127,10 +134,12 @@ class PDFRenderingQueue {
}
}

for (let i = 0; i < numVisible; i++) {
const { detailView } = visibleViews[i].view;
if (detailView && !this.isViewFinished(detailView)) {
return detailView;
if (!ignoreDetailViews) {
for (let i = 0; i < numVisible; i++) {
const { detailView } = visibleViews[i].view;
if (detailView && !this.isViewFinished(detailView)) {
return detailView;
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion web/pdf_thumbnail_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,9 @@ class PDFThumbnailViewer {
const thumbView = this.renderingQueue.getHighestPriority(
visibleThumbs,
this._thumbnails,
scrollAhead
scrollAhead,
/* preRenderExtra */ false,
/* ignoreDetailViews */ true
);
if (thumbView) {
this.#ensurePdfPageLoaded(thumbView).then(() => {
Expand Down
27 changes: 26 additions & 1 deletion web/pdf_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ class PDFViewer {

#mlManager = null;

#scrollTimeoutId = null;

#switchAnnotationEditorModeAC = null;

#switchAnnotationEditorModeTimeoutId = null;
Expand Down Expand Up @@ -1241,6 +1243,15 @@ class PDFViewer {
if (this.pagesCount === 0) {
return;
}

if (this.#scrollTimeoutId) {
clearTimeout(this.#scrollTimeoutId);
}
this.#scrollTimeoutId = setTimeout(() => {
this.#scrollTimeoutId = null;
this.update();
}, 100);

this.update();
}

Expand Down Expand Up @@ -1851,11 +1862,21 @@ class PDFViewer {
this._spreadMode !== SpreadMode.NONE &&
this._scrollMode !== ScrollMode.HORIZONTAL;

// If we are scrolling and the rendering of a detail view was just
// cancelled, it's because the user is scrolling too quickly and so
// we constantly need to re-render a different area.
// Don't attempt to re-render it: this will be done once the user
// stops scrolling.
const ignoreDetailViews =
this.#scrollTimeoutId !== null &&
visiblePages.views.some(page => page.detailView?.renderingCancelled);

const pageView = this.renderingQueue.getHighestPriority(
visiblePages,
this._pages,
scrollAhead,
preRenderExtra
preRenderExtra,
ignoreDetailViews
);

if (pageView) {
Expand Down Expand Up @@ -2449,6 +2470,10 @@ class PDFViewer {
clearTimeout(this.#scaleTimeoutId);
this.#scaleTimeoutId = null;
}
if (this.#scrollTimeoutId !== null) {
clearTimeout(this.#scrollTimeoutId);
this.#scrollTimeoutId = null;
}
if (!noUpdate) {
this.update();
}
Expand Down

0 comments on commit dc5d6aa

Please sign in to comment.