Skip to content

fix(annot): draw callouts upright on rotated pages - #5354

Draft
ExpandYourself wants to merge 1 commit into
storytold:mainfrom
ExpandYourself:fix/callout-upright-on-rotated-pages
Draft

ExpandYourself wants to merge 1 commit into
storytold:mainfrom
ExpandYourself:fix/callout-upright-on-rotated-pages

Conversation

@ExpandYourself

Copy link
Copy Markdown

What this changes

Fixes #4686.

A callout on a page with /Rotate 90, 180 or 270 was laid out in user space and skipped the /PCTextRotate turn that text boxes already get, so the box looked sideways. A new callout now uses that same turn. Its leader and text box are mapped into the displayed frame, and the UI picks the leader side in view space.

Supersedes #4690. That branch no longer merges; the comments UI has since moved from comments.rs to tools/comments/composer.rs. This is the same behaviour on current main.

Existing callouts saved without /PCTextRotate keep their current look. Unrotated pages are unchanged.

How you verified it

  • cargo test -p pdfcraft-annot callout — 4 passed, including callout_on_a_turned_page_is_drawn_upright_as_displayed and the three existing callout tests.

  • cargo fmt -p pdfcraft-annot -p pdfcraft-ui-egui -- --check

  • cargo fmt --check

  • clippy with -D warnings on the crates I changed

  • tests pass on the crates I changed

  • cargo xtask layers and cargo xtask assets (no crate boundary or asset change)

Tests. crates/annot/src/tests.rs::callout_on_a_turned_page_is_drawn_upright_as_displayed adds a callout to a page with /Rotate 90 and checks /PCTextRotate, the form /Matrix, and that the leader starts at the pointed-at spot in the displayed frame.

Never-crash

  • No new unwrap/expect/panic!/unreachable!/todo! outside tests, unless it is provably infallible with a comment saying why
  • No indexing or slicing with input-derived positions (get, and strings only at char boundaries); checked or saturating arithmetic; allocations sized by input are capped; recursion is bounded
  • No unsafe
  • Any crash this fixes has a small synthetic regression test (not a crash; the regression test pins the sideways callout)

Provenance

  • No asset from an Adobe product, and no Adobe icon recreated, traced or imitated
  • No font files committed here (they live in storytold/craft-fonts)
  • No GPL/AGPL code copied in. MuPDF, Ghostscript, Poppler, veraPDF and DSS may only be run as external oracle processes
  • Behaviour comes from the specification or black-box observation, not from reading another implementation's source
  • Every new asset has an ATTRIBUTION.toml entry and cargo xtask assets passes (no new asset)

Security

  • --root confinement still holds on every path, including ones reached by nested tool dispatch
  • MCP and the UI control channel stay opt-in, loopback-only and token-authenticated
  • No new outbound network access (that decision is the owner's)

Parity and reach

  • parity/acrobat-features.toml updated in this PR, with notes saying what is still missing. shipped means "exists and tested", not "as good as Acrobat"
  • Reachable headlessly through an automation tool where that applies (AGENTS.md §3), so agents can drive it and not just the UI (comment_add / add_annotation sets /PCTextRotate from the page)
  • Docs updated if this changes a documented behaviour (docs/, crate README) (parity notes only)

Anything a reviewer should look at first

  • Callouts on turned pages now render upright as displayed. Existing callouts without /PCTextRotate are left alone.
  • Unrotated pages, including a landscape MediaBox with /Rotate 0, are unchanged. /CL and /RD stay in user space.
  • Draw callouts upright on pages with /Rotate #4690 is the earlier attempt at this fix and should be closed in favour of this branch.

Callouts skipped the rotation text boxes already use, so a box on a turned page was laid out sideways.
@coderabbitai

coderabbitai Bot commented Oct 29, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
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.

[Bug Report] [PdfCraft] Wrong orientation when adding a callout on a landscape page

1 participant