Skip to content

fix: derive crop box from coordinate extent in save_elements#4371

Open
badGarnet wants to merge 1 commit into
mainfrom
fix/crop-image-for-save-elements
Open

fix: derive crop box from coordinate extent in save_elements#4371
badGarnet wants to merge 1 commit into
mainfrom
fix/crop-image-for-save-elements

Conversation

@badGarnet

@badGarnet badGarnet commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

save_elements assumed points[0] and points[2] were always the top-left and bottom-right corners. For elements whose coordinates carry (or were converted from) a y-up orientation, that ordering inverts the PIL crop box and raises ValueError: Coordinate 'lower' is less than 'upper'. The error was silently swallowed, so the figure/table image was dropped.

The crop box is now computed from the min/max of all coordinate points, so it's always valid regardless of point ordering.

Changes

  • pdf_image_utils.save_elements: use min/max over all points instead of fixed corner indices
  • Added a regression test for inverted/rotated coordinate ordering
  • CHANGELOG + version bump to 0.22.35

Summary by cubic

Fix image cropping in pdf_image_utils.save_elements by deriving the crop box from the coordinate extent. This prevents the PIL "Coordinate 'lower' is less than 'upper'" error and stops figure/table images from being dropped.

  • Bug Fixes
    • Compute the crop box with min/max over all points instead of assuming fixed corners.
    • Add a regression test for inverted/rotated coordinate ordering.

Written for commit 0ea49ae. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 4 files

Shadow auto-approve: would auto-approve. The change is a targeted bug fix in a utility function that replaces hardcoded corner indices with min/max extents, eliminating a silent crop-box inversion bug without altering core logic or adding risk.

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants