From ee5862bd81998177f82c95f7c3ac324c05d08eb2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 6 Oct 2017 17:26:54 +0200 Subject: [PATCH] Prevent the `annotationLayer` from, in some cases, becoming duplicated on the first page when the document loads I don't know if this is a regression, but I noticed earlier today that depending on the initial scale *and* sidebar state, the `annotationLayer` of the first rendered page may end up duplicated; please see screen-shot below. [screen-shot] I can reproduce this reliable with e.g. https://arxiv.org/pdf/1112.0542v1.pdf#zoom=page-width&pagemode=bookmarks. When the document loads, rendering of the first page begins immediately. When the sidebar is then opened, that forces re-rendering which thus aborts rendering of the first page. Note that calling `PDFPageView.draw()` will always, provided an `AnnotationLayerFactory` instance exists, call `AnnotationLayerBuilder.render()`. Hence the events described above will result in *two* such calls, where the actual annotation rendering/updating happens asynchronously. For reasons that I don't (at all) understand, when multiple `pdfPage.getAnnotations()` promises are handled back-to-back (in `AnnotationLayerBuilder.render()`), the `this.div` property seems to not update in time for the subsequent calls. This thus, at least in Firefox, result in double rendering of all annotations on the first page. Obviously it'd be good to find out why it breaks, since it *really* shouldn't, but this patch at least provides a (hopefully) acceptable work-around by ignoring `getAnnotations()` calls for `AnnotationLayerBuilder` instances that we're destroying (in `PDFPageView.reset()`). --- web/annotation_layer_builder.js | 9 +++++++++ web/pdf_page_view.js | 11 ++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/web/annotation_layer_builder.js b/web/annotation_layer_builder.js index 4612d65d4..2c96778ca 100644 --- a/web/annotation_layer_builder.js +++ b/web/annotation_layer_builder.js @@ -41,6 +41,7 @@ class AnnotationLayerBuilder { this.l10n = l10n; this.div = null; + this._cancelled = false; } /** @@ -49,6 +50,10 @@ class AnnotationLayerBuilder { */ render(viewport, intent = 'display') { this.pdfPage.getAnnotations({ intent, }).then((annotations) => { + if (this._cancelled) { + return; + } + let parameters = { viewport: viewport.clone({ dontFlip: true, }), div: this.div, @@ -80,6 +85,10 @@ class AnnotationLayerBuilder { }); } + cancel() { + this._cancelled = true; + } + hide() { if (!this.div) { return; diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index d236746c8..edd0ef582 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -136,7 +136,7 @@ class PDFPageView { } reset(keepZoomLayer = false, keepAnnotations = false) { - this.cancelRendering(); + this.cancelRendering(keepAnnotations); let div = this.div; div.style.width = Math.floor(this.viewport.width) + 'px'; @@ -159,7 +159,8 @@ class PDFPageView { // Hide the annotation layer until all elements are resized // so they are not displayed on the already resized page. this.annotationLayer.hide(); - } else { + } else if (this.annotationLayer) { + this.annotationLayer.cancel(); this.annotationLayer = null; } @@ -240,7 +241,7 @@ class PDFPageView { this.reset(/* keepZoomLayer = */ true, /* keepAnnotations = */ true); } - cancelRendering() { + cancelRendering(keepAnnotations = false) { if (this.paintTask) { this.paintTask.cancel(); this.paintTask = null; @@ -252,6 +253,10 @@ class PDFPageView { this.textLayer.cancel(); this.textLayer = null; } + if (!keepAnnotations && this.annotationLayer) { + this.annotationLayer.cancel(); + this.annotationLayer = null; + } } cssTransform(target, redrawAnnotations = false) {