From f86928d0d50e6fdee166d846ea1a3477ae2658ce Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 05:16:07 -0400 Subject: [PATCH] perf(pptx): index export lookups and deduplicate relationship parts (#1281) - Row 31: Use HashMap for comment author lookup in author_slot - Row 37: Index preserved parts by ID before resolving graphic frame relationships - Row 10: Index kept chart parts by normalized path and avoid duplicate payload scans - Row 39: Update existing relationship parts instead of appending duplicates in PackageOut --- crates/pptx/src/opc.rs | 7 ++++- crates/pptx/src/write/chart.rs | 10 +++++-- crates/pptx/src/write/mod.rs | 50 +++++++++++++++++++++------------ crates/pptx/src/write/shapes.rs | 24 +++++++++------- 4 files changed, 60 insertions(+), 31 deletions(-) diff --git a/crates/pptx/src/opc.rs b/crates/pptx/src/opc.rs index b8406dc8..f39b88bf 100644 --- a/crates/pptx/src/opc.rs +++ b/crates/pptx/src/opc.rs @@ -387,7 +387,12 @@ impl PackageOut { if rels.items.is_empty() { return; } - self.files.push((rels_path(part), rels.to_xml(), true)); + let path = rels_path(part); + if let Some(existing) = self.files.iter_mut().find(|(name, ..)| *name == path) { + existing.1 = rels.to_xml(); + } else { + self.files.push((path, rels.to_xml(), true)); + } } /// Add a part with no content type of its own (a relationships part). pub fn raw(&mut self, name: &str, data: Vec) { diff --git a/crates/pptx/src/write/chart.rs b/crates/pptx/src/write/chart.rs index 88917450..5dcbe33b 100644 --- a/crates/pptx/src/write/chart.rs +++ b/crates/pptx/src/write/chart.rs @@ -750,11 +750,17 @@ pub fn write_kept(pkg: &mut crate::opc::PackageOut, chart_name: &str, c: &Chart, } } pkg.rels(chart_name, &rels); + let parts_by_norm: std::collections::HashMap<_, _> = c.kept.parts.iter().map(|p| (norm(&p.name), p)).collect(); + let mut seen_payloads = std::collections::HashSet::new(); // The kept parts' own relationships, pointed at the new names. for p in c.kept.parts.iter().filter(|p| kept_payload(&p.name)) { - let Some(new_source) = names.get(&norm(&p.name)) else { continue }; + let norm_name = norm(&p.name); + if !seen_payloads.insert(norm_name.clone()) { + continue; + } + let Some(new_source) = names.get(&norm_name) else { continue }; let rp = norm(&rels_path(&p.name)); - let Some(rels_part) = c.kept.parts.iter().find(|q| norm(&q.name) == rp) else { continue }; + let Some(rels_part) = parts_by_norm.get(&rp) else { continue }; let mut out = RelsOut::default(); for r in crate::opc::rels_from_bytes(&rels_part.bytes, &p.name).items { if r.external { diff --git a/crates/pptx/src/write/mod.rs b/crates/pptx/src/write/mod.rs index f96f82c5..5d3e60e9 100644 --- a/crates/pptx/src/write/mod.rs +++ b/crates/pptx/src/write/mod.rs @@ -637,6 +637,7 @@ pub fn export(p: &Presentation, main_ct: &str) -> Result, PptxError> { // Comment authors. let mut authors: Vec<(String, String, u32)> = vec![]; + let mut author_map: std::collections::HashMap = std::collections::HashMap::new(); let mut n_comments = 0u32; // Slides. @@ -662,13 +663,13 @@ pub fn export(p: &Presentation, main_ct: &str) -> Result, PptxError> { n_comments += 1; let cname = format!("ppt/comments/comment{n_comments}.xml"); so.rels.add("comments", &opc::relative(&sname, &cname), false); - x.pkg.xml_part(&cname, CT_COMMENTS, comments_xml(&s.comments, &mut authors)); + x.pkg.xml_part(&cname, CT_COMMENTS, comments_xml(&s.comments, &mut authors, &mut author_map)); // The modern threaded part next to the flattened legacy one (issue #70): // PowerPoint 2021+ and DeckCraft read replies and resolved state from it, // older readers keep the legacy part. let mname = format!("ppt/comments/modernComment{n_comments}.xml"); let rid = so.rels.add(RT_COMMENTS_MODERN, &opc::relative(&sname, &mname), false); - x.pkg.xml_part(&mname, CT_COMMENTS_MODERN, modern_comments_xml(&s.comments, &cname, &mut authors)); + x.pkg.xml_part(&mname, CT_COMMENTS_MODERN, modern_comments_xml(&s.comments, &cname, &mut authors, &mut author_map)); comment_rel = Some(rid); } let xml = slide_xml(&mut x, &mut so, s, comment_rel.as_deref()); @@ -828,7 +829,7 @@ fn notes_xml(x: &mut Exp, o: &mut Out, notes: &TextBody) -> Vec { w.finish() } -fn comments_xml(list: &[Comment], authors: &mut Vec<(String, String, u32)>) -> Vec { +fn comments_xml(list: &[Comment], authors: &mut Vec<(String, String, u32)>, author_map: &mut std::collections::HashMap) -> Vec { let mut w = W::new(); root_open(&mut w, "p:cmLst", A::new()); let mut flat: Vec<&Comment> = vec![]; @@ -839,7 +840,7 @@ fn comments_xml(list: &[Comment], authors: &mut Vec<(String, String, u32)>) -> V } } for c in flat { - let ai = author_slot(c, authors); + let ai = author_slot(c, authors, author_map); let idx = match authors.get_mut(ai) { Some(a) => { a.2 += 1; @@ -860,32 +861,45 @@ fn comments_xml(list: &[Comment], authors: &mut Vec<(String, String, u32)>) -> V /// The author's slot in the shared bucket list, adding them on first use. Legacy parts index the /// list by number, the modern authors part gives every slot a GUID. -fn author_slot(c: &Comment, authors: &mut Vec<(String, String, u32)>) -> usize { - let author = if c.author.is_empty() { "Author".to_string() } else { c.author.clone() }; - match authors.iter().position(|(n, _, _)| *n == author) { - Some(i) => i, - None => { - let ini = if c.initials.is_empty() { author.chars().filter(|ch| ch.is_alphabetic()).take(2).collect() } else { c.initials.clone() }; - authors.push((author.clone(), ini, 0)); - authors.len() - 1 - } +fn author_slot(c: &Comment, authors: &mut Vec<(String, String, u32)>, map: &mut std::collections::HashMap) -> usize { + let author = if c.author.is_empty() { "Author" } else { &c.author }; + if let Some(&i) = map.get(author) { + return i; } + let ini = if c.initials.is_empty() { author.chars().filter(|ch| ch.is_alphabetic()).take(2).collect() } else { c.initials.clone() }; + let idx = authors.len(); + authors.push((author.to_string(), ini, 0)); + map.insert(author.to_string(), idx); + idx } /// The modern (p188) threaded-comment part: replies nest and the resolved state is kept, which /// the flattened legacy part cannot express (issue #70). -fn modern_comments_xml(list: &[Comment], part: &str, authors: &mut Vec<(String, String, u32)>) -> Vec { +fn modern_comments_xml( + list: &[Comment], + part: &str, + authors: &mut Vec<(String, String, u32)>, + author_map: &mut std::collections::HashMap, +) -> Vec { let mut w = W::new(); w.open("p188:cmLst", A::new().a("xmlns:p188", NS_P188).a("xmlns:a", NS_A).a("xmlns:r", NS_R)); for (ti, c) in list.iter().take(10_000).enumerate() { - modern_comment(&mut w, c, part, ti, authors, 0); + modern_comment(&mut w, c, part, ti, authors, author_map, 0); } w.close("p188:cmLst"); w.finish() } -fn modern_comment(w: &mut W, c: &Comment, part: &str, ti: usize, authors: &mut Vec<(String, String, u32)>, depth: usize) { - let ai = author_slot(c, authors); +fn modern_comment( + w: &mut W, + c: &Comment, + part: &str, + ti: usize, + authors: &mut Vec<(String, String, u32)>, + author_map: &mut std::collections::HashMap, + depth: usize, +) { + let ai = author_slot(c, authors, author_map); let author = authors.get(ai).map(|(n, _, _)| n.clone()).unwrap_or_default(); let dt = if valid_date(&c.date) { c.date.clone() } else { "2024-01-01T00:00:00.000".into() }; let mut a = A::new().a("id", crate::tables::guid(&format!("deckcraft-comment:{part}:{ti}"))).a("authorId", author_guid(&author)).a("created", dt); @@ -899,7 +913,7 @@ fn modern_comment(w: &mut W, c: &Comment, part: &str, ti: usize, authors: &mut V if depth < 4 && !c.replies.is_empty() { w.open0("p188:replyLst"); for (ri, r) in c.replies.iter().take(1000).enumerate() { - let ri_ai = author_slot(r, authors); + let ri_ai = author_slot(r, authors, author_map); let r_author = authors.get(ri_ai).map(|(n, _, _)| n.clone()).unwrap_or_default(); let rdt = if valid_date(&r.date) { r.date.clone() } else { "2024-01-01T00:00:00.000".into() }; let mut ra = A::new() diff --git a/crates/pptx/src/write/shapes.rs b/crates/pptx/src/write/shapes.rs index 14710cfa..38c71235 100644 --- a/crates/pptx/src/write/shapes.rs +++ b/crates/pptx/src/write/shapes.rs @@ -852,21 +852,25 @@ fn opaque( // parts re-added under this slide's relationship ids. if let Ok(doc) = crate::xml::parse(xml.as_bytes()) && matches!(doc.root.local(), "graphicFrame" | "sp" | "grpSp" | "cxnSp") - && let Some(ids) = crate::read::rel_refs(&doc.root) + { + let part_map: std::collections::HashMap<&str, (&str, deckcraft_model::MediaId)> = + parts.iter().map(|(id, kind, m)| (id.as_str(), (kind.as_str(), *m))).collect(); + if let Some(ids) = crate::read::rel_refs(&doc.root) .iter() .map(|r| { - let (_, kind, m) = parts.iter().find(|(id, ..)| id == r)?; + let (kind, m) = part_map.get(r.as_str())?; Some((r.clone(), x.media_rel(o, *m, kind)?)) }) .collect::>>() - { - let mut root = doc.root; - set_rel_ids(&mut root, &ids, 0); - ole_fallback(&mut root, x, o, preview); - let id = cx.id_for(s); - set_cnvpr_id(&mut root, id); - w.raw(&root.to_xml()); - return; + { + let mut root = doc.root; + set_rel_ids(&mut root, &ids, 0); + ole_fallback(&mut root, x, o, preview); + let id = cx.id_for(s); + set_cnvpr_id(&mut root, id); + w.raw(&root.to_xml()); + return; + } } log::warn!("pptx: object '{label}' written as its preview picture"); let id = cx.id_for(s);