diff --git a/crates/pptx/src/read/timing.rs b/crates/pptx/src/read/timing.rs index 1b6aa8b1b..1caa0001f 100644 --- a/crates/pptx/src/read/timing.rs +++ b/crates/pptx/src/read/timing.rs @@ -126,7 +126,7 @@ fn flat_starts(anims: &[&Animation]) -> Vec { /// a click group listed out of time order, whose flat reading disagrees with the file, is sorted by /// start time and its starts and delays rewritten to land every effect where PowerPoint plays it. /// Every other group is left untouched. -fn place_by_parent_delays(out: &mut [(Animation, u32, String, bool)], at: &[(Option, usize, u32)]) { +fn place_by_parent_delays(out: &mut [(Animation, u32, String, bool, usize, u32)], at: &[(Option, usize, u32)]) { let mut groups: Vec> = vec![]; let mut slot: std::collections::HashMap<(Option, usize), usize> = std::collections::HashMap::new(); for (i, &(t, g, _)) in at.iter().enumerate() { @@ -155,7 +155,7 @@ fn place_by_parent_delays(out: &mut [(Animation, u32, String, bool)], at: &[(Opt let had_click = out[idx[0]].0.start == AnimStart::OnClick; let mut order: Vec = (0..idx.len()).collect(); order.sort_by_key(|&k| abs[k]); - let mut sorted: Vec<(Animation, u32, String, bool)> = order.iter().map(|&k| out[idx[k]].clone()).collect(); + let mut sorted: Vec<(Animation, u32, String, bool, usize, u32)> = order.iter().map(|&k| out[idx[k]].clone()).collect(); let (mut begin, mut end) = (0u32, 0u32); for (n, (&k, item)) in order.iter().zip(sorted.iter_mut()).enumerate() { let a = &mut item.0; @@ -170,6 +170,7 @@ fn place_by_parent_delays(out: &mut [(Animation, u32, String, bool)], at: &[(Opt a.start = AnimStart::WithPrevious; a.delay_ms = t.saturating_sub(begin); } + item.5 = 0; end = end.max(t.saturating_add(span_ms(a))); } for (&i, item) in idx.iter().zip(sorted) { @@ -239,17 +240,19 @@ pub fn animations(imp: &mut Imp, part: &Part, ctx: &IdCtx, timing: &El, shapes: let Some(tn) = timing.child("tnLst") else { return vec![] }; let mut found = vec![]; collect(tn, At { trigger: None, group: 0, offset: 0 }, &mut 0, &mut found, 0); - // Paragraph builds (`bldP build="p"`) by (spid, grpId). - let by_para: Vec<(u32, String)> = timing + // Paragraph builds (`bldP`) by (spid, grpId) -> whether `build="p"`. + let bld_p: std::collections::HashMap<(u32, String), bool> = timing .child("bldLst") .map(|b| { b.children_named("bldP") - .filter(|p| p.attr("build") == Some("p")) - .filter_map(|p| Some((p.u32("spid")?, p.attr("grpId").unwrap_or("0").to_string()))) - .collect() + .filter_map(|p| Some(((p.u32("spid")?, p.attr("grpId").unwrap_or("0").to_string()), p.attr("build") == Some("p")))) + .fold(std::collections::HashMap::new(), |mut m, (k, v)| { + m.entry(k).and_modify(|e| *e |= v).or_insert(v); + m + }) }) .unwrap_or_default(); - let mut out: Vec<(Animation, u32, String, bool)> = vec![]; + let mut out: Vec<(Animation, u32, String, bool, usize, u32)> = vec![]; let mut at: Vec<(Option, usize, u32)> = vec![]; for f in found { let c = f.ctn; @@ -314,23 +317,27 @@ pub fn animations(imp: &mut Imp, part: &Part, ctx: &IdCtx, timing: &El, shapes: a.color = dml::color(clr); } let grp = c.attr("grpId").unwrap_or("0").to_string(); - let build = paragraph.is_some() && by_para.iter().any(|(s, g)| *s == file_id && *g == grp); - out.push((a, file_id, grp, build)); + let build = paragraph.is_some() && bld_p.get(&(file_id, grp.clone())) != Some(&false); + out.push((a, file_id, grp, build, f.group, f.offset)); at.push((f.trigger, f.group, f.offset)); } place_by_parent_delays(&mut out, &at); // A paragraph build's effects, one per paragraph, become one by-paragraph animation only when // the timeline plays that back exactly: PowerPoint lets each paragraph keep its own start and // effect, and other shapes' effects can sit between them (#844). - let same_build = - |x: &(Animation, u32, String, bool), y: &(Animation, u32, String, bool)| x.3 && y.3 && x.1 == y.1 && x.2 == y.2 && x.0.trigger == y.0.trigger; + let same_build = |x: &(Animation, u32, String, bool, usize, u32), y: &(Animation, u32, String, bool, usize, u32)| { + x.3 && y.3 && x.1 == y.1 && x.2 == y.2 && x.0.trigger == y.0.trigger + }; let mut anims = Vec::with_capacity(out.len()); for run in out.chunk_by(same_build) { let effects: Vec<&Animation> = run.iter().map(|(a, ..)| a).collect(); + let offsets: Vec = run.iter().map(|r| r.5).collect(); let build = run.first().is_some_and(|r| r.3); + let explicit_bld = run.first().is_some_and(|r| bld_p.get(&(r.1, r.2.clone())) == Some(&true)); + let same_click_group = run.first().is_some_and(|first| first.0.start == AnimStart::OnClick || run.iter().all(|r| r.4 == first.4)); let text = effects.first().and_then(|a| find_shape(shapes, a.shape)).and_then(|s| s.text.as_ref()); match (effects.first(), text) { - (Some(first), Some(text)) if build && plays_as_build(&effects, text) => { + (Some(first), Some(text)) if build && (explicit_bld || same_click_group) && plays_as_build(&effects, &offsets, text, explicit_bld) => { anims.push(Animation { text_build: TextBuild::ByParagraph, paragraph: None, ..(*first).clone() }); } _ => anims.extend(effects.into_iter().cloned()), @@ -343,7 +350,7 @@ pub fn animations(imp: &mut Imp, part: &Part, ctx: &IdCtx, timing: &El, shapes: /// first one plays: one effect per paragraph the build animates, in paragraph order, later top /// paragraphs on click (after previous when the build doesn't start on click), deeper ones with /// their parent, all with the same settings? -fn plays_as_build(effects: &[&Animation], text: &TextBody) -> bool { +fn plays_as_build(effects: &[&Animation], offsets: &[u32], text: &TextBody, explicit_bld: bool) -> bool { let roles = paragraph_build_roles(text); let expected = roles.iter().enumerate().filter(|(i, r)| match r { BuildRole::Top => true, @@ -351,6 +358,16 @@ fn plays_as_build(effects: &[&Animation], text: &TextBody) -> bool { BuildRole::Unowned => false, }); let Some(first) = effects.first() else { return false }; + let off0 = offsets.first().copied().unwrap_or(0); + let flat = flat_starts(effects); + let parent_starts_match = offsets + .iter() + .zip(effects.iter()) + .zip(flat.iter()) + .all(|((&off, a), &ft)| off == off0 || off.saturating_add(a.delay_ms) == off0.saturating_add(ft)); + if !explicit_bld && (!parent_starts_match || (first.start != AnimStart::OnClick && (span_ms(first) != 500 || first.delay_ms != 0 || off0 != 0))) { + return false; + } let settings = |a: &Animation| Animation { start: AnimStart::OnClick, paragraph: None, ..a.clone() }; let base = settings(first); let mut matched = 0; diff --git a/crates/pptx/tests/it/corpus_style.rs b/crates/pptx/tests/it/corpus_style.rs index e64e2f5e6..ab8606f4c 100644 --- a/crates/pptx/tests/it/corpus_style.rs +++ b/crates/pptx/tests/it/corpus_style.rs @@ -316,3 +316,99 @@ fn left_of(p: &Presentation) -> ShapeId { fn right_of(p: &Presentation) -> ShapeId { by_name(p, "Right").id } + +/// Return a copy of the `.pptx` archive `bytes` with `entry_name` rewritten by `f`. +fn rewrite_zip_entry(bytes: &[u8], entry_name: &str, f: impl Fn(&str) -> String) -> Vec { + use std::io::{Cursor, Read, Write}; + let mut r = zip::ZipArchive::new(Cursor::new(bytes)).unwrap(); + let mut out = Cursor::new(Vec::new()); + { + let mut w = zip::ZipWriter::new(&mut out); + let opts = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated); + for i in 0..r.len() { + let mut file = r.by_index(i).unwrap(); + let name = file.name().to_string(); + let mut buf = Vec::new(); + file.read_to_end(&mut buf).unwrap(); + w.start_file(&name, opts).unwrap(); + if name == entry_name { + let text = String::from_utf8(buf).unwrap(); + w.write_all(f(&text).as_bytes()).unwrap(); + } else { + w.write_all(&buf).unwrap(); + } + } + w.finish().unwrap(); + } + out.into_inner() +} + +#[test] +fn paragraph_build_without_bld_lst_still_folds_into_by_paragraph() { + use deckcraft_model::anim::{AnimClass, AnimStart, Animation, TextBuild}; + let mut pres = defaults::new_presentation(None); + let mut sh = shape(&mut pres, "Title 1", 50.0, 50.0, 400.0, 200.0); + let shape_id = sh.id; + sh.text = Some(TextBody { paragraphs: vec![Paragraph::new("First"), Paragraph::new("Second")], ..Default::default() }); + let s0 = Arc::make_mut(&mut pres.slides[0]); + s0.shapes.push(sh); + s0.animations.push(Animation { + shape: shape_id, + class: AnimClass::Entrance, + effect: "fade".into(), + start: AnimStart::OnClick, + duration_ms: 500, + text_build: TextBuild::ByParagraph, + ..Default::default() + }); + let bytes = deckcraft_pptx::export(&pres).unwrap(); + let without_bld = rewrite_zip_entry(&bytes, "ppt/slides/slide1.xml", |xml| { + let start = xml.find("").expect("bldLst"); + let end = xml.find("").expect("/bldLst") + "".len(); + format!("{}{}", &xml[..start], &xml[end..]) + }); + let back = deckcraft_pptx::import(&without_bld).unwrap(); + assert_eq!(back.slides[0].animations.len(), 1); + assert_eq!(back.slides[0].animations[0].text_build, TextBuild::ByParagraph); + + // Explicit bldP build="p" folds non-500ms After Previous builds, while without bldLst an 800ms + // After Previous run stays separate so playback does not alter its 800ms stagger to 500ms. + let mut pres_after = pres.clone(); + let s_after = Arc::make_mut(&mut pres_after.slides[0]); + s_after.animations[0].start = AnimStart::AfterPrevious; + s_after.animations[0].duration_ms = 800; + let bytes_after = deckcraft_pptx::export(&pres_after).unwrap(); + let back_explicit_after = deckcraft_pptx::import(&bytes_after).unwrap(); + assert_eq!(back_explicit_after.slides[0].animations.len(), 1); + assert_eq!(back_explicit_after.slides[0].animations[0].text_build, TextBuild::ByParagraph); + assert_eq!(back_explicit_after.slides[0].animations[0].duration_ms, 800); + + let without_bld_after = rewrite_zip_entry(&bytes_after, "ppt/slides/slide1.xml", |xml| { + let start = xml.find("").expect("bldLst"); + let end = xml.find("").expect("/bldLst") + "".len(); + format!("{}{}", &xml[..start], &xml[end..]) + }); + let back_after = deckcraft_pptx::import(&without_bld_after).unwrap(); + assert_eq!(back_after.slides[0].animations.len(), 2); + assert!(back_after.slides[0].animations.iter().all(|a| a.text_build == TextBuild::default())); + // Explicit bldP with build!="p" (e.g. build="cust") opts out of folding. + let custom_bld = rewrite_zip_entry(&bytes, "ppt/slides/slide1.xml", |xml| xml.replace(r#"build="p""#, r#"build="cust""#)); + let back_cust = deckcraft_pptx::import(&custom_bld).unwrap(); + assert_eq!(back_cust.slides[0].animations.len(), 2); + assert!(back_cust.slides[0].animations.iter().all(|a| a.text_build == TextBuild::default())); + + // Without bldLst, an AfterPrevious second paragraph whose parent container delay (1000ms) + // disagrees with the 500ms first paragraph span must not fold into a 500ms build. + let mut pres_with = pres.clone(); + Arc::make_mut(&mut pres_with.slides[0]).animations[0].start = AnimStart::WithPrevious; + let bytes_with = deckcraft_pptx::export(&pres_with).unwrap(); + let mismatched_offset = rewrite_zip_entry(&bytes_with, "ppt/slides/slide1.xml", |xml| { + let start = xml.find("").expect("bldLst"); + let end = xml.find("").expect("/bldLst") + "".len(); + let stripped = format!("{}{}", &xml[..start], &xml[end..]); + stripped.replacen(r#""#, r#""#, 1) + }); + let back_mismatch = deckcraft_pptx::import(&mismatched_offset).unwrap(); + assert_eq!(back_mismatch.slides[0].animations.len(), 2); + assert!(back_mismatch.slides[0].animations.iter().all(|a| a.text_build == TextBuild::default())); +}