Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically convert SVG to PDF images in the latex builder #2859

Closed
wants to merge 1 commit into from

Conversation

alyjak
Copy link
Contributor

@alyjak alyjak commented Aug 16, 2016

This adds the ability to automatically convert svg images to pdfs using Inkscape when using the latex builder. This gets around the fact that latexpdf cannot natively handle svg images.

This change should be non invasive -- no new dependencies are required. Only if Inkscape is already installed will the code spawn a subprocess to call it to convert svgs into pdfs. If Inkscape is not available, the existing code should be unaffected besides an extra console print that lets users know that if they install Inkscape they will get this new feature.

Notes:

  • I haven't tested this with windows.
  • Some users might find the extra print out to be spammy. Let me know what you think and I'd be happy to remove it.
  • I've run all the unit tests with and without Inkscape installed -- they all seem to pass in python 3.5, but when run in python 2.7 a couple seem to fail but they look to be unrelated to this change
  • The unit tests have different behavior based off of whether Inkscape is installed. If its not, test_environment.py test_images() works the same as it did. If it is installed, test_images() expects to see svgimg.svg: svgimg_svg2pdf.pdf in its image map.
  • I didn't see any place in the docs that needed to be updated for this change. Let me know if you think there's any documentation that needs updating.
  • I didn't think this was a big enough change to update CHANGES, I also wasn't sure which section should be updated anyways.

@tk0miya tk0miya added builder:latex type:enhancement enhance or introduce a new feature labels Aug 17, 2016
@tk0miya
Copy link
Member

tk0miya commented Aug 17, 2016

I'll take a look later.

refs: #2166, #2852

juga0 added a commit to juga0/autocrypt that referenced this pull request Jan 3, 2017
* replace .svg images by .* in order that `make latexpdf` not to fail.
* while [sphinx-doc #2859](sphinx-doc/sphinx#2859 is merged, add rule in Makefile to generate pdfs from svgs.
* `make latexpdf` will still fail if inkscape is not installed. The alternative will be to do not include the rule "images" in "latexpdf" and include the pdfs in git.
@anarcat
Copy link
Contributor

anarcat commented Jan 14, 2017

"This branch has conflicts that must be resolved" - otherwise what's the status on this?

@tk0miya
Copy link
Member

tk0miya commented Jan 15, 2017

Sorry for late response.

I feel this is very nice PR, but it would be nice if other builders also support SVG format.
So I'd like to improve this PR to use this feature from other builders.
I will try to improve this until next major release. If I could not, I will merge this as it is.

Note: It would be great if we can support other tools for image conversion.

@anarcat
Copy link
Contributor

anarcat commented Jan 15, 2017

the HTML builder already supports SVG. i suspect it may work with ePUB, depending on the reader, but I haven't tested that directly.

while it would be great to have more generic support for arbitrary file formats, this can quickly get into a very complicated project: witness imagemagick and how many security vulnerability it has. i would personally keep only to a few formats (PNG, JPG, SVG) and convert SVGs as necessary.

an alternative would be simply to refuse SVG files and explicitely require those to be converted by the user, since, in the end, that is what we effectively do for them...

also note that certain tools use librsvg's rsvg-convert to convert files. it may be better than Inkscape, because there are less dependencies...

in any case, conflicts need to be resolved here first. @alyjak are you still working on this PR?

thanks all!

@tk0miya
Copy link
Member

tk0miya commented Jan 15, 2017

the HTML builder already supports SVG. i suspect it may work with ePUB, depending on the reader, but I haven't tested that directly.

We already have many builders. And some builders also does not support SVG images. For example devhelp, texinfo and other builders. Also 3rd party builders might not support them.
It is ineffecient to add SVG support to each builder. I prefer to add conversion mechanism to sphinx-core.

Anyway, we still have 4-6 months duration until next major release; Sphinx-1.6. So I would not merge this for a while even if resolved.

@tk0miya tk0miya added this to the 1.6 milestone Jan 15, 2017
@alyjak
Copy link
Contributor Author

alyjak commented Jan 20, 2017

Sorry for the late response, i'll submit the updated PR later tonight

@jfbu
Copy link
Contributor

jfbu commented Feb 6, 2017

memo: for mac os x, a note could be added that users should install a compiled binary, because the bundled (from http://www.inkscape.org) Mac OS X application working under XQuartz requires full path filenames when executed from the command line for input and output https://bugs.launchpad.net/inkscape/+bug/1449251

edit: perhaps this changes with 0.92 release and its macports install (not tested)

hpk42 pushed a commit to hpk42/muacrypt that referenced this pull request Feb 23, 2017
* replace .svg images by .* in order that `make latexpdf` not to fail.
* while [sphinx-doc #2859](sphinx-doc/sphinx#2859 is merged, add rule in Makefile to generate pdfs from svgs.
* `make latexpdf` will still fail if inkscape is not installed. The alternative will be to do not include the rule "images" in "latexpdf" and include the pdfs in git.
hpk42 pushed a commit to hpk42/muacrypt that referenced this pull request Feb 23, 2017
* replace .svg images by .* in order that `make latexpdf` not to fail.
* while [sphinx-doc #2859](sphinx-doc/sphinx#2859 is merged, add rule in Makefile to generate pdfs from svgs.
* `make latexpdf` will still fail if inkscape is not installed. The alternative will be to do not include the rule "images" in "latexpdf" and include the pdfs in git.
@tk0miya tk0miya mentioned this pull request Mar 10, 2017
except OSError:
pass
if not self.svg_support:
self.info("Latex builder initialized without svg support. You can"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would print this line only if the project actually contains SVG images (i.e. move it below).

Also, please capitalize “LaTeX” and “SVG” properly.

@alyjak
Copy link
Contributor Author

alyjak commented Mar 18, 2017

@mitya57, thanks for the feedback, I have updated the warning to capitalize LaTeX and SVG correctly, and I have moved the warning to only display if inkscape is not present and there are SVG images in the build

@mitya57
Copy link
Contributor

mitya57 commented Mar 20, 2017

Thank you. In our project we would like to use some SVG images too, so I fully support the idea of this PR.

@tk0miya
Copy link
Member

tk0miya commented Apr 22, 2017

I just proposed a new extension; sphinx.ext.imgconverter in #3653.
I used Imagemagick to do that. But it also allows you to convert images with your tools if you make an extension.
Please give a comment if possible.
Thanks,

@tk0miya
Copy link
Member

tk0miya commented Apr 23, 2017

Now I merged #3653 instead.
Thank you for proposal. Actually, this affected to my PR.

@tk0miya tk0miya closed this Apr 23, 2017
@mitya57
Copy link
Contributor

mitya57 commented Apr 23, 2017

@tk0miya Sorry, but imagemagick is not a replacement for inkscape. Imagemagick rasterizes the SVG image, so the text layers are lost, and the resulting image is not scalable.

The correct way to use SVG images with LaTeX is converting them to PDF with Inkscape or other program that understands vector graphics.

@tk0miya
Copy link
Member

tk0miya commented Apr 25, 2017

I know, inkscape is a better choice. But there are many image converter tools. To use them, #3653 provides a way to enhance image converter. So you are able to make a converter using inkscape.
I think sphinx.ext.imgconverter is a reference implementation of image converters.

@mitya57
Copy link
Contributor

mitya57 commented Apr 26, 2017

OK, that is better. But it would be still nice if SVG to PDF using inkscape was available out of the box.

@tk0miya
Copy link
Member

tk0miya commented Apr 26, 2017

Absolutely, but we don't have enough time to maintain all of extensions as you know. So our basic strategy is make sphinx extensible and slim.

juga0 added a commit to juga0/autocrypt that referenced this pull request Nov 4, 2017
* replace .svg images by .* in order that `make latexpdf` not to fail.
* while [sphinx-doc #2859](sphinx-doc/sphinx#2859 is merged, add rule in Makefile to generate pdfs from svgs.
* `make latexpdf` will still fail if inkscape is not installed. The alternative will be to do not include the rule "images" in "latexpdf" and include the pdfs in git.
@Balletie
Copy link

Balletie commented Mar 13, 2018

Found this thread, made this to convert my SVG images to PDF using rsvg-convert:

https://gist.github.com/Balletie/a3ccf434f8ec492e47fff4589225d5cb

Just put it somewhere in sys.path so that it can be imported by Sphinx.

Edit: added inkscape one too

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
builder:latex type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants