collective
Browse / github.com/storytold/deckcraft / patch
fix(pptx): keep pictures that carry an SVG through Office's SVG extension
fix(pptx): keep pictures that carry an SVG through Office's SVG extension PowerPoint stores an SVG picture as an `a:blip` whose extension list holds an `asvg:svgBlip`, usually with a PNG fallback in the blip's own `r:embed`; some generators write no fallback. The reader only looked at `r:embed`, so saving turned an SVG-only picture into an empty shape and dropped the SVG, leaving PowerPoint an empty slide. A picture with a fallback lost its SVG and kept only the PNG. `PictureFill::svg` now keeps it, and the writer writes the extension back as PowerPoint does. Changing the picture, applying an artistic effect, removing the background or cropping the raster drops the
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- arc-uri-el (imported from pull request #1195, open)
- Base
a2435383b156b02278a63d09b0ffaa6bd7af3d4d- Size
- +196 −26 in 13 files
- Review
- pending
- Shared
- 2026-10-28
- Id
bbb247fe3052bba26371cf69
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/artistic.rs crates/engine/src/cmd/background.rs crates/engine/src/cmd/compress.rs crates/engine/src/cmd/edit.rs crates/engine/src/cmd/shape.rs crates/engine/src/media_refs.rs crates/engine/src/tests.rs crates/model/src/style.rs crates/pptx/src/opc.rs crates/pptx/src/read/dml.rs crates/pptx/src/write/dml.rs crates/pptx/tests/roundtrip.rs crates/pptx/tests/svg_pictures.rs
@@ -188,6 +188,8 @@ fn artistic_effect(s: &mut Session, p: &Value) -> Result<Value> { (Some(m), Some(e)) => { fill.media = m; fill.artistic = Some(Artistic { effect: e.clone(), original });+ // The effect is drawn on the raster picture; an SVG would show without it.+ fill.svg = None; } _ => { fill.media = original;@@ -175,6 +175,7 @@ fn remove_background(s: &mut Session, p: &Value) -> Result<Value> { && let ShapeKind::Picture { fill } = &mut sh.kind { fill.media = media;+ fill.svg = None; } } Ok(())@@ -189,6 +189,10 @@ fn compress(s: &mut Session, p: &Value) -> Result<Value> { if let Some((id, new_crop, _)) = hit { changed.push(media); fill.media = id;+ // A cropped-away area is gone from the raster but not from the SVG.+ if new_crop != crop {+ fill.svg = None;+ } fill.crop = new_crop; count += 1; touched = true;@@ -70,7 +70,10 @@ fn slides_scope(s: &Session, p: &Value) -> bool { pub(crate) fn media_of_shapes(shapes: &[Shape], out: &mut Vec<MediaId>) { deckcraft_model::walk(shapes, &mut |s, _| { match &s.kind {- deckcraft_model::ShapeKind::Picture { fill } => out.push(fill.media),+ deckcraft_model::ShapeKind::Picture { fill } => {+ out.push(fill.media);+ out.extend(fill.svg);+ } deckcraft_model::ShapeKind::Media(m) => { out.push(m.media); if let Some(p) = m.poster {@@ -87,6 +90,7 @@ pub(crate) fn media_of_shapes(shapes: &[Shape], out: &mut Vec<MediaId>) { } if let Some(deckcraft_model::Fill::Picture(pf)) = &s.fill { out.push(pf.media);+ out.extend(pf.svg); } }); }@@ -114,6 +118,7 @@ fn copy_inner(s: &mut Session, p: &Value) -> Result<String> { media_of_shapes(&sl.shapes, &mut mids); if let Some(deckcraft_model::Background::Fill { fill: deckcraft_model::Fill::Picture(pf) }) = &sl.background { mids.push(pf.media);+ mids.extend(pf.svg); } } let media = st.doc.media.iter().filter(|m| mids.contains(&m.id)).cloned().collect();@@ -260,7 +265,12 @@ fn remap_media(shapes: &mut [Shape], map: &HashMap<MediaId, MediaId>) { }; walk_mut(shapes, &mut |s| { match &mut s.kind {- deckcraft_model::ShapeKind::Picture { fill } => m(&mut fill.media),+ deckcraft_model::ShapeKind::Picture { fill } => {+ m(&mut fill.media);+ if let Some(svg) = fill.svg.as_mut() {+ m(svg);+ }+ } deckcraft_model::ShapeKind::Media(c) => { m(&mut c.media); if let Some(p) = c.poster.as_mut() {@@ -272,6 +282,9 @@ fn remap_media(shapes: &mut [Shape], map: &HashMap<MediaId, MediaId>) { } if let Some(deckcraft_model::Fill::Picture(pf)) = &mut s.fill { m(&mut pf.media);+ if let Some(svg) = pf.svg.as_mut() {+ m(svg);+ } } }); }@@ -327,10 +340,11 @@ fn paste(s: &mut Session, p: &Value) -> Result<Value> { let map = reid("edit.paste", doc, &mut sl.shapes)?; remap_animations(&mut sl.animations, &map); remap_media(&mut sl.shapes, &media);- if let Some(deckcraft_model::Background::Fill { fill: deckcraft_model::Fill::Picture(pf) }) = &mut sl.background- && let Some(n) = media.get(&pf.media)- {- pf.media = *n;+ if let Some(deckcraft_model::Background::Fill { fill: deckcraft_model::Fill::Picture(pf) }) = &mut sl.background {+ if let Some(n) = media.get(&pf.media) {+ pf.media = *n;+ }+ pf.svg = pf.svg.map(|s| media.get(&s).copied().unwrap_or(s)); } if doc.layout(sl.layout).is_none() { sl.layout = doc.masters.first().and_then(|m| m.layouts.first()).map(|l| l.id).unwrap_or_default();@@ -1117,6 +1117,7 @@ fn picture_change(s: &mut Session, p: &Value) -> Result<Value> { fill.media = media; fill.crop = [0.0; 4]; fill.artistic = None;+ fill.svg = None; } Ok(()) })@@ -31,6 +31,9 @@ macro_rules! walkers { if let Some(a) = &$($m)? p.artistic { f(&$($m)? a.original); }+ if let Some(s) = &$($m)? p.svg {+ f(s);+ } } pub fn fill(x: &$($m)? Fill, f: &mut F) {@@ -1721,6 +1721,30 @@ mod copy_remap { assert_eq!(picture_bytes(&b, &shape(&b, 0, pb)), b"deck b picture"); } + #[test]+ fn a_pasted_picture_brings_its_svg_along() {+ let mut a = Session::with_new();+ let mut b = Session::with_new();+ let (pa, ..) = populate(&mut a, b"raster");+ populate(&mut b, b"deck b picture");+ {+ let doc = Arc::make_mut(&mut a.doc_mut().unwrap().doc);+ let svg = doc.add_media("p.svg", "image/svg+xml", b"<svg/>".to_vec()).expect("media");+ let slide = Arc::make_mut(&mut doc.slides[0]);+ let Some(Shape { kind: ShapeKind::Picture { fill }, .. }) = slide.shapes.iter_mut().find(|x| x.id == pa) else { panic!() };+ fill.svg = Some(svg);+ }+ select(&mut a, &[pa]);+ a.execute("edit.copy", &json!({})).unwrap();+ b.clipboard = a.clipboard.clone();+ b.doc_mut().unwrap().selection.shapes.clear();+ let r = b.execute("edit.paste", &json!({})).unwrap();+ let new = shape(&b, 0, ShapeId(r["ids"][0].as_u64().unwrap() as u32));+ let ShapeKind::Picture { fill } = &new.kind else { panic!("not a picture") };+ let svg = fill.svg.and_then(|m| b.doc().unwrap().doc.media(m)).map(|m| m.data.as_ref().clone());+ assert_eq!(svg.as_deref(), Some(&b"<svg/>"[..]));+ }+ #[test] fn copied_connectors_follow_copied_ends_and_detach_from_the_rest() { let mut s = Session::with_new();@@ -147,6 +147,10 @@ pub struct PictureFill { /// picture; the original is kept to change or remove the effect. #[serde(skip_serializing_if = "Option::is_none")] pub artistic: Option<Artistic>,+ /// The picture's SVG, from Office's SVG extension (`asvg:svgBlip`), written back the same+ /// way. When the file has no raster fallback, `media` is this SVG too.+ #[serde(skip_serializing_if = "Option::is_none")]+ pub svg: Option<MediaId>, } /// An applied artistic effect: its `a14` name (e.g. `artisticBlur`) and the original picture.@@ -27,6 +27,7 @@ pub const NS_P14: &str = "http://schemas.microsoft.com/office/powerpoint/2010/ma pub const NS_P15: &str = "http://schemas.microsoft.com/office/powerpoint/2012/main"; pub const NS_P159: &str = "http://schemas.microsoft.com/office/powerpoint/2015/09/main"; pub const NS_A14: &str = "http://schemas.microsoft.com/office/drawing/2010/main";+pub const NS_ASVG: &str = "http://schemas.microsoft.com/office/drawing/2016/SVG/main"; /// Modern (threaded) comments, Office 2018/8 schema. pub const NS_P188: &str = "http://schemas.microsoft.com/office/powerpoint/2018/8/main"; @@ -123,9 +123,11 @@ pub fn gradient(f: &El) -> Gradient { /// `blipFill` (either `a:` or `p:`) → picture fill, registering the image as media. pub fn blip_fill(imp: &mut Imp, part: &Part, bf: &El) -> Option<PictureFill> { let blip = bf.child("blip");- let media = blip.and_then(|b| media_ref(imp, part, b));- let media = media?;- let mut pf = PictureFill { media, upright: !bf.bool("rotWithShape").unwrap_or(true), ..Default::default() };+ // Office's SVG extension carries the vector picture, with or without a raster fallback in+ // the blip's own `r:embed`.+ let svg = blip.and_then(|b| b.child("extLst")).and_then(|x| x.find("svgBlip")).and_then(|s| media_ref(imp, part, s));+ let media = blip.and_then(|b| media_ref(imp, part, b)).or(svg)?;+ let mut pf = PictureFill { media, svg, upright: !bf.bool("rotWithShape").unwrap_or(true), ..Default::default() }; if let Some(layer) = blip.and_then(|b| b.child("extLst")).and_then(|x| x.find("imgLayer")) && let Some(effect) = layer .elements()@@ -98,7 +98,12 @@ pub fn blip_fill(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, pf: &PictureFil // An image that lives outside the package is a linked picture: `a:blip` names it through // `r:link`; `r:embed` may only reference a part inside the package. let linked = x.p.media(pf.media).is_some_and(|m| m.data.is_empty() && m.link.is_some());- let rid = x.media_rel(o, pf.media, "image").unwrap_or_else(|| x.placeholder_rel(o));+ // An SVG rides in Office's SVG extension. Without a raster fallback the blip itself names no+ // image, as PowerPoint writes it.+ let svg_rel = pf.svg.and_then(|m| x.media_rel(o, m, "image"));+ let svg_linked = pf.svg.and_then(|m| x.p.media(m)).is_some_and(|m| m.data.is_empty() && m.link.is_some());+ let svg_only = svg_rel.is_some() && pf.svg == Some(pf.media);+ let rid = if svg_only { None } else { Some(x.media_rel(o, pf.media, "image").unwrap_or_else(|| x.placeholder_rel(o))) }; w.open(tag, A::new().a("rotWithShape", if pf.upright { "0" } else { "1" })); let adj = &pf.adjust; let has_kids = pf.alpha.is_some()@@ -109,8 +114,13 @@ pub fn blip_fill(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, pf: &PictureFil || adj.clear_color.is_some() || adj.bilevel.is_some() || adj.alpha_inv- || pf.artistic.is_some();- let a = if linked { A::new().a("r:link", &rid) } else { A::new().a("r:embed", &rid) };+ || pf.artistic.is_some()+ || svg_rel.is_some();+ let a = match &rid {+ None => A::new(),+ Some(rid) if linked => A::new().a("r:link", rid),+ Some(rid) => A::new().a("r:embed", rid),+ }; if has_kids { w.open("a:blip", a); if let Some(c) = adj.clear_color {@@ -146,21 +156,30 @@ pub fn blip_fill(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, pf: &PictureFil } // Artistic effect: the blip is the processed picture; the original rides in an // `a14:imgLayer` with the effect's name, as PowerPoint writes it.- if let Some(art) = &pf.artistic- && art.effect.starts_with("artistic")- && art.effect.chars().all(|c| c.is_ascii_alphanumeric())- && let Some(orig) = x.media_rel(o, art.original, "image")- {+ let art = pf+ .artistic+ .as_ref()+ .filter(|art| art.effect.starts_with("artistic") && art.effect.chars().all(|c| c.is_ascii_alphanumeric()))+ .and_then(|art| x.media_rel(o, art.original, "image").map(|orig| (art, orig)));+ if art.is_some() || svg_rel.is_some() { w.open0("a:extLst");- w.open("a:ext", A::new().a("uri", "{BEBA8EAE-BF5A-486C-A8C5-ECC9F3942E4B}"));- w.open("a14:imgProps", A::new().a("xmlns:a14", crate::opc::NS_A14));- w.open("a14:imgLayer", A::new().a("r:embed", &orig));- w.open0("a14:imgEffect");- w.empty0(&format!("a14:{}", art.effect));- w.close("a14:imgEffect");- w.close("a14:imgLayer");- w.close("a14:imgProps");- w.close("a:ext");+ if let Some((art, orig)) = &art {+ w.open("a:ext", A::new().a("uri", "{BEBA8EAE-BF5A-486C-A8C5-ECC9F3942E4B}"));+ w.open("a14:imgProps", A::new().a("xmlns:a14", crate::opc::NS_A14));+ let original_linked = x.p.media(art.original).is_some_and(|m| m.data.is_empty() && m.link.is_some());+ w.open("a14:imgLayer", A::new().a(if original_linked { "r:link" } else { "r:embed" }, orig));+ w.open0("a14:imgEffect");+ w.empty0(&format!("a14:{}", art.effect));+ w.close("a14:imgEffect");+ w.close("a14:imgLayer");+ w.close("a14:imgProps");+ w.close("a:ext");+ }+ if let Some(svg) = &svg_rel {+ w.open("a:ext", A::new().a("uri", "{96DAC541-7B7A-43D3-8B79-37D633B846F1}"));+ w.empty("asvg:svgBlip", A::new().a("xmlns:asvg", crate::opc::NS_ASVG).a(if svg_linked { "r:link" } else { "r:embed" }, svg));+ w.close("a:ext");+ } w.close("a:extLst"); } w.close("a:blip");@@ -243,6 +243,7 @@ pub fn rich_deck() -> Presentation { adjust: PictureAdjust { brightness: 0.2, contrast: -0.1, grayscale: true, ..Default::default() }, upright: false, artistic: None,+ svg: None, }, }; let mut tiled = shape(&mut p, "Tiled", 350.0, 120.0, 200.0, 150.0);new file mode 100644@@ -0,0 +1,94 @@+//! Pictures that carry an SVG through Office's SVG extension (`asvg:svgBlip`).++use std::io::{Cursor, Read, Write};++use deckcraft_geom::Xfrm;+use deckcraft_model::style::PictureFill;+use deckcraft_model::{Presentation, Shape, ShapeId, ShapeKind, defaults};++const SVG: &[u8] = br##"<svg xmlns="http://www.w3.org/2000/svg" width="40" height="20"><rect width="40" height="20" fill="#c2185b"/></svg>"##;++fn png() -> Vec<u8> {+ let mut out = Cursor::new(vec![]);+ image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel(4, 2, image::Rgb([194, 24, 91])))+ .write_to(&mut out, image::ImageFormat::Png)+ .expect("png");+ out.into_inner()+}++/// A deck with one picture showing an SVG with a PNG fallback, exported.+fn deck() -> Vec<u8> {+ let mut p = defaults::new_presentation(None);+ let media = p.add_media("step.png", "image/png", png()).expect("png");+ let svg = p.add_media("step.svg", "image/svg+xml", SVG.to_vec()).expect("svg");+ let id = ShapeId(p.alloc_id().expect("ids"));+ let first = p.slides[0].id;+ p.slide_mut(first).expect("slide").shapes.push(Shape {+ id,+ name: "Step".into(),+ xfrm: Some(Xfrm::new(10.0, 10.0, 200.0, 100.0)),+ kind: ShapeKind::Picture { fill: PictureFill { media, svg: Some(svg), ..Default::default() } },+ ..Default::default()+ });+ deckcraft_pptx::export(&p).expect("export")+}++/// `bytes` with `edit` applied to the first slide.+fn edit_slide(bytes: &[u8], edit: impl Fn(String) -> String) -> Vec<u8> {+ let mut z = zip::ZipArchive::new(Cursor::new(bytes)).expect("zip");+ let mut out = zip::ZipWriter::new(Cursor::new(Vec::new()));+ let o = zip::write::SimpleFileOptions::default();+ for i in 0..z.len() {+ let mut f = z.by_index(i).expect("entry");+ let mut d = vec![];+ f.read_to_end(&mut d).expect("read");+ let name = f.name().to_string();+ if name == "ppt/slides/slide1.xml" {+ d = edit(String::from_utf8(d).expect("utf8")).into_bytes();+ }+ out.start_file(name, o).expect("start");+ out.write_all(&d).expect("write");+ }+ out.finish().expect("finish").into_inner()+}++fn slide_xml(bytes: &[u8]) -> String {+ let mut z = zip::ZipArchive::new(Cursor::new(bytes)).expect("zip");+ let mut out = String::new();+ z.by_name("ppt/slides/slide1.xml").expect("slide").read_to_string(&mut out).expect("utf8");+ out+}++/// The picture named Step and the bytes of its image and SVG.+fn step(p: &Presentation) -> (Vec<u8>, Option<Vec<u8>>) {+ let s = p.slides[0].shapes.iter().find(|s| s.name == "Step").expect("picture");+ let ShapeKind::Picture { fill } = &s.kind else { panic!("not a picture: {:?}", s.kind) };+ let data = |m| p.media(m).map(|m| m.data.as_ref().clone()).expect("media");+ (data(fill.media), fill.svg.map(data))+}++#[test]+fn an_svg_with_a_png_fallback_round_trips() {+ let bytes = deck();+ let xml = slide_xml(&bytes);+ assert!(xml.contains("<asvg:svgBlip"), "{xml}");+ assert!(xml.contains("<a:blip r:embed="), "{xml}");+ let (image, svg) = step(&deckcraft_pptx::import(&bytes).expect("import"));+ assert_eq!((image, svg), (png(), Some(SVG.to_vec())));+}++#[test]+fn an_svg_without_a_fallback_stays_a_picture() {+ // Decks can name the SVG only, with no raster image in the blip itself.+ let bytes = edit_slide(&deck(), |x| {+ let start = x.find("<a:blip r:embed=\"").expect("blip");+ let end = start + x[start..].find('>').expect("end");+ format!("{}<a:blip>{}", &x[..start], &x[end + 1..])+ });+ let q = deckcraft_pptx::import(&bytes).expect("import");+ assert_eq!(step(&q), (SVG.to_vec(), Some(SVG.to_vec())));+ let again = deckcraft_pptx::export(&q).expect("export");+ let xml = slide_xml(&again);+ assert!(xml.contains("<p:pic>") && xml.contains("<a:blip><a:extLst>") && xml.contains("<asvg:svgBlip"), "{xml}");+ assert_eq!(step(&deckcraft_pptx::import(&again).expect("import")), (SVG.to_vec(), Some(SVG.to_vec())));+}