Offload canvas drawing operations to a worker #20729
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #20729 +/- ##
==========================================
- Coverage 89.75% 89.02% -0.74%
==========================================
Files 262 264 +2
Lines 66727 67117 +390
==========================================
- Hits 59893 59753 -140
- Misses 6834 7364 +530
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @nicolo-ribaudo received. Current queue size: 0 Live output at: http://54.193.163.58:8877/e0227fb9789b076/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @nicolo-ribaudo received. Current queue size: 0 Live output at: http://54.241.84.105:8877/b6e4acc21cb49a8/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/b6e4acc21cb49a8/output.txt Total script time: 46.91 mins
Image differences available at: http://54.241.84.105:8877/b6e4acc21cb49a8/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/e0227fb9789b076/output.txt Total script time: 90.47 mins
Image differences available at: http://54.193.163.58:8877/e0227fb9789b076/reftest-analyzer.html#web=eq.log |
03ad6c3 to
1afc3d4
Compare
|
I was looking through the reftest failures, it seems like the only real ones (the others are minor pixel differences due to the different rendering pipeline, but not visible to humans) are:
|
1d6de6c to
43a73d7
Compare
There's also gradient difference in 17069, also there are differences in 19022, same as issue8092 |
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @nicolo-ribaudo received. Current queue size: 0 Live output at: http://54.193.163.58:8877/832219967f9b55d/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @nicolo-ribaudo received. Current queue size: 0 Live output at: http://54.241.84.105:8877/659eb95d2835e52/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/659eb95d2835e52/output.txt Total script time: 46.69 mins
Image differences available at: http://54.241.84.105:8877/659eb95d2835e52/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/832219967f9b55d/output.txt Total script time: 78.06 mins
Image differences available at: http://54.193.163.58:8877/832219967f9b55d/reftest-analyzer.html#web=eq.log |
134955c to
ba0fa2c
Compare
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @nicolo-ribaudo received. Current queue size: 0 Live output at: http://54.193.163.58:8877/528a274cb665c1e/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @nicolo-ribaudo received. Current queue size: 0 Live output at: http://54.241.84.105:8877/67316c0b179c2dc/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/67316c0b179c2dc/output.txt Total script time: 46.49 mins
Image differences available at: http://54.241.84.105:8877/67316c0b179c2dc/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/528a274cb665c1e/output.txt Total script time: 75.41 mins
Image differences available at: http://54.193.163.58:8877/528a274cb665c1e/reftest-analyzer.html#web=eq.log |
96dda63 to
cca0198
Compare
GitHub Actions
Inactive
Introduce the RendererWorker class for offloading canvas rendering to a dedicated Web Worker. Alongside, it adds RendererMessageHandler, GlobalWorkerOptions.rendererSrc configuration, entrypoints for pdf.renderer.js bundle and build targets in gulpfile. No rendering changes are introduced in this commit, this is a setup for later commits that wire-up graphics execution and object forwarding.
Add a hasCanvasFilters method to PartialEvaluator that walks the page-level ExtGState dictionaries, and of Form XObject resources, to detect transfer functions (TR) that require DOM SVG filters. Such filters are unavailable on OffscreenCanvas, so detecting them up front lets the display layer fall back to main-thread rendering for affected pages. The detection is deliberately limited to TR: SMask rendering already has a pixel-buffer fallback in canvas.js, and TR inside tiling patterns, Type3 glyph streams or annotation appearance streams is rare enough in practice that walking those sub-resources (and gating first paint on annotation parsing) isn't worth it.
Add object forwarding so the renderer worker receives the same commonobj/obj messages as the main thread WorkerTransport now forwards commonobj/obj to the rendererHandler. Additionally, allow _startRenderPage propagates hasCanvasFilters from core layer.
…orker This decides whether to use worker rendering based on hasCanvasFilters, pageColors, dependency/image tracking. It transfers the canvas via transferControlToOffscreen and sends operator list chunks incrementally. In the renderer worker, it adds the functionality to initialize graphics and execute the operator list. Since operator lists are now posted across threads, Path2D objects are no longer materialized into argsArray; they are cached in a pathCache map on the operator list instead, keeping it structured-cloneable.
Update the viewer and tests to handle canvases that have been transferred to an OffscreenCanvas via the renderer worker.
Thread the enableWebGPU flag from getDocument() through WorkerTransport and InternalRenderTask to the renderer worker's InitializeGraphics handler, where it triggers GPU device initialization.
Forwarded CopyLocalImage messages carry no pixel data, so the renderer worker could stall waiting for an image the core worker never re-sent. Forward the main thread's decoded image when the renderer worker's local copy fails.
…derer worker Clear a page's objects before dropping them in cleanupPage, so ImageBitmaps are closed and not leaked.
…derer worker Register the render-task state before awaiting GPU initialization, so a cleanup arriving during initGPU() can abort the task instead of being ignored.
…derer worker A function-valued operationsFilter cannot be structured-cloned and was silently dropped. Send a precomputed mask for the ops in each chunk instead.
Rendering partialCrop tasks directly into the test canvas bypassed the renderer worker. Always render into a separate canvas and copy the result back for cropping.
getContext("2d") throws once a canvas has been transferred to the
renderer worker; read the pixels via createImageBitmap instead.
Read RendererWorker.rendererSrc inside the try-block, so a missing GlobalWorkerOptions.rendererSrc rejects the capability instead of throwing synchronously.
…derer worker A caller that supplies its own canvasContext expects the pixels to land there; gate worker rendering on the absence of canvasContext rather than on a canvas being passed.
Rename #rendererHandler to #messageHandler, matching PDFWorker.
Rename #worker to #webWorker, matching PDFWorker, and register the initialize() listeners in a consistent order.
Reorder the static class members alphabetically and move the public initializeFromPort() below the private methods.
Reorder the rendererSrc getter/setter alphabetically.
|
The bug and the patch for updating the number of prefs: https://bugzilla.mozilla.org/show_bug.cgi?id=2056102 |
| let initPromise = null; | ||
| if (useWorkerRendering) { | ||
| try { | ||
| const offscreen = this._canvas.transferControlToOffscreen(); |
There was a problem hiding this comment.
Once the control is transferred to the offscreen canvas it cannot be done again so I think it could be break the possibility of reusing the same canvas but in such a case we can reuse the associated offscreen one.
| processed.put(graphicState.objId); | ||
| } | ||
| try { | ||
| if (this._hasTransferMaps(graphicState.get("TR"))) { |
There was a problem hiding this comment.
I recently added support for TR2 too
| } | ||
| } | ||
|
|
||
| const xObjects = node.get("XObject"); |
There was a problem hiding this comment.
You only visit XObject but a TR/TR2 can be in a tiling pattern too.
Snuffleupagus
left a comment
Snuffleupagus
left a comment
There was a problem hiding this comment.
Given that this PR consists of a number of commits, please make sure that every single one of them works correctly on their own. Hence ensure, by testing locally, that all tests pass when run against each commit.
This is imperative to make sure that it's possible to bisect any future regressions to an exact commit, since the total size of the PR would otherwise make that really difficult.
8 participants
Commit descriptions
Note that commit hashes might change in the future due to commit edits later
1. 34ab2b6: Extract ObjectHandler from WorkerTransport …
Notable changes:
this.shouldCreatePageObjsis added in object handler, it does not affect the main-thread rendering, it is set to false, but for worker-rendering, the renderer worker keeps only Map<pageIndex, PDFObjects>, not PDFPageProxy instances. So object_handler.js (line 126) has to handle both shapes.The
shouldCreatePageObjspart exists because renderer-worker obj messages can arrive before InitializeGraphics has called#getPageObjs(pageIndex). In that case the renderer still must cache the image/pattern object, otherwise later CanvasGraphics will hit an unresolved dependency while executing the operator list.Relevent code:
Open questions:
ObjectHandlerclass?2. 741ca8d: Adds RendererWorker class for offloading canvas …
Notable changes
This commit just sets up the renderer worker while following the same pattern as PDFWorker setup.
disableWorkerRenderingfor disabling the worker-rendering. Note that the flag is only checked once to check whether we should set up theRendererHandler, all the further decisions to use the worker-rendering are deferred to whetherRendererHandleris notnull.WorkerAPI,OffScreenCanvasis not supported, a customownerDocumentis present, since custom ownerDocument can be iframe document etc. which cannot be transferred to a worker and the worker cannot create DOM nodes inside it. Same forstyleElement, it is used byFontLoader, and is also a DOM element.In the renderer worker, fonts would instead be loaded via the FontFace API (self.fonts.add()). When a custom styleElement is provided (testing scenarios), it forces CSS-based font loading instead of the FontFace API. Since CSS font loading doesn't work in a worker context, the renderer worker can't render fonts correctly if styleElement is in use.
3. c28992d: Add canvas filter detection in core layer …
Due to bug https://bugzilla.mozilla.org/show_bug.cgi?id=2011237, since there's no DOM access from a worker the filter has to be defined with an external URL which is not currently supported in
OffscreenCanvasRenderingContext2DNotable changes:
hasCanvasFiltersmethod is very similar tohasBlendModeswhere both of these methods traverse the resource graph to check the presence of canvas filters.hasCanvasFiltershowever returnstrueconservatively. I considererd reusing some of the code fromhasBlendModesbut that made the patch more complex and hard to read.4. ffe8e9a: Add object forwarding between main thread and renderer worker …
Open questions
At present the way renderer works is we use the main thread for forwarding the objs/commonObjs and font fallback instead of PDFworker directly sending it to the renderer worker. So
objs/commonObjsare duplicated and also adds an additional hop.The commonobjs/objs are still being duplicated for sending to each of main-thread and renderer, which can be fixed in the following two ways:
a. Use SAB
b. Move the InternalRenderTask entirely to the renderer worker, I think this is possible and this is the approach I would prefer, to not have main thread deal with objs/commonObjs at all, but this is a much larger refactor which I am not sure should be a part of this patch.
While fixing a. appears to be somewhat easy, I have tried with SAB and the current version of sending to both main and renderer is not the best approach because there is a race-condition that it introduces that causes browser tests to almost always timeout, I've tried fixing several but so far, the tests still timeout, if we want to remove the forwarding, I can spend more time looking into forwarding, however I think if we remove the dependency on main thread entirely, it should fix both issues.
For now there's a TODO comment to remove the forwarding in the future.
5. 9ddbd41: Add graphics initialization and operator list execution in renderer worker …
Notable changes
a.
keepRendererCanvas: This flag is added to to still clear normal page state, but ask the renderer worker to keep the transferred canvas alive. This lets PDF.js free operator lists, page objects, fonts/images tied to the page, etc., without making already-rendered visible pages go blank when scrolling etc.b. In main-thread rendering, CanvasGraphics gets the actual OptionalContentConfig instance directly. In renderer-worker rendering, that object cannot be sent as-is: it has class methods/private fields. There is a change to pass the plain data we receive from PDFWorker and rebuilding the object on the worker side.
c. We also need to transfer the annotation canvases, so we find the annotations with
hasOwnCanvas, and send it to renderer worker. It also caches the transferred canvases in_transferredAnnotationCanvasIds.d. Worker rendering is disabled when we have canvas filters and page colors for reasons described above and it falls back to main-thread rendering. It is also disabled when dependencyTracker and imagesTracker are present, this would require making them transferable, which is not too complex and can be done in future iterations.
e. Operator list is sent to renderer worker in chunks. Sending the full growing operator list every time would be very expensive. So the main thread only sends the new tail of the operator list: from the last sent index to the current length. After the renderer worker receives it, it appends that partition to its own worker-local operator list.
7. f085c07: Adapt viewer and tests for OffscreenCanvas renderer worker …
Notable changes
getImageDataon the temporary canvas.