Bug in mediabox expansion when applying non-right angle rotation #2282
Conversation
|
@MrinalJain17, can you confirm that when applying angle from -370 to +360, all results are as expected ? |
|
Yes @pubpub-zz In fact, given below is a comparison of the dimensions of a page after it has been rotated with I've also manually checked the results of course. Current (before fix):
New (after fix):
|
|
your results are very good! can you add a test in order to make sure that the issue will not come back ? |
tests/test_page.py
Outdated
| with open(expected_pdf_path, "rb") as f: | ||
| expected_pdf_bytes = BytesIO(f.read()) | ||
|
|
||
| assert rotated_pdf_bytes.getvalue() == expected_pdf_bytes.getvalue() |
There was a problem hiding this comment.
I would have just look at the box size as you did in your graphs with a criteria <1E-4
There was a problem hiding this comment.
That makes sense @pubpub-zz . I've updated the tests.
Due to rounding/interpolation, there's deviation of about 1-ish pixels compared to pillow. I've set an absolute tolerance of 2 pixels instead.
There was a problem hiding this comment.
Lmk if this looks good. Or if you want me to set the expected dimensions to be what pypdf actually ends up with -- and compare with a small tolerance instead.
|
Hi @pubpub-zz. Checking in on this -- lmk if there's anything else needed. |
|
This looks good to me👏 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2282 +/- ##
==========================================
- Coverage 94.45% 94.45% -0.01%
==========================================
Files 43 43
Lines 7651 7649 -2
Branches 1511 1511
==========================================
- Hits 7227 7225 -2
Misses 262 262
Partials 162 162 ☔ View full report in Codecov by Sentry. |
|
@MrinalJain17 Thank you for this fix and the explanation around it in the PR 🙏 If you want, I can add you to https://pypdf.readthedocs.io/en/latest/meta/CONTRIBUTORS.html :-) |
## What's new ### Bug Fixes (BUG) - Mediabox expansion size when applying non-right angle rotation (#2282) by @MrinalJain17 ### Robustness (ROB) - MissingWidth is IndirectObject (#2288) by @MartinThoma - Initialize states array with an empty value (#2280) by @alexey-v-paramonov ### Documentation (DOC) - Typo in example in extract-attachments.md (#2285) by @ageitgey - Add Alexey Paramonov as a contributor for #2280 by @MartinThoma ### Maintenance (MAINT) - Update sample-files by @MartinThoma [Full Changelog](3.17.0...3.17.1)
Fixes #2281