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

Allow change of svg-to-PNG converter [was: Why not use "svg2png" , pure node without JAVA depend] #59

Closed
everyx opened this issue Dec 11, 2014 · 15 comments

Comments

@everyx
Copy link

everyx commented Dec 11, 2014

I hava some trouble in my vps about java environment configuration, and i find the node package "svg2png" and modify some code, i'll submit a pull request about this for a review.

sorry for my poor english.

@pkra
Copy link
Contributor

pkra commented Dec 12, 2014

The reason is that svg2png depends on PhantomJS and one of core requirements of MathJax-node was to get away from PhantomJS.

AFAIK, there is no pure JS svg2png converter out there. In our experience, Batik performance is better and more stable compared to PhantomJS (but that might be worth re-evaluating).

In any case, we should make it easier to switch converters. I'm not sure how the others feel but simply dropping Batik does not seem ideal to me at this point.

@everyx
Copy link
Author

everyx commented Dec 12, 2014

@pkra Thanks for you reply, close this issue.

@everyx everyx closed this as completed Dec 12, 2014
@pkra
Copy link
Contributor

pkra commented Dec 12, 2014

Let me re-open and rename this. I definitely want us to abstract the PNG conversion; it should be as converter independent as possible. Since svg2png is authord by Domenic it would be a good model to follow for an NPM bridge to Batik.

@pkra pkra reopened this Dec 12, 2014
@pkra pkra changed the title Why not use "svg2png" , pure node without JAVA depend Allow change of svg-to-PNG converter [was: Why not use "svg2png" , pure node without JAVA depend] Dec 12, 2014
@everyx
Copy link
Author

everyx commented Dec 12, 2014

👍

@pkra
Copy link
Contributor

pkra commented Feb 5, 2015

See also #65 (comment)

@SamyPesse
Copy link

👍 svgexport works great

@pkra
Copy link
Contributor

pkra commented Feb 5, 2015

@SamyPesse see my earlier comment regarding our original requirements.

@physikerwelt
Copy link
Contributor

I agree that both solutions java and phantomjs are not ideal.
I think one of svgexports advantages is that no file needs to be written on "disk".
A node only solution would be awesome, but I guess it's a very challanging tasks, which can not be done a few after work sessions.
(ping @gwicke)

@pkra
Copy link
Contributor

pkra commented Feb 16, 2016

Some recent observations

@pkra
Copy link
Contributor

pkra commented Feb 16, 2016

Note: I haven't actually tested if the canvas solution for jsdom would be sufficient.

@gwicke
Copy link

gwicke commented Feb 16, 2016

Some quick notes from my end:

  • We use (a patched version of) rsvg for SVG thumbnailing in production. It is working okay, but lacks support for some features like HTML-in-SVG. There is some RSVG-related discussion on https://phabricator.wikimedia.org/T40010.
  • Latest phantomjs (2.0) is based on relatively recent Blink, so likely has similar SVG support as current Chrome. However, binding support for node is mostly still using phantomjs 1.9. We don't currently use phantomjs in production, but might reconsider it for tasks like HTML-to-PDF in the future.
  • I can't vouch for the quality or performance of the jsdom canvas solution. The dependencies are manageable (we have a few services using it).

@d00rman
Copy link

d00rman commented Feb 17, 2016

My 2 cents here would be to develop an extension point for MathJax so that people can hook up and test whatever suits them better, while still having a sane default. What is that sane default is arguable, ofc, but the way I see it is that it's sane if it doesn't introduce many (especially binary) external dependencies regardless of the performance penalties or feature set. These could be then mitigated on a case-per-case basis with specialised converters.

@pkra
Copy link
Contributor

pkra commented Feb 17, 2016

Thanks for the feedback, @gwicke @d00rman -- much appreciated!

@pkra
Copy link
Contributor

pkra commented Feb 18, 2016

See #174 for a first attempt.

@pkra pkra added this to the What comes next milestone Feb 18, 2016
@pkra
Copy link
Contributor

pkra commented Apr 11, 2016

Closed in favor of #205.

@pkra pkra closed this as completed Apr 11, 2016
@pkra pkra removed this from the What comes next -- towards v1.0 milestone Feb 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants