Skip to content

Commit

Permalink
[wdspec] browsingContext.print: fix rounding error in page.py test (w…
Browse files Browse the repository at this point in the history
…eb-platform-tests#40504)

* [wdspec] browsingContext.print: fix rounding error in page.py test

[pytest](https://github.com/web-platform-tests/wpt/blob/7a087d54be8b6c0ca0181a86dc1ff0b28461c383/webdriver/tests/support/image.py)
uses:

    def cm_to_px(cm): return round(cm * 96 / 2.54)

[js](https://github.com/web-platform-tests/wpt/blob/7a087d54be8b6c0ca0181a86dc1ff0b28461c383/tools/wptrunner/wptrunner/print_pdf_runner.html)
uses:

    const viewport = page.getViewport({ scale: 96. / 72. });
    ...
    canvas.height = viewport.height;
    canvas.width = viewport.width;

This produces a rounding error, even though the dimension is correct:

    >       assert cm_to_px(expected_dimensions["height"]) == height
    E       assert 454 == 453
    E         +454
    E         -453

The inconsistency of rounding in both ends becomes clear when we
eliminate "round" in the pytest side:

    >       assert cm_to_px(expected_dimensions["height"]) == height
    E       assert 453.54330708661416 == 453
    E         +453.54330708661416
    E         -453

There are multiple ways to fix this issue.

Option #1: Use "floor" instead of "round" in pytest.

Option #2: Use a range in the assertion comparison, allowing a
difference of up to +-1.0. This is what this PR does.

The comparison is performed in
[`assert_pdf_dimensions`](https://github.com/web-platform-tests/wpt/blob/b6107cc1ac8b9c2800b4c8e58af719b8e4d9b8db/webdriver/tests/support/fixtures_bidi.py#L210).

The problematic part is .96 / .72 which evaluates to 4/3 = 1.333333....

* use floor in cm_to_px instead of round

* compare to floor and to round instead of a range

* Revert "compare to floor and to round instead of a range"

This reverts commit 63f894e.

* Revert "use floor in cm_to_px instead of round"

This reverts commit 7e65d91.
  • Loading branch information
thiagowfx committed Jun 16, 2023
1 parent 3d537ab commit fb57353
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 8 deletions.
12 changes: 6 additions & 6 deletions tools/wptrunner/wptrunner/print_pdf_runner.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@
let loadingTask = pdfjsLib.getDocument({data: atob(pdfData)});
let pdf = await loadingTask.promise;
let rendered = [];
for (let pageNumber=1; pageNumber<=pdf.numPages; pageNumber++) {
for (let pageNumber = 1; pageNumber <= pdf.numPages; pageNumber++) {
let page = await pdf.getPage(pageNumber);
var viewport = page.getViewport({scale: 96./72.});
const viewport = page.getViewport({ scale: 96. / 72. });
// Prepare canvas using PDF page dimensions
var canvas = document.getElementsByTagName('canvas')[0];
var context = canvas.getContext('2d');
const canvas = document.getElementsByTagName('canvas')[0];
const context = canvas.getContext('2d');
canvas.height = viewport.height;
canvas.width = viewport.width;

// Render PDF page into canvas context
var renderContext = {
const renderContext = {
canvasContext: context,
viewport: viewport
viewport
};
await page.render(renderContext).promise;
rendered.push(canvas.toDataURL());
Expand Down
4 changes: 2 additions & 2 deletions webdriver/tests/support/fixtures_bidi.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ async def assert_pdf_dimensions(pdf, expected_dimensions):
png = await render_pdf_to_png_bidi(pdf)
width, height = png_dimensions(png)

assert cm_to_px(expected_dimensions["height"]) == height
assert cm_to_px(expected_dimensions["width"]) == width
assert (height - 1) <= cm_to_px(expected_dimensions["height"]) <= (height + 1)
assert (width - 1) <= cm_to_px(expected_dimensions["width"]) <= (width + 1)

return assert_pdf_dimensions

Expand Down

0 comments on commit fb57353

Please sign in to comment.