Skip to content

Commit

Permalink
Respect maxCanvasPixels when computing camvas dimensions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nicolo-ribaudo committed Jun 7, 2024
1 parent c21e73a commit 0807d1c
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 21 deletions.
13 changes: 7 additions & 6 deletions test/integration/viewer_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,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);

expect(canvasSize)
.withContext(`In ${browserName}, > MAX_CANVAS_PIXELS * 0.99`)
.toBeGreaterThan(MAX_CANVAS_PIXELS * 0.99);
})
);
});
Expand Down
10 changes: 5 additions & 5 deletions web/pdf_page_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {
DEFAULT_SCALE,
OutputScale,
RenderingStates,
roundToDivide,
floorToDivide,

Check failure on line 39 in web/pdf_page_view.js

View workflow job for this annotation

GitHub Actions / Lint (lts/*)

Member 'floorToDivide' of the import declaration should be sorted alphabetically
TextLayerMode,
} from "./ui_utils.js";
import { AnnotationEditorLayerBuilder } from "./annotation_editor_layer_builder.js";
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 18 additions & 10 deletions web/ui_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,11 @@ function binarySearchFirstItem(items, condition, start = 0) {
* Approximates float number as a fraction using Farey sequence (max order
* of 8).
* @param {number} x - Positive float number.
* @param {boolean} forceLower - If true, returns a franction that is always lower than x.
* @returns {Array} Estimated fraction: the first array item is a numerator,
* the second one is a denominator.
* the second one is a denominator. They are both natural numbers.
*/

Check failure on line 264 in web/ui_utils.js

View workflow job for this annotation

GitHub Actions / Lint (lts/*)

This line has a comment length of 91. Maximum allowed is 80
function approximateFraction(x) {
function approximateFraction(x, forceLower) {
// Fast paths for int numbers or their inversions.

Check failure on line 266 in web/ui_utils.js

View workflow job for this annotation

GitHub Actions / Lint (lts/*)

This line has a comment length of 84. Maximum allowed is 80
if (Math.floor(x) === x) {
return [x, 1];
Expand Down Expand Up @@ -297,18 +298,25 @@ function approximateFraction(x) {
}
}
let result;
// Select closest of the neighbours to x.
if (x_ - a / b < c / d - x_) {
result = x_ === x ? [a, b] : [b, a];
if (forceLower) {
result = x_ === x ? [a, b] : [d, c];
} else {
result = x_ === x ? [c, d] : [d, c];
// Select closest of the neighbours to x.
if (x_ - a / b < c / d - x_) {
result = x_ === x ? [a, b] : [b, a];
} else {
result = x_ === x ? [c, d] : [d, c];

Check failure on line 308 in web/ui_utils.js

View workflow job for this annotation

GitHub Actions / Lint (lts/*)

Unexpected if as the only statement in an else block
}
}
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;
}

/**

Check failure on line 322 in web/ui_utils.js

View workflow job for this annotation

GitHub Actions / Lint (lts/*)

Replace `x·%·div` with `(x·%·div)`
Expand Down Expand Up @@ -881,7 +889,7 @@ export {
ProgressBar,
removeNullCharacters,
RenderingStates,
roundToDivide,
floorToDivide,
SCROLLBAR_PADDING,
scrollIntoView,
ScrollMode,
Expand Down

0 comments on commit 0807d1c

Please sign in to comment.