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

Added pdf2image kwargs #6488

Merged
merged 6 commits into from Feb 15, 2024
Merged

Conversation

wolflu05
Copy link
Contributor

@wolflu05 wolflu05 commented Feb 14, 2024

While investigating some print quality issues, I noticed that there are some issues with the rendered png. So I first looked into to intermediately generated pdf, but everything looks fine there. So I suspect this has something todo with the png rendering. After a lot of playing with different options I found out that the pdf2image library has an optional setting (use_pdftocairo=True) to use pdftocairo instead of pdftoppm which is used by default.

pdftoppm (current) pdftocairo
image image
No sharp edges are rendered, which results in not always quadratical QR code pixels

So this PR adds an option to let the developer specify some custom options to pass over to pdf2image.convert_from_bytes(...). Not sure if we should use Cairo by default, as this may can break some printing plugins?

Copy link

netlify bot commented Feb 14, 2024

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit f3aed6a
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/65cdc78639b129000830e5ce

@wolflu05 wolflu05 added enhancement This is an suggested enhancement or new feature report Report generation labels Feb 14, 2024
@wolflu05 wolflu05 added this to the 0.14.0 milestone Feb 14, 2024
@SchrodingersGat
Copy link
Member

I would suggest that we add a specific optional argument such as use_cairo=False rather than making the plugin developers discover and pass the use_pdftocairo argument themselves. I'd also be on board with making it default to True given the better fidelity in your results

@wolflu05
Copy link
Contributor Author

I now added the direct use_cairo kwarg which is True by default now. But I still think it can have value to have all pdf2image arguments exposed to the developer, so I would keep that, what do you think?

@SchrodingersGat
Copy link
Member

I now added the direct use_cairo kwarg which is True by default now. But I still think it can have value to have all pdf2image arguments exposed to the developer, so I would keep that, what do you think?

I agree, I am happy with the implementation as it stands

@SchrodingersGat SchrodingersGat merged commit 35577fa into inventree:master Feb 15, 2024
25 of 26 checks passed
@wolflu05 wolflu05 deleted the pdf2image-kwargs branch February 15, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an suggested enhancement or new feature report Report generation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants