Create correct annotations - #5063
Conversation
Previously, we did not correctly handle coordinate transformations of annotations, links and widgets. This fix should cover all cases of the necessary calculations.
There was a problem hiding this comment.
🟡 Not ready to approve
There are a few correctness/maintainability issues in the updated remove_rotation() implementation (dead conditional logic, overly broad exception swallowing) plus minor test/module cleanup and a stated-links/widgets coverage gap to address.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR updates Page.remove_rotation() to better preserve visual placement when removing page rotation, specifically addressing annotations (including quads/vertices/ink and FreeText), links, and form widgets by applying coordinate transformations and updating related PDF keys/appearance.
Changes:
- Extend
Page.remove_rotation()to transform and rewrite annotation geometry (Rect, QuadPoints/Vertices/L/InkList/CL) and adjust annotation appearance matrices/rotation where applicable. - Transform link and widget rectangles during rotation removal to keep their on-page positions consistent.
- Add a new regression test that renders pages before/after
remove_rotation()across multiple rotations and compares pixel output.
File summaries
| File | Description |
|---|---|
src/__init__.py |
Reworks Page.remove_rotation() to persist correct geometry/appearance for multiple annotation types, links, and widgets after rotation removal. |
tests/test_5059.py |
Adds a rendering-based regression test to validate visual equivalence after remove_rotation() for rotated pages with many annotation types. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 4
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| mupdf.PDF_ANNOT_TEXT, | ||
| mupdf.PDF_ANNOT_INK, | ||
| ): | ||
| annot._update_appearance(rotate=0 if annot_type in (mupdf.PDF_ANNOT_STAMP, mupdf.PDF_ANNOT_TEXT) else -1) |
| except Exception: | ||
| # Annotation has no AP/N matrix to adjust. | ||
| pass |
| def test_5059(): | ||
| for rot in ROTATIONS: | ||
| src = _make_input_pdf(rot) | ||
| page = src[0] | ||
| pix_orig = page.get_pixmap(colorspace=pymupdf.csGRAY) | ||
| page.remove_rotation() | ||
| pix_rot0 = page.get_pixmap(colorspace=pymupdf.csGRAY) | ||
| rms_val = rms(pix_orig.samples, pix_rot0.samples, verbose=False) | ||
| assert rms_val < 2, f"RMS diff for rotation {rot}: {rms_val}" |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Previously, we did not correctly handle annotations, links and widgets in this process. This fix should cover all cases of the necessary coordinate transformations.