Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Ensure that we use the *correct* paintedViewport in PDFPageView.cssTransform, to avoid visual glitches on quick rotations (PR 7738 follow-up)

Browse Source 
*This fixes a regression from commit c9a0955c9c, i.e. PR 7738.*

Currently if you quickly rotate a document at least *twice*,[1] such that rendering of a page hasn't finished for the first rotation before the last rotation is triggered, the `cssTransform` method can fail to update the page correctly leading to it looking temporarily distorted.

The reason why things break is that previously we stored the `viewport` on the canvas DOM element, meaning that when it was accessed in `cssTransform` is was guaranteed to point to the `viewport` of the `zoomLayer` canvas.
Generally you want to avoid storing data on DOM elements this way, and during the `PDFPageView` refactoring needed to support SVG rendering, the previous `viewport` was instead stored directly on `PDFPageView`.
However, the problem is first of all that the `paintedViewport` only stores the *last* `viewport` computed, and second of all that there're no guarantees that it actually applies to the current `zoomLayer` canvas.
If a document is rotated slowly enough that rendering finishes *before* the next rotation then this problem doesn't exist, but for sufficiently quick rotations rendering will be cancelled at least once and the `paintedViewport` could thus be bogus.

The solution for the above problems is to ensure that we track the correct `viewport` for each DOM element (canvas or svg),[2] which seemed easist to do with a `WeakMap`.[3]

---
[1] I'm able to reproduce this using the `tracemonkey` file, but please note that for pages with few operations, i.e. that render very quickly, the effect may be hard to spot.

[2] One other possible solution that I briefly considered, was to wait until rendering finished before storing the current `viewport`. However, that would have caused issues with rotating a page before the *first* rendering operation had finished.

[3] This regression took me way longer to both figure out, and fix, than I'd like to admit :-)
This commit is contained in:
Jonas Jenwald 2017-01-21 12:27:57 +01:00
parent 1948a53ebb
commit 9eb9065c79
1 changed files with 8 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
web/pdf_page_view.js
View File

@ -97,7 +97,7 @@ var PDFPageView = (function PDFPageViewClosure() {
this.renderer = options.renderer || RendererType.CANVAS;
this.paintTask = null;
this.paintedViewport = null;
this.paintedViewportMap = new WeakMap();
this.renderingState = RenderingStates.INITIAL;
this.resume = null;
this.error = null;
@ -170,6 +170,7 @@ var PDFPageView = (function PDFPageViewClosure() {
}
if (this.canvas && !currentZoomLayerNode) {
this.paintedViewportMap.delete(this.canvas);
// Zeroing the width and height causes Firefox to release graphics
// resources immediately, which can greatly reduce memory consumption.
this.canvas.width = 0;
@ -177,11 +178,9 @@ var PDFPageView = (function PDFPageViewClosure() {
delete this.canvas;
}
if (this.svg) {
this.paintedViewportMap.delete(this.svg);
delete this.svg;
}
if (!currentZoomLayerNode) {
this.paintedViewport = null;
}
this.loadingIconDiv = document.createElement('div');
this.loadingIconDiv.className = 'loadingIcon';
@ -281,7 +280,7 @@ var PDFPageView = (function PDFPageViewClosure() {
Math.floor(height) + 'px';
// The canvas may have been originally rotated, rotate relative to that.
var relativeRotation = this.viewport.rotation -
this.paintedViewport.rotation;
this.paintedViewportMap.get(target).rotation;
var absRotation = Math.abs(relativeRotation);
var scaleX = 1, scaleY = 1;
if (absRotation === 90 || absRotation === 270) {
@ -434,9 +433,10 @@ var PDFPageView = (function PDFPageViewClosure() {
}
if (self.zoomLayer) {
var zoomLayerCanvas = self.zoomLayer.firstChild;
self.paintedViewportMap.delete(zoomLayerCanvas);
// Zeroing the width and height causes Firefox to release graphics
// resources immediately, which can greatly reduce memory consumption.
var zoomLayerCanvas = self.zoomLayer.firstChild;
zoomLayerCanvas.width = 0;
zoomLayerCanvas.height = 0;
@ -578,7 +578,7 @@ var PDFPageView = (function PDFPageViewClosure() {
canvas.style.width = roundToDivide(viewport.width, sfx[1]) + 'px';
canvas.style.height = roundToDivide(viewport.height, sfy[1]) + 'px';
// Add the viewport so it's known what it was originally drawn with.
this.paintedViewport = viewport;
this.paintedViewportMap.set(canvas, viewport);
// Rendering area
var transform = !outputScale.scaled ? null :
@ -643,7 +643,7 @@ var PDFPageView = (function PDFPageViewClosure() {
return svgGfx.getSVG(opList, actualSizeViewport).then(function (svg) {
ensureNotCancelled();
self.svg = svg;
self.paintedViewport = actualSizeViewport;
self.paintedViewportMap.set(svg, actualSizeViewport);
svg.style.width = wrapper.style.width;
svg.style.height = wrapper.style.height;