[WIP] Modularization of PNG generation

[pkra]

• abstract the png generation
  • mj-single
  • mj-page
• move batik-specific code into a separate module
• add librsvg and svg2png based modules.
• decide on new default
  • update package.json
• update Readme

This fixes #59.

[pkra]

@dpvc @zorkow the main changes are ready for review. I marked it WIP for the organizational details.

[pkra]

FYI, the tests fail because Batik won't be installed on Travis at the moment. They pass locally for me.

[pkra]

To clarify: the PR currently only modifies mj-single. Once that part is reviewed, I'll reproduce it for mj-page.

[pkra]

While the test that's in this PR is universal (in that it succeeds with all three converter modules), I'm wondering if this approach is not actually taking things far enough.

Should the PNG conversion be ripped out completely instead? Just pass the SVG to your own lib and done. It's a bit radical, I admit. But for mj-single it seems sensible; PNG generation is really not mathjax-y.

[d00rman]

You can avoid the stream altogether here since you already have the data to render in svg, so simply:

var svgRender = new rsvg(svg);

[d00rman]

Note that this still allows you to keep the block on L11 (error checking), because the rsvg object is a stream itself.

[pkra]

Thanks! Since I'm not too experienced with streams: does L11 catch only errors from the streaming process (which is nice if the string was corrupted somehow) or does it also catch errors that rsvg itself has produced?

[d00rman]

According to the lib's code, both would trigger L11.

[pkra]

Thanks, @d00rman !

[d00rman]

Should the PNG conversion be ripped out completely instead? Just pass the SVG to your own lib and done. It's a bit radical, I admit. But for mj-single it seems sensible; PNG generation is really not mathjax-y.

I'm inclined to think the same. Conversion in itself is not (or should not, IMO) be MathJax' problem. But, a pragmatic solution might be to allow users to specify which converter to use in the init phase. Ideally, we'd want the conversion engine or lib to be a peer dependency of MathJax, i.e. it should not be required by MathJax itself, because, as we see from this PR, that entails a strong dependency on either:

• a binary module / package; or
• a large Java-based solution

If there were a really slow, but light dependency that could be used as the sane default, that would be awesome, but I have a feeling that here explaining to users how to achieve the conversion is the right way.

In that context clearly defining an interface for the conversion (which I think has been achieved implicitly in this PR) is a good practice, because that means additional converters can be only an npm install away, as it makes it rather easy to package different converters separately: one can easily imagine lib/mj-batik and lib/mj-librsvg being separate npm packages, and then the user just supplies them to MathJax during the initialisation phase.

[pkra]

@d00rman thanks for the comments!

Ideally, we'd want the conversion engine or lib to be a peer dependency of MathJax

I've never used peer dependencies. How would that work?

one can easily imagine lib/mj-batik and lib/mj-librsvg being separate npm packages, and then the user just supplies them to MathJax during the initialisation phase.

I think I agree but I'm not sure. FWIW, I had been thinking a separate module mj-svg2png could look something like

var mj = require('./lib/mj-single');
var svg2png = require('svg2png');

exports.mj2png = function(options, callback) {
  mj.start();
  mj.typeset(options, function(data) {
    var sourceBuffer = new Buffer(data.svg, "utf-8");
    var returnBuffer = svg2png.sync(sourceBuffer, {
      width: data.width,
      height: data.height
    });
    data.png = "data:image/png;base64," + returnBuffer.toString('base64');
    callback(data);
  });
};

(note: just a mockup).

[dpvc]

You've lost the use of the dpi setting that is part of the current API by changing it to scale*72 here, so that is a breaking change. (Also, the default dpi passed by the command-line tools is based on the ex size, with the default ex of 6 producing a dpi of 96, so this will produce different sized images from the current version. I worked a bit to get the dpi values to do the right thing for the given ex size.)

[dpvc]

If you want to use a width and height rather than dpi, batik can do that (according to the man page), so it would be possible to use those instead (and change or remove the command line dpi API, or base the scaling off the dpi, e.g., make scale = dpi / 96). In that case, however, I'd only pass width and height and have the scale multiplied in before svg2png is called so that the width and height here are the true sizes that are being requested.

[pkra]

@dpvc thanks for the review. But before I fix this, perhaps we can first discuss the larger question of moving the entire PNG workflow out of mathjax-node itself.

[dpvc]

I'm OK with removing the PNG stuff (it was a pain to implement), though I suspect you will get some push-back from people currently using it. It might be nice to keep some *2png command-line tools (but with the svg-to-png conversion being done through an external tool in the command-line program rather than in mj-single itself).

[d00rman]

I've never used peer dependencies. How would that work?

NPM supports peerDependencies where one can specify the modules that should be included by the application. A big note here is that the peerDependencies sections will disappear with npm 3.x.

The general idea is that an external package provides the svg2png functionality, which is then related to MathJax-node when require()-ing it (or setting a global variable that would be picked up by it).

I think I agree but I'm not sure. FWIW, I had been thinking a separate module mj-svg2png could look something like

var mj = require('./lib/mj-single');
var svg2png = require('svg2png');

exports.mj2png = function(options, callback) {
mj.start();
mj.typeset(options, function(data) {
var sourceBuffer = new Buffer(data.svg, "utf-8");
var returnBuffer = svg2png.sync(sourceBuffer, {
width: data.width,
height: data.height
});
data.png = "data:image/png;base64," + returnBuffer.toString('base64');
callback(data);
});
};
(note: just a mockup).

Yup, something like that 👍

[pkra]

A big note here is that the peerDependencies sections will disappear with npm 3.x.

Ah, right. I'm too used to 3.x already, so I got confused when I read up on peerDepdencies. Makes sense now, thank you!

[pkra]

Closing in favor of #205