Skip to content

M7: the paragraph editor draws on its page only (#3061) - #3070

Merged
aiongg merged 2 commits into
storytold:mainfrom
mtlcl410:fix/edit-text-clip-to-page-3061
Oct 22, 2026
Merged

aiongg merged 2 commits into
storytold:mainfrom
mtlcl410:fix/edit-text-clip-to-page-3061

Conversation

@mtlcl410

@mtlcl410 mtlcl410 commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

What this changes

Fixes #3061.

In the reporter's file, page 1's text runs past the page's bottom edge. HTML-to-PDF converters write this when they rely on the page clip. Normal view clips it at the edge, as it should.

In Edit text & images, clicking that paragraph opened an editor that drew its box and all its text over page 2, on top of page 2's own text. That is the second screenshot in the issue. The paragraphs' faint outlines also crossed the gap between the pages.

Now:

  • The editor is clipped to its page (edit_text_ui::overlay), and also to the document area. The editor is a foreground egui::Area, so a page scrolled up under the tool bar used to draw it over the tool bar too.
  • Paragraph outlines are clipped to their page (page_input).
  • Unchanged: the paragraph still holds its off-page lines, and editing and applying work as before. Clicks were already limited to the page (xf.rect.contains).

Before and after, on a synthetic two-page file whose page-1 paragraph runs to y = −270:

before after
the editor's lines 43–56 drawn over page 2 the editor ends at page 1's edge

How you verified it

  • cargo fmt --all -- --check: clean
  • cargo clippy -p pdfcraft-ui-egui --all-targets -- -D warnings: clean
  • cargo test -p pdfcraft-ui-egui --test editing (130) and --test content, with CRAFT_FONTS_DIR: pass

Test: a_paragraph_running_off_its_page_edits_without_covering_the_next_page. It builds a two-page PDF whose page-1 paragraph runs past the bottom edge, renders the window and checks pixels:

  1. Before opening the paragraph, the gap between the pages and the top of page 2 are unchanged across the paragraph's columns, so its outline stays on page 1.
  2. After a click opens it, and the editor holds the off-page lines, the same pixels are still unchanged.
  3. After scrolling the page 300 pt up past the document area, the row just above the document area is the same as before the scroll.

Mutation check: 3 mutants, all caught, each reverted on its own:

  • the editor's page clip, caught at check 2;
  • the outline clip, caught at check 1;
  • the document-area part of the editor clip, caught at check 3.

Never-crash / provenance / parity

No new unwrap, indexing or unsafe. Rect intersection only. No assets, no third-party code. No path, network or MCP change. No parity status changes: this is a display fix inside edit.text-edit.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Paragraph outlines are now clipped to their page boundaries.
    • Inline text editors remain limited to the intersection of the page and visible document area.
    • Paragraphs extending below a PDF page remain fully editable without covering page gaps or adjacent pages.
    • Scrolling no longer allows off-page editor content to appear outside the visible document viewport.

@mtlcl410

Copy link
Copy Markdown
Contributor Author

The test (windows-latest) failure is image_pixels_follow_the_frame_while_a_corner_is_dragged, an image-drag test. It found no picture drawn at all (not only at []), i.e. the page raster hadn't arrived yet in that frame. That's a timing issue, not a misplaced drawing. This PR only clips the paragraph editor and paragraph outlines, not image drawing, and the test passes locally on this branch (Windows 11). dependency licences and advisories is the repo-wide RUSTSEC-2026-0370/0371/0372 failure that #3071 fixes.

@mtlcl410

Copy link
Copy Markdown
Contributor Author

Follow-up: the same image_pixels_follow_the_frame_while_a_corner_is_dragged failure hit #3071, which only changes deny.toml. The cause is a race: the test drags before the asynchronous page raster arrives. #3077 makes it wait; with a slow render simulated locally, main's test fails with this exact message.

@mtlcl410
mtlcl410 force-pushed the fix/edit-text-clip-to-page-3061 branch from 091d582 to 9779384 Compare October 17, 2026 23:42
mtlcl410 added a commit to mtlcl410/pdfcraft that referenced this pull request Oct 17, 2026
image_pixels_follow_the_frame_while_a_corner_is_dragged drags before the
asynchronous page raster has arrived on a slow runner, and the preview
(drawn from that raster) then draws no picture: 'not only at []' on
windows-latest for storytold#3070 and storytold#3071 (a deny.toml-only change). With a
300 ms render delay added locally, main's test fails the same way and
this one passes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz

@ahmadmuj ahmadmuj 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.

Checked page and viewport clipping with crop, rotation and zoom cases. Removing each bound independently makes the pixel regression fail. UI apply and in-memory save/reopen preserve all 56 paragraph rows and the next page's text. All 130 active editing tests pass.

The clipping change looks good in these headless Linux checks. I did not verify native Windows or rotated text appearance. Local WASM verification stopped at an uncached dependency; the hosted WASM check passes.

A paragraph running past its page's bottom edge, as HTML-to-PDF converters
write when they rely on the page clip, opened in an editor that covered the
next page with its box and text, and paragraph outlines crossed the gap
between pages. The editor is clipped to its page within the document area
(it also covered the tool bars when scrolled up), and outlines to their page.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz
@mtlcl410
mtlcl410 force-pushed the fix/edit-text-clip-to-page-3061 branch from 9779384 to 9194ec4 Compare October 18, 2026 16:56
@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8e6b8b71-eab5-4fa1-baba-1c99a3d8bdd3
📥 Commits

Reviewing files that changed from the base of the PR and between 9194ec4 and de25be1.

📒 Files selected for processing: 1
  • crates/ui-egui/tests/editing.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

📝 Walkthrough

Paragraph outlines and the inline text editor are clipped to page-related bounds. A regression test checks rendering when a paragraph extends beyond its page and when scrolling moves the page above the viewport.

Changes: Page-bounded text editing

Layer / File(s) Summary
Apply and verify page clipping
crates/ui-egui/src/edit_text_ui.rs, crates/ui-egui/tests/editing.rs
Paragraph outlines are clipped to the page and UI clip bounds. The editor is clipped to the intersection of the page and viewport. Regression tests check the inter-page gap, the second page, off-page text, and viewport scrolling.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: echelon, marcoarnulfo

Merge Risk: ⚪ Minimal · up to de25b

The page-bounded editing change is supported by focused source and regression-test evidence, with no actionable merge-blocking risk identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly identifies the main change: restricting paragraph editor drawing to its page. It is concise and specific.
Linked Issues check Passed Issue #3061 requires PDF text editing to avoid hiding or incorrectly displaying nearby text. page_input clips paragraph outlines to the page rectangle. overlay clips the foreground editor to the i…
Out of Scope Changes check Passed The changes are limited to clipping logic in crates/ui-egui/src/edit_text_ui.rs and a regression test in crates/ui-egui/tests/editing.rs. Both changes directly support issue #3061. No unrelated pr…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.
Description check Passed The pull request description is complete and relevant. It explains the behavior change, references issue #3061, lists verification commands and results, describes the regression test and mutation chec…
Full details: Linked Issues check

Explanation

Issue #3061 requires PDF text editing to avoid hiding or incorrectly displaying nearby text. page_input clips paragraph outlines to the page rectangle. overlay clips the foreground editor to the intersection of the page and document viewport. The regression test a_paragraph_running_off_its_page_edits_without_covering_the_next_page verifies the page gap, page 2, off-page editor lines, and document-area scrolling. The changes preserve the existing editor behavior and page-limited interaction.

Full details: Out of Scope Changes check

Explanation

The changes are limited to clipping logic in crates/ui-egui/src/edit_text_ui.rs and a regression test in crates/ui-egui/tests/editing.rs. Both changes directly support issue #3061. No unrelated product change is shown.

Full details: Description check

Explanation

The pull request description is complete and relevant. It explains the behavior change, references issue #3061, lists verification commands and results, describes the regression test and mutation checks, and addresses never-crash, provenance, security, and parity considerations. The optional reviewer-focus section is omitted, but no critical information is missing.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@FredApps FredApps 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.

The new test fails in CI at this head (test (ubuntu-latest)): a_paragraph_running_off_its_page_edits_without_covering_the_next_page panics at crates/ui-egui/tests/editing.rs:4915 in the first shown_on_its_page_only call ("the paragraph's outline: between the pages at x = 416"): left: [232, 232, 234, 255], right: [233, 233, 235, 255].

So the gap pixel at x = 416 is one level off the colour sampled at first.right() - 40, before the editor is even opened. Either something still paints in the gap there (the outline's clip rect is xf.rect.expand(1.0), so the stroke is clipped by the page but the page shadow is not part of it) or the reference pixel is not a clean gap colour. Please make the test compare against the same pixel without the paragraph (render once with the outlines off) rather than a fixed sample, or fix whatever is drawing there, and push a green run.

The clipping approach itself (with_clip_rect for the outlines, set_clip_rect on the editor Area) is the right place for it.

…ls without Edit text (storytold#3061)

Review on storytold#3070: the gap between pages isn't one colour on every platform;
on ubuntu CI the pixel at x = 416 was one level off the fixed sample taken
40 px from the page's right edge. The test now takes a screenshot of the
same pages before Edit text is on (no paragraph outline), checks that the
pages stay where they were, and compares every gap pixel with the same
pixel there. Removing either clip still fails it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R3niW7kVeT8mi7TpWc8eAJ
@mtlcl410

Copy link
Copy Markdown
Contributor Author

@FredApps thanks, done in de25be1 the way you suggested. The test now screenshots the same pages before Edit text is on, so with no paragraph outline, and checks that turning Edit text on leaves both page rects where they were. shown_on_its_page_only then compares every gap pixel with the same pixel in that screenshot instead of one sample 40 px from the page's right edge. The page-2 rows still have to be white paper.

It still catches the bug it was written for. Removing the outline painter's with_clip_rect fails it at "the paragraph's outline: between the pages", and removing the editor Area's set_clip_rect fails it at "the open editor: between the pages". Gate here: 41 suites, 1307 passed. I can't reproduce the one-level difference on Windows, so the ubuntu job on this head is the real check.

@FredApps FredApps 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.

Read the whole diff. It is a small, targeted fix: the Edit text outlines (page_input) and the open paragraph editor are clipped to the page rectangle intersected with the viewport, so a paragraph that runs past the page edge (relying on the page clip) no longer paints over the next page or the tool bars.

  • The outline painter uses with_clip_rect on the existing clip, so it only ever narrows the area and respects scroll containers.
  • The editor Area clips to xf.rect ∩ viewport_rect() computed before line_editor is borrowed, so it does not conflict with the mutable borrow, and the viewport term keeps it out of the toolbar when scrolled.
  • The regression test builds the real case (a 56-line paragraph that runs to y = -270 above an ordinary page 2). It compares the gap between the pages with the same render taken without Edit text, checks page 2 stays white, confirms the off-page lines are still in the editor, and checks the row of pixels above the document area after scrolling.

CI is green on this head.

aiongg added a commit to aiongg/pdfcraft that referenced this pull request Oct 22, 2026
…s it from its whole handle, not just the inner half (storytold#2276)

crates/ui-egui/src/edit_text_ui.rs: storytold#3008's paragraph_box for the outlines with storytold#3070's page clip on their painter.
@aiongg
aiongg merged commit 30ac04a into storytold:main Oct 22, 2026
6 checks passed
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.

PDFCraft edit txt image

4 participants