Fix SVG resize in cairosvg and inkscape builder

algoo / preview-generator

generates previews of files with cache management

https://pypi.org/project/preview-generator/

MIT License

228 stars 50 forks source link

Fix SVG resize in cairosvg and inkscape builder #184

Open L3o-pold opened 4 years ago

raphj commented 4 years ago

@L3o-pold ping.

I haven't reviewed this PR. Can you provide us with some insight on this PR so we can review it in the best conditions? What problem are you solving?

L3o-pold commented 4 years ago

@inkhey it depends on how cairosvg.svg2png handle ratio.

It prevents deforming svg. You can try to before/after with an svg icon for example. Before the patch if you ask a big size it will not be extended as it should.

inkhey commented 4 years ago

Just tested with or without this patch: test_result.zip

For cairosvg:

Based on svg "view"
Preserve ratio, if ratio doesn't match file size, will change the view and also show elements outside of the "standard" view.

For inkscape:

Based on drawing view.
setting both height and width make inkscape don't preserve ratio anymore.

Not sure what should be the correct behavior here.

L3o-pold commented 4 years ago

@inkhey you can see that with patch file svg_tesselation_test_inkscape1 and svg_tesselation_test_inkscape2.jpg are much better with the patch.

inkhey commented 4 years ago

@inkhey you can see that with patch file svg_tesselation_test_inkscape1 and svg_tesselation_test_inkscape2.jpg are much better with the patch.

As behavior are unclear, i will create an issue to make decision. I do think new behavior with cairosvg is ok but i do not like the fact that ratio is not preserved in inkscape file.

As we will not fix the fully problem here (should we use view or drawing view, what about some person that do need not preserving ratio), i suggest you to fix inkscape builder in a way ratio is preserved. A not perfect but nice enough solution may be setting only height or width, as inkscape does not provide command line out of the box to do so. If this is ok, i will merge this.

inkhey commented 3 years ago

Inkscape issue is https://gitlab.com/inkscape/inbox/-/issues/2111 related.