From 751ac26d280f59eaf6be64690465eb562b141f7b Mon Sep 17 00:00:00 2001 From: Naseq Date: Tue, 3 Nov 2026 17:57:57 +0430 Subject: [PATCH 1/2] fix(pptx): place effects by their parent time node delays (fixes #1744) --- crates/pptx/src/read/timing.rs | 115 +++++++++++++++++++++-- crates/pptx/tests/it/main.rs | 1 + crates/pptx/tests/it/parent_delays.rs | 128 ++++++++++++++++++++++++++ 3 files changed, 238 insertions(+), 6 deletions(-) create mode 100644 crates/pptx/tests/it/parent_delays.rs diff --git a/crates/pptx/src/read/timing.rs b/crates/pptx/src/read/timing.rs index 9c748328..f3166d62 100644 --- a/crates/pptx/src/read/timing.rs +++ b/crates/pptx/src/read/timing.rs @@ -53,25 +53,125 @@ pub fn transition(imp: &mut Imp, part: &Part, t: &El) -> Option { struct Found<'a> { ctn: &'a El, trigger: Option, + /// Click group the effect sits in: a new one starts at every time node whose start is `indefinite`. + group: usize, + /// Sum of the start delays of the time nodes between the click group and the effect, in ms. + offset: u32, } -fn collect<'a>(e: &'a El, trigger: Option, out: &mut Vec>, depth: usize) { +/// Where `collect` is in the time tree: the click group and the delay accumulated inside it. +#[derive(Clone, Copy)] +struct At { + trigger: Option, + group: usize, + offset: u32, +} + +fn collect<'a>(e: &'a El, at: At, groups: &mut usize, out: &mut Vec>, depth: usize) { if depth > 64 || out.len() > 10_000 { return; } for c in e.elements() { if c.is("cTn") && c.has_attr("presetClass") { - out.push(Found { ctn: c, trigger }); + out.push(Found { ctn: c, trigger: at.trigger, group: at.group, offset: at.offset }); continue; } - let mut trig = trigger; + let mut at = at; if c.is("seq") && let Some(ctn) = c.child("cTn") && ctn.attr("nodeType") == Some("interactiveSeq") { - trig = ctn.path(&["stCondLst", "cond", "tgtEl", "spTgt"]).and_then(|s| s.u32("spid")); + at.trigger = ctn.path(&["stCondLst", "cond", "tgtEl", "spTgt"]).and_then(|s| s.u32("spid")); + } + if c.is("cTn") + && let Some(cond) = c.path(&["stCondLst", "cond"]) + { + if cond.attr("delay").map(str::trim) == Some("indefinite") { + *groups += 1; + at.group = *groups; + at.offset = 0; + } else { + at.offset = at.offset.saturating_add(ms_attr(cond, "delay").unwrap_or(0)); + } + } + collect(c, at, groups, out, depth + 1); + } +} + +/// How long an effect holds back an After Previous one, as the show timeline counts it. +fn span_ms(a: &Animation) -> u32 { + let iterations = if a.repeat == u32::MAX || a.repeat <= 1 { 1 } else { a.repeat.min(1_000) }; + a.duration_ms.saturating_mul(if a.auto_reverse { 2 } else { 1 }).saturating_mul(iterations) +} + +/// Start times (ms after the click) that the flat effect list plays at, effect after effect. +fn flat_starts(anims: &[&Animation]) -> Vec { + let (mut begin, mut end) = (0u32, 0u32); + anims + .iter() + .map(|a| { + if a.start == AnimStart::AfterPrevious { + begin = end; + } + let start = begin.saturating_add(a.delay_ms); + end = end.max(start.saturating_add(span_ms(a))); + start + }) + .collect() +} + +/// PowerPoint places each effect at the delays of all its enclosing time nodes, so a file may list +/// effects out of time order (a countdown written first, then effects at 5 s placed only by their +/// parent's delay). The flat list only knows "after/with previous" plus the effect's own delay, so +/// 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)]) { + 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() { + let n = *slot.entry((t, g)).or_insert_with(|| { + groups.push(vec![]); + groups.len() - 1 + }); + groups[n].push(i); + } + for idx in groups { + // More than one click in a group means the tree isn't one click per group: leave it. + if idx.iter().skip(1).any(|&i| out[i].0.start == AnimStart::OnClick) { + continue; + } + let abs: Vec = idx.iter().map(|&i| at[i].2.saturating_add(out[i].0.delay_ms)).collect(); + // Only a file that lists effects out of time order needs this: files PowerPoint or DeckCraft + // save are in order, and their reading stays exactly as before. + if abs.is_sorted() { + continue; + } + let flat = flat_starts(&idx.iter().map(|&i| &out[i].0).collect::>()); + if flat == abs { + continue; + } + 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 begin, mut end) = (0u32, 0u32); + for (n, (&k, item)) in order.iter().zip(sorted.iter_mut()).enumerate() { + let a = &mut item.0; + let t = abs[k]; + if n == 0 && a.start == AnimStart::OnClick { + a.delay_ms = t; + } else if a.start == AnimStart::AfterPrevious && t >= end { + begin = end; + a.delay_ms = t - end; + } else { + a.start = AnimStart::WithPrevious; + a.delay_ms = t.saturating_sub(begin); + } + end = end.max(t.saturating_add(span_ms(a))); + } + for (&i, item) in idx.iter().zip(sorted) { + out[i] = item; } - collect(c, trig, out, depth + 1); } } @@ -135,7 +235,7 @@ pub fn animations(imp: &mut Imp, part: &Part, ctx: &IdCtx, timing: &El, shapes: let _ = (imp, part); let Some(tn) = timing.child("tnLst") else { return vec![] }; let mut found = vec![]; - collect(tn, None, &mut found, 0); + 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 .child("bldLst") @@ -147,6 +247,7 @@ pub fn animations(imp: &mut Imp, part: &Part, ctx: &IdCtx, timing: &El, shapes: }) .unwrap_or_default(); let mut out: Vec<(Animation, u32, String, bool)> = vec![]; + let mut at: Vec<(Option, usize, u32)> = vec![]; for f in found { let c = f.ctn; let class = AnimClass::from_xml(c.attr("presetClass").unwrap_or("entr")); @@ -212,7 +313,9 @@ pub fn animations(imp: &mut Imp, part: &Part, ctx: &IdCtx, timing: &El, shapes: 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)); + 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). diff --git a/crates/pptx/tests/it/main.rs b/crates/pptx/tests/it/main.rs index df7065cb..71a8d377 100644 --- a/crates/pptx/tests/it/main.rs +++ b/crates/pptx/tests/it/main.rs @@ -18,6 +18,7 @@ mod media_timing; mod notes; mod ole; mod ole_vml; +mod parent_delays; mod placeholders; mod roundtrip; mod sample; diff --git a/crates/pptx/tests/it/parent_delays.rs b/crates/pptx/tests/it/parent_delays.rs new file mode 100644 index 00000000..9dcaa851 --- /dev/null +++ b/crates/pptx/tests/it/parent_delays.rs @@ -0,0 +1,128 @@ +//! PowerPoint places an effect at the sum of its enclosing time nodes' delays, so a file can list +//! effects out of time order. The BNI meeting-deck generator writes a 1 s countdown chain first, +//! then "swap to the visual at 5 s" effects placed only by their parent `p:par` delay: read in file +//! order, those all fired together when the countdown ended. + +use std::io::{Cursor, Read, Write}; + +use deckcraft_geom::Xfrm; +use deckcraft_model::anim::AnimStart; +use deckcraft_model::{Shape, ShapeId, defaults}; + +/// An "appear"/"disappear" effect (preset 1) in its own `p:par`, as PowerPoint writes it. +fn eff(id: u32, class: &str, node: &str, delay: u32, spid: u32) -> String { + let vis = if class == "entr" { "visible" } else { "hidden" }; + format!( + r#"style.visibility"#, + id + 1000 + ) +} + +/// A group `p:par` starting `delay` ms into the click, holding `inner`. +fn group(id: u32, delay: u32, inner: &str) -> String { + format!( + r#"{inner}"# + ) +} + +fn import_with_timing(build: impl Fn(&[u32]) -> String) -> Vec<(String, AnimStart, u32)> { + let mut p = defaults::new_presentation(None); + let shapes: Vec = ["Three", "Two", "One", "Visual"] + .into_iter() + .map(|name| Shape { + id: ShapeId(p.alloc_id().expect("id")), + name: name.into(), + xfrm: Some(Xfrm::new(10.0, 10.0, 100.0, 50.0)), + ..Default::default() + }) + .collect(); + std::sync::Arc::make_mut(&mut p.slides[0]).shapes = shapes; + let mut z = zip::ZipArchive::new(Cursor::new(deckcraft_pptx::export(&p).expect("export"))).expect("zip"); + let mut out = zip::ZipWriter::new(Cursor::new(Vec::new())); + for i in 0..z.len() { + let mut f = z.by_index(i).expect("entry"); + let name = f.name().to_string(); + let mut d = vec![]; + f.read_to_end(&mut d).expect("read"); + if name == "ppt/slides/slide1.xml" { + let xml = String::from_utf8(d).expect("utf8"); + let spids: Vec = ["Three", "Two", "One", "Visual"] + .iter() + .map(|n| { + let at = xml.find(&format!(r#"name="{n}""#)).expect("shape written"); + let open = xml[..at].rfind("{}"#, + build(&spids) + ); + let xml = match (xml.find(""), xml.find("")) { + (Some(a), Some(b)) => format!("{}{}{}", &xml[..a], timing, &xml[b + "".len()..]), + _ => xml.replacen("", &format!("{timing}"), 1), + }; + d = xml.into_bytes(); + } + out.start_file(name, zip::write::SimpleFileOptions::default()).expect("start"); + out.write_all(&d).expect("write"); + } + let q = deckcraft_pptx::import(&out.finish().expect("finish").into_inner()).expect("import"); + let s = &q.slides[0]; + s.animations + .iter() + .map(|a| { + let name = s.shapes.iter().find(|x| x.id == a.shape).map(|x| x.name.clone()).unwrap_or_default(); + (name, a.start, a.delay_ms) + }) + .collect() +} + +/// Start (ms after the click) of each effect when the flat list is played effect after effect. +fn flat(effects: &[(String, AnimStart, u32)]) -> Vec<(String, u32)> { + let (mut begin, mut end) = (0u32, 0u32); + effects + .iter() + .map(|(n, start, delay)| { + if *start == AnimStart::AfterPrevious { + begin = end; + } + let t = begin + delay; + end = end.max(t); + (n.clone(), t) + }) + .collect() +} + +#[test] +fn effects_placed_by_parent_delay_play_when_powerpoint_plays_them() { + // Click: "Three" goes, then "Two" after 1 s, "One" after 1 s more (a countdown chain), then — + // written last — the visual appears at 1 s, placed only by its parent's delay. + let fx = import_with_timing(|s| { + [ + group(10, 0, &eff(11, "exit", "clickEffect", 0, s[0])), + group(20, 0, &eff(21, "exit", "afterEffect", 1000, s[1])), + group(30, 1000, &eff(31, "exit", "afterEffect", 1000, s[2])), + group(40, 1000, &eff(41, "entr", "afterEffect", 0, s[3])), + ] + .concat() + }); + let got = flat(&fx); + let at = |n: &str| got.iter().find(|(x, _)| x == n).map(|(_, t)| *t); + assert_eq!((at("Three"), at("Two"), at("Visual"), at("One")), (Some(0), Some(1000), Some(1000), Some(2000)), "{fx:?}"); +} + +#[test] +fn a_timeline_already_in_order_is_read_unchanged() { + let fx = import_with_timing(|s| { + [ + group(10, 0, &[eff(11, "exit", "clickEffect", 0, s[0]), eff(12, "entr", "withEffect", 500, s[3])].concat()), + group(20, 500, &eff(21, "exit", "afterEffect", 1000, s[1])), + ] + .concat() + }); + let starts: Vec<_> = fx.iter().map(|(n, st, d)| (n.as_str(), *st, *d)).collect(); + assert_eq!(starts, [("Three", AnimStart::OnClick, 0), ("Visual", AnimStart::WithPrevious, 500), ("Two", AnimStart::AfterPrevious, 1000)]); +} From 6546136a4217e1b3ba67e28e7aaf03bc0f04db34 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Tue, 3 Nov 2026 08:49:18 -0500 Subject: [PATCH 2/2] PPTX timing: a sorted click group keeps its click on the effect that now plays first --- crates/pptx/src/read/timing.rs | 5 ++++- crates/pptx/tests/it/parent_delays.rs | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/pptx/src/read/timing.rs b/crates/pptx/src/read/timing.rs index f3166d62..1b6aa8b1 100644 --- a/crates/pptx/src/read/timing.rs +++ b/crates/pptx/src/read/timing.rs @@ -151,6 +151,8 @@ fn place_by_parent_delays(out: &mut [(Animation, u32, String, bool)], at: &[(Opt if flat == abs { continue; } + // The group's click stays on whichever effect now plays first. + 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(); @@ -158,7 +160,8 @@ fn place_by_parent_delays(out: &mut [(Animation, u32, String, bool)], at: &[(Opt for (n, (&k, item)) in order.iter().zip(sorted.iter_mut()).enumerate() { let a = &mut item.0; let t = abs[k]; - if n == 0 && a.start == AnimStart::OnClick { + if n == 0 && had_click { + a.start = AnimStart::OnClick; a.delay_ms = t; } else if a.start == AnimStart::AfterPrevious && t >= end { begin = end; diff --git a/crates/pptx/tests/it/parent_delays.rs b/crates/pptx/tests/it/parent_delays.rs index 9dcaa851..8d9f25c6 100644 --- a/crates/pptx/tests/it/parent_delays.rs +++ b/crates/pptx/tests/it/parent_delays.rs @@ -126,3 +126,20 @@ fn a_timeline_already_in_order_is_read_unchanged() { let starts: Vec<_> = fx.iter().map(|(n, st, d)| (n.as_str(), *st, *d)).collect(); assert_eq!(starts, [("Three", AnimStart::OnClick, 0), ("Visual", AnimStart::WithPrevious, 500), ("Two", AnimStart::AfterPrevious, 1000)]); } + +#[test] +fn a_delayed_click_effect_sorted_after_a_with_effect_keeps_the_click_on_the_group() { + // The click effect waits 500 ms; a with-effect in the same click plays at once, and a later + // effect is placed only by its parent's delay. Sorted, the with-effect plays first and takes + // the click, so the group still waits for one. + let fx = import_with_timing(|s| { + [ + group(10, 0, &[eff(11, "exit", "clickEffect", 500, s[0]), eff(12, "entr", "withEffect", 0, s[3])].concat()), + group(20, 1000, &eff(21, "exit", "afterEffect", 0, s[1])), + ] + .concat() + }); + let starts: Vec<_> = fx.iter().map(|(n, st, _)| (n.as_str(), *st)).collect(); + assert_eq!(starts[0], ("Visual", AnimStart::OnClick), "{fx:?}"); + assert_eq!(starts.iter().filter(|(_, st)| *st == AnimStart::OnClick).count(), 1, "{fx:?}"); +}