Use @napi-rs/canvas in native engine to match pdfjs-dist#56
Open
ywang-clarify wants to merge 1 commit into
Open
Use @napi-rs/canvas in native engine to match pdfjs-dist#56ywang-clarify wants to merge 1 commit into
ywang-clarify wants to merge 1 commit into
Conversation
pdfjs-dist 4.x ships its own @napi-rs/canvas-based NodeCanvasFactory and polyfills (DOMMatrix, ImageData, Path2D) when running on Node. If the native engine hands pdfjs a Cairo-backed canvas v3 context, the intermediate image objects pdfjs constructs fail canvas v3's strict instanceOf type check in Context2d::DrawImage and throw "Image or Canvas expected" -- which the outer try/catch in comparePdfByImage swallows into a generic "An error occurred". Reproduces on any PDF that contains an embedded raster image; text-only PDFs pass. Swap the native engine to @napi-rs/canvas so both sides of the boundary use the same module. Same fix the pdf.js maintainers recommend in mozilla/pdf.js#19566 and #19794. @napi-rs/canvas's toBuffer() requires an explicit mime type, so the three call sites are updated to pass "image/png". Adds @napi-rs/canvas as a direct dependency. Today it's installed indirectly via pdfjs-dist's optionalDependencies; declaring it directly makes the dependency explicit and guaranteed. graphicsMagick engine unchanged; public API and types unchanged. The `canvas` dep becomes unused for the native engine but is left in dependencies in this PR to keep the diff minimal -- happy to drop it in a follow-up if you'd prefer. Fixes marcdacz#55.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #55.
pdfjs-dist@4.xuses@napi-rs/canvasin Node for its internal polyfills (DOMMatrix,ImageData,Path2D) and intermediate canvases (pdf.mjs:11411-11443,pdf.mjs:11451-11456). When the native engine hands pdfjs a Cairo-backedcanvasv3 rendering context, the intermediate image objects pdfjs constructs failcanvas's strictinstanceOftype check insideContext2d::DrawImage, throwingTypeError: Image or Canvas expected. The error is then swallowed bycomparePdfByImage's outertry/catchand surfaces only as a genericstatus: failed, message: "An error occurred.".Switching the native engine to
@napi-rs/canvasso both sides of the boundary speak the same module fixes it. Same fix recommended by the pdf.js maintainers in mozilla/pdf.js#19566 and #19794.Changes
functions/engines/NodeCanvasFactory.jsrequire("canvas")→require("@napi-rs/canvas")functions/engines/native.jsrequire("canvas")→require("@napi-rs/canvas")canvas.toBuffer()→canvas.toBuffer("image/png")(×3) —@napi-rs/canvasrequires an explicit mime type.package.json@napi-rs/canvasas a direct dependency. Today it's installed indirectly via pdfjs-dist'soptionalDependencies; declaring it directly makes the dependency explicit and guaranteed.graphicsMagick engine unchanged. Public API and types unchanged.
Verification
Self-compare of a PDF with embedded raster images:
{ status: "failed", message: "An error occurred.\nTypeError: Image or Canvas expected" }{ status: "passed" }Local
npm testagainst the modified branch passes all the native-engine specs that pass on master (the same 2 native specs and ~22 graphicsMagick specs fail on master too — appears to be environmental, e.g. GraphicsMagick/Ghostscript not installed on a Windows machine).Follow-up (optional)
canvas(Cairo v3) becomes unused after this change. Left independenciesin this PR to keep the diff minimal — happy to drop it in a follow-up if you'd prefer.