From 0159ec0a1234cd101bcae172799f2b8608b095b2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 17 Feb 2022 22:21:59 +0100 Subject: [PATCH 1/3] Remove the `backingStorePixelRatio`-part of the `getOutputScale` helper function The `CanvasRenderingContext2D.backingStorePixelRatio` property was never standardized, and only Safari set (its prefixed version of) it to anything other than `1`. Note that e.g. MDN doesn't contain any information about this property, and one of the few sources of information (at this point) is the following post: https://stackoverflow.com/questions/24332639/why-context2d-backingstorepixelratio-deprecated Hence we can simplify the `getOutputScale` helper function, by removing some dead code, and now it no longer requires any parameters when called. --- web/pdf_page_view.js | 9 ++++----- web/pdf_thumbnail_view.js | 2 +- web/ui_utils.js | 15 ++++----------- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index f7e7dad3d..1bb4f1bc8 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -806,8 +806,7 @@ class PDFPageView { } const ctx = canvas.getContext("2d", { alpha: false }); - const outputScale = getOutputScale(ctx); - this.outputScale = outputScale; + const outputScale = (this.outputScale = getOutputScale()); if (this.useOnlyCssZoom) { const actualSizeViewport = viewport.clone({ @@ -844,9 +843,9 @@ class PDFPageView { this.paintedViewportMap.set(canvas, viewport); // Rendering area - const transform = !outputScale.scaled - ? null - : [outputScale.sx, 0, 0, outputScale.sy, 0, 0]; + const transform = outputScale.scaled + ? [outputScale.sx, 0, 0, outputScale.sy, 0, 0] + : null; const renderContext = { canvasContext: ctx, transform, diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index b2d5541fa..91895eff5 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -233,7 +233,7 @@ class PDFThumbnailView { canvas.mozOpaque = true; } const ctx = canvas.getContext("2d", { alpha: false }); - const outputScale = getOutputScale(ctx); + const outputScale = getOutputScale(); canvas.width = (upscaleFactor * this.canvasWidth * outputScale.sx) | 0; canvas.height = (upscaleFactor * this.canvasHeight * outputScale.sy) | 0; diff --git a/web/ui_utils.js b/web/ui_utils.js index 86b4bd907..64450b372 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -79,18 +79,11 @@ const AutoPrintRegExp = /\bprint\s*\(/; /** * Returns scale factor for the canvas. It makes sense for the HiDPI displays. - * @returns {Object} The object with horizontal (sx) and vertical (sy) - * scales. The scaled property is set to false if scaling is - * not required, true otherwise. + * @returns {Object} The object with horizontal (sx) and vertical (sy) scales. + * The scaled property is false if scaling is not required, true otherwise. */ -function getOutputScale(ctx) { - const devicePixelRatio = window.devicePixelRatio || 1; - const backingStoreRatio = - ctx.webkitBackingStorePixelRatio || - ctx.mozBackingStorePixelRatio || - ctx.backingStorePixelRatio || - 1; - const pixelRatio = devicePixelRatio / backingStoreRatio; +function getOutputScale() { + const pixelRatio = window.devicePixelRatio || 1; return { sx: pixelRatio, sy: pixelRatio, From 0928d26d542f4fb1ecc1caf195224eceec928e5f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 17 Feb 2022 22:28:03 +0100 Subject: [PATCH 2/3] Replace the `scaled` property, in the `getOutputScale` return, with a getter In some cases, in the `PDFPageView` implementation, we're modifying the `sx`/`sy` properties when CSS-only zooming is being used. Currently this requires that you remember to *manually* update the `scaled` property to prevent issues, which doesn't feel all that nice and also seems error-prone. By replacing the `scaled` property with a getter, this is now handled automatically instead. --- web/pdf_page_view.js | 2 -- web/ui_utils.js | 5 ++++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 1bb4f1bc8..5777d1042 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -816,7 +816,6 @@ class PDFPageView { // of the page. outputScale.sx *= actualSizeViewport.width / viewport.width; outputScale.sy *= actualSizeViewport.height / viewport.height; - outputScale.scaled = true; } if (this.maxCanvasPixels > 0) { @@ -825,7 +824,6 @@ class PDFPageView { if (outputScale.sx > maxScale || outputScale.sy > maxScale) { outputScale.sx = maxScale; outputScale.sy = maxScale; - outputScale.scaled = true; this.hasRestrictedScaling = true; } else { this.hasRestrictedScaling = false; diff --git a/web/ui_utils.js b/web/ui_utils.js index 64450b372..e3479d500 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -87,7 +87,10 @@ function getOutputScale() { return { sx: pixelRatio, sy: pixelRatio, - scaled: pixelRatio !== 1, + + get scaled() { + return this.sx !== 1 || this.sy !== 1; + }, }; } From 36cb82e517f43b437551f55d06cf46bc4e04b993 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 18 Feb 2022 16:38:25 +0100 Subject: [PATCH 3/3] Convert the `getOutputScale` helper function into a `OutputScale` class Given the previous patch in particular, this seems like an overall nicer format since it avoids duplicating the `scaled` getter in each instance. --- web/pdf_page_view.js | 4 ++-- web/pdf_thumbnail_view.js | 4 ++-- web/ui_utils.js | 35 ++++++++++++++++++++++------------- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 5777d1042..386349ffb 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -40,7 +40,7 @@ import { import { approximateFraction, DEFAULT_SCALE, - getOutputScale, + OutputScale, RendererType, RenderingStates, roundToDivide, @@ -806,7 +806,7 @@ class PDFPageView { } const ctx = canvas.getContext("2d", { alpha: false }); - const outputScale = (this.outputScale = getOutputScale()); + const outputScale = (this.outputScale = new OutputScale()); if (this.useOnlyCssZoom) { const actualSizeViewport = viewport.clone({ diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index 91895eff5..293bb6be0 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -19,7 +19,7 @@ // eslint-disable-next-line max-len /** @typedef {import("./pdf_rendering_queue").PDFRenderingQueue} PDFRenderingQueue */ -import { getOutputScale, RenderingStates } from "./ui_utils.js"; +import { OutputScale, RenderingStates } from "./ui_utils.js"; import { RenderingCancelledException } from "pdfjs-lib"; const DRAW_UPSCALE_FACTOR = 2; // See comment in `PDFThumbnailView.draw` below. @@ -233,7 +233,7 @@ class PDFThumbnailView { canvas.mozOpaque = true; } const ctx = canvas.getContext("2d", { alpha: false }); - const outputScale = getOutputScale(); + const outputScale = new OutputScale(); canvas.width = (upscaleFactor * this.canvasWidth * outputScale.sx) | 0; canvas.height = (upscaleFactor * this.canvasHeight * outputScale.sy) | 0; diff --git a/web/ui_utils.js b/web/ui_utils.js index e3479d500..0e362b3da 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -78,20 +78,29 @@ const SpreadMode = { const AutoPrintRegExp = /\bprint\s*\(/; /** - * Returns scale factor for the canvas. It makes sense for the HiDPI displays. - * @returns {Object} The object with horizontal (sx) and vertical (sy) scales. - * The scaled property is false if scaling is not required, true otherwise. + * Scale factors for the canvas, necessary with HiDPI displays. */ -function getOutputScale() { - const pixelRatio = window.devicePixelRatio || 1; - return { - sx: pixelRatio, - sy: pixelRatio, +class OutputScale { + constructor() { + const pixelRatio = window.devicePixelRatio || 1; - get scaled() { - return this.sx !== 1 || this.sy !== 1; - }, - }; + /** + * @type {number} Horizontal scale. + */ + this.sx = pixelRatio; + + /** + * @type {number} Vertical scale. + */ + this.sy = pixelRatio; + } + + /** + * @type {boolean} Returns `true` when scaling is required, `false` otherwise. + */ + get scaled() { + return this.sx !== 1 || this.sy !== 1; + } } /** @@ -836,7 +845,6 @@ export { DEFAULT_SCALE_DELTA, DEFAULT_SCALE_VALUE, getActiveOrFocusedElement, - getOutputScale, getPageSizeInches, getVisibleElements, isPortraitOrientation, @@ -849,6 +857,7 @@ export { noContextMenuHandler, normalizeWheelEventDelta, normalizeWheelEventDirection, + OutputScale, parseQueryString, PresentationModeState, ProgressBar,