From 3909bebeece5f382054cc5a524a3f74578260ae8 Mon Sep 17 00:00:00 2001 From: hcchung1 <131962610+hcchung1@users.noreply.github.com> Date: Fri, 16 Oct 2026 12:09:10 +0800 Subject: [PATCH] M7: a paragraph number keeps naming the paragraph text_paragraphs listed (#2516) Paragraph numbers are positions in a list regrouped from geometry after every edit, so an edit that merged a paragraph into its neighbour made the next edit by number rewrite another paragraph and lose its text. After text_paragraphs, text_edit and text_edit_layout find the listed paragraph again by its text and box, and refuse one that no longer exists on its own (list the page again). An edit's result names the numbers that are gone in stale_paragraphs. The inline editor's switch-paragraph click holds the same key instead of an index. Without a listing a number is a position, as before. An identity that survives save and reopen is still #2508. --- ROADMAP.md | 2 +- crates/automation/src/lib.rs | 90 ++++++++++++++++++----- crates/automation/src/tools.rs | 4 +- crates/automation/tests/automation.rs | 74 +++++++++++++++++++ crates/edit/src/lib.rs | 4 +- crates/edit/src/text.rs | 100 ++++++++++++++++++++++++++ crates/engine/src/lib.rs | 2 +- crates/ui-egui/src/canvas.rs | 3 +- crates/ui-egui/src/edit_text_ui.rs | 11 ++- docs/editing-existing-text.md | 13 ++++ 10 files changed, 277 insertions(+), 26 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index b1283e7751..90fe90f4e6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -171,7 +171,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md | 8 | 1.0 polish: performance, right-to-left, missing languages, keyboard-only (M14) | 55–95 | ## Progress log - +- **2026-10-16 (M7, #2516):** A paragraph number from `text_paragraphs` keeps naming the paragraph it listed after edits regroup the page: `text_edit` and `text_edit_layout` find it again by its text and box, and refuse one that was merged into a neighbour instead of rewriting the paragraph that took its number (`stale_paragraphs` lists the numbers gone). The inline editor's switch-paragraph click holds the same key. No stable identity across save and reopen yet (#2508). - **2026-10-19 (M14, #3373):** The main menu and its long submenus scroll in short windows, keeping their commands reachable. Estimates unchanged. - **2026-10-19 (M3, #3387):** Save-prompt shortcuts yield to later consent and utility dialogs; a synthetic UI regression checks that an edited document stays open. Estimates unchanged. diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 0e2e0dd698..067c9dac33 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -36,7 +36,7 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::Arc; -use pdfcraft_engine::{CombineSource, DocId, Document, Edit, Session, commands}; +use pdfcraft_engine::{BlockKey, CombineSource, DocId, Document, Edit, Session, TextBlock, block_overlapping, commands}; use pdfcraft_platform::staging::{StagingName, staging_suffixes}; use pdfcraft_render::{PageRenderer, PageText, RenderConfig, RenderRequest, RequestKind}; use serde_json::{Value, json}; @@ -118,6 +118,9 @@ pub struct Automation { renderers: HashMap>, PageRenderer)>, /// Extracted page text, under one memory budget across all open documents. texts: TextCache, + /// The paragraphs of the last `text_paragraphs` listing of each page, so a paragraph number + /// keeps naming that paragraph after edits regroup the page (#2516). + listed: HashMap<(DocId, usize), Vec>, } impl Default for Automation { @@ -128,7 +131,7 @@ impl Default for Automation { impl Automation { pub fn new() -> Self { - Self { session: Session::new(), root: None, roots: None, renderers: HashMap::new(), texts: TextCache::default() } + Self { session: Session::new(), root: None, roots: None, renderers: HashMap::new(), texts: TextCache::default(), listed: HashMap::new() } } /// Confine every path the tools read or write to `root` (relative paths resolve inside it). @@ -530,8 +533,9 @@ impl Automation { let doc = self.doc_to_copy(&a)?; let info = &doc.info.pages[page]; let r = |x: f32| (x as f64 * 100.0).round() / 100.0; - let blocks: Vec = doc - .text_blocks(page) + let current = doc.text_blocks(page); + let id = doc.id; + let blocks: Vec = current .iter() .enumerate() .map(|(i, b)| { @@ -548,6 +552,7 @@ impl Automation { }) }) .collect(); + self.record_listing(id, page, ¤t); json!({ "page": page + 1, "count": blocks.len(), "paragraphs": blocks }) } "text_font_files" => { @@ -605,10 +610,8 @@ impl Automation { let doc = self.doc(&a)?; let k = a.int("paragraph")?; let blocks = doc.text_blocks(page); - if k < 1 || k as usize > blocks.len() { - return Err(ToolError::InvalidArgs("paragraph is out of range".into())); - } - let source = &blocks[k as usize - 1]; + let index = self.paragraph_index(doc.id, page, k, &blocks)?; + let source = blocks.get(index).ok_or_else(|| ToolError::InvalidArgs("paragraph is out of range".into()))?; let text = a.opt_str("text")?.unwrap_or(&source.editor_text); let style = pdfcraft_engine::BlockStyle { size: a.opt_num("size")?, @@ -617,7 +620,7 @@ impl Automation { ranges: parse_style_ranges(a.get("ranges"))?, ..Default::default() }; - let layout = doc.text_edit_layout(page, k as usize - 1, text, &style).map_err(failed)?; + let layout = doc.text_edit_layout(page, index, text, &style).map_err(failed)?; let info = &doc.info.pages[page]; let view_rect = |r: [f64; 4]| { let a = info.user_to_view(r[0] as f32, r[1] as f32); @@ -630,12 +633,13 @@ impl Automation { } "text_edit" if a.get("paragraph").is_some() => { let page = self.page(&a)?; - let n = self.doc(&a)?.text_blocks(page).len(); let k = a.int("paragraph")?; - if k < 1 || k as usize > n { - return Err(ToolError::InvalidArgs(format!("paragraph {k} is out of range: page {} has {n} paragraphs", page + 1))); - } - let block = self.doc(&a)?.text_blocks(page)[k as usize - 1].clone(); + let (id, blocks) = { + let doc = self.doc(&a)?; + (doc.id, doc.text_blocks(page)) + }; + let index = self.paragraph_index(id, page, k, &blocks)?; + let block = blocks.get(index).cloned().ok_or_else(|| ToolError::InvalidArgs(format!("paragraph {k} is out of range")))?; let text = a.opt_str("text")?.map(str::to_owned).unwrap_or(block.text); let mut style = pdfcraft_engine::BlockStyle { ranges: parse_style_ranges(a.get("ranges"))?, @@ -674,10 +678,19 @@ impl Automation { other => return Err(ToolError::InvalidArgs(format!("unknown align {other:?}"))), }); } - let mut out = self.apply(&a, Edit::EditTextBlock { page, block: k as usize - 1, text, style })?; - if let Some(b) = self.doc(&a)?.text_blocks(page).get(k as usize - 1) { + // Where the paragraph is drawn after the edit: moved by dx, dy (up is +dy). + let [dx, dy] = style.offset.unwrap_or([0.0, 0.0]); + let moved = [block.rect[0] + dx, block.rect[1] + dy, block.rect[2] + dx, block.rect[3] + dy]; + let mut out = self.apply(&a, Edit::EditTextBlock { page, block: index, text, style })?; + let after = self.doc(&a)?.text_blocks(page); + let edited = block_overlapping(&after, moved); + if let Some(b) = edited.and_then(|i| after.get(i)) { out["paragraph"] = json!({ "text": b.text, "lines": b.lines.len(), "font": b.base_font }); } + let stale = self.follow_edit(id, page, k, edited.and_then(|i| after.get(i)), &after); + if !stale.is_empty() { + out["stale_paragraphs"] = json!(stale); + } out } "text_edit" => { @@ -882,6 +895,51 @@ impl Automation { self.session.get(DocId(id)).ok_or_else(|| ToolError::Failed(format!("no open document with id {id} (see doc_list)"))) } + /// Most pages whose paragraph listing is remembered. + const MAX_LISTINGS: usize = 256; + + /// Remember the paragraphs `text_paragraphs` just listed for `page`, so their numbers can be + /// resolved against that listing. + fn record_listing(&mut self, doc: DocId, page: usize, blocks: &[TextBlock]) { + if self.listed.len() >= Self::MAX_LISTINGS && !self.listed.contains_key(&(doc, page)) { + self.listed.clear(); + } + self.listed.insert((doc, page), blocks.iter().map(BlockKey::of).collect()); + } + + /// The index in `blocks` (the page's paragraphs now) of paragraph number `k` (1-based). + /// + /// After `text_paragraphs` a number names the paragraph that listing showed, wherever it is + /// now. Without a listing it is a position in `blocks`. A paragraph an edit merged into a + /// neighbour (or changed) is refused: the number would otherwise rewrite another paragraph + /// and lose its text (#2516). + fn paragraph_index(&self, doc: DocId, page: usize, k: i64, blocks: &[TextBlock]) -> Result { + let keys = self.listed.get(&(doc, page)); + let n = keys.map_or(blocks.len(), Vec::len); + let i = usize::try_from(k) + .ok() + .and_then(|k| k.checked_sub(1)) + .filter(|i| *i < n) + .ok_or_else(|| ToolError::InvalidArgs(format!("paragraph {k} is out of range: page {} has {n} paragraphs", page + 1)))?; + let Some(keys) = keys else { return Ok(i) }; + keys.get(i).and_then(|key| key.find(blocks, i)).ok_or_else(|| { + ToolError::InvalidArgs(format!( + "paragraph {k} of page {} is no longer a paragraph of its own: an edit changed it or how the page is grouped. List the page again with text_paragraphs", + page + 1 + )) + }) + } + + /// Update the listing after paragraph `k` was edited into `edited`, one of the page's + /// paragraphs `after`. Returns the numbers of the listed paragraphs that are gone. + fn follow_edit(&mut self, doc: DocId, page: usize, k: i64, edited: Option<&TextBlock>, after: &[TextBlock]) -> Vec { + let Some(keys) = self.listed.get_mut(&(doc, page)) else { return Vec::new() }; + if let (Some(edited), Some(key)) = (edited, usize::try_from(k).ok().and_then(|k| k.checked_sub(1)).and_then(|i| keys.get_mut(i))) { + *key = BlockKey::of(edited); + } + keys.iter().enumerate().filter(|(i, key)| key.find(after, *i).is_none()).map(|(i, _)| i + 1).collect() + } + /// The document, if its security allows copying text and images out of it (Table 22, bit 5). fn doc_to_copy(&self, a: &Args) -> Result<&Document> { let doc = self.doc(a)?; diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index e154e1eebc..276b61cd3d 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -1273,7 +1273,7 @@ pub fn tools() -> Vec { t("text_fonts", "List document fonts", "List validated embedded source faces and approved full preview faces for a page. Full registered resources can be selected by text_edit.font_resource; preview_only faces are not PDF resource names.") .ro().cmd("edit.edit_text") .with(schema(json!({"doc":doc(), "page":{"type":"integer","minimum":1}}), &["doc","page"])), - t("text_paragraphs", "List paragraphs", "The paragraphs on a page (adjacent source runs grouped into aligned visual rows, including mixed fonts, sizes and colours): number, text, editor_text (visual rows separated by newlines), rows and source line numbers, box (top-left-origin points), font and size. Use the number with text_edit's paragraph.") + t("text_paragraphs", "List paragraphs", "The paragraphs on a page (adjacent source runs grouped into aligned visual rows, including mixed fonts, sizes and colours): number, text, editor_text (visual rows separated by newlines), rows and source line numbers, box (top-left-origin points), font and size. Use the number with text_edit's paragraph: until you list the page again, a number names the paragraph listed here, even after edits regroup the page.") .ro() .cmd("edit.edit_text") .with(schema(json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 } }), &["doc", "page"])), @@ -1281,7 +1281,7 @@ pub fn tools() -> Vec { .ro() .cmd("edit.edit_text") .with(schema(json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "paragraph": { "type": "integer", "minimum": 1 }, "text": { "type": "string" }, "size": { "type": "number" }, "width": { "type": "number" }, "font_resource":{"type":"string"}, "ranges":{"type":"array","maxItems":1024,"items":{"type":"object"}} }), &["doc", "page", "paragraph"])), - t("text_edit", "Edit text", "Replace the text of one paragraph (paragraph, from text_paragraphs: rewrapped to the paragraph's width with its line spacing) or one line (line, from text_lines) in place, keeping position, size and colour. For a mixed-style region, editor_text supplies visual row breaks; surviving text keeps its source-run styles and inserted text inherits the surrounding run. For a paragraph, also change its formatting: font (helvetica, times, courier) with bold/italic, size (points), color (#rrggbb), align (left, center, right, justify), underline, line_spacing (× size), char_spacing (points) and scale (horizontal, percent), or move it (dx, dy in points; up is +dy) and rewrap it to a new width (points); text may then be omitted. Its own font is reused when it can show every character and no font/weight change is requested; otherwise it uses a standard font matching the source style or, for Japanese and other text no standard font can show (Cyrillic, Greek), real Mincho/Gothic fallback outlines matching the requested/source family and available weight, from a face that has every character (the result shows the font used). Missing Japanese weights use Regular, not synthetic bold; the small web build has only Gothic Regular. Paragraph bold may be set without font to keep the source family. Text that no available font can show is refused. Undoable.") + t("text_edit", "Edit text", "Replace the text of one paragraph (paragraph, from text_paragraphs: rewrapped to the paragraph's width with its line spacing) or one line (line, from text_lines) in place, keeping position, size and colour. For a mixed-style region, editor_text supplies visual row breaks; surviving text keeps its source-run styles and inserted text inherits the surrounding run. For a paragraph, also change its formatting: font (helvetica, times, courier) with bold/italic, size (points), color (#rrggbb), align (left, center, right, justify), underline, line_spacing (× size), char_spacing (points) and scale (horizontal, percent), or move it (dx, dy in points; up is +dy) and rewrap it to a new width (points); text may then be omitted. Its own font is reused when it can show every character and no font/weight change is requested; otherwise it uses a standard font matching the source style or, for Japanese and other text no standard font can show (Cyrillic, Greek), real Mincho/Gothic fallback outlines matching the requested/source family and available weight, from a face that has every character (the result shows the font used). Missing Japanese weights use Regular, not synthetic bold; the small web build has only Gothic Regular. Paragraph bold may be set without font to keep the source family. Text that no available font can show is refused. A paragraph number is refused when an earlier edit merged that paragraph into a neighbour (list the page again); the result's stale_paragraphs names the numbers gone. Undoable.") .cmd("edit.edit_text") .with(schema( json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "line": { "type": "integer", "minimum": 1 }, diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index cbf40f1962..ad9c84c14d 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -6855,3 +6855,77 @@ fn text_box_and_callout_fill_and_border_through_tools() { assert_eq!(session.get(saved).unwrap().comment_props(0, index).unwrap().fill, None); } } + +/// Three paragraphs: Alpha (two rows), Bravo (one row) and Charlie (two rows), at the leading +/// of #2516. A third Alpha row brings its last baseline within 2.5 × size of Bravo's, so the next +/// read groups the two as one paragraph. +fn three_paragraph_pdf() -> Vec { + use pdfcraft_cos::{Dict, Document, ObjRef, Object, SaveOptions, Stream, write_full}; + use std::sync::Arc; + + let content = "BT /F1 12 Tf 14.4 TL 1 0 0 1 10 250 Tm (Alpha paragraph, first row.) Tj T* (Alpha paragraph, second row.) Tj ET \ + BT /F1 12 Tf 14.4 TL 1 0 0 1 10 200 Tm (Bravo paragraph, a single row.) Tj ET \ + BT /F1 12 Tf 14.4 TL 1 0 0 1 10 150 Tm (Charlie paragraph, first row.) Tj T* (Charlie paragraph, second row.) Tj ET"; + let mut doc = Document::open(Arc::new(fixture(1))).unwrap(); + doc.set(ObjRef::new(5, 0), Object::Stream(Stream::flate(Dict::new(), content.as_bytes()))); + write_full(&doc, &SaveOptions::default()).unwrap() +} + +/// #2516: a paragraph number names the paragraph `text_paragraphs` listed. When an edit regroups +/// the page so that paragraph no longer exists on its own, the next edit by that number is +/// refused. It used to rewrite whichever paragraph had taken the number, and that text was gone. +#[test] +fn text_edit_refuses_a_paragraph_number_an_earlier_edit_regrouped() { + let dir = workdir("text-edit-stale-number"); + std::fs::write(dir.join("p.pdf"), three_paragraph_pdf()).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({"path":"p.pdf"}))["doc"].as_u64().unwrap(); + let listed = ok(&mut a, "text_paragraphs", json!({"doc":doc,"page":1})); + assert_eq!(listed["count"], 3); + assert!(listed["paragraphs"][1]["text"].as_str().unwrap().starts_with("Bravo")); + + // Listing again here would renumber the page, so the first edit's result says what moved. + let first = ok( + &mut a, + "text_edit", + json!({"doc":doc,"page":1,"paragraph":1,"text":"Alpha paragraph, first row.\nAlpha paragraph, second row.\nAlpha paragraph, a third row."}), + ); + assert_eq!(first["stale_paragraphs"], json!([2]), "the third row regroups Alpha with Bravo: {first}"); + + // Paragraph 2 of the earlier listing was Bravo, which is now part of Alpha's paragraph. + let stale = a.call("text_edit", &json!({"doc":doc,"page":1,"paragraph":2,"text":"Bravo paragraph, edited."})); + assert!(matches!(&stale, Err(ToolError::InvalidArgs(m)) if m.contains("text_paragraphs")), "{stale:?}"); + let text = page_text(&mut a, doc).join(" "); + assert!(text.contains("Charlie paragraph, first row."), "Charlie is untouched: {text}"); + assert!(text.contains("Charlie paragraph, second row."), "Charlie is untouched: {text}"); + assert!(!text.contains("Bravo paragraph, edited."), "nothing was rewritten: {text}"); + + // After listing again, the number names what the listing says. + let relisted = ok(&mut a, "text_paragraphs", json!({"doc":doc,"page":1})); + assert!(relisted["paragraphs"][1]["text"].as_str().unwrap().starts_with("Charlie")); + ok(&mut a, "text_edit", json!({"doc":doc,"page":1,"paragraph":2,"text":"Charlie paragraph, edited."})); + let text = page_text(&mut a, doc).join(" "); + assert!(text.contains("Charlie paragraph, edited.") && text.contains("Alpha paragraph, a third row."), "{text}"); + std::fs::remove_dir_all(dir).unwrap(); +} + +/// A number from the listing keeps naming its paragraph while other paragraphs are edited, and +/// a paragraph can be edited twice in a row by the same number. +#[test] +fn text_edit_numbers_survive_edits_that_do_not_regroup() { + let dir = workdir("text-edit-stable-number"); + std::fs::write(dir.join("p.pdf"), three_paragraph_pdf()).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({"path":"p.pdf"}))["doc"].as_u64().unwrap(); + ok(&mut a, "text_paragraphs", json!({"doc":doc,"page":1})); + ok(&mut a, "text_edit", json!({"doc":doc,"page":1,"paragraph":3,"text":"Charlie one.\nCharlie two."})); + ok(&mut a, "text_edit", json!({"doc":doc,"page":1,"paragraph":1,"text":"Alpha one.\nAlpha two."})); + ok(&mut a, "text_edit", json!({"doc":doc,"page":1,"paragraph":1,"text":"Alpha again.\nAlpha two."})); + ok(&mut a, "text_edit", json!({"doc":doc,"page":1,"paragraph":3,"text":"Charlie once more.\nCharlie two."})); + // A rewrite rewraps at the width of the widest row, so compare without line breaks. + let text = page_text(&mut a, doc).join(" ").split_whitespace().collect::>().join(" "); + for want in ["Alpha again.", "Bravo paragraph, a single row.", "Charlie once more."] { + assert!(text.contains(want), "{want}: {text}"); + } + std::fs::remove_dir_all(dir).unwrap(); +} diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index c1062b4105..e53aa6be38 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -859,8 +859,8 @@ pub mod indic; mod linebreak; pub mod text; pub use text::{ - BlockStyle, LineEdit, MAX_FORM_DEPTH, MAX_FORM_VISITS, RangeStyle, ReadingBlocks, TextBlock, TextLine, clear_block, reading_blocks, - reading_blocks_checked, replace_block, replace_line, rewrite_block, text_blocks, text_lines, + BlockKey, BlockStyle, LineEdit, MAX_FORM_DEPTH, MAX_FORM_VISITS, RangeStyle, ReadingBlocks, TextBlock, TextLine, block_overlapping, clear_block, + reading_blocks, reading_blocks_checked, replace_block, replace_line, rewrite_block, text_blocks, text_lines, }; pub mod added; pub use added::{ diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index 2672592178..b4696ba60a 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -2010,6 +2010,57 @@ fn on_same_baseline(line: &TextLine, target: &TextLine) -> bool { same_dir && same_size && offset.is_finite() && offset < size * 0.3 } +/// How a listing of [`text_blocks`] records a paragraph so that it can be found again. +/// +/// The index of a paragraph is its position in a list regrouped from geometry on every read, so +/// an edit that merges or splits paragraphs makes the same index name another one (#2516). A key +/// names the paragraph by what it says and where it is; a paragraph that no longer exists as +/// such has no match, instead of a neighbour taking its place. +#[derive(Clone, Debug, PartialEq)] +pub struct BlockKey { + text: String, + rect: [f64; 4], +} + +impl BlockKey { + /// Two boxes are the same box within this many points. + const TOLERANCE: f64 = 0.01; + + pub fn of(block: &TextBlock) -> Self { + Self { text: block.text.clone(), rect: block.rect } + } + + /// The paragraph's box when it was recorded (user space). + pub fn rect(&self) -> [f64; 4] { + self.rect + } + + /// The paragraph in `blocks` with this text and box. `hint` is tried first, so two + /// paragraphs drawn on top of one another are told apart by their order. + pub fn find(&self, blocks: &[TextBlock], hint: usize) -> Option { + let matches = |b: &TextBlock| b.text == self.text && b.rect.iter().zip(&self.rect).all(|(a, b)| (a - b).abs() <= Self::TOLERANCE); + match blocks.get(hint) { + Some(b) if matches(b) => Some(hint), + _ => blocks.iter().position(matches), + } + } +} + +/// The paragraph in `blocks` whose box overlaps `rect` the most, if any overlaps it at all. An +/// edit rewrites a paragraph in place, so this is the paragraph it became. +pub fn block_overlapping(blocks: &[TextBlock], rect: [f64; 4]) -> Option { + // `f64::min` and `max` skip a NaN, which would make a box that isn't a number overlap anything. + let area = |b: &TextBlock| { + if !rect.iter().chain(&b.rect).all(|v| v.is_finite()) { + return 0.0; + } + let w = b.rect[2].min(rect[2]) - b.rect[0].max(rect[0]); + let h = b.rect[3].min(rect[3]) - b.rect[1].max(rect[1]); + if w > 0.0 && h > 0.0 { w * h } else { 0.0 } + }; + blocks.iter().enumerate().map(|(i, b)| (i, area(b))).filter(|(_, a)| *a > 0.0).max_by(|(_, a), (_, b)| a.total_cmp(b)).map(|(i, _)| i) +} + /// The paragraphs on a page (0-based). pub fn text_blocks(doc: &Document, page: usize) -> Result, EditError> { let lines = text_lines(doc, page)?; @@ -2789,6 +2840,55 @@ pub(crate) fn translate_blocks(doc: &mut Document, page: usize, indexes: &[usize mod tests { use super::*; + fn block(text: &str, rect: [f64; 4]) -> TextBlock { + TextBlock { + text: text.into(), + rect, + base_font: String::new(), + size: 12.0, + bold: false, + italic: false, + color: [0.0; 3], + rotation: 0.0, + lines: Vec::new(), + rows: Vec::new(), + editor_text: text.into(), + spans: Vec::new(), + invisible: false, + } + } + + #[test] + fn a_block_key_finds_its_paragraph_after_others_move_and_not_a_neighbour_in_its_place() { + let (a, b, c) = (block("Alpha", [0.0, 80.0, 50.0, 100.0]), block("Bravo", [0.0, 50.0, 50.0, 60.0]), block("Charlie", [0.0, 0.0, 50.0, 20.0])); + let key = BlockKey::of(&b); + assert_eq!(key.find(&[a.clone(), b.clone(), c.clone()], 1), Some(1)); + // Alpha and Bravo became one paragraph: Bravo's number now belongs to Charlie. + let merged = block("Alpha Bravo", [0.0, 50.0, 50.0, 100.0]); + assert_eq!(key.find(&[merged, c.clone()], 1), None); + // A paragraph that split in two shifts the others down. + assert_eq!(key.find(&[a.clone(), block("x", [0.0; 4]), b.clone(), c], 1), Some(2)); + // The same words somewhere else, or other words in the same place, are not the paragraph. + assert_eq!(key.find(&[block("Bravo", [0.0, 40.0, 50.0, 50.0])], 0), None); + assert_eq!(key.find(&[block("Bravo!", b.rect)], 0), None); + assert_eq!(key.find(&[], 0), None); + // Two paragraphs drawn on top of one another are told apart by the hint. + assert_eq!(key.find(&[b.clone(), b], 1), Some(1)); + // A box that is not a number matches nothing, and cannot panic. + let nan = BlockKey::of(&block("Bravo", [f64::NAN; 4])); + assert_eq!(nan.find(&[block("Bravo", [f64::NAN; 4])], 0), None); + } + + #[test] + fn the_block_that_overlaps_most_is_the_edited_paragraph() { + let blocks = [block("a", [0.0, 80.0, 50.0, 100.0]), block("b", [0.0, 40.0, 50.0, 70.0])]; + assert_eq!(block_overlapping(&blocks, [0.0, 60.0, 50.0, 95.0]), Some(0)); + assert_eq!(block_overlapping(&blocks, [0.0, 35.0, 50.0, 75.0]), Some(1)); + assert_eq!(block_overlapping(&blocks, [200.0, 0.0, 250.0, 10.0]), None, "a box that overlaps nothing"); + assert_eq!(block_overlapping(&[], [0.0, 0.0, 1.0, 1.0]), None); + assert_eq!(block_overlapping(&blocks, [f64::NAN; 4]), None); + } + #[test] fn paragraph_wrapping_keeps_ideographic_spaces() { let width = |s: &str| s.chars().count() as f64; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index dfe11b25c6..ff70dd7d2c 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -62,7 +62,7 @@ pub use pdfcraft_forms::{ }; pub use pdfcraft_a11y as a11y; -pub use pdfcraft_edit::{BlockStyle, PageImage, RangeStyle, TextBlock, TextLine, first_undrawable}; +pub use pdfcraft_edit::{BlockKey, BlockStyle, PageImage, RangeStyle, TextBlock, TextLine, block_overlapping, first_undrawable}; pub use pdfcraft_measure as measure; pub use pdfcraft_xfa::Report as XfaLayout; diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index eb9244a284..456d37bc9b 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -501,7 +501,8 @@ pub struct DocView { pub line_editor: Option, /// The box a click switched to while another paragraph was being edited: it opens once the /// queued edit has applied, so its index and geometry come from the document it now is (#2022). - pub(crate) pending_text_open: Option<(usize, usize)>, + /// It is held as the paragraph's text and box, not an index: the edit can regroup the page. + pub(crate) pending_text_open: Option<(usize, pdfcraft_engine::BlockKey)>, pub(crate) text_background: crate::edit_text_ui::TextBackground, pub(crate) move_background: crate::content_ui::MoveBackground, /// Edit text & images: the images per page (by document generation), and the selected one. diff --git a/crates/ui-egui/src/edit_text_ui.rs b/crates/ui-egui/src/edit_text_ui.rs index 66ad98ffe6..442a05f7ea 100644 --- a/crates/ui-egui/src/edit_text_ui.rs +++ b/crates/ui-egui/src/edit_text_ui.rs @@ -1000,10 +1000,15 @@ pub(crate) fn page_input( // applied and `lines`/`boxes` are the document's new inventory (#2022). if allow_interaction && active.is_none() - && let Some((open_page, block)) = view.pending_text_open + && let Some((open_page, key)) = view.pending_text_open.clone() && open_page == page { - open_editor(ui, xf, doc, lines, &boxes, page, block, view); + // The edit may have regrouped the page: open the paragraph that was clicked, wherever it + // is now, and none if it was merged into another (#2516). + match key.find(lines, 0) { + Some(block) => open_editor(ui, xf, doc, lines, &boxes, page, block, view), + None => view.pending_text_open = None, + } } if !allow_interaction { return false; @@ -1105,7 +1110,7 @@ pub(crate) fn page_input( } } // The clicked box opens next frame, from the document with that edit applied. - view.pending_text_open = Some((page, hit)); + view.pending_text_open = lines.get(hit).map(|l| (page, pdfcraft_engine::BlockKey::of(l))); return true; } open_editor(ui, xf, doc, lines, &boxes, page, hit, view); diff --git a/docs/editing-existing-text.md b/docs/editing-existing-text.md index e8bb03b9a4..af213a6e5e 100644 --- a/docs/editing-existing-text.md +++ b/docs/editing-existing-text.md @@ -94,3 +94,16 @@ invalid ranges, font selection and root confinement. CLI acceptance repeats inse and two saves/reopens on four pages; independent PDFium checks extraction and rendering. No private document, font or screenshot is included in the repository. + +## Paragraph numbers (automation) + +A paragraph's number is its position in a list that is regrouped from geometry on every read, +so an edit that merges two paragraphs, or splits one, moves the others. After `text_paragraphs` +the numbers name the paragraphs *that listing showed*: `text_edit` and `text_edit_layout` find +the paragraph again by its text and box, so editing paragraph 3 then paragraph 1 still edits the +two the listing showed. A paragraph an edit merged into a neighbour (or changed) is refused with +a message to list the page again, instead of rewriting whichever paragraph took its number +(#2516). The result of an edit names the numbers that are gone in `stale_paragraphs`. Listing +a page again renumbers it. Without a listing, a number is a position in the page's current +paragraphs, as before. The inline editor holds the paragraph a switching click asked for the +same way. What is still missing is an identity that survives save and reopen (design: #2508).