Skip to content

fix(render): emit clip-only text (7 Tr) to devices for text extraction (#3109) - #3116

Open
jonathanwxh-cell wants to merge 5 commits into
storytold:mainfrom
jonathanwxh-cell:fix/clip-mode-text-extraction
Open

jonathanwxh-cell wants to merge 5 commits into
storytold:mainfrom
jonathanwxh-cell:fix/clip-mode-text-extraction

Conversation

@jonathanwxh-cell

@jonathanwxh-cell jonathanwxh-cell commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Text drawn in clipping mode (7 Tr, TextRenderingMode::Clip) shapes the clipping path for visual rendering, but was previously omitted from device calls and thus dropped from text extraction, Search, and Redaction. This fix emits glyphs drawn under 7 Tr to text devices with a dedicated GlyphDrawMode::Clip variant (distinct from GlyphDrawMode::Invisible), ensuring clipping-only text (such as gradient- or image-filled titles) is extracted and searchable while still allowing filtered extraction (TextFilter { images: true }) to preserve clipping-mode titles without retaining invisible OCR layers.

Fixes #3109

What this changes

  • Introduces GlyphDrawMode::Clip in hayro-interpret::types.
  • In hayro-interpret::interpret::text::show_glyph, calls device.draw_glyph(..., &GlyphDrawMode::Clip) under TextRenderingMode::Clip after clip_glyph.
  • In hayro::renderer, ignores GlyphDrawMode::Clip alongside GlyphDrawMode::Invisible so visual rendering does not paint clip-only text directly.
  • Adds regression tests in crates/render/src/text.rs for 7 Tr text extraction across search and TextFilter filtering, plus visual raster clipping assertions in crates/render/src/raster.rs.

How you verified it

  • cargo fmt --all -- --check
  • cargo clippy -p pdfcraft-render -j 2 --all-targets -- -D warnings
  • cargo test -p pdfcraft-render -j 2 (309 passed)
  • cargo xtask layers
  • cargo xtask assets

Never-crash

  • No new unwrap/expect/panic!/unreachable!/todo! outside tests
  • No indexing or slicing with input-derived positions
  • No unsafe
  • Regression tests added in crates/render

Summary by CodeRabbit

  • Bug Fixes
    • Clipping-only text in PDFs is now available for text extraction, search, and geometry-based highlights without being painted as visible text.
    • Clipped headings are retained when filtering out invisible OCR text, while invisible OCR text remains excluded.
    • Text clipping continues to constrain subsequent fills, including when the text is rotated or uses replacement text.

@mtlcl410

Copy link
Copy Markdown
Contributor

Heads-up: #3191 fixes the same issue (#3109) with the same show_glyph call, but reports the glyph as a new GlyphDrawMode::Clip rather than Invisible. The difference shows with filtered extraction (TextFilter.images, used to leave out OCR/invisible text). There, Invisible also drops a clip-only title that is visible on the page. Swapping #3191's mode to Invisible fails its clipping_only_text_survives_image_filter test. One of the two will have to give way; details are in my review on #3191.

@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: 1fd0ca17-f6ba-45a1-ae71-9afda4ea2ce7
📥 Commits

Reviewing files that changed from the base of the PR and between e22d706 and 52dd6b1.

📒 Files selected for processing: 2
  • crates/render/src/raster.rs
  • vendor/hayro/src/renderer.rs

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

📝 Walkthrough

The interpreter now reports text drawn in clipping mode to the device after applying its clipping path. The renderer does not paint these glyphs. Tests cover text extraction, image filtering, and raster clipping.

Changes: Clipping-mode text extraction

Layer / File(s) Summary
Report clipping-mode glyphs
vendor/hayro-interpret/src/types.rs, vendor/hayro-interpret/src/interpret/text.rs, vendor/hayro/src/renderer.rs
GlyphDrawMode adds Clip. The interpreter reports clipping-mode glyphs after applying the clipping path. The renderer handles Clip and Invisible glyphs without painting them.
Validate and document extraction behavior
crates/render/src/text.rs, crates/render/src/raster.rs, vendor/README.md
Tests cover extraction, search, glyph geometry, highlights, /ActualText, redraws, image filtering, and clipping without direct painting. The README describes clipping-mode text reporting and related test coverage.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: echelon

Merge Risk: ⚪ Minimal · up to 52dd6

Clipping-only outline text is available for extraction and search without being painted. No actionable change-introduced risk remains; the PR is mergeable subject to normal checks.

### Architecture Summary

Architecture risk: 🔵 Low · up to 52dd6

The change affects 1 system.

Changed systems: crates

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — crates (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in crates/render/src/text.rs: Added a test asserting that text in rendering modes 0–7 has the same extracted text, search results, glyphs, and highlight rectangles as fill mode. It also asserts expectations for clipping-mode /ActualText, redraw deduplication, turned text, text without fill-through, and rendered pixels with and without a fill-through operation.
  • observed — Modified behavior in crates/render/src/text.rs: Added a test asserting that image filtering retains clipped headings, including a heading represented by /ActualText, while excluding separate OCR text.
  • observed — Modified behavior in crates/render/src/raster.rs: Added test_clip_only_text_raster_preserves_clipping_without_painting, which renders PDFs to verify that 7 Tr text paints no pixels by itself, but clips later painting to the glyph-shaped region.
  • observed — Modified behavior in vendor/hayro-interpret/src/interpret/text.rs: The Clip rendering-mode branch now reports the glyph to the device with GlyphDrawMode::Clip after applying its clipping path; previously it only updated the path.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description check Passed The description clearly explains the behavior change, linked issue, implementation, tests, and Never-crash checks. It omits the optional or less relevant template sections for provenance, security, pa…
Title check Passed The title is concise, specific, and accurately describes the main change: emitting clip-only text to devices for text extraction.
Linked Issues check Passed The change satisfies [#3109]. show_glyph now clips each TextRenderingMode::Clip glyph and reports it with GlyphDrawMode::Clip. The renderer ignores Clip, so it does not paint the glyph directl…
Out of Scope Changes check Passed The changed interpreter, draw-mode type, renderer branch, documentation, and regression tests support [#3109]. The raster tests verify that the fix preserves visual clipping and does not paint clip-on…
Full details: Description check

Explanation

The description clearly explains the behavior change, linked issue, implementation, tests, and Never-crash checks. It omits the optional or less relevant template sections for provenance, security, parity, and reviewer focus, but the required change and verification details are substantially complete.

Full details: Linked Issues check

Explanation

The change satisfies [#3109]. show_glyph now clips each TextRenderingMode::Clip glyph and reports it with GlyphDrawMode::Clip. The renderer ignores Clip, so it does not paint the glyph directly. The added tests cover extraction, search, filtered extraction, and raster clipping behavior. The shared text-device path therefore supplies the glyphs needed by word count and search-redaction.

Full details: Out of Scope Changes check

Explanation

The changed interpreter, draw-mode type, renderer branch, documentation, and regression tests support [#3109]. The raster tests verify that the fix preserves visual clipping and does not paint clip-only text. No unrelated functional change is established by the reviewed changes.

  • Fix all pre-merge checks with AI
✨ 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 code reads correctly on this head: the Clip arm now reports the glyph with a new GlyphDrawMode::Clip (vendor/hayro-interpret/src/interpret/text.rs:129-138), and the renderer paints neither Invisible nor Clip (vendor/hayro/src/renderer.rs:1689). CI is green. Two things need settling before I can approve:

  • Same fix as #3191. It reports the same glyph from the same place with the same GlyphDrawMode::Clip, and it already has seven reviews. Only one of the two should land. Please agree with that author which one is kept, and carry over any test the other one has that this one lacks.
  • The PR description is stale. It says the fix calls draw_glyph with GlyphDrawMode::Invisible, "matching 3 Tr". The diff adds a new Clip variant instead, because Invisible would drop clip-only titles under TextFilter { images: true }. Please update the text to describe what the code does, since that reason is the point of the change.

@jonathanwxh-cell

Copy link
Copy Markdown
Contributor Author

Thanks @FredApps! Updated the PR description to accurately describe GlyphDrawMode::Clip and why it is kept distinct from GlyphDrawMode::Invisible under TextFilter { images: true }. This PR (#3116, opened earlier) also already covers all the test cases from #3191 (

@jonathanwxh-cell

Copy link
Copy Markdown
Contributor Author

Thanks @FredApps!

  1. Refreshed against upstream/main (4db6ac4), resolving the append-only ROADMAP.md conflict.
  2. Carried over the raster test est_clip_only_text_raster_preserves_clipping_without_painting from Report clipping-only (7 Tr) text to the text device for extraction #3191 into crates/render/src/raster.rs to verify that 7 Tr alone leaves the page blank while a subsequent fill clips to the text outlines.
  3. Updated the PR description to describe GlyphDrawMode::Clip and document why it is distinct from Invisible (retaining visible clipped headings when TextFilter.images excludes invisible OCR layers).

All 309 render tests, clippy, fmt, layers, and assets are green.

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.

Text drawn in clipping mode (7 Tr) is missing from text extraction, Find, word count and search-redaction, though it shows on the page

3 participants