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: page dpi #2771

Merged
merged 2 commits into from
Jun 24, 2024
Merged

fix: page dpi #2771

merged 2 commits into from
Jun 24, 2024

Conversation

nikischin
Copy link
Contributor

@nikischin nikischin commented Jun 2, 2024

Fix for dpi issue implemented with #1869

fixes #1953
fixes #2006
fixes #2668

Unfortunately wasn't able to get it linked locally so this is untested tested. Maybe someone could test or have some tip how to get the package tested locally. I followed the steps in the CONTRIBUTION.md and it says it's linked but it does not detect changes using yarn watch and doesn't make any changes in my test setup. yarn bootstrap is not working for me but for my understanding also should not be needed anymore?

Edit: I was able to test this not in local development but in my build and it does the desired job.

How to test:

  • Set up PDF with <Image style={{width: 100}} /> and <Text style={{fontSize: 12}}/> and some basic style. <page size="A4">
  • Have a look at the result (it's with 72 dpi)
  • Change page dpi to 300: <page size="A4" dpi={300}>

Current behavior: (bug)

  • After dpi change font gets smaller and page size gets multiplied. So instead of 210x297mm we have 875 x 1238 mm.

Expected behavior:

  • After dpi change layout should remain visually unchanged. Text should be same size and also image.
  • Page size should remain 210x297mm, while resulution should be 595x842px in 72 dpi and 2.480x3.508px in 300dpi.

Copy link

changeset-bot bot commented Jun 2, 2024

🦋 Changeset detected

Latest commit: f5b8eca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@react-pdf/layout Minor
@react-pdf/pdfkit Minor
@react-pdf/render Minor
@react-pdf/stylesheet Minor
@react-pdf/types Minor
@react-pdf/renderer Patch
@react-pdf/svgkit Patch
@react-pdf/font Patch
@react-pdf/examples Patch
@react-pdf/e2e-node-cjs Patch
@react-pdf/e2e-node-esm Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@nikischin
Copy link
Contributor Author

nikischin commented Jun 2, 2024

@wojtekmaj @diegomura do you have some hint for me maybe how to get the link working locally in order to test this?

Edit: While I did not get it run locally I managed to include my link implemented in my build and tested it there and it does the job and I also implemented some improvement. So you can either pass pixel size or the pre-defined strings.

Like this I am currently testing: cd ../layout;yarn build;cd ../renderer;yarn build And then yarn build in my test repo.

I was wondering if we would like to implement support for other units as well, as we do have for styling already. Shouldn't be too complicated, I could easily implement this.

@nikischin nikischin marked this pull request as ready for review June 2, 2024 12:54
@nikischin nikischin changed the title Possible fix for dpi issue implemented with #1869 Fix dpi issue implemented with #1869 Jun 2, 2024
@diegomura diegomura changed the title Fix dpi issue implemented with #1869 fix: dpi issue implemented with #1869 Jun 24, 2024
@diegomura diegomura changed the title fix: dpi issue implemented with #1869 fix: page dpi Jun 24, 2024
Copy link
Owner

@diegomura diegomura left a comment

Choose a reason for hiding this comment

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

utACK

@diegomura diegomura merged commit 8e6a832 into diegomura:master Jun 24, 2024
@nikischin nikischin deleted the bugfix/customDpi branch June 24, 2024 06:34
@whitestardeveloper
Copy link

I'm looking forward to the next version.
When will you publish it? DPI correction will be very useful to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants