diff --git a/crates/model/src/chart.rs b/crates/model/src/chart.rs index fbca876f..36dd9708 100644 --- a/crates/model/src/chart.rs +++ b/crates/model/src/chart.rs @@ -404,6 +404,10 @@ pub struct ChartRel { pub id: String, pub rel_type: String, pub target: String, + /// Absolute name of the part an internal relationship reaches (matches a `ChartPart::name` + /// up to case); empty for external ones. The writer renames kept parts and uses this to + /// point the relationship at the new name. + pub part: String, pub external: bool, } diff --git a/crates/pptx/src/opc.rs b/crates/pptx/src/opc.rs index 0e7c5e64..0e7f86fe 100644 --- a/crates/pptx/src/opc.rs +++ b/crates/pptx/src/opc.rs @@ -127,23 +127,7 @@ impl Package { pub fn rels(&self, part: &str) -> Rels { let rp = rels_path(part); let Some(doc) = self.xml(&rp) else { return Rels::default() }; - let base = dir_of(part); - let mut out = Rels::default(); - for r in doc.root.children_named("Relationship") { - let (Some(id), Some(ty), Some(target)) = (r.attr("Id"), r.attr("Type"), r.attr("Target")) else { continue }; - let external = r.attr("TargetMode").is_some_and(|m| m.eq_ignore_ascii_case("external")); - let target_raw = target.to_string(); - let target = if external { target_raw.clone() } else { resolve(&base, target) }; - out.items.push(Rel { - id: id.to_string(), - kind: ty.rsplit('/').next().unwrap_or(ty).to_string(), - ty: ty.to_string(), - target_raw, - target, - external, - }); - } - out + rels_of(&doc, part) } /// Content types: (extension defaults, part overrides). pub fn content_type(&self, part: &str) -> Option { @@ -165,6 +149,38 @@ impl Package { } } +/// The relationships in a parsed rels part, with internal targets resolved against `part`'s folder. +fn rels_of(doc: &xml::Doc, part: &str) -> Rels { + let base = dir_of(part); + let mut out = Rels::default(); + for r in doc.root.children_named("Relationship") { + let (Some(id), Some(ty), Some(target)) = (r.attr("Id"), r.attr("Type"), r.attr("Target")) else { continue }; + let external = r.attr("TargetMode").is_some_and(|m| m.eq_ignore_ascii_case("external")); + let target_raw = target.to_string(); + let target = if external { target_raw.clone() } else { resolve(&base, target) }; + out.items.push(Rel { + id: id.to_string(), + kind: ty.rsplit('/').next().unwrap_or(ty).to_string(), + ty: ty.to_string(), + target_raw, + target, + external, + }); + } + out +} + +/// The relationships of `part` from the bytes of its rels part (kept parts carried by a chart). +pub fn rels_from_bytes(bytes: &[u8], part: &str) -> Rels { + match xml::parse(bytes) { + Ok(doc) => rels_of(&doc, part), + Err(e) => { + log::warn!("pptx: relationships of {part}: {e}"); + Rels::default() + } + } +} + #[derive(Clone, Debug, Default)] pub struct Rel { pub id: String, @@ -377,10 +393,6 @@ impl PackageOut { pub fn raw(&mut self, name: &str, data: Vec) { self.files.push((name.to_string(), data, true)); } - pub fn part_data(&self, name: &str) -> Option<&[u8]> { - let n = norm(name); - self.files.iter().find(|(f, _, _)| norm(f) == n).map(|(_, d, _)| d.as_slice()) - } /// Add a binary part; its extension gets a Default content type. pub fn binary(&mut self, name: &str, content_type: &str, data: Vec) { let ext = name.rsplit('.').next().unwrap_or("bin").to_ascii_lowercase(); @@ -407,7 +419,14 @@ impl PackageOut { let err = |e: &dyn std::fmt::Display| PptxError::Write(e.to_string()); zw.start_file("[Content_Types].xml", deflate).map_err(|e| err(&e))?; zw.write_all(&ct.finish()).map_err(|e| err(&e))?; + let mut written = std::collections::HashSet::new(); for (name, data, compress) in &self.files { + // Zip entry names must be unique: a second part under a name already written is + // dropped rather than producing a package readers reject or resolve differently. + if !written.insert(norm(name)) { + log::warn!("pptx: part {name} written twice; keeping the first"); + continue; + } let large = data.len() as u64 >= u32::MAX as u64 / 2; let opts = if *compress { deflate } else { stored }.large_file(large); zw.start_file(name.as_str(), opts).map_err(|e| err(&e))?; diff --git a/crates/pptx/src/read/chart.rs b/crates/pptx/src/read/chart.rs index bda93537..9fef9984 100644 --- a/crates/pptx/src/read/chart.rs +++ b/crates/pptx/src/read/chart.rs @@ -421,11 +421,11 @@ pub(crate) fn capture_kept(pkg: &crate::opc::Package, chart_part: &str) -> deckc continue; } if r.external { - kept.rels.push(ChartRel { id: r.id, rel_type: r.ty, target: r.target_raw, external: true }); + kept.rels.push(ChartRel { id: r.id, rel_type: r.ty, target: r.target_raw, part: String::new(), external: true }); continue; } if copy_part(pkg, &r.target, &mut kept.parts, &mut seen, 0) { - kept.rels.push(ChartRel { id: r.id, rel_type: r.ty, target: r.target_raw, external: false }); + kept.rels.push(ChartRel { id: r.id, rel_type: r.ty, target: r.target_raw, part: r.target, external: false }); } } kept diff --git a/crates/pptx/src/write/chart.rs b/crates/pptx/src/write/chart.rs index 38b16099..88917450 100644 --- a/crates/pptx/src/write/chart.rs +++ b/crates/pptx/src/write/chart.rs @@ -512,10 +512,15 @@ fn plot_group(w: &mut W, c: &Chart, kind: ChartType, list: &[(usize, &Series)]) } /// The `chartEx` part of an Office 2016 chart (waterfall, funnel…): the kept part when the chart -/// is unchanged since it was read (without its workbook link), else one written from the model. -/// `None` for other chart types. +/// is unchanged since it was read, else one written from the model. `None` for other chart types. +#[cfg(test)] pub fn chartex_xml(c: &Chart) -> Option> { - kept_chartex(c).or_else(|| c.kind.is_chartex().then(|| build_chartex(c))) + chartex_part(c).map(|(x, _)| x) +} + +/// [`chartex_xml`], and whether it is the original part (so its kept relationships still apply). +pub fn chartex_part(c: &Chart) -> Option<(Vec, bool)> { + kept_chartex(c).map(|x| (x, true)).or_else(|| c.kind.is_chartex().then(|| (build_chartex(c), false))) } fn kept_chartex(c: &Chart) -> Option> { @@ -525,11 +530,14 @@ fn kept_chartex(c: &Chart) -> Option> { crate::read::resolve_mc(&mut root, 0); let mut back = crate::read::chartex_from(&root)?; back.raw = c.raw.clone(); + // The sidecar is not modelled chart state. + back.kept = c.kept.clone(); if &back != c { return None; } let mut keep = doc.root; - keep.remove_all(&["externalData"]); + // The workbook link stays only when its part is written back. + drop_unkept_refs(&mut keep, c); let mut w = W::new(); w.raw(&keep.to_xml()); Some(w.finish()) @@ -553,9 +561,7 @@ pub fn chart_xml(c: &Chart) -> (Vec, bool) { if back == me { let mut keep = doc.root; // Without the parts those elements point at, PowerPoint repairs the file. - if c.kept.rels.is_empty() { - keep.remove_all(&["externalData", "userShapes"]); - } + drop_unkept_refs(&mut keep, c); let mut w = W::new(); w.raw(&keep.to_xml()); return (w.finish(), true); @@ -660,31 +666,106 @@ pub fn chart_xml(c: &Chart) -> (Vec, bool) { (w.finish(), false) } -/// Write the chart's own relationships and the parts they reach. -pub fn write_kept(pkg: &mut crate::opc::PackageOut, chart_name: &str, c: &Chart) { - if c.kept.rels.is_empty() && c.kept.parts.is_empty() { +/// The relationship ids of an unchanged chart that [`write_kept`] writes back: external ones, and +/// internal ones whose part was kept. +fn kept_rel_ids(c: &Chart) -> std::collections::HashSet<&str> { + let parts: std::collections::HashSet = c.kept.parts.iter().filter(|p| kept_payload(&p.name)).map(|p| crate::opc::norm(&p.name)).collect(); + c.kept.rels.iter().filter(|r| r.external || parts.contains(&crate::opc::norm(&r.part))).map(|r| r.id.as_str()).collect() +} + +/// A kept part that is written as a part of its own (rels parts are rewritten instead). +fn kept_payload(name: &str) -> bool { + !name.to_ascii_lowercase().ends_with(".rels") && crate::opc::sidecar_part(name) +} + +/// Drop `c:externalData` / `c:userShapes` (and the `cx:` ones) that point at a relationship that +/// is not written back: without the part PowerPoint repairs the file. +fn drop_unkept_refs(root: &mut crate::xml::El, c: &Chart) { + let ids = kept_rel_ids(c); + fn rec(e: &mut crate::xml::El, ids: &std::collections::HashSet<&str>, depth: usize) { + if depth > crate::xml::MAX_DEPTH { + return; + } + e.children.retain(|n| match n { + crate::xml::Node::El(x) if x.is("externalData") || x.is("userShapes") => x.attr("r:id").is_some_and(|id| ids.contains(id)), + _ => true, + }); + for ch in e.elements_mut() { + rec(ch, ids, depth + 1); + } + } + rec(root, &ids, 0); +} + +/// A fresh name for a kept part: `ppt//keptN.`. The writer generates no `kept…` +/// names and `n` is unique within the package, so a kept part never lands on (or duplicates) a +/// part the writer makes, whatever the source file called it. +fn kept_name(original: &str, n: u32) -> String { + let norm = crate::opc::norm(original); + let area = norm.split('/').nth(1).unwrap_or("embeddings"); + let area = if matches!(area, "embeddings" | "charts" | "drawings" | "media") { area } else { "embeddings" }; + let file = crate::opc::file_of(&norm); + let ext = file + .rsplit_once('.') + .map(|(_, e)| e) + .filter(|e| !e.is_empty() && e.len() <= 8 && e.chars().all(|c| c.is_ascii_alphanumeric())) + .unwrap_or("bin"); + format!("ppt/{area}/kept{n}.{ext}") +} + +/// Write an unchanged chart's own relationships and the parts they reach. Each kept part is +/// written under a fresh name ([`kept_name`]) and the relationships between kept parts are +/// rewritten to those names; a relationship to anything that was not kept is dropped. +/// `n_kept` numbers kept parts across the package. +pub fn write_kept(pkg: &mut crate::opc::PackageOut, chart_name: &str, c: &Chart, n_kept: &mut u32) { + use std::collections::HashMap; + + use crate::opc::{RelsOut, norm, relative, rels_path}; + + if c.kept.rels.is_empty() { return; } - let mut rels = crate::opc::RelsOut::default(); - for r in &c.kept.rels { - rels.items.push((r.id.clone(), r.rel_type.clone(), r.target.clone(), r.external)); - } - pkg.rels(chart_name, &rels); - for p in &c.kept.parts { - match pkg.part_data(&p.name) { - Some(existing) if existing != p.bytes.as_slice() => { - log::warn!("pptx: chart part {} already written with different bytes; keeping the first", p.name); - continue; - } - Some(_) => continue, - None => {} - } - if !crate::opc::sidecar_part(&p.name) { - log::warn!("pptx: not writing chart part {}", p.name); + // Original part name (normalized) → the name written. + let mut names: HashMap = HashMap::new(); + for p in c.kept.parts.iter().filter(|p| kept_payload(&p.name)) { + let key = norm(&p.name); + if names.contains_key(&key) { continue; } + *n_kept += 1; + let new = kept_name(&p.name, *n_kept); let ct = if !p.content_type.is_empty() { p.content_type.clone() } else { crate::mime_for(&p.name).to_string() }; - pkg.binary(&p.name, &ct, p.bytes.clone()); + // An override per part: a kept part never sets the package-wide default for its extension. + pkg.xml_part(&new, &ct, p.bytes.clone()); + names.insert(key, new); + } + let mut rels = RelsOut::default(); + for r in &c.kept.rels { + if r.external { + rels.items.push((r.id.clone(), r.rel_type.clone(), r.target.clone(), true)); + } else if let Some(new) = names.get(&norm(&r.part)) { + rels.items.push((r.id.clone(), r.rel_type.clone(), relative(chart_name, new), false)); + } else { + log::warn!("pptx: chart relationship {} has no kept part; dropped", r.id); + } + } + pkg.rels(chart_name, &rels); + // 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 rp = norm(&rels_path(&p.name)); + let Some(rels_part) = c.kept.parts.iter().find(|q| norm(&q.name) == 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 { + out.items.push((r.id, r.ty, r.target_raw, true)); + } else if let Some(new) = names.get(&norm(&r.target)) { + out.items.push((r.id, r.ty, relative(new_source, new), false)); + } else { + log::warn!("pptx: relationship {} of kept chart part {} has no kept target; dropped", r.id, p.name); + } + } + pkg.rels(new_source, &out); } } diff --git a/crates/pptx/src/write/mod.rs b/crates/pptx/src/write/mod.rs index 6e895a3d..dd222d3b 100644 --- a/crates/pptx/src/write/mod.rs +++ b/crates/pptx/src/write/mod.rs @@ -59,6 +59,8 @@ pub struct Exp<'a> { n_diagram: u32, n_tags: u32, n_chartex: u32, + /// Parts kept from an unchanged chart (`ppt/…/keptN.ext`). + n_kept: u32, pub slide_parts: HashMap, pub table_styles: Vec, placeholder: Option, @@ -195,10 +197,15 @@ impl Exp<'_> { } /// An unchanged Office 2016 chart's `chartEx` part and its relationship id. pub fn chartex_rel(&mut self, o: &mut Out, c: &deckcraft_model::Chart) -> Option { - let xml = chart::chartex_xml(c)?; + let (xml, preserved) = chart::chartex_part(c)?; self.n_chartex += 1; let name = format!("ppt/charts/chartEx{}.xml", self.n_chartex); self.pkg.xml_part(&name, CT_CHARTEX, xml); + if preserved { + chart::write_kept(&mut self.pkg, &name, c, &mut self.n_kept); + } else if !c.kept.parts.is_empty() { + log::warn!("pptx: chart data changed; the embedded workbook was not written back"); + } Some(o.rels.add_new(RT_CHARTEX, &opc::relative(&o.name, &name), false)) } @@ -208,7 +215,7 @@ impl Exp<'_> { let (xml, preserved) = chart::chart_xml(c); self.pkg.xml_part(&name, CT_CHART, xml); if preserved { - chart::write_kept(&mut self.pkg, &name, c); + chart::write_kept(&mut self.pkg, &name, c, &mut self.n_kept); } else if !c.kept.parts.is_empty() { log::warn!("pptx: chart data changed; the embedded workbook was not written back"); } @@ -528,6 +535,7 @@ pub fn export(p: &Presentation, main_ct: &str) -> Result, PptxError> { n_diagram: 0, n_tags: 0, n_chartex: 0, + n_kept: 0, slide_parts: HashMap::new(), table_styles: vec![], placeholder: None, diff --git a/crates/pptx/src/write/shapes.rs b/crates/pptx/src/write/shapes.rs index 770ee775..aff238b7 100644 --- a/crates/pptx/src/write/shapes.rs +++ b/crates/pptx/src/write/shapes.rs @@ -401,6 +401,10 @@ pub fn shape(w: &mut W, x: &mut Exp, o: &mut Out, cx: &mut Ctx, s: &Shape, depth } let mut plain = (**c).clone(); plain.raw = plain.raw.filter(|r| !r.contains(super::chart::NS_CX)); + if ex.is_some() { + // The kept workbook belongs to the `chartEx` part; the fallback has none. + plain.kept = Default::default(); + } let rid = x.chart_rel(o, &plain); frame_open(w, x, o, id, s); w.open("a:graphicData", A::new().a("uri", "http://schemas.openxmlformats.org/drawingml/2006/chart")); diff --git a/crates/pptx/src/xml.rs b/crates/pptx/src/xml.rs index eceb0708..9484863a 100644 --- a/crates/pptx/src/xml.rs +++ b/crates/pptx/src/xml.rs @@ -171,19 +171,6 @@ impl El { write_el(self, &mut s, 0); s } - /// Remove descendant elements with any of these local names (bounded). - pub fn remove_all(&mut self, locals: &[&str]) { - fn rec(e: &mut El, locals: &[&str], depth: usize) { - if depth > MAX_DEPTH { - return; - } - e.children.retain(|c| !matches!(c, Node::El(x) if locals.contains(&x.local()))); - for c in e.elements_mut() { - rec(c, locals, depth + 1); - } - } - rec(self, locals, 0) - } } fn write_el(e: &El, s: &mut String, depth: usize) { diff --git a/crates/pptx/tests/chart_embed.rs b/crates/pptx/tests/chart_embed.rs index ddf56731..d80d4d6d 100644 --- a/crates/pptx/tests/chart_embed.rs +++ b/crates/pptx/tests/chart_embed.rs @@ -98,14 +98,15 @@ fn an_unchanged_chart_keeps_its_embedded_workbook() { let saved = deckcraft_pptx::export(&deckcraft_pptx::import(&src).expect("import")).expect("export"); let saved_parts = parts(&saved); - let workbook = saved_parts.iter().find(|(n, _)| n == EMBED).map(|(_, d)| d.as_slice()); + // Kept parts are written under fresh names, so they can't land on a part the writer makes. + let workbook = saved_parts.iter().find(|(n, _)| n == "ppt/embeddings/kept1.xlsx").map(|(_, d)| d.as_slice()); assert_eq!(workbook, Some(WORKBOOK), "the embedding part is dropped on save"); let rels = saved_parts .iter() .find(|(n, _)| n.starts_with("ppt/charts/_rels/chart") && n.ends_with(".xml.rels")) .map(|(_, d)| String::from_utf8_lossy(d).into_owned()) .expect("chart relationships"); - assert!(rels.contains("../embeddings/Microsoft_Excel_Worksheet1.xlsx"), "{rels}"); + assert!(rels.contains(r#"Id="rId1""#) && rels.contains("../embeddings/kept1.xlsx"), "{rels}"); assert!(!rels.contains("slide1.xml"), "a chart relationship must not replace the slide: {rels}"); let slide = saved_parts.iter().find(|(n, _)| n == "ppt/slides/slide1.xml").map(|(_, d)| String::from_utf8_lossy(d).into_owned()).expect("slide"); assert!(slide.contains("sld"), "{slide}"); @@ -131,7 +132,7 @@ fn an_edited_chart_does_not_write_a_stale_workbook() { let value = chart.series.get_mut(0).and_then(|s| s.values.get_mut(0)).expect("value"); *value = Some(99.0); let saved = parts(&deckcraft_pptx::export(&p).expect("export")); - assert!(saved.iter().all(|(n, _)| n != EMBED), "a stale workbook was written after the chart changed"); + assert!(saved.iter().all(|(n, _)| !n.starts_with("ppt/embeddings/")), "a stale workbook was written after the chart changed"); let xml = saved .iter() .find(|(n, _)| n.starts_with("ppt/charts/chart") && n.ends_with(".xml")) @@ -139,3 +140,187 @@ fn an_edited_chart_does_not_write_a_stale_workbook() { .expect("chart xml"); assert!(!xml.contains("externalData"), "{xml}"); } + +fn png(rgb: [u8; 3]) -> Vec { + let img = image::RgbImage::from_pixel(4, 4, image::Rgb(rgb)); + let mut out = Cursor::new(vec![]); + image::DynamicImage::ImageRgb8(img).write_to(&mut out, image::ImageFormat::Png).expect("png"); + out.into_inner() +} + +/// Rewrite `src`: `edit` may change any entry (name, bytes → new bytes), then `extra` entries are added. +fn rewrite(src: &[u8], edit: impl Fn(&str, Vec) -> Vec, extra: &[(&str, &[u8])]) -> Vec { + let mut zip = zip::ZipArchive::new(Cursor::new(src)).expect("zip"); + let mut out = zip::ZipWriter::new(Cursor::new(Vec::new())); + let opts = zip::write::SimpleFileOptions::default(); + for i in 0..zip.len() { + let mut f = zip.by_index(i).expect("entry"); + let name = f.name().to_string(); + let mut data = vec![]; + f.read_to_end(&mut data).expect("read"); + let data = edit(&name, data); + out.start_file(&name, opts).expect("start"); + out.write_all(&data).expect("write"); + } + for (name, data) in extra { + out.start_file(*name, opts).expect("start"); + out.write_all(data).expect("write"); + } + out.finish().expect("finish").into_inner() +} + +fn unique_names(parts: &[(String, Vec)]) { + let mut seen = std::collections::HashSet::new(); + for (n, _) in parts { + assert!(seen.insert(n.to_ascii_lowercase()), "duplicate zip entry {n}"); + } +} + +fn text_of(parts: &[(String, Vec)], name: &str) -> String { + parts.iter().find(|(n, _)| n == name).map(|(_, d)| String::from_utf8_lossy(d).into_owned()).unwrap_or_else(|| panic!("no {name}")) +} + +const RT: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships"; + +/// A chart whose relationships reach parts named like the ones the writer generates +/// (`ppt/media/image1.png`, `ppt/drawings/…`) must not collide with them on save: no duplicate +/// zip entries, and the slide's own picture keeps its bytes. +#[test] +fn kept_chart_parts_never_collide_with_written_parts() { + let picture = png([10, 200, 30]); + let mut p = defaults::new_presentation(None); + p.slides.clear(); + let img = p.add_media("image1.png", "image/png", picture.clone()); + let layout = defaults::layout_of_kind(&p, LayoutType::Blank).unwrap_or_default(); + let mut slide = defaults::new_slide(&mut p, layout); + // The chart comes first, so its kept parts are written before the picture. + let mut chart = Shape { id: ShapeId(p.alloc_id()), name: "Pie".into(), xfrm: Some(Xfrm::new(40.0, 40.0, 400.0, 300.0)), ..Default::default() }; + chart.kind = ShapeKind::Chart(Box::new(Chart::sample(ChartType::Pie))); + slide.shapes.push(chart); + let mut pic = Shape { id: ShapeId(p.alloc_id()), name: "Pic".into(), xfrm: Some(Xfrm::new(500.0, 40.0, 80.0, 80.0)), ..Default::default() }; + pic.kind = ShapeKind::Picture { fill: deckcraft_model::style::PictureFill { media: img, ..Default::default() } }; + slide.shapes.push(pic); + p.slides.push(Arc::new(slide)); + let base = deckcraft_pptx::export(&p).expect("export"); + assert!(parts(&base).iter().any(|(n, d)| n == "ppt/media/image1.png" && *d == picture)); + + // The chart's own relationships reach the slide picture's part and a drawing whose + // relationships reach it too (and, hostile, the slide). + let chart_rels = format!( + r#""# + ); + let drawing_rels = format!( + r#""# + ); + let drawing = br#""#; + let src = rewrite( + &base, + |name, data| { + if name.starts_with("ppt/charts/chart") && name.ends_with(".xml") { + String::from_utf8(data) + .expect("chart") + .replacen("", r#""#, 1) + .into_bytes() + } else { + data + } + }, + &[ + ("ppt/charts/_rels/chart1.xml.rels", chart_rels.as_bytes()), + ("ppt/drawings/drawing1.xml", drawing), + ("ppt/drawings/_rels/drawing1.xml.rels", drawing_rels.as_bytes()), + (EMBED, WORKBOOK), + ], + ); + + let saved = parts(&deckcraft_pptx::export(&deckcraft_pptx::import(&src).expect("import")).expect("export")); + unique_names(&saved); + let picture_parts: Vec<&String> = saved.iter().filter(|(_, d)| *d == picture).map(|(n, _)| n).collect(); + assert!(picture_parts.iter().any(|n| *n == "ppt/media/image1.png"), "{picture_parts:?}"); + let slide_rels = text_of(&saved, "ppt/slides/_rels/slide1.xml.rels"); + assert!(slide_rels.contains("../media/image1.png"), "{slide_rels}"); + + // The chart's relationships point at the renamed parts, which hold the kept bytes. + let rels = text_of(&saved, "ppt/charts/_rels/chart1.xml.rels"); + assert!(!rels.contains("image1.png") && !rels.contains("drawing1.xml") && !rels.contains("Worksheet1"), "{rels}"); + let target = |rels: &str, id: &str| -> String { + let at = rels.find(&format!(r#"Id="{id}""#)).unwrap_or_else(|| panic!("{id} in {rels}")); + let rest = &rels[at..]; + let t = rest.find(r#"Target=""#).expect("target") + 8; + rest[t..].split('"').next().unwrap_or("").to_string() + }; + let resolve = |from_dir: &str, t: &str| format!("ppt/{}", t.trim_start_matches("../").trim_start_matches(from_dir)); + let img_part = resolve("", &target(&rels, "rId3")); + assert!(img_part.starts_with("ppt/media/kept"), "{img_part}"); + assert!(saved.iter().any(|(n, d)| *n == img_part && *d == picture), "{img_part}"); + let drawing_part = resolve("", &target(&rels, "rId2")); + assert!(saved.iter().any(|(n, d)| *n == drawing_part && d.as_slice() == drawing.as_slice()), "{drawing_part}"); + let wb = resolve("", &target(&rels, "rId1")); + assert!(saved.iter().any(|(n, d)| *n == wb && d.as_slice() == WORKBOOK), "{wb}"); + + // The drawing's own relationships follow the rename; the one to the slide is dropped. + let file = drawing_part.rsplit('/').next().unwrap_or(""); + let drels = text_of(&saved, &format!("ppt/drawings/_rels/{file}.rels")); + assert_eq!(resolve("", &target(&drels, "rId1")), img_part, "{drels}"); + assert!(!drels.contains("slide"), "{drels}"); + + // And the result reads back with the same kept parts. + let again = deckcraft_pptx::export(&deckcraft_pptx::import(&saved_zip(&saved)).expect("reimport")).expect("export again"); + unique_names(&parts(&again)); +} + +fn saved_zip(parts: &[(String, Vec)]) -> Vec { + let mut out = zip::ZipWriter::new(Cursor::new(Vec::new())); + let opts = zip::write::SimpleFileOptions::default(); + for (n, d) in parts { + out.start_file(n.as_str(), opts).expect("start"); + out.write_all(d).expect("write"); + } + out.finish().expect("finish").into_inner() +} + +/// An unchanged Office 2016 chart keeps its embedded workbook too; its plain-chart fallback +/// carries none. +#[test] +fn an_unchanged_chartex_keeps_its_embedded_workbook() { + let mut p = defaults::new_presentation(None); + p.slides.clear(); + let layout = defaults::layout_of_kind(&p, LayoutType::Blank).unwrap_or_default(); + let mut slide = defaults::new_slide(&mut p, layout); + let mut shape = Shape { id: ShapeId(p.alloc_id()), name: "Pie".into(), xfrm: Some(Xfrm::new(40.0, 40.0, 400.0, 300.0)), ..Default::default() }; + shape.kind = ShapeKind::Chart(Box::new(Chart::sample(ChartType::Waterfall))); + slide.shapes.push(shape); + p.slides.push(Arc::new(slide)); + let base = deckcraft_pptx::export(&p).expect("export"); + assert!(parts(&base).iter().any(|(n, _)| n == "ppt/charts/chartEx1.xml")); + let rels = format!( + r#""# + ); + let src = rewrite( + &base, + |name, data| { + if name == "ppt/charts/chartEx1.xml" { + String::from_utf8(data) + .expect("chartex") + .replacen("", r#""#, 1) + .into_bytes() + } else { + data + } + }, + &[("ppt/charts/_rels/chartEx1.xml.rels", rels.as_bytes()), (EMBED, WORKBOOK)], + ); + let chart = chart_of(&src); + assert_eq!(chart.kind, ChartType::Waterfall); + assert!(chart.kept.parts.iter().any(|p| p.bytes == WORKBOOK)); + + let saved = parts(&deckcraft_pptx::export(&deckcraft_pptx::import(&src).expect("import")).expect("export")); + unique_names(&saved); + let xml = text_of(&saved, "ppt/charts/chartEx1.xml"); + assert!(xml.contains("externalData"), "{xml}"); + let rels = text_of(&saved, "ppt/charts/_rels/chartEx1.xml.rels"); + assert!(rels.contains("../embeddings/kept1.xlsx"), "{rels}"); + assert!(saved.iter().any(|(n, d)| n == "ppt/embeddings/kept1.xlsx" && d.as_slice() == WORKBOOK)); + assert!(saved.iter().all(|(n, _)| n != "ppt/charts/_rels/chart1.xml.rels"), "the fallback chart has no workbook"); + assert!(!text_of(&saved, "ppt/charts/chart1.xml").contains("externalData")); +} diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 1e6e103d..163a3dfa 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -95,16 +95,12 @@ the round trip even though DeckCraft doesn't model it. | Text warp (`prstTxWarp`) | yes | yes | All 40 presets rendered (`crates/geom/src/warp.rs`); adjust values (`avLst`) round-trip | | Pictures with crop, recolour, duotone, alpha, luminance | yes | yes | | | Tables with styles | yes | yes | Merges and both diagonal borders round-trip | -| Charts (`c:chartSpace`) | yes | yes | Original XML kept in `Chart.raw`. Trendlines (type, order or period, forecast, intercept, equation and R²) are read, written and drawn. A combo chart's secondary value axis is read and is not written back when the chart is rebuilt | -| Charts (`cx:` chartex: waterfall, funnel, treemap, sunburst, histogram, Pareto, box) | yes | yes | Unchanged parts kept; edited and new ones written from the model with a plain-chart fallback. Treemap and sunburst hierarchy levels kept; region maps kept, drawn as tiles (no map geometry) | +| Charts (`c:chartSpace`) | yes | yes | Original XML kept in `Chart.raw`. Trendlines (type, order or period, forecast, intercept, equation and R²) are read, written and drawn. A combo chart's secondary value axis is read and is not written back when the chart is rebuilt. An unchanged chart also keeps its embedded workbook and the other parts its relationships point at, under fresh `keptN` names so they never replace a part DeckCraft writes (#335); editing the chart drops that workbook instead of writing a stale one | +| Charts (`cx:` chartex: waterfall, funnel, treemap, sunburst, histogram, Pareto, box) | yes | yes | Unchanged parts kept, with their embedded workbook (#335); edited and new ones written from the model with a plain-chart fallback. Treemap and sunburst hierarchy levels kept; region maps kept, drawn as tiles (no map geometry) | | SmartArt (`dgm`) | drawing part only, as a group; text laid out on its `dsp:txXfrm` rectangle | as shapes | Diagram data model lost on save | | SmartArt (`dgm`) | drawing part only, as a group | as shapes | Diagram data model lost on save. A diagram with no relationship parts keeps its XML; the drawing and data parts are still dropped (#203) | | SmartArt (`dgm`) | drawing part only, as a group | as shapes | Diagram data model lost on save (DeckCraft keeps its own outline in `.deckcraft` only) | | SmartArt (`dgm`) | the drawing part as a group, and the outline from the data model (Text Pane) | as a diagram: data model from the outline; layout, quick style and colour parts (the file's own kept until the layout is switched); the group as the cached drawing | DeckCraft's layout definitions are simple; a diagram without a cached drawing isn't drawn | -| Tables with styles | yes | yes | | -| Charts (`c:chartSpace`) | yes | yes | Original XML kept in `Chart.raw`. An unchanged chart also keeps its embedded workbook and the other parts its relationships point at (#335); editing the chart drops that workbook instead of writing a stale one | -| Charts (`cx:` chartex: waterfall, funnel, treemap, sunburst, histogram, box) | no | no | | -| SmartArt (`dgm`) | drawing part only, as a group | as shapes | Diagram data model lost on save | | Equations (OMML, `a14:m`) | no | no | PR #55 open | | Audio / video (`p14:media`, trim, fades, bookmarks) | yes | partly | Playback options lost on save (#74) | | Ink (`contentPart`) | yes | as a group of freeform lines | PowerPoint sees shapes, not ink | diff --git a/docs/gaps.md b/docs/gaps.md index 0d161449..e7454e85 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -244,9 +244,8 @@ Detail in [file-format-parity.md](file-format-parity.md). moving average, with forecast and the equation and R² label. A moving average is drawn with neither label. A combo chart's secondary value axis is read (`y2_*`, `Series.secondary_axis`) and is not written back when the chart is rebuilt. - chart types" (partial); `chartex` parts (waterfall, funnel, treemap, sunburst, histogram, box & - whisker) aren't read. An unchanged chart's embedded workbook now survives save (#335); editing - the chart still drops it rather than writing a stale spreadsheet. + An unchanged chart's embedded workbook now survives save (#335); editing the chart still + drops it rather than writing a stale spreadsheet. - **Impact:** most business decks have charts; editing their data is routine. - **Estimate:** 12–20 h. Data and elements are **beta blockers** (6–10 h).