aleleba/deckcraft
1
0
Fork 0
mirror of https://github.com/storytold/deckcraft.git synced 2026-10-24 10:37:03 -06:00
Code Issues Packages Projects Releases Wiki Activity

PPTX: SmartArt and OLE objects we can't draw keep their parts on save (#203)

Browse Source 
An object DeckCraft doesn't model kept its XML, but the parts that XML
points to (diagram data, layout, style and colours; an OLE embedding)
were not kept, so a save wrote it as an empty rectangle or its preview.

Opaque now carries those parts (relationship id, full relationship type,
the part as a media item) and the writer re-adds them under the slide's
new relationship ids. A part with relationships of its own, or an
external link, still falls back to the preview with the existing
warning. A diagram's data part loses its link to the cached drawing,
which is a slide relationship id a save renumbers; only diagrams whose
drawing is missing or empty reach this path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
justinjohn0306
2026-10-12 17:09:09 +05:30
co-authored by Claude Opus 5.5
parent 0ec0cc5e81
commit c462859f6f
7 changed files with 146 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/model/src/lib.rs
+4
View File
@@ -286,6 +286,10 @@ pub enum ShapeKind {
xml: String,
preview: Option<MediaId>,
label: String,
/// The parts its XML points to (SmartArt data, an OLE embedding…), so a save writes them
/// back: (relationship id used in `xml`, relationship type, the part as a media item).
#[serde(default, skip_serializing_if = "Vec::is_empty")]
parts: Vec<(String, String, MediaId)>,
},
}
crates/pptx/src/opc.rs
+3 -1
View File
@@ -105,7 +105,7 @@ impl Package {
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 = if external { target.to_string() } else { resolve(&base, target) };
out.items.push(Rel { id: id.to_string(), kind: ty.rsplit('/').next().unwrap_or(ty).to_string(), target, external });
out.items.push(Rel { id: id.to_string(), kind: ty.rsplit('/').next().unwrap_or(ty).to_string(), ty: ty.to_string(), target, external });
}
out
}
@@ -134,6 +134,8 @@ pub struct Rel {
pub id: String,
/// Last segment of the relationship type (`slide`, `image`, `media`…).
pub kind: String,
/// The whole relationship type URI, to write it back as it was.
pub ty: String,
/// Absolute part name for internal targets; the URL for external ones.
pub target: String,
pub external: bool,
crates/pptx/src/read/mod.rs
+24 -10
View File
@@ -400,19 +400,33 @@ fn sane_len(v: f64, dflt: f64) -> f64 {
/// Does the element (or a descendant) reference a relationship?
pub fn has_rel_refs(el: &El) -> bool {
fn rec(e: &El, depth: usize) -> bool {
!rel_refs(el).is_empty()
}
/// Is this attribute a relationship id (`r:id`, `r:embed`, `r:dm`…)?
pub fn is_rel_attr(k: &str, v: &str) -> bool {
!k.starts_with("xmlns")
&& k.contains(':')
&& matches!(k.rsplit(':').next(), Some("id" | "embed" | "link" | "dm" | "lo" | "qs" | "cs" | "pict" | "href"))
&& !v.is_empty()
}
/// Every relationship id `el` and its descendants use, sorted and without repeats.
pub fn rel_refs(el: &El) -> Vec<String> {
fn rec(e: &El, out: &mut Vec<String>, depth: usize) {
if depth > crate::xml::MAX_DEPTH {
return false;
return;
}
out.extend(e.attrs.iter().filter(|(k, v)| is_rel_attr(k, v)).map(|(_, v)| v.clone()));
for c in e.elements() {
rec(c, out, depth + 1);
}
let own = e.attrs.iter().any(|(k, v)| {
!k.starts_with("xmlns")
&& k.contains(':')
&& matches!(k.rsplit(':').next(), Some("id" | "embed" | "link" | "dm" | "lo" | "qs" | "cs" | "pict" | "href"))
&& !v.is_empty()
});
own || e.elements().any(|c| rec(c, depth + 1))
}
rec(el, 0)
let mut out = vec![];
rec(el, &mut out, 0);
out.sort();
out.dedup();
out
}
pub fn color_map(cm: &El) -> Vec<(String, String)> {
crates/pptx/src/read/shapes.rs
+44 -1
View File
@@ -356,10 +356,53 @@ fn frame(imp: &mut Imp, part: &Part, ctx: &mut IdCtx, e: &El, depth: usize) -> S
} else {
uri.rsplit('/').next().unwrap_or("Object").to_string()
};
s.kind = ShapeKind::Opaque { xml: part.keep(e), preview, label };
// Keep the parts it points to so a save writes it back whole (#203). A part with relationships
// of its own, or an external link, can't be carried: the save then falls back to the preview.
let parts = super::rel_refs(e)
.into_iter()
.map(|id| {
let rel = part.rels.get(&id).filter(|r| !r.external)?;
if !imp.pkg.rels(&rel.target).items.is_empty() {
return None;
}
let (kind, target) = (rel.ty.clone(), rel.target.clone());
let media = if rel.kind == "diagramData" { diagram_data(imp, &target) } else { imp.media_for(&target) };
Some((id, kind, media?))
})
.collect::<Option<Vec<_>>>()
.unwrap_or_default();
s.kind = ShapeKind::Opaque { xml: part.keep(e), preview, label, parts };
s
}
/// A diagram's data part, without the link to its cached drawing: that link is a slide
/// relationship id, which a save renumbers. Only diagrams whose drawing is missing or empty get
/// here, and those are drawn from their data.
fn diagram_data(imp: &mut Imp, target: &str) -> Option<deckcraft_model::MediaId> {
fn strip(e: &mut El, depth: usize) {
if depth > crate::xml::MAX_DEPTH {
return;
}
e.children.retain(|n| !matches!(n, crate::xml::Node::El(c) if c.is("ext") && c.find("dataModelExt").is_some()));
for c in e.elements_mut() {
strip(c, depth + 1);
}
}
let mut doc = imp.pkg.xml(target)?;
strip(&mut doc.root, 0);
let xml = format!("<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\n{}", doc.root.to_xml());
let id = deckcraft_model::MediaId(imp.alloc());
let content_type = imp.pkg.content_type(target)?;
imp.media.push(deckcraft_model::MediaItem {
id,
name: crate::opc::file_of(target).to_string(),
content_type,
data: std::sync::Arc::new(xml.into_bytes()),
link: None,
});
Some(id)
}
/// SmartArt: read the pre-drawn shapes of its drawing part as a group.
fn smartart(imp: &mut Imp, part: &Part, gd: &El, frame: &Shape, depth: usize) -> Option<Shape> {
let rel_ids = gd.child("relIds")?;
crates/pptx/src/write/shapes.rs
+38 -4
View File
@@ -380,7 +380,7 @@ pub fn shape(w: &mut W, x: &mut Exp, o: &mut Out, cx: &mut Ctx, s: &Shape, depth
}
w.close("p:grpSp");
}
ShapeKind::Opaque { xml, preview, label } => opaque(w, x, o, cx, s, xml, *preview, label),
ShapeKind::Opaque { xml, preview, label, parts } => opaque(w, x, o, cx, s, xml, *preview, label, parts),
}
}
@@ -612,13 +612,31 @@ fn media(w: &mut W, x: &mut Exp, o: &mut Out, id: u32, s: &Shape, m: &deckcraft_
}
#[allow(clippy::too_many_arguments)]
fn opaque(w: &mut W, x: &mut Exp, o: &mut Out, cx: &mut Ctx, s: &Shape, xml: &str, preview: Option<deckcraft_model::MediaId>, label: &str) {
// Kept XML that references no relationships can be written back as it was.
fn opaque(
w: &mut W,
x: &mut Exp,
o: &mut Out,
cx: &mut Ctx,
s: &Shape,
xml: &str,
preview: Option<deckcraft_model::MediaId>,
label: &str,
parts: &[(String, String, deckcraft_model::MediaId)],
) {
// Kept XML is written back as it was when every part it references was kept too, with those
// parts re-added under this slide's relationship ids.
if let Ok(doc) = crate::xml::parse(xml.as_bytes())
&& !crate::read::has_rel_refs(&doc.root)
&& matches!(doc.root.local(), "graphicFrame" | "sp" | "grpSp" | "cxnSp")
&& let Some(ids) = crate::read::rel_refs(&doc.root)
.iter()
.map(|r| {
let (_, kind, m) = parts.iter().find(|(id, ..)| id == r)?;
Some((r.clone(), x.media_rel(o, *m, kind)?))
})
.collect::<Option<std::collections::HashMap<_, _>>>()
{
let mut root = doc.root;
set_rel_ids(&mut root, &ids, 0);
let id = cx.id_for(s);
set_cnvpr_id(&mut root, id);
w.raw(&root.to_xml());
@@ -668,6 +686,22 @@ fn shape_with_id(w: &mut W, x: &mut Exp, o: &mut Out, cx: &mut Ctx, s: &Shape, i
}
}
fn set_rel_ids(e: &mut crate::xml::El, ids: &std::collections::HashMap<String, String>, depth: usize) {
if depth > crate::xml::MAX_DEPTH {
return;
}
for (k, v) in &mut e.attrs {
if crate::read::is_rel_attr(k, v)
&& let Some(new) = ids.get(v.as_str())
{
v.clone_from(new);
}
}
for c in e.elements_mut() {
set_rel_ids(c, ids, depth + 1);
}
}
fn set_cnvpr_id(el: &mut crate::xml::El, id: u32) {
fn rec(e: &mut crate::xml::El, id: u32, depth: usize) -> bool {
if depth > 8 {
crates/pptx/tests/malformed.rs
+1 -1
View File
@@ -251,7 +251,7 @@ fn export_of_odd_models_is_valid() {
}
s.shapes.push(Shape {
id: ShapeId(7),
kind: ShapeKind::Opaque { xml: "<not xml".into(), preview: None, label: "Thing".into() },
kind: ShapeKind::Opaque { xml: "<not xml".into(), preview: None, label: "Thing".into(), parts: vec![] },
..Default::default()
});
p.slides.push(Arc::new(s));
crates/pptx/tests/roundtrip.rs
+32
View File
@@ -730,3 +730,35 @@ fn date_fields_keep_their_type_and_saved_text() {
assert_eq!(by_name(&q.slides[0], name).text, by_name(&p.slides[0], name).text, "{name}");
}
}
/// #203: SmartArt we can't draw kept its XML but lost its diagram parts on save.
#[test]
fn smartart_without_a_drawing_keeps_its_parts() {
let mut p = defaults::new_presentation(None);
let data = p.add_media(
"data1.xml",
"application/vnd.openxmlformats-officedocument.drawingml.diagramData+xml",
br#"<dgm:dataModel xmlns:dgm="http://schemas.openxmlformats.org/drawingml/2006/diagram" xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main"><dgm:ptLst><dgm:pt modelId="1"><dgm:t><a:p><a:r><a:t>Manager</a:t></a:r></a:p></dgm:t></dgm:pt></dgm:ptLst><dgm:extLst><a:ext uri="http://schemas.microsoft.com/office/drawing/2008/diagram"><dsp:dataModelExt xmlns:dsp="http://schemas.microsoft.com/office/drawing/2008/diagram" relId="rId6"/></a:ext></dgm:extLst></dgm:dataModel>"#.to_vec(),
);
let mut parts = vec![("rId2".to_string(), "http://schemas.openxmlformats.org/officeDocument/2006/relationships/diagramData".to_string(), data)];
for (i, kind) in ["Layout", "QuickStyle", "Colors"].into_iter().enumerate() {
let xml = format!(r#"<dgm:{kind} xmlns:dgm="http://schemas.openxmlformats.org/drawingml/2006/diagram"/>"#).into_bytes();
let m = p.add_media(&format!("{kind}1.xml"), &format!("application/vnd.openxmlformats-officedocument.drawingml.diagram{kind}+xml"), xml);
parts.push((format!("rId{}", i + 3), format!("http://schemas.openxmlformats.org/officeDocument/2006/relationships/diagram{kind}"), m));
}
let xml = r#"<p:graphicFrame xmlns:p="http://schemas.openxmlformats.org/presentationml/2006/main" xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships"><p:nvGraphicFramePr><p:cNvPr id="4" name="Diagram 3"/><p:cNvGraphicFramePr/><p:nvPr/></p:nvGraphicFramePr><p:xfrm><a:off x="0" y="0"/><a:ext cx="914400" cy="914400"/></p:xfrm><a:graphic><a:graphicData uri="http://schemas.openxmlformats.org/drawingml/2006/diagram"><dgm:relIds xmlns:dgm="http://schemas.openxmlformats.org/drawingml/2006/diagram" r:dm="rId2" r:lo="rId3" r:qs="rId4" r:cs="rId5"/></a:graphicData></a:graphic></p:graphicFrame>"#;
let mut s = new_slide(&mut p, "SmartArt");
let mut frame = shape(&mut p, "Diagram 3", 0.0, 0.0, 72.0, 72.0);
frame.kind = ShapeKind::Opaque { xml: xml.into(), preview: None, label: "SmartArt".into(), parts };
s.shapes.push(frame);
p.slides.push(Arc::new(s));
// Twice: the second save starts from what the first one wrote.
let q = round(&round(&p));
let ShapeKind::Opaque { parts, .. } = &by_name(&q.slides[1], "Diagram 3").kind else { panic!("not kept as an object") };
assert_eq!(parts.len(), 4, "data, layout, style and colours all kept");
let data = q.media(parts.iter().find(|(_, k, _)| k.ends_with("/diagramData")).expect("data").2).expect("media");
let data = String::from_utf8_lossy(&data.data);
assert!(data.contains("Manager"), "{data}");
assert!(!data.contains("dataModelExt"), "the link to a drawing that isn't written is dropped: {data}");
}