typst / svg2pdf

Converts SVG files to PDF.
Apache License 2.0
287 stars 34 forks source link

line width/height defined with % is wrong #23

Closed Legonaftik closed 1 year ago

Legonaftik commented 1 year ago

Trying to define lines with syntax like 100% generates output PDFs with wrong geometry: <line x1="0" x2="100%" y1="10%" y2="10%" stroke="red"/>

height.svg -> height.pdf width.svg -> width.pdf

How to reproduce: Input SVG's aspect ratio should be such that width > height to see the wrong width calculation <line x1="0" x2="100%>. In order to reproduce the height issue, use an SVG with height > width.

Workaround: Using rect with the same percentage-based syntax leads to correct width/height, as you can see from the attached examples.

Other SVG2PDF converters work fine with such syntax.

LaurenzV commented 1 year ago

This is also a bug in the underlying resvg library, but it seems to work correctly with the newest version of resvg. So once svg2pdf updates its dependency to the newer version of resvg, this issue should be resolved.

Legonaftik commented 1 year ago

@LaurenzV @reknih Are there any plans of updating to the latest version of resvg soon? As I can see, there's an existing attempt in this branch but the last update was 2 weeks ago, before these geometry-related issues were opened.

Updating resvg looks promising because it should fix at least these open issues: https://github.com/typst/svg2pdf/issues/21 https://github.com/typst/svg2pdf/issues/23 https://github.com/typst/svg2pdf/issues/24 Sorry, I am not familiar with Rust so I wanted to better understand the contributors plans before considering investing my own time.

LaurenzV commented 1 year ago

Not sure, maybe @laurmaedje can shed some light on this, considering that he started the branch with the port. If he doesn't plan on continuing the port for now maybe I could give it a shot, but no promises.

reknih commented 1 year ago

Hello! I'm sorry for my inactivity on the repo, but due to time constraints, I will not be able to commit to fixing this before April. Furthermore, we are considering bringing the functionality of this library into the core of Typst (it will be open source at that point) and archive this repo, just fyi.

laurmaedje commented 1 year ago

To give some context on why we're considering this: This library doesn't deal well with text in SVGs and by directly integrating it into Typst, we could provide way better text handling (such that you can control the font selection and copy from the resulting PDF).

Legonaftik commented 1 year ago

Not sure, maybe @laurmaedje can shed some light on this, considering that he started the branch with the port. If he doesn't plan on continuing the port for now maybe I could give it a shot, but no promises.

@laurmaedje the current progress at branch port-to-usvg-0.29 is not compilable yet so could you please share whether it's potentially difficult to finish a proper migration or is it just a matter of spare time? If you think that the rest of migration is trivial, please let me know so I can try to handle it, even though I don't have prior experience with Rust. 🙏

laurmaedje commented 1 year ago

Mostly a matter of time. Shouldn't be super hard, but it still requires a little bit of restructuring I think. Currently there's a preregister pass, which allocates PDF ids for gradients. In usvg 0.29 gradients aren't accessed via id anymore so this would need to be changed.

Legonaftik commented 1 year ago

This library doesn't deal well with text in SVGs and by directly integrating it into Typst, we could provide way better text handling (such that you can control the font selection and copy from the resulting PDF).

Did you consider using usvg 0.28+ which includes support of actual text without converting it to paths? Sorry, it's out of the scope of this issue and might belong https://github.com/typst/svg2pdf/issues/21#issue-1515609305

Legonaftik commented 1 year ago

In usvg 0.29 gradients aren't accessed via id anymore so this would need to be changed.

@laurmaedje if port-to-usvg-0.29 is time consuming then maybe updating to usvg 0.27 is also good enough for this project? usvg 0.27 is the first version that fixes this issue (https://github.com/typst/svg2pdf/issues/23).

laurmaedje commented 1 year ago

Integrating it into Typst would use usvg 0.28+'s capabilities. What usvg 0.28+ does is directly expose the text as a String instead of converting to paths. However, to embed into PDF, you still need to perform text layout (which is complex). By integrating into Typst we can use Typst text layout instead of rolling a second one here.

RazrFalcon commented 1 year ago

In usvg 0.29 gradients aren't accessed via id anymore so this would need to be changed.

Hi! Got here via https://github.com/RazrFalcon/resvg/issues/604. Since 0.31 you can use usvg::Tree::paint_servers too get all gradients.

RazrFalcon commented 1 year ago

Congrats! Feel free to report any API feedback.