Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix text height rendering for PDF generation #196

Merged

Conversation

steffancarrington
Copy link
Collaborator

@steffancarrington steffancarrington commented Jul 17, 2023

  • Font position adjustment logic added to the UI to ensure it displays in the same places as the generated PDF
  • Custom heightOfFontAtSize() method added to correctly render text in generated PDFs.

Before

Screen.Recording.2023-07-17.at.12.32.30.mov

After

Screen.Recording.2023-07-17.at.12.28.57.mov

Copy link
Collaborator

@hand-dot hand-dot left a comment

Choose a reason for hiding this comment

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

Just minor fix, other than this, Looks good.

packages/common/src/helper.ts Outdated Show resolved Hide resolved
@hand-dot
Copy link
Collaborator

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

@hand-dot
Copy link
Collaborator

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

I got some errors on the production build.

packages/generator/src/helper.ts Outdated Show resolved Hide resolved
packages/ui/src/components/Schemas/TextSchema.tsx Outdated Show resolved Hide resolved
@steffancarrington
Copy link
Collaborator Author

Hey @hand-dot ,

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

@hand-dot
Copy link
Collaborator

Hey @steffancarrington ,
Thanks!

@hand-dot
Copy link
Collaborator

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

@hand-dot
Copy link
Collaborator

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

CleanShot.2023-07-18.at.22.25.19.mp4

@steffancarrington
Copy link
Collaborator Author

@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;

@hand-dot hand-dot mentioned this pull request Jul 25, 2023
2 tasks
@hand-dot hand-dot changed the title Fix text height rendering for PDF generation heightOfFontAtSize bug Jul 25, 2023
@hand-dot hand-dot changed the title heightOfFontAtSize bug Fix text height rendering for PDF generation Jul 25, 2023
@hand-dot hand-dot self-requested a review July 27, 2023 01:02
@peteward
Copy link
Collaborator

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

@hand-dot hand-dot merged commit 1bed97e into pdfme:main Jul 27, 2023
hand-dot added a commit that referenced this pull request Jul 30, 2023
* chore: Update font tests & calculateDynamicFontSize

- Updated the font tests and added tests for calculateDynamicFontSize.
- Also refactored code for readability and maintainability.

* [WIP] Update font test and font module to support font alignment.

- The font test was updated to use a shorter input string.
- The `calculateDynamicFontSize` function in the font module was updated to include font alignment calculations.
- The `heightOfFontAtSize` function in the font module was updated to use the provided font size parameter.
- The `getFontKitFont` function in the font module was updated to use the provided fontName from the text schema.
- The `drawInputByTextSchema` function in the generator module was updated to use the `getFontKitFont` and `getFontAlignmentValue` functions from the font module.

* Update generator snapshot pdf

* Update generator snapshot pdf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants