Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Respect maxCanvasPixels when computing canvas dimensions #18218

Merged
merged 2 commits into from
Jun 19, 2024
Merged

Respect maxCanvasPixels when computing canvas dimensions #18218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
12692d2
Add test for CSS-only zoom above maxCanvasPixels
nicolo-ribaudo Jun 7, 2024
de23bb9
Respect `maxCanvasPixels` when computing canvas dimensions
nicolo-ribaudo Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion test/integration/test_utils.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`;
}
}
Expand Down
220 changes: 168 additions & 52 deletions test/integration/viewer_spec.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,24 +94,6 @@ describe("PDF viewer", () => {
});

describe("CSS-only zoom", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait(
"tracemonkey.pdf",
".textLayer .endOfContent",
null,
null,
{
maxCanvasPixels: 0,
}
);
});

afterAll(async () => {
await closePages(pages);
});

function createPromiseForFirstPageRendered(page) {
return createPromise(page, (resolve, reject) => {
const controller = new AbortController();
Expand All @@ -129,50 +111,184 @@ describe("PDF viewer", () => {
});
}

it("respects drawing delay when zooming out", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const promise = await createPromiseForFirstPageRendered(page);
describe("forced (maxCanvasPixels: 0)", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait(
"tracemonkey.pdf",
".textLayer .endOfContent",
null,
null,
{ maxCanvasPixels: 0 }
);
});

afterAll(async () => {
await closePages(pages);
});

const start = await page.evaluate(() => {
const startTime = performance.now();
window.PDFViewerApplication.pdfViewer.decreaseScale({
drawingDelay: 100,
scaleFactor: 0.9,
it("respects drawing delay when zooming out", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const promise = await createPromiseForFirstPageRendered(page);

const start = await page.evaluate(() => {
const startTime = performance.now();
window.PDFViewerApplication.pdfViewer.decreaseScale({
drawingDelay: 100,
scaleFactor: 0.9,
});
return startTime;
});
return startTime;
});

const end = await awaitPromise(promise);
const end = await awaitPromise(promise);

expect(end - start)
.withContext(`In ${browserName}`)
.toBeGreaterThanOrEqual(100);
})
);
expect(end - start)
.withContext(`In ${browserName}`)
.toBeGreaterThanOrEqual(100);
})
);
});

it("respects drawing delay when zooming in", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const promise = await createPromiseForFirstPageRendered(page);

const start = await page.evaluate(() => {
const startTime = performance.now();
window.PDFViewerApplication.pdfViewer.increaseScale({
drawingDelay: 100,
scaleFactor: 1.1,
});
return startTime;
});

const end = await awaitPromise(promise);

expect(end - start)
.withContext(`In ${browserName}`)
.toBeGreaterThanOrEqual(100);
})
);
});
});

it("respects drawing delay when zooming in", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const promise = await createPromiseForFirstPageRendered(page);
describe("triggers when going bigger than maxCanvasPixels", () => {
let pages;

const MAX_CANVAS_PIXELS = new Map();
nicolo-ribaudo marked this conversation as resolved.
Show resolved Hide resolved

const start = await page.evaluate(() => {
const startTime = performance.now();
window.PDFViewerApplication.pdfViewer.increaseScale({
drawingDelay: 100,
scaleFactor: 1.1,
beforeAll(async () => {
pages = await loadAndWait(
"tracemonkey.pdf",
".textLayer .endOfContent",
null,
null,
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 };
}
);
});

beforeEach(async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await page.evaluate(() => {
window.PDFViewerApplication.pdfViewer.currentScale = 0.5;
});
return startTime;
});
})
);
});

const end = await awaitPromise(promise);
afterAll(async () => {
await closePages(pages);
});

expect(end - start)
.withContext(`In ${browserName}`)
.toBeGreaterThanOrEqual(100);
})
);
function getCanvasSize(page) {
return page.evaluate(() => {
const canvas = window.document.querySelector(".canvasWrapper canvas");
return canvas.width * canvas.height;
});
}

// MAX_CANVAS_PIXELS must be big enough that the originally rendered
// canvas still has enough space to grow before triggering CSS-only zoom
it("test correctly configured", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const canvasSize = await getCanvasSize(page);

expect(canvasSize)
.withContext(`In ${browserName}`)
.toBeLessThan(MAX_CANVAS_PIXELS.get(browserName) / 4);
expect(canvasSize)
.withContext(`In ${browserName}`)
.toBeGreaterThan(MAX_CANVAS_PIXELS.get(browserName) / 16);
})
);
});

it("does not trigger CSS-only zoom below maxCanvasPixels", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const originalCanvasSize = await getCanvasSize(page);
const factor = 2;

await page.evaluate(scaleFactor => {
window.PDFViewerApplication.pdfViewer.increaseScale({
drawingDelay: 0,
scaleFactor,
});
}, factor);

const canvasSize = await getCanvasSize(page);

expect(canvasSize)
.withContext(`In ${browserName}`)
.toBe(originalCanvasSize * factor ** 2);

expect(canvasSize)
.withContext(`In ${browserName}, MAX_CANVAS_PIXELS`)
.toBeLessThan(MAX_CANVAS_PIXELS.get(browserName));
})
);
});

it("triggers CSS-only zoom above maxCanvasPixels", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const originalCanvasSize = await getCanvasSize(page);
const factor = 4;

await page.evaluate(scaleFactor => {
window.PDFViewerApplication.pdfViewer.increaseScale({
drawingDelay: 0,
scaleFactor,
});
}, factor);

const canvasSize = await getCanvasSize(page);

expect(canvasSize)
.withContext(`In ${browserName}`)
.toBeLessThan(originalCanvasSize * factor ** 2);

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);
})
);
});
});
});
});
10 changes: 5 additions & 5 deletions web/pdf_page_view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
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
12 changes: 8 additions & 4 deletions web/ui_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
timvandermeij marked this conversation as resolved.
Show resolved Hide resolved
/**
* @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);
}

/**
Expand Down Expand Up @@ -863,6 +867,7 @@ export {
DEFAULT_SCALE_DELTA,
DEFAULT_SCALE_VALUE,
docStyle,
floorToDivide,
getActiveOrFocusedElement,
getPageSizeInches,
getVisibleElements,
Expand All @@ -881,7 +886,6 @@ export {
ProgressBar,
removeNullCharacters,
RenderingStates,
roundToDivide,
SCROLLBAR_PADDING,
scrollIntoView,
ScrollMode,
Expand Down