IconJar / IJSVG

MacOS SVG rendering and exporting library
MIT License
172 stars 34 forks source link

Unit is ignored in svg size #12

Closed erikronstrom closed 3 years ago

erikronstrom commented 3 years ago

Thanks for a wonderful library!

I tried to use IJSVG to convert SVG to PDF and it works very well. But I noticed that the attached file failed; the output is just a small empty rectangle.

Lion.svg.zip

I haven't spent too much time on this but I believe the issue is in the SVG header:

<svg xmlns="http://www.w3.org/2000/svg"
     xmlns:xlink="http://www.w3.org/1999/xlink"
     width="15cm" height="15cm">

Note that 1) there is no explicit viewBox 2) the specified size is given in centimeters

This causes IJSVG to treat the SVG as being 15x15 "internal units". Adding a viewBox to the SVG file solves the problem, as does changing "15cm" to "570" (assuming a default of 96/2.54≈38 pixels per cm).

Does this make sense?

curthard89 commented 3 years ago

Hello,

IJSVG is very much tailored towards basic icons, and thus does not implement the full spec, specially when it comes to units. It just happens to handle most SVG's pretty darn well.

That being said, if no view box is supplied it falls back to 0 0 widthAttribute heightAttribute, these are parsed fairly rudimentarily and not using IJSVGUnitLength.

What would need to happen is IJSVGUnitLength would need to detect the cm unit and multiple its float value by the pixel -> cm conversion (not sure if that needs to be based on DPI or not). From that, the width and height attributes can use those units to when create its viewBox from, which will probably sort the issue out.

However, that is very specific to this one case and probably wont work for every use case.

curthard89 commented 3 years ago

@erikronstrom

Ive pushed a test branch to try: here

Screenshot 2021-01-20 at 20 59 35

Seems to yield good results

erikronstrom commented 3 years ago

Nice work!

IIRC, units in SVG used to be dependent on some DPI setting but at some point they changed the spec so that an inch is always 96 internal units and so on.

curthard89 commented 3 years ago

These changes have been merged into master along with a whole suite of other changes (mainly memory and performance improvements)