Respect maxCanvasPixels when computing canvas dimensions

Ensure that we never round the canvas dimensions above `maxCanvasPixels` by rounding them to the preceeding multiple of the display ratio rather than the succeeding one.
mozilla · Jun 18, 2024 · 6db460c · 6db460c
1 parent c21e73a
commit 6db460c
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 21 deletions.
diff --git a/test/integration/test_utils.mjs b/test/integration/test_utils.mjs
@@ -38,8 +38,13 @@ function loadAndWait(filename, selector, zoom, pageSetup, options) {
 
       let app_options = "";
       if (options) {
+        const optionsObject =
+          typeof options === "function"
+            ? await options(page, session.name)
+            : options;
+
         // Options must be handled in app.js::_parseHashParams.
-        for (const [key, value] of Object.entries(options)) {
+        for (const [key, value] of Object.entries(optionsObject)) {
           app_options += `&${key}=${encodeURIComponent(value)}`;
         }
       }

diff --git a/test/integration/viewer_spec.mjs b/test/integration/viewer_spec.mjs
@@ -178,15 +178,21 @@ describe("PDF viewer", () => {
     describe("triggers when going bigger than maxCanvasPixels", () => {
       let pages;
 
-      const MAX_CANVAS_PIXELS = 4_000_000;
+      const MAX_CANVAS_PIXELS = new Map();
 
       beforeAll(async () => {
         pages = await loadAndWait(
           "tracemonkey.pdf",
           ".textLayer .endOfContent",
           null,
           null,
-          { maxCanvasPixels: MAX_CANVAS_PIXELS }
+          async (page, browserName) => {
+            const ratio = await page.evaluate(() => window.devicePixelRatio);
+            const maxCanvasPixels = 1_000_000 * ratio ** 2;
+            MAX_CANVAS_PIXELS.set(browserName, maxCanvasPixels);
+
+            return { maxCanvasPixels };
+          }
         );
       });
 
@@ -220,10 +226,10 @@ describe("PDF viewer", () => {
 
             expect(canvasSize)
               .withContext(`In ${browserName}`)
-              .toBeLessThan(MAX_CANVAS_PIXELS / 4);
+              .toBeLessThan(MAX_CANVAS_PIXELS.get(browserName) / 4);
             expect(canvasSize)
               .withContext(`In ${browserName}`)
-              .toBeGreaterThan(MAX_CANVAS_PIXELS / 16);
+              .toBeGreaterThan(MAX_CANVAS_PIXELS.get(browserName) / 16);
           })
         );
       });
@@ -249,7 +255,7 @@ describe("PDF viewer", () => {
 
             expect(canvasSize)
               .withContext(`In ${browserName}, MAX_CANVAS_PIXELS`)
-              .toBeLessThan(MAX_CANVAS_PIXELS);
+              .toBeLessThan(MAX_CANVAS_PIXELS.get(browserName));
           })
         );
       });
@@ -273,12 +279,13 @@ describe("PDF viewer", () => {
               .withContext(`In ${browserName}`)
               .toBeLessThan(originalCanvasSize * factor ** 2);
 
-            // Disabled because `canvasSize` is `4_012_800`, which is
-            // close to the limit but somehow a bit more.
-            //
-            //  expect(canvasSize)
-            //    .withContext(`In ${browserName}, MAX_CANVAS_PIXELS`)
-            //    .toBeLessThan(MAX_CANVAS_PIXELS);
+            expect(canvasSize)
+              .withContext(`In ${browserName}, <= MAX_CANVAS_PIXELS`)
+              .toBeLessThanOrEqual(MAX_CANVAS_PIXELS.get(browserName));
+
+            expect(canvasSize)
+              .withContext(`In ${browserName}, > MAX_CANVAS_PIXELS * 0.99`)
+              .toBeGreaterThan(MAX_CANVAS_PIXELS.get(browserName) * 0.99);
           })
         );
       });

diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js
@@ -34,9 +34,9 @@ import {
 import {
   approximateFraction,
   DEFAULT_SCALE,
+  floorToDivide,
   OutputScale,
   RenderingStates,
-  roundToDivide,
   TextLayerMode,
 } from "./ui_utils.js";
 import { AnnotationEditorLayerBuilder } from "./annotation_editor_layer_builder.js";
@@ -1025,11 +1025,11 @@ class PDFPageView {
     const sfx = approximateFraction(outputScale.sx);
     const sfy = approximateFraction(outputScale.sy);
 
-    canvas.width = roundToDivide(width * outputScale.sx, sfx[0]);
-    canvas.height = roundToDivide(height * outputScale.sy, sfy[0]);
+    canvas.width = floorToDivide(width * outputScale.sx, sfx[0]);
+    canvas.height = floorToDivide(height * outputScale.sy, sfy[0]);
     const { style } = canvas;
-    style.width = roundToDivide(width, sfx[1]) + "px";
-    style.height = roundToDivide(height, sfy[1]) + "px";
+    style.width = floorToDivide(width, sfx[1]) + "px";
+    style.height = floorToDivide(height, sfy[1]) + "px";
 
     // Add the viewport so it's known what it was originally drawn with.
     this.#viewportMap.set(canvas, viewport);

diff --git a/web/ui_utils.js b/web/ui_utils.js
@@ -260,6 +260,7 @@ function binarySearchFirstItem(items, condition, start = 0) {
  *  @param {number} x - Positive float number.
  *  @returns {Array} Estimated fraction: the first array item is a numerator,
  *                   the second one is a denominator.
+ *                   They are both natural numbers.
  */
 function approximateFraction(x) {
   // Fast paths for int numbers or their inversions.
@@ -306,9 +307,12 @@ function approximateFraction(x) {
   return result;
 }
 
-function roundToDivide(x, div) {
-  const r = x % div;
-  return r === 0 ? x : Math.round(x - r + div);
+/**
+ * @param {number} x - A positive number to round to a multiple of `div`.
+ * @param {number} div - A natural number.
+ */
+function floorToDivide(x, div) {
+  return x - (x % div);
 }
 
 /**
@@ -863,6 +867,7 @@ export {
   DEFAULT_SCALE_DELTA,
   DEFAULT_SCALE_VALUE,
   docStyle,
+  floorToDivide,
   getActiveOrFocusedElement,
   getPageSizeInches,
   getVisibleElements,
@@ -881,7 +886,6 @@ export {
   ProgressBar,
   removeNullCharacters,
   RenderingStates,
-  roundToDivide,
   SCROLLBAR_PADDING,
   scrollIntoView,
   ScrollMode,