search

yWorks / jsPDF

JavaScript PDF generation to work with SVG
MIT License
56 stars 23 forks source link

Kerning string size bug in getCharWidthsArray #30

Closed chrisregnier closed 4 years ago

chrisregnier commented 4 years ago

From the original project https://github.com/MrRio/jsPDF/issues/2647 Since there doesn't seem to be a lot of movement on the original project I've tried your branch as well and found the same issue.

I'm kinda surprised I couldn't find any other issues that seemed to point to this as a problem with string sizes, so I'm curious if this is actually the expected kerning behaviour that I'm misunderstanding?

yGuy commented 4 years ago

That's probably because this fork was originally created to support svg2pdf and it does not depend on the suspiciuous functionality. I am unsure whether this problem (if it is one) affects svg2pdf but your code sample does look suspicious to me, too. Do you know what kind of code/plugin/jspdf functionality would be affected if this is a bug? Or is this only a convenience method used by devs and not internally?

yGuy commented 4 years ago

I'd say the code is wrong, and this is rather obvious:

https://github.com/MrRio/jsPDF/blob/7d4e2eb075d832407b8113881808b5475234f0aa/src/modules/split_text_to_size.js#L74

If the font has a kerning in the lookup for a pair of characters, it will keep that kerning "correction value" for all upcoming pairs of characters if there is no value in the lookup, which is probably the case for most character pairs. So as soon as you come across a "correction value"/ pair of characters in your string that have a kerning value, all others that follow will be wrong until there is another pair that has a match in the table.

The fix is probably is to simply reset the value to 0 after it has been applied and just pull in the declaration and assingment into the loop or put an else there that sets the value to 0.

HackbrettXXX commented 4 years ago

I agree with @yGuy. We would be happy about a pull request @chrisregnier :)

chrisregnier commented 4 years ago

OK perfect I'll add create a PR for both repos. Thanks