Skip to content

Commit

Permalink
Merge pull request #18698 from calixteman/issue18694
Browse files Browse the repository at this point in the history
Avoid to have a white line around the canvas
  • Loading branch information
calixteman authored Sep 7, 2024
2 parents 77c7ec6 + 68332ec commit 5369a24
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 7 deletions.
8 changes: 6 additions & 2 deletions src/display/display_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1103,8 +1103,12 @@ function setLayerDimensions(

const w = `var(--scale-factor) * ${pageWidth}px`,
h = `var(--scale-factor) * ${pageHeight}px`;
const widthStr = useRound ? `round(${w}, 1px)` : `calc(${w})`,
heightStr = useRound ? `round(${h}, 1px)` : `calc(${h})`;
const widthStr = useRound
? `round(down, ${w}, var(--scale-round-x, 1px))`
: `calc(${w})`,
heightStr = useRound
? `round(down, ${h}, var(--scale-round-y, 1px))`
: `calc(${h})`;

if (!mustFlip || viewport.rotation % 180 === 0) {
style.width = widthStr;
Expand Down
74 changes: 74 additions & 0 deletions test/integration/viewer_spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ import {
createPromise,
getSpanRectFromText,
loadAndWait,
scrollIntoView,
waitForPageRendered,
} from "./test_utils.mjs";
import { PNG } from "pngjs";

describe("PDF viewer", () => {
describe("Zoom origin", () => {
Expand Down Expand Up @@ -365,4 +368,75 @@ describe("PDF viewer", () => {
});
});
});

describe("Canvas fits the page", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait(
"issue18694.pdf",
".textLayer .endOfContent",
"page-width"
);
});

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

it("must check that canvas perfectly fits the page whatever the zoom level is", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
const debug = false;

// The pdf has a single page with a red background.
// We set the viewer background to red, because when screenshoting
// some part of the viewer background can be visible.
// But here we don't care about the viewer background: we only
// care about the page background and the canvas default color.

await page.evaluate(() => {
document.body.style.background = "#ff0000";
const toolbar = document.querySelector(".toolbar");
toolbar.style.display = "none";
});
await page.waitForSelector(".toolbar", { visible: false });
await page.evaluate(() => {
const p = document.querySelector(`.page[data-page-number="1"]`);
p.style.border = "none";
});

for (let i = 0; ; i++) {
const handle = await waitForPageRendered(page);
await page.evaluate(() => window.PDFViewerApplication.zoomOut());
await awaitPromise(handle);
await scrollIntoView(page, `.page[data-page-number="1"]`);

const element = await page.$(`.page[data-page-number="1"]`);
const png = await element.screenshot({
type: "png",
path: debug ? `foo${i}.png` : "",
});
const pageImage = PNG.sync.read(Buffer.from(png));
let buffer = new Uint32Array(pageImage.data.buffer);

// Search for the first red pixel.
const j = buffer.indexOf(0xff0000ff);
buffer = buffer.slice(j);

expect(buffer.every(x => x === 0xff0000ff))
.withContext(`In ${browserName}, in the ${i}th zoom in`)
.toBe(true);

const currentScale = await page.evaluate(
() => window.PDFViewerApplication.pdfViewer.currentScale
);
if (currentScale <= 0.1) {
break;
}
}
})
);
});
});
});
1 change: 1 addition & 0 deletions test/pdfs/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -665,3 +665,4 @@
!highlights.pdf
!highlight.pdf
!bug1708040.pdf
!issue18694.pdf
Binary file added test/pdfs/issue18694.pdf
Binary file not shown.
14 changes: 14 additions & 0 deletions test/unit/ui_utils_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import {
backtrackBeforeAllVisibleElements,
binarySearchFirstItem,
calcRound,
getPageSizeInches,
getVisibleElements,
isPortraitOrientation,
Expand Down Expand Up @@ -627,4 +628,17 @@ describe("ui_utils", function () {
});
});
});

describe("calcRound", function () {
it("should handle different browsers/environments correctly", function () {
if (
typeof window !== "undefined" &&
window.navigator?.userAgent?.includes("Firefox")
) {
expect(calcRound(1.6)).not.toEqual(1.6);
} else {
expect(calcRound(1.6)).toEqual(1.6);
}
});
});
});
31 changes: 26 additions & 5 deletions web/pdf_page_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
} from "pdfjs-lib";
import {
approximateFraction,
calcRound,
DEFAULT_SCALE,
floorToDivide,
OutputScale,
Expand Down Expand Up @@ -127,6 +128,10 @@ class PDFPageView {

#previousRotation = null;

#scaleRoundX = 1;

#scaleRoundY = 1;

#renderError = null;

#renderingState = RenderingStates.INITIAL;
Expand Down Expand Up @@ -1039,11 +1044,27 @@ class PDFPageView {
const sfx = approximateFraction(outputScale.sx);
const sfy = approximateFraction(outputScale.sy);

canvas.width = floorToDivide(width * outputScale.sx, sfx[0]);
canvas.height = floorToDivide(height * outputScale.sy, sfy[0]);
const { style } = canvas;
style.width = floorToDivide(width, sfx[1]) + "px";
style.height = floorToDivide(height, sfy[1]) + "px";
const canvasWidth = (canvas.width = floorToDivide(
calcRound(width * outputScale.sx),
sfx[0]
));
const canvasHeight = (canvas.height = floorToDivide(
calcRound(height * outputScale.sy),
sfy[0]
));
const pageWidth = floorToDivide(calcRound(width), sfx[1]);
const pageHeight = floorToDivide(calcRound(height), sfy[1]);
outputScale.sx = canvasWidth / pageWidth;
outputScale.sy = canvasHeight / pageHeight;

if (this.#scaleRoundX !== sfx[1]) {
div.style.setProperty("--scale-round-x", `${sfx[1]}px`);
this.#scaleRoundX = sfx[1];
}
if (this.#scaleRoundY !== sfy[1]) {
div.style.setProperty("--scale-round-y", `${sfy[1]}px`);
this.#scaleRoundY = sfy[1];
}

// Add the viewport so it's known what it was originally drawn with.
this.#viewportMap.set(canvas, viewport);
Expand Down
5 changes: 5 additions & 0 deletions web/pdf_viewer.css
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@
canvas {
margin: 0;
display: block;
width: 100%;
height: 100%;

&[hidden] {
display: none;
Expand All @@ -101,6 +103,9 @@
}

.pdfViewer .page {
--scale-round-x: 1px;
--scale-round-y: 1px;

direction: ltr;
width: 816px;
height: 1056px;
Expand Down
20 changes: 20 additions & 0 deletions web/ui_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,25 @@ function toggleExpandedBtn(button, toggle, view = null) {
view?.classList.toggle("hidden", !toggle);
}

// In Firefox, the css calc function uses f32 precision but the Chrome or Safari
// are using f64 one. So in order to have the same rendering in all browsers, we
// need to use the right precision in order to have correct dimensions.
const calcRound =
typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")
? Math.fround
: (function () {
if (
typeof PDFJSDev !== "undefined" &&
PDFJSDev.test("LIB") &&
typeof document === "undefined"
) {
return x => x;
}
const e = document.createElement("div");
e.style.width = "round(down, calc(1.6666666666666665 * 792px), 1px)";
return e.style.width === "calc(1320px)" ? Math.fround : x => x;
})();

export {
animationStarted,
apiPageLayoutToViewerModes,
Expand All @@ -870,6 +889,7 @@ export {
AutoPrintRegExp,
backtrackBeforeAllVisibleElements, // only exported for testing
binarySearchFirstItem,
calcRound,
CursorTool,
DEFAULT_SCALE,
DEFAULT_SCALE_DELTA,
Expand Down

0 comments on commit 5369a24

Please sign in to comment.