Update y0/y1 when clipping objects - #1393
Open
dylanpulver wants to merge 1 commit into
Open
dylanpulver wants to merge 1 commit into
dylanpulver wants to merge 1 commit into
Conversation
`utils.clip_obj` adjusted `top`, `bottom`, `height` and `doctop` but left `y0`/`y1` at their pre-crop values, so objects that straddle a crop boundary came back with `y1 - y0 != height`. The sibling helpers `move_object` and `resize_object` already keep the bottom-up coordinates in sync; this applies the same convention to `clip_obj`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
utils.clip_objupdatestop,bottom,heightanddoctopbut noty0/y1, so an object that straddles aPage.crop(...)boundary keeps its pre-crop bottom-up coordinates.On
tests/pdfs/pdffill-demo.pdf(792pt tall), cropping through the middle of the first char:y1 - y0is 18.0 whileheightis 9.0. The README documentsy1as the distance of the object's top from the bottom of the page, i.e. 792 - 126.18 = 665.82.The two sibling helpers in the same module already handle this:
resize_objectdoesy1 -= difffortopandy0 -= diffforbottom, andmove_objectshifts both. This applies the same convention inclip_obj.How I found it: reading
utils/geometry.pyfor helpers that maintain the two coordinate systems inconsistently — not from a PDF in hand.clip_objis the only one of the three that touchestop/bottomwithout touchingy0/y1.geometry.pyreports 100% line coverage andtest_resize_objectasserts they0/y1behaviour explicitly, but there was no unit test forclip_obj.Measured:
python -m pytest tests/→ 174 passed on this branch, 173 before. Reverting onlygeometry.pyand keeping the new test → 1 failed / 173 passed, differing ony0andy1. A naive variant that shifts bothy0andy1by thetopdelta also fails, ony0.Not addressed:
clip_objstill leaves a curve'sptslist uncropped, andcurve_to_edgesoutput carries noy0/y1at all.black,isort,flake8andmypy --strictare clean; I did not run the notebook examples.Disclosure: this patch was written with AI assistance (Claude).