Repository navigation
Compute operator bounding boxes in the worker - #22289
calixteman wants to merge 3 commits into
Conversation
Viewer previewCommit 34f4838 (build logs).
Previews are removed when the PR closes and may be pruned after 30 days without an update. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #22289 +/- ##
==========================================
+ Coverage 90.46% 90.53% +0.06%
==========================================
Files 277 279 +2
Lines 68317 68495 +178
==========================================
+ Hits 61805 62010 +205
+ Misses 6512 6485 -27
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:
|
Merging this PR will degrade performance by 3.78%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
| * Writes the transformed rectangle's axis-aligned bounds into `output`. | ||
| * @returns {boolean} Whether the result is a valid box. | ||
| */ | ||
| function transformRect(x0, y0, x1, y1, matrix, output) { |
There was a problem hiding this comment.
With the exception of the last line, this function looks like a verbatim copy of Util.axialAlignedBoundingBox?
| ); | ||
| } | ||
|
|
||
| function intersectBBoxes(first, second) { |
There was a problem hiding this comment.
With the exception of the last line, how similar is this to Util.intersect?
343cf11 to
3207050
Compare
| * Operation bounds stored as `[minX, minY, maxX, maxY]` bytes in 1/256 of the | ||
| * page view box. Decoding adds one unit to the upper bounds. | ||
| */ | ||
| class OperatorBBoxes { |
There was a problem hiding this comment.
Is this class unused in the renderer-worker?
If so you might want to place this in its own file instead, to avoid this code being bundled into the pdf.renderer.mjs file.
| { | ||
| /** @type {boolean} */ | ||
| value: false, | ||
| value: true, |
There was a problem hiding this comment.
Should this perhaps be enabled by default in the PDFViewer and BasePDFPageView classes as well?
3207050 to
932615e
Compare
932615e to
6f38415
Compare
Snuffleupagus
left a comment
Snuffleupagus
left a comment
There was a problem hiding this comment.
It looks like this made Util.multiplyByDOMMatrix unused, so that can probably be removed now?
6f38415 to
99ccb6b
Compare
| @@ -802,46 +790,53 @@ class Util { | |||
| // Applies the transform to the rectangle and finds the minimum axially | |||
| // aligned bounding box. | |||
| static axialAlignedBoundingBox(rect, transform, output) { | |||
There was a problem hiding this comment.
It's probably better suited for a follow-up, but should all existing Util.axialAlignedBoundingBox calls be replaced with Util.transformedRectBoundingBox instead?
Propagate bounds to operator dependencies and send packed boxes with the final operator-list chunk. Expose them through OperatorBBoxes.
Migrate operation filters and debugger highlights to worker bounds. Remove display-side bounding-box tracking and recordOperations.
99ccb6b to
34f4838
Compare
Compute operator bounds and propagate them to dependencies in the worker. Send packed bounds to the display layer for partial rendering and debugger highlights.
Remove display-side bounding-box tracking and recordOperations, and enable optimized partial rendering by default.