tskit-dev / tskit

Population-scale genomics
MIT License
155 stars 73 forks source link

SVG can no longer be converted with ImageMagick #980

Open molpopgen opened 4 years ago

molpopgen commented 4 years ago

This affects creating static content (like manuscripts). I guess this just needs to be documented somewhere?

With 0.3.2, I get the following via convert -density 300 foo.svg foo.png:

foo

Adding -background none doesn't change anything.

Inkscape solves the problem via inkscape -z foo.svg -d 300 --export-filename foo2.png:

foo2

rsvg-convert from librsvg2-bin also works, but it is more fiddly to get the final DPI right.

benjeffery commented 4 years ago

I think this is worth fixing - seems a fill attribute should do it?

jeromekelleher commented 4 years ago

Yes, this is a bug. @hyanwong, can you comment please?

hyanwong commented 4 years ago

We have a fill style set in the CSS. It's probable that IM is ignoring the CSS spec for some reason. I'll have a look if there's an easy fix.

hyanwong commented 4 years ago

My version of ImageMagick doesn't like it when the style is wrapped in <![CDATA[ ... ]]>, which is, by default, how the SVG library we use wraps styles. In the CSS docs it says:

Note how the CSS style sheet is placed within a CDATA construct (i.e., <![CDATA[ ... ]]>). Placing internal CSS style sheets within CDATA blocks is sometimes necessary since CSS style sheets can include characters, such as ">", which conflict with XML parsers. Even if a given style sheet does not use characters that conflict with XML parsing, it is highly recommended that internal style sheets be placed inside CDATA blocks.

We can probably omit it if we want to. See https://stackoverflow.com/questions/55465703/is-cdata-necessary-when-styling-an-inline-svg . Note that wrapping the <![CDATA[ markup in /* ... */, as shown in that SO question doesn't seem to help with ImageMagick, in my testing.

hyanwong commented 4 years ago

I've just submitted it as a bug to ImageMagick: https://www.imagemagick.org/discourse-server/viewtopic.php?f=3&t=36936 . Dunno what you want to do in the meantime, @benjeffery / @jeromekelleher ?

molpopgen commented 4 years ago

Thanks everyone. In the meantime, I'll change my Makefiles to use inkscape.

hyanwong commented 4 years ago

Thanks everyone. In the meantime, I'll change my Makefiles to use inkscape.

If you want, you could use ImageMagick but simply cut out the <![CDATA[ strings. At the risk of incurring the wrath of Pythonistas, this should do it:

perl -pi -e 's/<!\[CDATA\[|\]\]>//g' myfile.svg

🤯

molpopgen commented 4 years ago

"Perl PIE" like that saved my PhD on several occasions!

benjeffery commented 4 years ago

If we have a way that works (inkscape) then add it to the docs and move on I think. Hopefully that bug will be fixed eventually.

hyanwong commented 4 years ago

If we have a way that works (inkscape) then add it to the docs and move on I think. Hopefully that bug will be fixed eventually.

Yep. Also, apparently, ImageMagick should use the Inkscape libs if they are available, so there's probably a way to get ImageMagick to work on @molpopgen 's system.

molpopgen commented 4 years ago

If we have a way that works (inkscape) then add it to the docs and move on I think. Hopefully that bug will be fixed eventually.

Yep. Also, apparently, ImageMagick should use the Inkscape libs if they are available, so there's probably a way to get ImageMagick to work on @molpopgen 's system.

I found something about that when first exploring this, but haven't been able to find it again. In the end, I'll just update the Makefiles in various projects. I'd prefer to use convert, though, as it is a smaller dependency, but oh well.

grahamgower commented 4 years ago

I haven't tried it, but mupdf/mutool convert has added svg support recently (at least it's in the git repo, not sure about any releases).

molpopgen commented 4 years ago

Thanks @grahamgower. Version 1.16.1 does convert svg to png but has the same problem that IM does.