plotly-graph PDF with electron's printToPDF #6

etpinard · 2017-08-22T18:06:26Z

See commit 54b4173 for the details and see comparison with the batik outputs here.

Note that, electron's printToPDF is way faster too. Generating all 411 mocks took about 8 minutes using batik and just under 3 minutes using printToPDF.

- adding util/remote makes the renderer's remote module easily mockable using sinon. - add stubProp common test util, to e.g. reduce iframeLoadDelay in tests

- when *batik* option isn't defined, use win.webContents.printToPDF - step-by-step: + use svg image string to render image in DOM + call window.getSelection.selectAllChidren to grab only img div + call printToPDF on img div

etpinard · 2017-08-22T18:08:30Z

To me 👀 , printToPDF and batik output very similar PDFs, but comparing PDFs side-by-side isn't the most precise.

For all images below: left: batik / right: printToPDF

printToPDF seems to give better results for graphs with outlined legends:

bounding boxed in general look a lot better in the printToPDF outputs:

Both outputs are wrong (they both show little gap(s)) for bar_bargap0:

batik is bad at doing carpets:

batik doesn't like parcoords tick fonts:

printToPDF has issues with some gl3d grid lines:

printToPDF doesn't seem to like polar text bg:

etpinard · 2017-08-22T18:14:02Z

src/component/plotly-graph/render.js

+    printSelectionOnly: true,
+    pageSize: {
+      width: (imgOpts.width) / 0.0035,
+      height: (imgOpts.height) / 0.0035


pageSize here expects numbers in micrometer, plotly.js set width and height in pixels. I took this number from this source. I gives ok results, but not perfect.

Comparing the batik and electron outputs of a graph with non-white bg illustrates the issue:

batik on the left / printToPDF on the right.

Commit 5992a8b improves this.

But printToPDF still output a (small white) margin:

Would this be a deal-breaker for printToPDF?

with 135cb1b

those white margins are gone (actually, they're still there, but now they appear with background-color: fullLayout.paper_bgcolor)

Thanks for the deep dive into this one @etpinard - given all the other improvements we get with printToPDF I think this extra bit of margin, now that it keeps the right color, is acceptable.

Might be worthwhile at some point taking another look at this but for the first release this is great.

Might be worthwhile at some point taking another look at this

For sure. I'll open an issue as soon as this PR is merged.

bpostlethwaite · 2017-08-23T14:24:48Z

Thanks for the gist https://gist.github.com/etpinard/1ce4f6e4c089d5bfc426485638d0abda - nice work on the comparison.

I am impressed how well the Electron printToPDF did without much tweaking. My gut says that it is close enough "out of the box" to invest the time making it even better.

I'll let @alexcjohnson's discerning 👀 scrutinize those pdfs and for the final judgement on that one :)

etpinard · 2017-08-23T14:31:50Z

My gut says that it is close enough "out of the box" to invest the time making it even better.

I think so too, especially that the out-of-the-box is much faster 🐎

- which is functional on branch https://github.com/plotly/plotly.js/compare/to-image-scale

- only expose svg2pdf routine! - i.e. 🔪 eps logic (will be in own pdftops util) and scale logic (to be done in renderer hopefully)

etpinard · 2017-08-28T22:04:03Z

by the way, gistclone is pretty awesome for cloning gists w/o having to go to the gist page (which might take a long time to load).

- use correct px-by-micron logic - add margin offset (that appears to work ok)

etpinard · 2017-08-30T17:00:01Z

Fonts (after installing them locally - ref #11) are looking sharp:

etpinard · 2017-08-31T02:27:47Z

Oh and I should add, switching to electron's printToPDF will ✅ plotly.js' oldest bug plotly/plotly.js#109 🎉 ⏲️ 🎉

Screenshot of pdf output:

alexcjohnson · 2017-08-31T13:32:06Z

Some vacation-delayed comments on the images above:

printToPDF seems to give better results for graphs with outlined legends:
bounding boxed in general look a lot better in the printToPDF outputs:

So much better to be generating PDFs in the same environment the fonts were originally rendered in - major win!

Both outputs are wrong (they both show little gap(s)) for bar_bargap0:

We can ignore this for now as it's not new, but would be worth investigating later. This is happening because we apply shape-rendering: crispEdges via CSS (classed('crisp', true)) rather than inline styling. Though I notice that's not applied consistently anymore, I see a lot of places we've used inline styling. Anyway the idea, which may need refining particularly in light of electron, was to explicitly drop it from vector output so we get higher precision when the user can do stuff like put the vector image into another container and zoom or rotate it. Unfortunately that means these subpixel gaps between bars return, as it was only crispEdges that hid them. Not sure that this even has a solution in theory, when you consider the possibility of rotating the vector image on screen.

FYI Drawing.crispRound is also explicitly short-circuited in static plots - this is even more important in fact, so that vector output has items positioned & sized correctly to sub-pixel precision.

batik is bad at doing carpets:
batik doesn't like parcoords tick fonts:

How did we miss these for so long? Nice wins for electron anyway... but I worry that svg exports brought into another environment (illustrator and its ilk?) might cause this to surface again - perhaps we could figure out what carpet & parcoords do differently here and make them match everything else?

printToPDF has issues with some gl3d grid lines:

Is it not getting the same plotGlPixelRatio as we use normally?

printToPDF doesn't seem to like polar text bg:

polar text gets a white shadow behind it, which seems to get corrupted by printToPDF... but it looks like Batik is dropping the shadow entirely, which is arguably less bad but still not correct. Parcoords is supposed to have a similar shadow on tick labels, which it appears both printToPDF and Batik miss. Would be worth spending some time figuring out if there's a more robust way to apply this shadowing.

- open new window for each pdf export which: + 🔪 the extraneous margin-left in printSelectionOnly exports + no longer freezes up when parallelLimit > 1 - we still need to add a few pixel to the export pageSize + but now hide those artificial margin by setting and exporting the graphs' bgColor.

etpinard · 2017-08-31T15:02:03Z

src/component/plotly-graph/render.js

  const pxByMicrometer = 0.00377957517575025
-  const inducedMarginOffset = 18
+  const offset = 6


We still need this unfortunately to make sure the exported pdf always fits on one page.

Like I said to @alexcjohnson in a private convo, I believe electron (or chromium?) is messing up their pixel to micron computation somewhere. I should try to write up a bug report for them. Oh well, in the meantime hacks appears to work.

To see it the results:

npm i -g gistclone gistclone https://gist.github.com/etpinard/1ce4f6e4c089d5bfc426485638d0abda cd "plotly-graph PDF comparison" <pdf-viewer-of-your-choice> all-electron.pdf

etpinard · 2017-08-31T15:04:11Z

src/component/plotly-graph/render.js

+    // we can't set image src into html as chromium has a 2MB URL limit
+    // https://craignicol.wordpress.com/2016/07/19/excellent-export-and-the-chrome-url-limit/
+
+    win.loadURL(`data:text/html,${html}`)


Setting this to data:image/svg+xml,${imgData} wasn't an option unfortunately as Chromium sets a limit of 2MB for URLs, and yes, SVGs of size > 2MB do happen.

etpinard · 2017-08-31T15:24:37Z

How did we miss these for so long?

We don't test PDF generation for all our mocks (just these mocks to be precise) and even then we only check if the export file size is bigger than some number.

It might worth spending some time adding pdf diff test to our suite (e.g. using diffpdf). I suspect once plotly-image-exporter is out, pdf exports will be pretty popular.

but I worry that svg exports brought into another environment (illustrator and its ilk?)

Good point about Illustrator, we should check 🔬

printToPDF has issues with some gl3d grid lines:
Is it not getting the same plotGlPixelRatio as we use normally?

This is probably the last blocking issue remaining (well this and the polar font thing, but it's polar so 😏 ). To make matters worse, those grid lines look different when generated with xvfb and regular headfull electron. Left w/o xfvb / right w/ xfvb:

Hopefully tweaking plotGlPixelRatio will do the trick 🙂

etpinard · 2017-08-31T18:08:09Z

src/component/plotly-graph/constants.js

@@ -30,6 +30,9 @@ module.exports = {

  mathJaxConfigQuery: '?config=TeX-AMS-MML_SVG',

+  // config option passed in render step
+  plotGlPixelRatio: 3,


Hopefully tweaking plotGlPixelRatio will do the trick 🙂

Yep, it did:

etpinard · 2017-08-31T20:49:28Z

src/util/batik.js

- * - https://xmlgraphics.apache.org/batik/tools/rasterizer.html
- * - http://archive.apache.org/dist/xmlgraphics/batik/
- */
-class Batik {


One for the ages.

🌅 It's a brand new day 💯

- 🔪 plotly-graph-exporter stdout msg test (for now, this fails on CI)

etpinard · 2017-08-31T21:49:24Z

.circleci/config.yml

@@ -0,0 +1,33 @@
+version: 2


Circle 2.0 is blazing fast 🐎 - see https://circleci.com/docs/2.0/

etpinard · 2017-09-01T22:03:42Z

Ok. I gonna go ahead and merge this thing to keep the wheels spinning.

Feel free to open an issue if you want to discuss anything printToPDF related.

etpinard added 2 commits August 22, 2017 11:34

add util/remote + mucho plotly-dashboard tests

090b200

- adding util/remote makes the renderer's remote module easily mockable using sinon. - add stubProp common test util, to e.g. reduce iframeLoadDelay in tests

add printToPDF renderer logic for plotly-graph

54b4173

- when *batik* option isn't defined, use win.webContents.printToPDF - step-by-step: + use svg image string to render image in DOM + call window.getSelection.selectAllChidren to grab only img div + call printToPDF on img div

etpinard commented Aug 22, 2017

View reviewed changes

pass 'scale' to Plotly.toImage

406b920

- which is functional on branch https://github.com/plotly/plotly.js/compare/to-image-scale

etpinard mentioned this pull request Aug 23, 2017

Alternatives to Batik for scaling images #9

Closed

etpinard added 5 commits August 24, 2017 12:39

rewrite batik util

f69172b

- only expose svg2pdf routine! - i.e. 🔪 eps logic (will be in own pdftops util) and scale logic (to be done in renderer hopefully)

add pdftops util (to convert pdf2eps)

b2b3ea3

add pretest script

b90d7fc

scale PDF pageSize in render

828d2a3

update plotly-graph convert with new batik + pdftops logic

1c9193e

etpinard mentioned this pull request Aug 28, 2017

Image testing with image-exporter electron runner plotly/plotly.js#1972

Closed

Merge branch 'master' into plotly-graph-pdf

f104ad3

improve printToPDF page size logic

5992a8b

- use correct px-by-micron logic - add margin offset (that appears to work ok)

etpinard commented Aug 31, 2017

View reviewed changes

etpinard added 2 commits August 31, 2017 14:05

put pdf page image load timeout value in constants.js

649f97f

add plotGlPixelRatio to improve look of gl3d grid lines

23cfc95

etpinard commented Aug 31, 2017

View reviewed changes

etpinard added 3 commits August 31, 2017 16:15

fixup tests

acb007b

🔪 batik as printToPDF 🏆

96b1fbc

add test convert + Pdftops instance test

f8708a2

etpinard commented Aug 31, 2017

View reviewed changes

etpinard force-pushed the plotly-graph-pdf branch from 1aeea7b to 880ffd3 Compare August 31, 2017 21:27

add circleci 2.0 config

11b67c3

- 🔪 plotly-graph-exporter stdout msg test (for now, this fails on CI)

etpinard force-pushed the plotly-graph-pdf branch from 4b18d37 to 11b67c3 Compare August 31, 2017 21:46

etpinard commented Aug 31, 2017

View reviewed changes

.circleci/config.yml

@@ -0,0 +1,33 @@

version: 2

Copy link

Contributor Author

etpinard Aug 31, 2017

Choose a reason for hiding this comment

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

Circle 2.0 is blazing fast 🐎 - see https://circleci.com/docs/2.0/

etpinard added 2 commits August 31, 2017 18:25

📚

47614f9

rm unnecessary else branch

f93bd57

etpinard merged commit ce681ad into master Sep 1, 2017

etpinard deleted the plotly-graph-pdf branch September 1, 2017 22:03

This was referenced Sep 1, 2017

Stay aware of electron printToPDF improvements, to get rid of print background hack #12

Open

Investigate ways to do text shadowing #14

Open

Look for ways to improve PDF output of 'crispEdges' SVG shape rendering #15

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plotly-graph PDF with electron's printToPDF #6

plotly-graph PDF with electron's printToPDF #6

etpinard commented Aug 22, 2017

etpinard commented Aug 22, 2017 •

edited

Loading

etpinard Aug 22, 2017 •

edited

Loading

etpinard Aug 30, 2017

etpinard Aug 31, 2017

alexcjohnson Aug 31, 2017

etpinard Aug 31, 2017

bpostlethwaite commented Aug 23, 2017 •

edited

Loading

etpinard commented Aug 23, 2017

etpinard commented Aug 28, 2017 •

edited

Loading

etpinard commented Aug 30, 2017

etpinard commented Aug 31, 2017

alexcjohnson commented Aug 31, 2017

etpinard Aug 31, 2017

etpinard Aug 31, 2017 •

edited

Loading

etpinard commented Aug 31, 2017

etpinard Aug 31, 2017

etpinard Aug 31, 2017

alexcjohnson Aug 31, 2017

etpinard Aug 31, 2017

etpinard commented Sep 1, 2017 •

edited

Loading

plotly-graph PDF with electron's printToPDF #6

plotly-graph PDF with electron's printToPDF #6

Conversation

etpinard commented Aug 22, 2017

etpinard commented Aug 22, 2017 • edited Loading

etpinard Aug 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bpostlethwaite commented Aug 23, 2017 • edited Loading

etpinard commented Aug 23, 2017

etpinard commented Aug 28, 2017 • edited Loading

etpinard commented Aug 30, 2017

etpinard commented Aug 31, 2017

alexcjohnson commented Aug 31, 2017

Choose a reason for hiding this comment

etpinard Aug 31, 2017 • edited Loading

Choose a reason for hiding this comment

etpinard commented Aug 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard commented Sep 1, 2017 • edited Loading

etpinard commented Aug 22, 2017 •

edited

Loading

etpinard Aug 22, 2017 •

edited

Loading

bpostlethwaite commented Aug 23, 2017 •

edited

Loading

etpinard commented Aug 28, 2017 •

edited

Loading

etpinard Aug 31, 2017 •

edited

Loading

etpinard commented Sep 1, 2017 •

edited

Loading