pdfme / pdfme

A TypeScript based PDF generator library, made with React.
https://pdfme.com
MIT License
2.19k stars 194 forks source link

Fix text height rendering for PDF generation #196

Closed steffancarrington closed 10 months ago

steffancarrington commented 10 months ago

Before

https://github.com/pdfme/pdfme/assets/45564901/31bf079c-d667-4c62-afe6-381667e7ccd2

After

https://github.com/pdfme/pdfme/assets/45564901/b68da68b-34f0-40e1-88b7-f2a31aec9e81

hand-dot commented 10 months ago

I'll check this PR behavior on the playground from now. after check, I'll comment.

hand-dot commented 10 months ago

Hey @steffancarrington Did you try npm run build on play-ground?

I got some errors on the production build.

steffancarrington commented 10 months ago

Hey @hand-dot ,

Thanks for reviewing, will look at addressing this feedback today 👍

hand-dot commented 10 months ago

Hey @steffancarrington , Thanks!

hand-dot commented 10 months ago

@steffancarrington @jbarton123 Could you please explain what exactly the point of issue in the pdfme/pdf-lib library when calculating the heightOfFontAtSize()

hand-dot commented 10 months ago

I found another problem. once click text type schema, the text position slightly shifts down from the initial text position.

https://github.com/pdfme/pdfme/assets/24843808/5412874a-ce3a-4674-a1bd-a56b91c54db1

steffancarrington commented 10 months ago

@steffancarrington @jbarton123 Could you please explain what exactly the point of issue in the pdfme/pdf-lib library when calculating the heightOfFontAtSize()

Hey @hand-dot ,

The update that was made to the heightOfFontAtSize() in the helper was to account for the descent value * the font scale, as this wasn't being considered in the pdf-lib version of the method.

The change that was introduced was the following (to ensure that we were always taking into account the descender value of the text):

Before height -= Math.abs(descent) || 0;

After height -= Math.abs(descent * scale) || 0;

peteward commented 10 months ago

Hi @hand-dot, We did some visual tests with this updated code. The vertical position of fonts is much better, although not perfect, especially at smaller font sizes. I don't think there's anything we can do to improve it further though due to the difference in browser font rendering. Testing Artwork Builder pdfme fonts.pdf