feat(annotations): add logic for drawing/region annotation cursor input on rotated files#748
Conversation
|
Duncan Hsu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
f70e8c5 to
4039be9
Compare
| const local = rotatePoint({ x: relX, y: relY }, -rotation); | ||
|
|
||
| // Convert from center-relative to top-left-relative using pre-transform dimensions | ||
| return [local.x + creatorEl.offsetWidth / 2, local.y + creatorEl.offsetHeight / 2]; |
There was a problem hiding this comment.
Can we have a comment explaining this whole algorithm? I'm finding it hard to follow even with the comments here
There was a problem hiding this comment.
I tried incorporating clearer comments explaining the algorithm in the shared helper function within utils/rotate.ts. Let me know if it's easier to follow now, even with clear comments the logic can be quite confusing without diagrams so I could also walk through it in a watercooler session if that's helpful
jmcbgaston
left a comment
There was a problem hiding this comment.
Nice fix — the inverse-rotation approach is the correct way to handle CSS-rotated interaction layers, and using offsetWidth/offsetHeight for normalization is the right call. The math is duplicated almost verbatim between DrawingCreator and RegionCreator, and the rotated code path has no unit test coverage — worth addressing before merge. A few other smaller concerns inline.
Approve with minor changes.
When the annotation layer has a CSS rotation transform, mouse coordinates from getBoundingClientRect() are in screen space but the drawing layer interprets them in its rotated local frame. This causes live drawing to appear offset by the rotation angle. Fix by applying the inverse rotation to convert screen-space coordinates into the element local coordinate system, and use offsetWidth/offsetHeight for percentage normalization since these report pre-transform dimensions. Also removes the isRotated guard that previously blocked region creation on rotated files, enabling annotations at all rotation angles. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6f500bf to
516d9c4
Compare
Summary
Adds support for live annotation drawing on rotated pages/images. This is in preparation for enabling annotation creation on rotated PDFs and images, where the annotation controls were previously hidden. Without this change, simply re-enabling annotation creation controls in the rotated state would cause the live drawing input to be offset from the user’s cursor.
This PR:
offsetWidth/offsetHeight(pre-transform dimensions) for percentage instead ofgetBoundingClientRect()which returns post-transform axis-aligned bounding box dimensionsisRotatedguard that blocked region annotation creation on rotated filesFor further clarity, here is the live pointer capture flow when drawing annotations:
getPositionapplies the inverse rotation transform to convert window/browser coordinates into the annotation layer’s local coordinate space.getPoints/getShapenormalize those local-space pixel coordinates into percentages using the annotation layer’s pre-transform (unrotated)offsetWidth/offsetHeight.Test plan