search

typst / svg2pdf

Converts SVG files to PDF.
Apache License 2.0
273 stars 32 forks source link

Error in `dpi_ratio` #55

Closed Ultraxime closed 7 months ago

Ultraxime commented 7 months ago

Hello,

I was trying to convert a 1500x2300 pixels SVG into a PDF using svg2pdf. I want it to be 62.7x96.2mm. So by simple math, it should be at a resolution around 607dpi. But while reading the output, my PDF reader (okular and brave) told me it was 4461x6840mm. Using the trial and error method, I found that the dpi I had to provide to svg2pdf was 8.5.

So dug into the code to find where I could have made a mistake, or it was svg2pdf that had one.

It seems to me that line 396 and 397 in lib.rs you multiply by dpi_ratio instead of dividing by it. dpi_ratio is defined in util.helper.rs as dpi / 72. I understand the user unit in PDF is 1/72nd of an inch, hence the division by 72. So dpi_ratio returns a value in dots per inch time inch per user unit, i.e. dots per user unit. Hence, multiply dots by dots per user unit gave us dot² per user unit instead of user unit.

If I made a mistake in my reasoning, and I'm wrong, I would be happy to be corrected; else I can either make a pull request with the modification, or wait for you to correct it.

LaurenzV commented 7 months ago

Yes, I think you're right! Feel free to open a PR.

Unfortunately the CLI is not very well-tested (unlike the core library itself), which is why this probably went unnoticed for so long. 😅 The main reason being that svg2pdf is mainly used for typst as a Rust library, so I haven't put much time into improving the CLI. Once I feel like more people are using it I definitely plan on investing more into that, though.

Ultraxime commented 7 months ago

I made the pull request. It's only two lines change and I added the comments for them

By the way, I wasn't using the CLI but the lib directly as a dependency for my rust project

LaurenzV commented 7 months ago

I'm not the owner of this repo, so you'll have to wait a bit until they see it. 😄