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

SVG path is incorrectly broken into 2 nodes #10522

Closed
johnkingsley opened this issue Feb 6, 2019 · 4 comments
Closed

SVG path is incorrectly broken into 2 nodes #10522

johnkingsley opened this issue Feb 6, 2019 · 4 comments

Comments

@johnkingsley
Copy link

Attach (recommended) or Link to PDF file here:
test4.pdf

Configuration:

  • Web browser and its version: Chrome 56.0.2924.76
  • Operating system and its version: Linux Mint 18.3
  • PDF.js version: commit c0d6e46
  • Is a browser extension: no

Steps to reproduce the problem:

  1. run "node pdf2svg.js test4.pdf"
  2. load the output test4.1.svg into Chrome
  3. Check the console error log. You will see this:
    chrome_console_error

What is the expected behavior? (add screenshot)
No errors.

What went wrong? (add screenshot)
One path in the output got split into 2 parts.
The initial "M" is in one node and the following "L" is in the next node.
These should be in the same node.
bad_svg_node

Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension):

@johnkingsley
Copy link
Author

This may be related to bug #9167.

@janpe2
Copy link
Contributor

janpe2 commented Feb 12, 2019

I think the culprit is here:

var OperatorList = (function OperatorListClosure() {
var CHUNK_SIZE = 1000;

I'm not very familiar with the workings of OperatorList but it looks like operator lists are split into chunks of about 1000 operators. Sometimes the chunk boundary is placed in the middle of a PDF path definition. This produces two OPS.constructPath operators and the latter one doesn't start with a moveTo.

Does this sound plausible?

Instead of modifying OperatorList and its constant CHUNK_SIZE, svg.js should be fixed. Maybe like this: Consecutive OPS.constructPath operators should be combined into one <svg:path> node if there is no intervening path painting operator...

@Snuffleupagus
Copy link
Collaborator

I'm not very familiar with the workings of OperatorList but it looks like operator lists are split into chunks of about 1000 operators.

Correct; this is to allow rendering to begin before the entire OperatorList (i.e. the page) has been parsed, thus reducing overall time needed from the page loading to it being fully rendered.

Does this sound plausible?

Yes, and it should be easy to verify (just increase the value a lot, effectively disabling this chunking).

Instead of modifying OperatorList and its constant CHUNK_SIZE, svg.js should be fixed.

Agreed, modifying the constant is definitely not an acceptable solution. First of all, it would do nothing more than move the error elsewhere. Second of all, and much more importantly, changing it could have far-reaching implications for the general rendering performance in the canvas back-end.

Maybe like this: Consecutive OPS.constructPath operators should be combined into one <svg:path> node if there is no intervening path painting operator...

Again, that sounds totally reasonable.

@johnkingsley
Copy link
Author

I can confirm that if I increase CHUNK_SIZE to 10000000 then the problem goes away.
(and I agree that this isn't a proper solution)
Thanks for your help.

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

No branches or pull requests

4 participants