mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-15 15:22:54 +00:00
Hector Villarreal
aef22858a9
## What this changes Fixes #3850. Partly addresses #3682: drags within one column of its two-column page now select that column's text (the tester's harness measured 3/9 → 7/9 on `cols.pdf`). Drags from one column into the next still follow reading order, as on `main`; they need column-correct reading order (#3676). Selecting text in a table selected the wrong things. A table reads column by column, so a plain drag along one row, or down and across a few rows, took the rest of the first column and the top of the next ones between its ends. On some rows the highlight jumped to the column headers and half the table. Copy gave the cells out of order. Preview selects these by rows, and this makes PdfCraft do the same. Rebased on `main` after the module split (#4889): the row selection is in `crates/render/src/text.rs`, the padded-cell join and barcode rows in `text/order.rs` and `text/layout.rs`, the drag's pointer end in `crates/ui-egui/src/canvas/selection.rs` and `canvas.rs`. - **Visual rows** (`PageText::visual_between`, used by `Selection::rects` and `Selection::text`). The drag selects by rows when reading order between its ends would not follow them: the rest of the first row, the rows between, and the start of the last. Copy gives the cells tab-separated, as Column select does. Rows are used when either of these holds: - Reading order strays above or below the rows dragged over. - The drag ends in another column. That means across a gutter that no text of those rows crosses (checked a few lines above and below as well, so a justified line's wide word gap does not count), or with the pointer past the end of both lines. - **Limits on the rows.** Rows never take lines left of everywhere the drag has been, and they stay in the column when the drag does. A drag in the right column of side-by-side text that ends on a hanging list marker does not pull in the left column. Running text keeps reading order whenever it covers the same glyphs, and so does a column of running text beside a sidebar. A drag down one article or table column keeps reading order too. A line's extent for these limits is its ink: spaces padding it out don't count. - **Pointer position.** `Span::Run` records where the drag's pointer ended (`head_x`). It can lie well past the nearest glyph, as in the empty end of a table row. Select all, a clicked word, line or page, and four-click page selection keep reading order. - **Hit testing.** `PageText::nearest` still takes the glyph whose em box holds the point first (#3693). Failing that, it prefers the row level with the pointer, so past the end of a short row a longer row just below no longer captures the drag. Between rows, the old distance rule still applies. - **Padded cells.** Report generators pad each cell of a row with trailing spaces. When a cell's padding stops short of the next cell (more than the 3-height continuation gap), the next cell on its baseline now joins the row, within 8 line heights (`PADDED_FIELD`, `PADDED_REACH`). Without this, the row's last cells were read after the whole table, in selection, copy and `text_extract` alike. - **Barcodes drawn with a font** (new since the last review). Mexican CFDI invoices and receipts print their QR or Aztec code with a barcode font: rows of glyphs stacked flush, 5 pt from the seal's text and on nearly the same baselines. Each code row joined the seal line beside it, so the seal read interleaved with the code, some seal lines came after the whole code, and a drag down the seal jumped through it. Rows of at least 8 glyphs, 4 or more stacked flush with one glyph count and the same edges (`barcode_rows`), now join no other text and read after the page's text. A point beside the code is not taken by the code row level with it. ## How you verified it - [x] `cargo fmt --all --check` - [x] `cargo clippy --workspace --all-targets --locked -- -D warnings` - [x] tests pass on the crates I changed, and on the crates that use text extraction: | Crate or suite | Passed | |---|---| | `pdfcraft-render` (lib + integration) | 476 + 64 | | `pdfcraft-ui-egui --lib` | 696 | | `pdfcraft-ui-egui --test it` (`column_select`, `text_select`, `drag_selects`, `selection_works`) | 33 | | `pdfcraft-automation` | 359 | | `pdfcraft-engine` | 442 | | `pdfcraft-export` | 43 | - [x] `cargo xtask layers`, `cargo xtask assets`, `cargo check -p pdfcraft-render --target wasm32-unknown-unknown`, `cargo xtask parity` **Tests.** I reverted each fix and watched these fail: - `column_select.rs::a_plain_drag_along_one_table_row_selects_only_that_row` (UI, select tool and highlighter). Before the fix it got `"Apple\nPear\nQty\n12"` instead of `"Apple\t12"`. - `column_select.rs::a_drag_ending_between_the_columns_selects_the_rows` (UI). Without `head_x` it got `"Name\nApple\nPear"`. - `column_select.rs::a_drag_down_text_beside_a_barcode_selects_only_the_text` (UI). Without the barcode rows it got `"ABCDEFGHIJKL one two three\nFGHIJKLMNOPQ\n…"`. - `text/tests.rs::a_barcode_drawn_with_a_font_reads_apart_from_the_text_beside_it` - `text/tests.rs::a_point_beside_a_barcode_takes_the_text_not_the_code_row` - `text/tests.rs::a_point_level_with_a_row_takes_that_rows_nearest_glyph` - `text/tests.rs::a_drag_in_the_right_column_takes_nothing_from_the_left` Also new in `text/tests.rs`: `a_drag_along_one_table_row_selects_that_row`, `a_drag_over_table_rows_selects_those_rows`, `a_drag_that_reading_order_already_covers_keeps_it`, `a_pointer_ending_beside_the_column_takes_the_rows`, `a_cell_padded_with_spaces_joins_the_next_cell_of_its_row`, `a_justified_gap_is_not_a_gutter`, and the running-text, sidebar, heading and vertical-text cases from earlier review rounds. In `column_select.rs`: double-click drags, select all and four-click page selection on uneven columns. All fixtures are synthetic. **Corpus.** Before the rebase, text extraction of the pinned pdf.js corpus was byte-identical with and without this PR. The barcode change leaves it identical too: the plain text of the first 60 pages of each of its 989 PDFs (1,525 pages) is the same with and without it. **Real-world files** (private, not attached): - **Text extraction:** on a personal folder of PDFs (bank and hotel reports, payslips, policies, contracts, slide decks), the only files that differed from `main` before the rebase were bank statements whose padded rows now keep their amounts. The barcode change alters one page across 105 files: a statement's CFDI page, whose seal now reads in order. - **Simulated drags:** about 53,700 random drags across those files. Selection takes at most about 145 µs per frame. - **Two-column contract:** on a bilingual two-column contract, 2,649 drags kept inside one column leak into the other column less often than before (1,224 vs 1,591). In no file does the new code leak a two-column drag where the old one did not, except across table rows, where rows running across the page is intended. ## Never-crash - [x] No new `unwrap`/`expect`/`panic!`/`unreachable!`/`todo!` outside tests - [x] No indexing with input-derived positions in the new code (`get`). Allocations are bounded by the page's glyphs, and `side_by_side` compares at most `SIDE_BY_SIDE_LINES` lines. Barcode rows are found with one hash map over the page's rows. NaN boxes rank last in `nearest` (`total_cmp`) and are never barcode rows. No recursion. - [x] No `unsafe` - [ ] Any crash this fixes has a small synthetic regression test (n/a, no crash) ## Provenance - [x] No asset from an Adobe product, and no Adobe icon recreated, traced or imitated - [x] No font files committed here - [x] No GPL/AGPL code copied in - [x] Behaviour comes from black-box observation (Preview's selection on the same files), not from reading another implementation's source - [x] No new assets ## Security - [x] `--root` confinement untouched - [x] MCP and the UI control channel untouched - [x] No new outbound network access ## Parity and reach - [x] `parity/acrobat-features.toml`: `view.text-select` gains the new evidence and a note. Selection across pages is still missing, as before. - [x] Reachable headlessly: the padded-cell join and barcode rows change `text_extract` too. Visual-row selection is a pointer interaction, like the rest of plain drag selection. - [x] `ROADMAP.md` progress log line ## Anything a reviewer should look at first 1. **A deliberate behaviour change.** `alt_drag_selects_only_the_text_inside_the_rectangle` asserted that a plain drag from "Name" to "12" selects in reading order: `"Name\nApple\nPear\nQty\n12"`. It now selects the two rows dragged over, `"Name\tQty\nApple\t12"`, and I changed the assertion. Reading order itself is unchanged (`a_column_selection_takes_one_table_column` still asserts it), and so is a drag down one column. 2. **The gutter test is a heuristic.** "Another column" means a vertical strip at least one line height wide that no text in the dragged rows, or 4 line heights around them, crosses. That catches table columns and side-by-side text, and ignores justified word gaps in practice. Thin columns with ragged edges may still fool it. 3. **The padded-cell join changes reading order.** It applies only to a segment whose box runs on at least 2 line heights of trailing spaces past its last visible glyph, and only towards a cell on the same baseline within 8 line heights. 4. **Barcode rows change reading order.** A stack of 4+ flush rows of 8+ glyphs, each with the same glyph count, height and edges, reads after the page's text. Text set solid still has lines of different lengths and counts; a hex dump set solid in a monospaced font would also match, and would then read last. 5. **Overlap with #2431** (narrow article gutters, #2387). The two PRs touch different code paths and should rebase cleanly. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Text drags across table-like layouts now select cells by visual row, with copied cells separated by tabs. Multi-click and Shift-click selections can also follow visual rows where applicable; select-all and other non-row selections retain reading order. * Text selection and pointer placement work more accurately around columns, gutters, and barcode-like text, reducing unintended selection of nearby content. Barcode text is kept separate from adjacent text. * **Documentation** * Updated feature coverage and progress notes to reflect text-selection behavior and regression testing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->