From 1e2ac1d85ac3fb69739aa7693b85d86c8fe185f1 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 20:21:21 +0000 Subject: [PATCH] fix(xfa): copy embedded field values out only as far as the text budget The text layout cut each embedded value to the bytes its block had left, but the resolver cloned the whole value first. With a byte or two left and a value starting with a multibyte character, every later run cloned the full value and kept none of it. The resolver is given the bytes left and returns only a prefix that fits. --- crates/xfa/src/layout.rs | 41 +++++++++++++++++++++++++--------------- crates/xfa/src/tests.rs | 30 +++++++++++++++++++++++++---- crates/xfa/src/text.rs | 25 ++++++++++++++++-------- 3 files changed, 69 insertions(+), 27 deletions(-) diff --git a/crates/xfa/src/layout.rs b/crates/xfa/src/layout.rs index 326e7f741..c99fc8c92 100644 --- a/crates/xfa/src/layout.rs +++ b/crates/xfa/src/layout.rs @@ -278,6 +278,12 @@ pub fn layout(tpl: &Template, data: Option<&DataNode>) -> Result Ok(Form { pages: l.pages, fields: l.fields, warnings: l.warnings }) } +// Bytes of embedded field values copied out on this thread, for tests that bound layout work. +#[cfg(test)] +thread_local! { + pub(crate) static EMBED_COPIED: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + fn collect_values(nodes: &[Node], out: &mut HashMap, depth: usize) { if depth > MAX_DEPTH { return; @@ -478,12 +484,17 @@ impl Layouter<'_> { self.pages.len() } - fn embed(&self, id: &str) -> String { - match self.tpl.page_roles.iter().find(|(i, _)| i == id).map(|(_, r)| *r) { + /// The text an embedded field `id` shows, at most `max` bytes of a field value (the page + /// number and count are a few bytes; the text layout cuts them). + fn embed(&self, id: &str, max: usize) -> String { + let v = match self.tpl.page_roles.iter().find(|(i, _)| i == id).map(|(_, r)| *r) { Some(PageRole::Number) => self.page_no().to_string(), Some(PageRole::Count) => PAGE_COUNT_MARK.to_string(), - None => self.values.get(id).cloned().unwrap_or_default(), - } + None => self.values.get(id).map(|v| text::prefix(v, max).to_string()).unwrap_or_default(), + }; + #[cfg(test)] + EMBED_COPIED.with(|c| c.set(c.get().saturating_add(v.len() as u64))); + v } /// `TooLarge` once the measurement budget is spent (sizes measured after that are guesses). @@ -1050,7 +1061,7 @@ impl Layouter<'_> { _ => return None, }; // No text (a bordered spacer or rule): nothing to size to, so it keeps what it has room for. - let w = text::natural_width(&rich, para, &Face::of(font), &|id| self.embed(id)); + let w = text::natural_width(&rich, para, &Face::of(font), &|id, max| self.embed(id, max)); (w > 0.0).then_some(w + 0.01) } @@ -1058,8 +1069,8 @@ impl Layouter<'_> { /// an image at its aspect ratio, and nothing for lines, rectangles and empty values. fn value_height(&self, v: &Value, w: f64, ctx: &Ctx) -> f64 { match v { - Value::Text(t) => text::measure_height(&text::plain(t), w, &ctx.para, &Face::of(&ctx.font), &|id| self.embed(id)), - Value::Rich(r) => text::measure_height(r, w, &ctx.para, &Face::of(&ctx.font), &|id| self.embed(id)), + Value::Text(t) => text::measure_height(&text::plain(t), w, &ctx.para, &Face::of(&ctx.font), &|id, max| self.embed(id, max)), + Value::Rich(r) => text::measure_height(r, w, &ctx.para, &Face::of(&ctx.font), &|id, max| self.embed(id, max)), Value::Image { data, .. } => crate::image::size(data).map_or(w, |(iw, ih)| if iw > 0 { w * ih as f64 / iw as f64 } else { w }), Value::Line { .. } | Value::Rectangle(_) | Value::Empty => 0.0, } @@ -1084,7 +1095,7 @@ impl Layouter<'_> { Value::Rich(r) => r.clone(), other => text::plain(&other.plain().unwrap_or_default()), }; - text::measure_height(&rich, inner_w, ¶, &Face::of(&font), &|id| self.embed(id)) + text::measure_height(&rich, inner_w, ¶, &Face::of(&font), &|id, max| self.embed(id, max)) } _ => (Face::of(&font).width(&cap.value.plain().unwrap_or_default()) + 2.0).min(inner_w * 0.6), }); @@ -1107,7 +1118,7 @@ impl Layouter<'_> { other => text::plain(&other.plain().unwrap_or_default()), }; let w = (inner_w - f.ui_margin.horizontal()).max(1.0); - text::measure_height(&rich, w, &ctx.para.apply(&c.para), &Face::of(&ctx.font.apply(&c.font)), &|id| self.embed(id)) + text::measure_height(&rich, w, &ctx.para.apply(&c.para), &Face::of(&ctx.font.apply(&c.font)), &|id, max| self.embed(id, max)) }); cap + f.ui_margin.vertical() } @@ -1121,8 +1132,8 @@ impl Layouter<'_> { Value::Rich(r) => r.clone(), other => text::plain(&other.plain().unwrap_or_default()), }; - text::measure_height(&rich, (inner_w - r).max(1.0), &ctx.para.apply(&c.para), &Face::of(&ctx.font.apply(&c.font)), &|id| { - self.embed(id) + text::measure_height(&rich, (inner_w - r).max(1.0), &ctx.para.apply(&c.para), &Face::of(&ctx.font.apply(&c.font)), &|id, max| { + self.embed(id, max) }) }); ui.max(cap.unwrap_or(0.0)) @@ -1158,7 +1169,7 @@ impl Layouter<'_> { let padding = if f.access == Access::NonInteractive { 0.0 } else { 4.0 }; let w = (inner_w - beside - f.ui_margin.horizontal() - padding).max(1.0); let font = ctx.font.apply(&f.common.font); - text::measure_height(&text::plain(value.trim_end()), w, &ctx.para, &Self::field_text_face(f, &font), &|_| String::new()) + text::measure_height(&text::plain(value.trim_end()), w, &ctx.para, &Self::field_text_face(f, &font), &|_, _| String::new()) } /// The height of an exclusion group's fields at `inner_w`, by the group's layout: stacked @@ -1427,9 +1438,9 @@ impl Layouter<'_> { // Text that doesn't fit the height it was given (the template's face is narrower than // the standard one drawn in its place, so it wraps to more lines) is drawn smaller, // down to 70%, rather than over what lies below. - let fits = |k: f64| text::measure_height(&rich, rect.w / k, para, &face, &|id| self.embed(id)) <= rect.h / k + 1.0; + let fits = |k: f64| text::measure_height(&rich, rect.w / k, para, &face, &|id, max| self.embed(id, max)) <= rect.h / k + 1.0; let scale = if rect.h < 4.0 || fits(1.0) { 1.0 } else { [0.95, 0.9, 0.85, 0.8, 0.75].into_iter().find(|k| fits(*k)).unwrap_or(0.7) }; - let mut block = text::layout(&rich, Rect::new(rect.x, rect.y, rect.w / scale, rect.h / scale), para, &face, &|id| self.embed(id)); + let mut block = text::layout(&rich, Rect::new(rect.x, rect.y, rect.w / scale, rect.h / scale), para, &face, &|id, max| self.embed(id, max)); if scale < 1.0 { for s in &mut block.spans { s.x = rect.x + (s.x - rect.x) * scale; @@ -1672,7 +1683,7 @@ impl Layouter<'_> { let para = Para { v_align: VAlign::Top, ..fctx.para.clone() }; // In the face it was measured in, so it takes the lines it was given. let face = Self::field_text_face(f, &fctx.font); - let block = text::layout(&text::plain(value.trim_end()), ui.inset(&m), ¶, &face, &|_| String::new()); + let block = text::layout(&text::plain(value.trim_end()), ui.inset(&m), ¶, &face, &|_, _| String::new()); return self.push_block(block); } } diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index a4ebacafd..5ada20e5d 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -2638,7 +2638,7 @@ fn a_space_at_the_end_of_a_line_does_not_count_when_fitting_it() { assert_eq!(baselines.len(), 1, "{spans:?}"); // Nor do they add a blank line: the text measures as tall as without them. let face = text::Face::of(&Font { typeface: "Arial".into(), size: 10.0, ..Font::default() }); - let height = |t: &str| text::measure_height(&text::plain(t), w, &Para::default(), &face, &|_| String::new()); + let height = |t: &str| text::measure_height(&text::plain(t), w, &Para::default(), &face, &|_, _| String::new()); assert_eq!(height("a. DATE "), height("a. DATE")); // Text that is wider than the box still wraps. let spans = draw_spans(&one_draw("a. DATES ", w, "left")); @@ -2664,7 +2664,7 @@ fn a_long_word_breaks_in_linear_work() { let face = text::Face::of(&Font { typeface: "Arial".into(), size: 1.0, ..Font::default() }); let word = "i".repeat(20_000); text::MEASURED.with(|m| m.set(0)); - let b = text::layout(&text::plain(&word), Rect::new(0.0, 0.0, 1000.0, 0.0), &Para::default(), &face, &|_| String::new()); + let b = text::layout(&text::plain(&word), Rect::new(0.0, 0.0, 1000.0, 0.0), &Para::default(), &face, &|_, _| String::new()); let measured = text::MEASURED.with(|m| m.get()); assert!(b.spans.len() > 2, "{} lines", b.spans.len()); assert_eq!(b.spans.iter().map(|s| s.text.len()).sum::(), 20_000); @@ -2678,11 +2678,33 @@ fn embedded_field_values_expand_to_a_bounded_amount_of_text() { let value = "word ".repeat(3_300); let run = Run { embed: Some("f".into()), ..Run::default() }; let rich = RichText { paragraphs: vec![Paragraph { runs: vec![run; 100], h_align: None }] }; - let b = text::layout(&rich, Rect::new(0.0, 0.0, 500.0, 0.0), &Para::default(), &face, &|_| value.clone()); + let b = text::layout(&rich, Rect::new(0.0, 0.0, 500.0, 0.0), &Para::default(), &face, &|_, _| value.clone()); let kept: usize = b.spans.iter().map(|s| s.text.len()).sum(); assert!(kept > 0); assert!(kept <= text::MAX_EMBEDDED, "{kept} bytes of embedded text laid out"); - assert!(text::natural_width(&rich, &Para::default(), &face, &|_| value.clone()) <= face.width(&value) * 70.0); + assert!(text::natural_width(&rich, &Para::default(), &face, &|_, _| value.clone()) <= face.width(&value) * 70.0); +} + +/// Once the embedded-text budget is nearly spent, a long value embedded again is copied out only +/// as far as the budget reaches, not whole each time. +#[test] +fn embedded_values_are_copied_out_only_as_far_as_the_budget() { + let first: String = "xxxxxxx ".repeat(text::MAX_EMBEDDED / 8 + 1).chars().take(text::MAX_EMBEDDED - 1).collect(); + let second = "\u{e9}".repeat(100_000); + let span = |id: &str| format!(r##""##); + let spans = format!("{}{}", span("a"), span("b").repeat(200)); + let tpl = format!( + r#""# + ); + crate::layout::EMBED_COPIED.with(|c| c.set(0)); + layout_xml(&tpl).unwrap(); + let copied = crate::layout::EMBED_COPIED.with(|c| c.get()); + // The draw is measured at most seven times before it is laid out once. + assert!(copied <= 8 * text::MAX_EMBEDDED as u64 + 8 * 4 * 201, "{copied} bytes of embedded values copied"); } /// Repeated subforms each carry their own copy of a long script: the live form holds a bounded diff --git a/crates/xfa/src/text.rs b/crates/xfa/src/text.rs index 63de87244..7e8321268 100644 --- a/crates/xfa/src/text.rs +++ b/crates/xfa/src/text.rs @@ -181,19 +181,28 @@ fn ink_width(w: &Word) -> f64 { /// holds: a value embedded over and over is expanded only so far. pub(crate) const MAX_EMBEDDED: usize = 1 << 20; +/// The longest prefix of `s` at most `max` bytes long that ends on a character boundary. +pub fn prefix(s: &str, max: usize) -> &str { + if s.len() <= max { + return s; + } + let end = s.char_indices().map(|(i, _)| i).take_while(|i| *i <= max).last().unwrap_or(0); + s.get(..end).unwrap_or_default() +} + /// Break the runs of a paragraph into words (spaces stay attached to the word before them). -/// Embedded values take from `embed_left` bytes and are cut when it runs out. -fn words(runs: &[Run], base: &Face, embed: &dyn Fn(&str) -> String, embed_left: &mut usize) -> Vec { +/// Embedded values take from `embed_left` bytes and are cut when it runs out; `embed` is asked +/// for at most that many bytes of each. +fn words(runs: &[Run], base: &Face, embed: &dyn Fn(&str, usize) -> String, embed_left: &mut usize) -> Vec { let mut out = Vec::new(); for run in runs { let face = base.with_run(run); let text: String = match &run.embed { Some(_) if *embed_left == 0 => String::new(), Some(id) => { - let mut v = embed(id); + let mut v = embed(id, *embed_left); if v.len() > *embed_left { - let end = v.char_indices().map(|(i, _)| i).take_while(|i| *i <= *embed_left).last().unwrap_or(0); - v.truncate(end); + v = prefix(&v, *embed_left).to_string(); } *embed_left = embed_left.saturating_sub(v.len()); v @@ -228,7 +237,7 @@ fn words(runs: &[Run], base: &Face, embed: &dyn Fn(&str) -> String, embed_left: /// Lay `rich` out inside `rect` with `para` and `base` face. `embed` resolves embedded fields to /// their text. Returns the spans (absolute coordinates) and the height the text takes. -pub fn layout(rich: &RichText, rect: Rect, para: &Para, base: &Face, embed: &dyn Fn(&str) -> String) -> Block { +pub fn layout(rich: &RichText, rect: Rect, para: &Para, base: &Face, embed: &dyn Fn(&str, usize) -> String) -> Block { struct Line { words: Vec, width: f64, @@ -349,7 +358,7 @@ pub fn layout(rich: &RichText, rect: Rect, para: &Para, base: &Face, embed: &dyn /// The width `rich` takes with no line wrapped (its longest paragraph, with the paragraph's /// margins and first-line indent): how wide a box without an explicit width grows. -pub fn natural_width(rich: &RichText, para: &Para, base: &Face, embed: &dyn Fn(&str) -> String) -> f64 { +pub fn natural_width(rich: &RichText, para: &Para, base: &Face, embed: &dyn Fn(&str, usize) -> String) -> f64 { let indent = para.text_indent.max(0.0); let mut embed_left = MAX_EMBEDDED; let widest = @@ -358,7 +367,7 @@ pub fn natural_width(rich: &RichText, para: &Para, base: &Face, embed: &dyn Fn(& } /// Height `rich` needs at `width` (for boxes without an explicit height). -pub fn measure_height(rich: &RichText, width: f64, para: &Para, base: &Face, embed: &dyn Fn(&str) -> String) -> f64 { +pub fn measure_height(rich: &RichText, width: f64, para: &Para, base: &Face, embed: &dyn Fn(&str, usize) -> String) -> f64 { let p = Para { v_align: crate::model::VAlign::Top, ..para.clone() }; layout(rich, Rect::new(0.0, 0.0, width.max(1.0), 0.0), &p, base, embed).height }