Fix annotation rectangle under a rotated transformation matrix - #1794
Open
MaxFreedomPollard wants to merge 1 commit into
Open
Fix annotation rectangle under a rotated transformation matrix#1794MaxFreedomPollard wants to merge 1 commit into
MaxFreedomPollard wants to merge 1 commit into
Conversation
`_convertRect` in lib/mixins/annotations.js reassigned `x1` on the line before it used `x1` to compute `y1`, and did the same with `x2` and `y2`, so each corner's y came out of the already transformed x. It also mapped only two of the four corners, which cannot describe the axis aligned `Rect` that ISO 32000-1 12.5.2 asks for once the matrix rotates or skews. Both defects surface together the moment `doc.rotate()` is in effect: a quarter turn of a 100x100 box about its own top left corner produced `/Rect [0 872 100 772]`, which is 200 points above the content and inverted, instead of `/Rect [0 672 100 772]`. Reported in foliojs#1153. Transform all four corners and return their bounding box. Every matrix that keeps the axes aligned and both scale factors positive - the default page matrix, `translate` and positive `scale` - yields the same two extremes as before, so existing output does not move; a mirroring matrix now returns a normalized `[llx lly urx ury]` rectangle as ISO 32000-1 7.9.5 requires.
blikblum
requested changes
Sep 7, 2026
| ]; | ||
| const xs = corners.map((corner) => corner[0]); | ||
| const ys = corners.map((corner) => corner[1]); | ||
|
|
Member
There was a problem hiding this comment.
Can you refactor to remove the inner function and extra map and Math calls?
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
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.
What kind of change does this PR introduce?
Bug fix. Every annotation added while a rotated transformation matrix is in effect gets a rectangle that does not line up with the content it marks, so links added after
doc.rotate()are clickable in the wrong place. Reported in #1153, which was closed without the code changing.The bug
_convertRectinlib/mixins/annotations.jsassigns tox1on line 191 and then readsx1on line 192 to computey1, soy1is derived from the already transformed x. Lines 193 and 194 do the same withx2andy2.m1is_ctm[1], which is zero for the default page matrix, fortranslateand forscale. That zeroes the wrong term, which is why the defect stays hidden until something rotates or skews the matrix.Only two of the four corners are mapped at all, which is the second half of the problem.
Rectis an axis aligned rectangle in default user space (ISO 32000-1, 12.5.2), so once the matrix rotates the content, a single corner pair no longer bounds the rotated quadrilateral, and a matrix that mirrors an axis returns the pair in the wrong order for the[llx lly urx ury]form ISO 32000-1, 7.9.5 requires.Reproduction, turning a 100 by 100 box a quarter turn about its own top left corner:
master writes
/Rect [0 872 100 772]. The rotated box occupies x 0 to 100 and y 672 to 772, so the rectangle sits 200 points above the content and is inverted on top of that.The gentler rotation from #1153,
doc.rotate(20, { origin: [150, 70] })around the same box, produces/Rect [85.914362 696.933948 214.085638 747.066052]. The four transformed corners span x 85.914362 to 214.085638 and y 657.914362 to 786.085638, so the annotation ends up a 50 point tall band across the middle of a shape that is 128 points tall, and the top and bottom of the drawn box are not clickable. The width is right and the height is not, which is the scaled rather than rotated rectangle that issue describes.The fix
Map all four corners and return their bounding box.
Every matrix that keeps the axes aligned with both scale factors positive, which covers the default page matrix,
translateand positivescale, yields the same two extremes the old code returned, so existing output does not move. A mirroring matrix now comes back normalized, and a rotated matrix comes back covering the rotated area._markupbuildsQuadPointsout of the same four numbers, so highlight, underline and strike are corrected with it, and their quadrilateral is now exactly the corners ofRectrather than a shape that can fall outside it, which is what ISO 32000-1, Table 179 asks for.Testing
Two tests added to
tests/unit/annotations.spec.js. The first is the defect and fails on master:The second pins the untransformed rectangle at
/Rect [100 672 200 772]and passes both before and after, so a change that moved ordinary annotations would be caught.npx vitest run tests/unit/annotations.spec.js: 16 passed, 16 total.npm test: 60 test files passed, 493 tests passed, 0 failed, includingtests/visual.npm run prettier: "All matched files use Prettier code style!".npm run lint: no output, no errors.Checklist: