Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken node support? (CanvasPattern.setTransform) #13724

Closed
kmcclellan opened this issue Jul 12, 2021 · 5 comments
Closed

Broken node support? (CanvasPattern.setTransform) #13724

kmcclellan opened this issue Jul 12, 2021 · 5 comments

Comments

@kmcclellan
Copy link

Attach (recommended) or Link to PDF file here https://github.com/mozilla/pdf.js/blob/8ab65ed32d0f99a5fd5bf6ce302acb25082910c0/test/pdfs/issue7847_radial.pdf

Configuration:

  • Web browser and its version: NodeJS v14.17.1, node-canvas v2.8.0
  • Operating system and its version: Windows v10.0.18363
  • PDF.js version: 0afc785
  • Is a browser extension: no

Steps to reproduce the problem:

$ npm install
$ gulp dist-install
$ npm install canvas
$ cd examples/node/pdf2png
$ node pdf2png.js ../../../test/pdfs/issue7847_radial.pdf

What is the expected behavior? (add screenshot)

As of ac44afa, pattern_helper.js makes use of CanvasPattern.setTransform(...), which has unfortunately not been implemented yet in the node canvas (Automattic/node-canvas#1411). Previously I've been able to use the pattern helper from within Node contexts.

What went wrong? (add screenshot)

# PDF document loaded.
(node:1608) UnhandledPromiseRejectionWarning: TypeError: pattern.setTransform is not a function
    at RadialAxialShadingPattern.getPattern (/tmp/pdf.js/build/dist/legacy/build/pdf.js:15919:15)
    at CanvasGraphics.fill (/tmp/pdf.js/build/dist/legacy/build/pdf.js:14612:37)
    at CanvasGraphics.executeOperatorList (/tmp/pdf.js/build/dist/legacy/build/pdf.js:14116:24)
    at InternalRenderTask._callee4$ (/tmp/pdf.js/build/dist/legacy/build/pdf.js:12335:51)
    at tryCatch (/tmp/pdf.js/build/dist/legacy/build/pdf.js:832:17)
    at Generator.invoke [as _invoke] (/tmp/pdf.js/build/dist/legacy/build/pdf.js:1005:22)
    at Generator.next (/tmp/pdf.js/build/dist/legacy/build/pdf.js:875:21)
    at asyncGeneratorStep (/tmp/pdf.js/build/dist/legacy/build/pdf.js:9508:103)
    at _next (/tmp/pdf.js/build/dist/legacy/build/pdf.js:9510:194)
    at /tmp/pdf.js/build/dist/legacy/build/pdf.js:9510:364
(Use `node --trace-warnings ...` to show where the warning was created)
(node:1608) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:1608) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Jul 12, 2021

[...] which has unfortunately not been implemented yet in the node canvas (Automattic/node-canvas#1411). Previously I've been able to use the pattern helper from within Node contexts.

There's even a possible patch in Automattic/node-canvas#1623, but it apparently hasn't landed yet (however I cannot tell if it'd fix this issue).

Given the sheer number of long-standing issues fixed by the recent patches in src/display/pattern_helper.js and that browsers are the main development target for the PDF.js library, removing usage of CanvasPattern.setTransform seems somewhat difficult at this point in time unfortunately.
(Note that it'd probably not be all that difficult to avoid the breaking errors, and allow the rest of the document to continue rendering, although that'd of course result in "broken" patterns.)

/cc @brendandahl Can you see any possible work-around, specifically for Node.js environments?

@brendandahl
Copy link
Contributor

We could have an alternative path that goes back to the old way of adjusting the main canvas transform before fill/stroke. Some pdfs will be broken this way, but the majority will still work. I don't imagine this will a very clean approach.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Jul 13, 2021

We could have an alternative path that goes back to the old way of adjusting the main canvas transform before fill/stroke. Some pdfs will be broken this way, but the majority will still work. I don't imagine this will a very clean approach.

Yeah, agreed that it does sound quite messy and thus not great overall. What we could, and likely even should, do is probably to just catch the errors to prevent completely broken rendering.

@kmcclellan
Copy link
Author

I can confirm that running with a local build of Automattic/node-canvas#1623 resolves this.

@Snuffleupagus
Copy link
Collaborator

Closing this issue, since there's not much more we can do here now, given that:

  1. We'll now at least catch these errors, such that the rest of the page can finish rendering, please see PR Avoid all rendering breaking completely when CanvasPattern.setTransform() is unsupported #13725.
  2. The upstream node-canvas PR, see Add setTransform method to CanvasPattern Automattic/node-canvas#1623, was just merged; hence this should thus be completely fixed with a future release of that package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants