[api-minor] Render high-res partial page views when falling back to CSS zoom (bug 1492303)

[nicolo-ribaudo]

Commit:

Render high-res partial page views when falling back to CSS zoom

When rendering big PDF pages at high zoom levels, we currently fall back to CSS zoom to avoid rendering canvases with too many pixels. This causes zoomed in PDF to look blurry, and the text to be potentially unreadable.

This commit adds support for rendering part of a page (called PDFPageDetailView in the code), so that we can render portion of a page in a smaller canvas without hiting the maximun canvas size limit.

Specifically, we render an area of that page that is slightly larger than the area that is visible on the screen (100% larger in each direction, unless we have to limit it due to the maximum canvas size). As the user scrolls around the page, we re-render a new area centered around what is currently visible.

To play around with this patch, you need to use a large PDF (the one I'm using is https://www.gtt.to.it/cms/risorse/urbana/mappa/mapparete.pdf) and zoom in: in the current release the text becomes blurry, while with this patch it remains sharp. Also, if you set maxCanvasPixels to a smaller value (for example, 8M) you'll more clearly see the effects also at lower zoom levels. Note however that if maxCanvasPixels is set to less than 9 time the number of pixels visible in your window you'll get more frequent re-renders as you scroll.

The PDFPageDetailView class still process every single operation of the PDF, even if many of them will actually be outside of the canvas and thus not be actually rendered. This is acceptable because we are only rendering one "detail view" per page at a time, and we are not splitting the page in multiple tiles that might appear at the same time on the screen. In the future however #19043 could improve the performance of this patch, and it would enable (for example) tiling a page into multiple detail views when printing it.

There are some very minor TODOs that I left in the code, for some areas I need review feedback for. Also, I still need to find the best way to write tests.

Fixes #14147

Before:

After:

https://www.gtt.to.it/cms/risorse/urbana/mappa/mapparete.pdf at 400% zoom

Before:

After:

Fixes #14193

[nicolo-ribaudo]

After discussing this patch with @calixteman, I updated it to prioritize rendering the full css-zoomed canvas rather than the high-resolution one. This means that, when zooming in, you'll first see the css-zoomed canvas, and then it will be replaced by the high-res one once it's ready. It's a slightly worse experience, however it guarantees that we do not regress in cases where the user starts moving around before that we are done rendering the background canvas (because if only the high-res canvas is there, they'll see just white until when the new high-res canvas is rendered).

I also changed the logic that decides when to re-render the high-res canvas to not only do it once the user scrolls past its edges, but when the user is close to scrolling past the edges.

[nicolo-ribaudo]

Unfortunately this approach has a problem right now: since the various SVG paths for drawings are inserted inside the .canvasWrapper, they are now behind the detail layer and thus not visible. Marking as draft until this is fixed.

[nicolo-ribaudo]

Fixing the drawing problem required changing the new canvas to be in the existing .canvasWrapper, rather than in a separate layer. This actually ended up simplifying the code a bit, and the canvasWrapper management logic is now fully contained in PDFPageView.

[Snuffleupagus]

Leaving a few more comments, mostly for the first patch since I've only begun looking at the Render high-res partial page views when falling back to CSS zoom commit (and that one requires careful checking).

Also, is this new functionality anything that we could "easily" test with e.g. integration-tests?

[nicolo-ribaudo]

I'm working on adding some meaningful tests.

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Failed

Full output at http://54.241.84.105:8877/8ba1a6512246b8a/output.txt

Total script time: 29.65 mins

• Unit tests: Passed
• Integration Tests: FAILED
• Regression tests: Passed

[moz-tools-bot]

From: Bot.io (Windows)

---

Failed

Full output at http://54.193.163.58:8877/3fbf252fd1d6f6e/output.txt

Total script time: 60.00 mins

[Snuffleupagus]

/botio integrationtest

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/abb80cfe2b1f79c/output.txt

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/62ca8d25aae0fb7/output.txt

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Failed

Full output at http://54.241.84.105:8877/abb80cfe2b1f79c/output.txt

Total script time: 13.26 mins

• Integration Tests: FAILED

[nicolo-ribaudo]

Whops sorry, the test now should be fixed for real. It was accidentally relying on the first expect() call happen as soon as the PDF viewer opens, before that it finishes rendering anything (which happens for me locally, but the bots are slower). We don't actually need that expect().

[moz-tools-bot]

From: Bot.io (Windows)

---

Failed

Full output at http://54.193.163.58:8877/62ca8d25aae0fb7/output.txt

Total script time: 27.42 mins

• Integration Tests: FAILED

[nicolo-ribaudo]

@Snuffleupagus please wait before re-running the tests — @calixteman messaged me a video where the detail canvas gets left behind even when it's not needed anymore, and I'm now investigating it.

[nicolo-ribaudo]

Wow this was a difficult bug to track down, and to write tests for. I added two commits (that I will squash), fixing two different race conditions that causes the detail canvas to not be properly removed from the DOM when it's not needed anymore. Each commit includes a test.

The first commit fixes a race condition that can only be triggered via the API (since it requires two synchronous calls to .updateScale). The second commit fixes one that can also appear while manually zooming in/out quickly, and I believe it's the bug that @calixteman noticed.

I'm pasting here the message of the second fixup! commit, so that it doesn't get lost when squashing:

Clean up the PDFDetailView after cancellation synchronously.

This ensures that, when rendering is cancelled, we always restore tracking
the canvas that is still in the DOM (done in the `onCancel` callback of
`_drawCanvas`) before calling `.remove()` on whatever the tracked canvas
is.

Given this algorithm that starts with a 1000% zoom already rendered,
with a detail view canvas that we'll call #1:
- zoom 0.3x (!needs detail view)
- await 1 microtick (Promise.resolve())
- zoom 0.3x (!does not need detail view)

Before this patch, the order of operations would be:
- Create a new detail canvas #2 for the 300% zoom level
- Change detailView.canvas from #1 to #2
- await 1 microtick (between the two zoom changes)
- In .reset(), cancel the rendering
- In .reset(), call detailView.canvas.remove() (removing #2, even if the
  one in the dom is actually still #1)
- Set .detailView to `null`, because we don't need it anymore.
- await, because the `onCancel` logic was run asyncronously
- Change detailView.canvas from #2 to #1

After this patch, the order becomes:
- Create a new detail canvas #2 for the 300% zoom level
- Change detailView.canvas from #1 to #2
- await 1 microtick (between the two zoom changes)
- In .reset(), cancel the rendering
  - It runs `onCancel`, changing detailView.canvas from #2 to #1
- In .reset(), call detailView.canvas.remove() (which now removes #1)
- Set .detailView to `null`, because we don't need it anymore.

[Snuffleupagus]

/botio integrationtest

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/026343628fae9c8/output.txt

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/7a79d253a24948d/output.txt

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Failed

Full output at http://54.241.84.105:8877/026343628fae9c8/output.txt

Total script time: 11.39 mins

• Integration Tests: FAILED

[moz-tools-bot]

From: Bot.io (Windows)

---

Failed

Full output at http://54.193.163.58:8877/7a79d253a24948d/output.txt

Total script time: 26.77 mins

• Integration Tests: FAILED

[nicolo-ribaudo]

The windows-only failure for "PDF viewer CSS-only zoom triggers when going bigger than maxCanvasPixels triggers CSS-only zoom above maxCanvasPixels" could be related, I'm taking a look