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
649 stars 98 forks source link

Implement textLength support #160

Closed Mrfence97 closed 3 years ago

Mrfence97 commented 3 years ago

This PR adds support for the textLength attribute for <text> nodes, solving issue #69.

The length is set by inserting appropriate spaces of length charSpace between characters. To calculate charSpace, the length ('default length') of the text is calculated using measureTextWidth() as if textLength wasn't applied.

The implementation when the <text> node just contains plain text is straightforward. However, if <tspan>s are present, the implementation is more complicated to account for e.g. different font sizes, as the default length must be calculated for each <tspan> at a time and then summed. Note this means that TextChunks aren't immediately put() to the PDF when a new chunk starts, instead they are stored in an array so these width calculations can occur beforehand.

Note textLength implementations differ between browsers, this PR follows the Firefox implementation:

Mrfence97 commented 3 years ago
  • The textLength attribute is also allowed on tSpan elements. Did you consider this? I think this is not too important for now, but if you have time you may have a look at this.

Although the SVG spec states that it's allowed on <tspan>s, implementations appear to differ. Firefox simply ignores any textLengths that appear in a <tspan> and only ever considers the textLength in the parent <text> element. In Chrome, however, if a <tspan> has a textLength and has at least one of x/y/dx/dy set, then the prior content is stretched to fit the original textLength and the subsequent text is fit to the new textLength.

For example, this is how the following SVG is rendered in both Firefox and Chrome:

<rect y="0" width="200" height="20" fill="transparent" stroke="black"/>
<text textLength="200" y="20" font-family="times">Hello, <tspan y="40" textLength="100" fill="red">wor</tspan><tspan fill="yellow">ld!</tspan></text>

image

The PR matches Firefox and it I think it would be significant additional work to also implement the Chrome behaviour.

I'll add a test case that includes this example and also ones that set x and y. I'll also comment that it doesn't render correctly in Chrome!

I'll also verify what's causing the checks to fail, there should be no changes to any elements that don't have a textLength set so I'm assuming it'll just be floating point errors.

Mrfence97 commented 3 years ago

After resetting back to using tSpanCount, the test errors were all due to small numerical errors in four tests:

  1. complete-diagram2
  2. display-none-and-visibility-inheritance
  3. title-element
  4. xml-space

Switching back to this.element.childNodes.length - 1 a total of 10 tests fail (including those ones above). I haven't included the updated reference PDFs in the commit in case you wanted to review the differences (which will likely be more than just floating point errors). The very latest commit just adds the updated reference PDFs so you could checkout ea316e0 to look at the errors yourself.

I've also changed the regex and included a test case that sets x/y in a <tspan>.

Mrfence97 commented 3 years ago

It appears that my machine generates different reference PDFs to the test server...

I've pushed the reference PDFs that make all tests pass, but it seems the server generates different PDFs. Could this be because I'm running Windows? I've deleted and reinstalled all node packages and still all my tests pass. Strange goings on indeed.

HackbrettXXX commented 3 years ago

Seems like it did... thanks for this PR :)

Mrfence97 commented 3 years ago

Great, thanks for merging! Is there a road-map for when the next release will be made?

HackbrettXXX commented 3 years ago

I would like to coordinate this release with the next jsPDF release, which I will hopefully be able to release within the next 2-3 weeks or so.

yGuy commented 3 years ago

@Mrfence97 - thank you very much for the great work and your contribution! 'would love to see more quality PR requests. Let us know if there is a way we can say thank you and provide you with more awesome tasks ;-)