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

pdf.set_display_mode(zoom=...) not working #926

Closed
macdeport opened this issue Sep 17, 2023 · 4 comments · Fixed by #928
Closed

pdf.set_display_mode(zoom=...) not working #926

macdeport opened this issue Sep 17, 2023 · 4 comments · Fixed by #928
Labels

Comments

@macdeport
Copy link

I discovered fpdf2 and tried it out. Thanks for this interesting library.

Describe the bug

Error details
I couldn't make

pdf.set_display_mode(zoom=25)

works to produce a PDF that opens at a scale of 25%...

Minimal code
Please include some minimal Python code reproducing your issue:

from fpdf import FPDF
pdf = FPDF()
pdf.add_page()
pdf.set_display_mode(zoom=50)
pdf.output('test_zoom.pdf')

If you don't know how to build a minimal reproducible example, please check this tutorial: https://stackoverflow.com/help/minimal-reproducible-example

Environment
Please provide the following information:

  • Operating System: Mac OSX
  • Python version: Python 3.9.18
  • fpdf2 version used: 2.7.5

Proposed fix
If I'm not mistaken...

In output.py > _finalize_catalog() after :

 # zoom_mode is a number, not one of the allowed strings:
 zoom_config = ["/XYZ", "null", "null", str(fpdf.zoom_mode / 100)]

add :

    zoom_config = [
        pdf_ref(first_page_obj.id),
        *zoom_config]
@macdeport macdeport added the bug label Sep 17, 2023
@andersonhc
Copy link
Collaborator

Thank you @macdeport for reporting this bug.

I was able to reproduce it and your proposed fix seems to solve the problem. Do you want to submit a PR fixing the bug?

We have documented our development guidelines if you want to take a look.

@Lucas-C
Copy link
Member

Lucas-C commented Sep 18, 2023

Thank you for reporting this @macdeport

We already have some unit tests regarding the zoom:
https://github.com/py-pdf/fpdf2/blob/master/test/metadata/test_display_mode.py
But it seems that we did not test setting a numeric value.

An extra test could be added regarding this case.
If you do not wish to submit a PR, one of the current maintainers can do it,
but we often like to ask fpdf2 users if they want to use this opportunity to contribute to the project 🙂

@macdeport
Copy link
Author

macdeport commented Sep 18, 2023

@Lucas-C @andersonhc I would have been happy to contribute more, and it could have been instructive for me, but I'm too busy at the moment... thank you for what you do and for your offer.

@andersonhc
Copy link
Collaborator

@Lucas-C @andersonhc I would have been happy to contribute more, and it could have been instructive for me, but I'm too busy at the moment... thank you for what you do and for your offer.

No worries. I just opened a PR to fix the bug and added an automated test on this feature to reduce the chances of it breaking again in the future.

Thanks again for taking the time to create the minimal code and investigate the fix.

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

Successfully merging a pull request may close this issue.

3 participants