From ae21773fefa919340e1b910e4eca24264007ba6f Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 8 Nov 2026 19:40:50 -0500 Subject: [PATCH] fix(edit): stop rescanning what a loop already holds in wrapping, grouping and flattening (#5963) --- crates/edit/src/added/layout.rs | 17 +++++++++++------ crates/edit/src/added/store.rs | 11 +++++++++-- crates/edit/src/flatten.rs | 17 +++++++++++++++-- crates/edit/src/tests.rs | 14 ++++++++++++++ crates/edit/src/text_regions/grouping.rs | 6 +++++- 5 files changed, 54 insertions(+), 11 deletions(-) diff --git a/crates/edit/src/added/layout.rs b/crates/edit/src/added/layout.rs index d1be9ef521..a6a55224d9 100644 --- a/crates/edit/src/added/layout.rs +++ b/crates/edit/src/added/layout.rs @@ -54,16 +54,21 @@ pub(super) fn wrapped(t: &AddedText) -> Vec<(String, bool, usize)> { let measure = |s: &str| if rtl_script || thai { shaped_width(t, s, rtl) } else { latin_width(t, s) }; let space = measure(" "); let (mut line, mut line_w) = (String::new(), 0.0); + // Widths add up (the standard-14 widths have no kerning), so each word is measured once + // and appended in place: a line that never fills (negative character spacing) stays + // linear instead of being copied and measured again for every word. for word in para.split(' ') { - let candidate = if line.is_empty() { word.to_string() } else { format!("{line} {word}") }; - let candidate_w = - if rtl_script || thai { if line.is_empty() { measure(word) } else { line_w + space + measure(word) } } else { measure(&candidate) }; + let word_w = measure(word); + let candidate_w = if line.is_empty() { word_w } else { line_w + space + word_w }; if !line.is_empty() && candidate_w > width { out.push((std::mem::take(&mut line), rtl, index)); - line = word.to_string(); - line_w = measure(word); + line.push_str(word); + line_w = word_w; } else { - line = candidate; + if !line.is_empty() { + line.push(' '); + } + line.push_str(word); line_w = candidate_w; } } diff --git a/crates/edit/src/added/store.rs b/crates/edit/src/added/store.rs index 3555910f04..a5b53581fc 100644 --- a/crates/edit/src/added/store.rs +++ b/crates/edit/src/added/store.rs @@ -328,8 +328,15 @@ pub fn isolate_content(doc: &mut Document, page: usize, index: usize) -> Result< /// names them so), so they go when the item is replaced or deleted. fn own_fonts(doc: &Document, obj: ObjRef) -> Vec> { let Object::Stream(s) = &*doc.get(obj) else { return Vec::new() }; - let Ok(data) = s.decoded() else { return Vec::new() }; - data.split(u8::is_ascii_whitespace).filter_map(|w| w.strip_prefix(b"/")).filter(|w| w.starts_with(b"PCAr")).map(<[u8]>::to_vec).collect() + // The stream is one this writer put there, so its decode is bounded by the stream itself: the + // page limit alone would refuse an item larger than a page's content, and its fonts would stay + // on the page's /Font, which `write` and `delete_content` are supposed to take them off. + let limit = pdfcraft_cos::MAX_PAGE_DECODED.max(s.raw.len().saturating_mul(16)); + let Ok(data) = s.decoded_within(limit) else { return Vec::new() }; + // Each name once: a stream that repeats a name is not copied out once per use. + let names: std::collections::BTreeSet<&[u8]> = + data.split(u8::is_ascii_whitespace).filter_map(|w| w.strip_prefix(b"/")).filter(|w| w.starts_with(b"PCAr")).collect(); + names.into_iter().map(<[u8]>::to_vec).collect() } fn drop_fonts(doc: &Document, res: &mut Dict, names: &[Vec]) { diff --git a/crates/edit/src/flatten.rs b/crates/edit/src/flatten.rs index ef86d6c67a..c54b062af6 100644 --- a/crates/edit/src/flatten.rs +++ b/crates/edit/src/flatten.rs @@ -183,6 +183,19 @@ fn flatten_which(doc: &mut Document, pages: &[usize], which: Which) -> Result pdfcraft_model::Page { + let mut dict = page.dict.clone(); + if let Some(own) = doc.get(page.obj).as_dict() { + for (k, v) in own.iter() { + dict.set(k.clone(), v.clone()); + } + } + pdfcraft_model::Page { obj: page.obj, dict } +} + fn flatten_in(doc: &mut Document, pages: &[usize], which: Which) -> Result { let all = page_list(doc); check(pages, all.len())?; @@ -322,10 +335,10 @@ fn flatten_in(doc: &mut Document, pages: &[usize], which: Which) -> Result doc.set(r, Object::Array(kept)), None => doc.update_dict(page.obj, |d| { diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 1c4829b418..700117ac6f 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -9748,6 +9748,20 @@ fn added_text_is_wrapped_once_per_write() { assert!(calls <= 4, "the text was wrapped {calls} times for 40 lines"); } +/// Negative character spacing keeps a line from ever filling its box. Wrapping then holds every +/// word on one line, appended and measured once each rather than copied and measured again for +/// each word (which took minutes for this text); the widths still decide the breaks. +#[test] +fn added_text_that_never_fills_its_line_wraps_in_linear_time() { + let words = vec!["word"; 200_000].join(" "); + let packed = AddedText { rect: [0.0, 0.0, 100.0, 100.0], text: words.clone(), char_spacing: -10.0, ..AddedText::default() }; + let start = std::time::Instant::now(); + assert_eq!(added::lines(&packed), [words]); + assert!(start.elapsed() < std::time::Duration::from_secs(10), "{:?}", start.elapsed()); + let plain = AddedText { rect: [0.0, 0.0, 60.0, 100.0], text: "one two three four".into(), size: 10.0, ..AddedText::default() }; + assert_eq!(added::lines(&plain), ["one two", "three four"]); +} + /// Character spacing on added text (#4245) is the PDF's `Tc`, kept in `/PCAdded` so the item /// reopens with it; an item written without it reads 0. #[test] diff --git a/crates/edit/src/text_regions/grouping.rs b/crates/edit/src/text_regions/grouping.rs index 69b3a16850..1dcb9a9373 100644 --- a/crates/edit/src/text_regions/grouping.rs +++ b/crates/edit/src/text_regions/grouping.rs @@ -366,10 +366,13 @@ fn group_layout(lines: &[TextLine], mixed: bool) -> Vec { if !block.rows.is_empty() { block.editor_text.push('\n'); } + // The run placed before this one in the row, tracked rather than searched for: rows + // don't share runs, so it is the last one this loop pushed. + let mut prev_in_row: Option<&TextLine> = None; for &index in row { let Some(line) = lines.get(index) else { continue }; // Some producers position separate words without encoding a space. - if let Some(prev) = block.lines.last().filter(|i| row.contains(i)).and_then(|i| lines.get(*i)) + if let Some(prev) = prev_in_row && !row_text.ends_with(char::is_whitespace) && !line.text.starts_with(char::is_whitespace) && line.frame_rect()[0] - prev.frame_rect()[2] > line.size * 0.12 @@ -383,6 +386,7 @@ fn group_layout(lines: &[TextLine], mixed: bool) -> Vec { row_text.push_str(&line.text); block.invisible &= line.origin.state.render_mode == 3; block.lines.push(index); + prev_in_row = Some(line); } if !block.text.is_empty() { if block.text.ends_with('-') {