yWorks / svg2pdf.js

A javascript-only SVG to PDF conversion utility that runs in the browser. Brought to you by yWorks - the diagramming experts
MIT License
654 stars 101 forks source link

fails to render part of svg-diagram on pdf #94

Closed Fadiabb closed 5 years ago

Fadiabb commented 5 years ago

Hallo,

Using this html page draws a svg-diagram on screen: https://gist.github.com/Fadiabb/33ba13aa4ce52baaf6d5030a1a8f5fe0 fails-to-render-part-of-diagram

after rendering to pdf using svg2pdf() function the lower part does not render:

fails-to-render-part-of-diagram-pdf

Which is expected to render the complete svg element on pdf

I'm using git version: 38f10d329807d2a671e78695a4504dc3ebbb7a33 Firefox: 60.5.1esr (64-bit)

yGuy commented 5 years ago

Please create a reduced test-case - ideally executable using jsfiddle or a similar tool. But make sure that the test-case is reduced to the max. This will probably already help you understand what feature specifically isn't working as expected. Thank you for your help.

Fadiabb commented 5 years ago

Analysis: the element <rect rx="0.25em" ry="0.25em" fill="#fff" stroke="#ccc"></rect> leads to stop render the next coming elements. adding width and height will solve the problem and render all the elements. https://gist.github.com/Fadiabb/33ba13aa4ce52baaf6d5030a1a8f5fe0#file-svg_2_pdf_example3-html-L363

Reproduce with reduced simple example

<svg>
        <rect rx="0.25em" ry="0.25em" fill="red"></rect>
        <rect
          rx="0.25em"
          ry="0.25em"
          fill="red"
          width="30px"
          height="30px"
        ></rect>
</svg>

the missing width and height in the first element causes svg2pdf() to stop render the next elements

HackbrettXXX commented 5 years ago

The SVG spec does not specify a default value for the width/height values. Intuitively, the defaults should be zero, of course. The spec also does not clearly say if these attributes are required or not. So I'm not sure what the expected behavior should be. Could you verify what the expected behavior is?

Fadiabb commented 5 years ago

It would be better if the following svg-elements get rendered normally on pdf in case these values are missing somewhere.

yGuy commented 5 years ago

It would be better if we only had to deal with valid svg files :-)

Pull requests are welcome!

bernhardreiter commented 5 years ago

Some background: The SVG is created by using d3js, so we probably will not be the only ones encountering it. Other SVG viewers like the browser just keep rendering.

What about logging a warning and then continue rendering?

yGuy commented 5 years ago

Some background: The SVG is created by using d3js, so we probably will not be the only ones encountering it. Other SVG viewers like the browser just keep rendering.

We're not saying that this should not or cannot be improved. However the workaround is obvious: add the attribute.

What about logging a warning and then continue rendering?

That might be a good idea. Actually fixing it would be even better. We are open to accept pull-requests as the tag indicates! I did half of the work already by researching the spec and existing implementations ;-) We're not saying we don't want the situation to be improved, here, however, and I think the actual behavior should be like this:

The attribute is missing, therefor the default value auto should apply (according to https://developer.mozilla.org/en-US/docs/Web/SVG/Element/rect ) And auto in the context of SVG mostly (except the rectangle has children) means that value will be zero and this again means that the rectangle will not be painted at all (see https://www.w3.org/TR/SVG11/shapes.html#RectElementWidthAttribute )

Here is a possible implementation that considers auto: https://github.com/NYTimes/svg-crowbar/pull/26/commits/9aac11527bdc0644b7106c90af3bac29945f238d

bernhardreiter commented 5 years ago

@yGuy @HackbrettXXX thanks again for svg2pdf, we like the component. And thanks for taking the time and looking into the issues.

We (as in @Fadiabb and me) do report these points for documentation and discussion. I agree that is it not the most important point because it is not really useful to have in an SVG. :)