From 1a059304f93173bc0cfed506d9b0091f355a13ea Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 21:29:14 -0500 Subject: [PATCH] PPTX export: table border and paragraph lookups by index, direct charmap lookups (#2256) * pptx: table border lookups by position, paragraph lines grouped once * pptx: look a character up in the face's map, don't expand it * Apply review fixes - crates/pptx/src/table.rs --------- Co-authored-by: aiongg <67543387+aiongg@users.noreply.github.com> --- crates/pptx/src/fonts.rs | 11 ++---- crates/pptx/src/table.rs | 80 +++++++++++++++++++++++++++++----------- crates/pptx/src/text.rs | 29 ++++++++++++--- 3 files changed, 85 insertions(+), 35 deletions(-) diff --git a/crates/pptx/src/fonts.rs b/crates/pptx/src/fonts.rs index 3d5eb4c9b..27c976aaf 100644 --- a/crates/pptx/src/fonts.rs +++ b/crates/pptx/src/fonts.rs @@ -18,7 +18,6 @@ pub struct RunFont { #[derive(Default)] pub struct Fonts { names: HashMap, - cmaps: HashMap>, /// Glyph → the lowest character mapping to it. reverse: HashMap>, /// Faces the slides use, by (family, bold, italic), in first-use order. @@ -106,11 +105,13 @@ impl Fonts { /// Nominal advance of `c` in font units (no kerning or features), as an application without /// the composer's spacing would set it. pub fn advance(&mut self, face: &FontFace, c: char) -> Option { - self.cmap(face).get(&c).map(|g| face.advance(*g)) + self.glyph(face, c).map(|g| face.advance(g)) } + /// The glyph for `c`, looked up in the face's character map directly: expanding the whole map + /// into a table made one lookup cost (and keep) every mapping of the font. pub fn glyph(&mut self, face: &FontFace, c: char) -> Option { - self.cmap(face).get(&c).copied() + Some(face.glyph_for(c)).filter(|g| *g != 0) } /// The character a glyph shows (the lowest code point mapping to it). @@ -124,10 +125,6 @@ impl Fonts { }); map.get(&gid).copied() } - - fn cmap(&mut self, face: &FontFace) -> &HashMap { - self.cmaps.entry(face.id()).or_insert_with(|| face.chars().into_iter().collect()) - } } /// A family embedded in the presentation: its regular, bold, italic and bold italic faces. diff --git a/crates/pptx/src/table.rs b/crates/pptx/src/table.rs index 0c8bd27d7..b4a04d96d 100644 --- a/crates/pptx/src/table.rs +++ b/crates/pptx/src/table.rs @@ -19,8 +19,18 @@ fn edges(mut v: Vec) -> Vec { v } +/// The grid line nearest `x` (the first of two equally near). fn index(edges: &[f64], x: f64) -> usize { - edges.iter().enumerate().min_by(|a, b| (a.1 - x).abs().total_cmp(&(b.1 - x).abs())).map_or(0, |(i, _)| i) + if !x.is_finite() { + return edges.iter().enumerate().min_by(|a, b| (a.1 - x).abs().total_cmp(&(b.1 - x).abs())).map_or(0, |(i, _)| i); + } + // `edges` is sorted: the nearest is one of the two around `x`. + let at = edges.partition_point(|e| *e < x); + match (at.checked_sub(1).and_then(|i| edges.get(i)), edges.get(at)) { + (Some(lo), Some(hi)) if x - lo > hi - x => at, + (Some(_), _) => at - 1, + _ => at.min(edges.len().saturating_sub(1)), + } } pub fn tables(ex: &mut Exporter, ft: &FrameText, m: Affine, k: f64, opacity: f32) { @@ -29,26 +39,51 @@ pub fn tables(ex: &mut Exporter, ft: &FrameText, m: Affine, k: f64, opacity: f32 } } -/// The stroke along one side of a cell (`a`–`b`), if the segments on that line cover most of it -/// (a merged cell's side is drawn in pieces, one per grid cell). -fn edge_stroke(strokes: &[StrokeSeg], a: (f64, f64), b: (f64, f64)) -> Option<&StrokeSeg> { - let vertical = (a.0 - b.0).abs() < 0.25; - let (lo, hi) = if vertical { (a.1, b.1) } else { (a.0, b.0) }; - let mut covered = 0.0; - let mut first = None; - for s in strokes { - let (on, s0, s1) = if vertical { - ((s.a.x - a.0).abs() < 0.75 && (s.b.x - a.0).abs() < 0.75, s.a.y.min(s.b.y), s.a.y.max(s.b.y)) - } else { - ((s.a.y - a.1).abs() < 0.75 && (s.b.y - a.1).abs() < 0.75, s.a.x.min(s.b.x), s.a.x.max(s.b.x)) - }; - let overlap = s1.min(hi) - s0.max(lo); - if on && overlap > 0.0 { - covered += overlap; - first.get_or_insert(s); +/// A table's stroke segments by where they lie, so the sides of each cell don't search them all. +struct EdgeIndex<'a> { + strokes: &'a [StrokeSeg], + /// (x of the first end, index), for segments whose first end is finite; sorted by x. + by_x: Vec<(f64, usize)>, + by_y: Vec<(f64, usize)>, +} + +impl<'a> EdgeIndex<'a> { + fn new(strokes: &'a [StrokeSeg]) -> Self { + let mut by_x: Vec<(f64, usize)> = strokes.iter().enumerate().filter(|(_, s)| s.a.x.is_finite()).map(|(i, s)| (s.a.x, i)).collect(); + let mut by_y: Vec<(f64, usize)> = strokes.iter().enumerate().filter(|(_, s)| s.a.y.is_finite()).map(|(i, s)| (s.a.y, i)).collect(); + by_x.sort_by(|p, q| p.0.total_cmp(&q.0)); + by_y.sort_by(|p, q| p.0.total_cmp(&q.0)); + EdgeIndex { strokes, by_x, by_y } + } + + /// The stroke along one side of a cell (`a`-`b`), if the segments on that line cover most of it + /// (a merged cell's side is drawn in pieces, one per grid cell). + fn stroke(&self, a: (f64, f64), b: (f64, f64)) -> Option<&'a StrokeSeg> { + let vertical = (a.0 - b.0).abs() < 0.25; + let (lo, hi) = if vertical { (a.1, b.1) } else { (a.0, b.0) }; + let (list, line) = if vertical { (&self.by_x, a.0) } else { (&self.by_y, a.1) }; + // Segments whose first end is within 0.75 of the line: the only ones that can be on it. + let from = list.partition_point(|(v, _)| line - *v >= 0.75); + let to = list.partition_point(|(v, _)| *v - line < 0.75); + let mut near: Vec = list.get(from..to).unwrap_or_default().iter().map(|(_, i)| *i).collect(); + // Back in the segments' own order: the first one on the line is the one that shows. + near.sort_unstable(); + let mut covered = 0.0; + let mut first = None; + for s in near.into_iter().filter_map(|i| self.strokes.get(i)) { + let (on, s0, s1) = if vertical { + ((s.a.x - a.0).abs() < 0.75 && (s.b.x - a.0).abs() < 0.75, s.a.y.min(s.b.y), s.a.y.max(s.b.y)) + } else { + ((s.a.y - a.1).abs() < 0.75 && (s.b.y - a.1).abs() < 0.75, s.a.x.min(s.b.x), s.a.x.max(s.b.x)) + }; + let overlap = s1.min(hi) - s0.max(lo); + if on && overlap > 0.0 { + covered += overlap; + first.get_or_insert(s); + } } + first.filter(|_| covered > 0.5 * (hi - lo)) } - first.filter(|_| covered > 0.5 * (hi - lo)) } fn border(ex: &mut Exporter, tag: &str, s: Option<&StrokeSeg>, k: f64, opacity: f32) -> String { @@ -108,6 +143,7 @@ fn table(ex: &mut Exporter, t: &TableFrag, m: Affine, k: f64, opacity: f32) { if t.cells.iter().any(|c| c.graphic.is_some()) { ex.warn("graphic cells are exported empty"); } + let edges_of = EdgeIndex::new(&t.strokes); let id = ex.id(); let mut s = format!( "\ @@ -151,7 +187,7 @@ fn table(ex: &mut Exporter, t: &TableFrag, m: Affine, k: f64, opacity: f32) { ); continue; }; - s.push_str(&cell(ex, c, &attrs, t, k, opacity)); + s.push_str(&cell(ex, c, &attrs, &edges_of, k, opacity)); } s.push_str(""); } @@ -159,7 +195,7 @@ fn table(ex: &mut Exporter, t: &TableFrag, m: Affine, k: f64, opacity: f32) { ex.out.push_str(&s); } -fn cell(ex: &mut Exporter, c: &PlacedCell, attrs: &str, t: &TableFrag, k: f64, opacity: f32) -> String { +fn cell(ex: &mut Exporter, c: &PlacedCell, attrs: &str, edges: &EdgeIndex, k: f64, opacity: f32) -> String { let rc = c.rect; let mut body = String::new(); let mut margins = [0.0f64; 4]; @@ -194,7 +230,7 @@ fn cell(ex: &mut Exporter, c: &PlacedCell, attrs: &str, t: &TableFrag, k: f64, o let mut pr = format!(""); for (tag, side) in [("lnL", 'L'), ("lnR", 'R'), ("lnT", 'T'), ("lnB", 'B')] { let (a, bb) = ln(side); - let seg = edge_stroke(&t.strokes, a, bb); + let seg = edges.stroke(a, bb); pr.push_str(&border(ex, tag, seg, k, opacity)); } match c.fill.as_ref().and_then(|(sw, tint)| ex.color(sw, *tint, opacity)) { diff --git a/crates/pptx/src/text.rs b/crates/pptx/src/text.rs index b15cbe714..005e651a5 100644 --- a/crates/pptx/src/text.rs +++ b/crates/pptx/src/text.rs @@ -187,6 +187,10 @@ fn line_height(seg: &Segment, i: usize) -> f64 { fn para_step(seg: &Segment, i: usize) -> Option { let para = seg.lines.get(i)?.para; let ls: Vec<&OutLine> = seg.lines.iter().filter(|l| l.para == para).collect(); + median_step(&ls) +} + +fn median_step(ls: &[&OutLine]) -> Option { let mut d: Vec = ls.windows(2).map(|w| w[1].baseline - w[0].baseline).filter(|v| *v > 0.0).collect(); if d.is_empty() { return None; @@ -195,6 +199,15 @@ fn para_step(seg: &Segment, i: usize) -> Option { d.get(d.len() / 2).copied() } +/// [`para_step`] for every paragraph of a segment, in one pass (it is asked once per paragraph). +fn para_steps(seg: &Segment) -> std::collections::HashMap> { + let mut by_para: std::collections::HashMap> = std::collections::HashMap::new(); + for l in &seg.lines { + by_para.entry(l.para).or_default().push(l); + } + by_para.into_iter().map(|(p, ls)| (p, median_step(&ls))).collect() +} + fn text_box(ex: &mut Exporter, seg: &Segment, m: Affine, k: f64, opacity: f32, name: &str) { let r = seg_rect(seg); let Some((xfrm, _)) = box_of(r, m) else { return }; @@ -215,6 +228,7 @@ fn write_box(ex: &mut Exporter, xfrm: XfrmBox, body: &str, name: &str) { pub(crate) fn segments(ex: &mut Exporter, cs: &ComposedStory, ft: &FrameText, story: &str, skip: &[usize]) -> Vec { let mut out: Vec = Vec::new(); let mut prev: Option<&Line> = None; + let skip: std::collections::HashSet = skip.iter().copied().collect(); for (i, l) in ft.lines.iter().enumerate() { if skip.contains(&i) { prev = None; @@ -379,13 +393,11 @@ fn body(ex: &mut Exporter, seg: &Segment, r: Rect, k: f64, opacity: f32, insets: pub(crate) fn paragraphs(ex: &mut Exporter, seg: &Segment, r: Rect, k: f64, opacity: f32) -> String { let mut s = String::new(); let mut prev_end: Option<(f64, f64)> = None; - // The segment's lines, a paragraph at a time; `at` is the index of each one's first line. - let mut at = 0; + // The segment's lines, a paragraph at a time. + let steps = para_steps(seg); for lines in seg.lines.chunk_by(|a, b| a.para == b.para && !a.last_in_para) { - let i = at; - at += lines.len(); let (Some(first), Some(last)) = (lines.first(), lines.last()) else { continue }; - let mut h = line_height(seg, i); + let mut h = steps.get(&first.para).copied().flatten().unwrap_or(if first.leading > 0.0 { first.leading } else { first.size * SINGLE }); let r_in = BASELINE_IN_LINE; if let Some((prev_base, prev_h)) = prev_end && lines.len() == 1 @@ -537,12 +549,17 @@ fn reflow(ex: &mut Exporter, it: &Item, tfr: &TextFrame, ft: &FrameText, cs: &Co let _ = write!(s, " anchor=\"{anchor}\">"); // The paragraphs this frame shows, with the runs of their composed glyphs. let ranges = st.para_ranges(); + // The frame's lines by paragraph, once (a filter per paragraph made many paragraphs quadratic). + let mut by_para: std::collections::HashMap> = std::collections::HashMap::new(); + for l in &ft.lines { + by_para.entry(l.para).or_default().push(l); + } let mut first = true; for (pi, range) in ranges.iter().enumerate() { if range.end < ft.range.start || range.start >= ft.range.end.max(ft.range.start + 1) { continue; } - let lines: Vec<&Line> = ft.lines.iter().filter(|l| l.para == pi).collect(); + let lines: Vec<&Line> = by_para.get(&pi).cloned().unwrap_or_default(); let Some(pf) = st.paras.get(pi) else { continue }; let (pp, chars) = doc.styles.resolve_para(pf); let algn = match pp.align {