From 2d6582163f02489f2fb678e17bb1bbc81f89f2a4 Mon Sep 17 00:00:00 2001 From: Faabyy <68284158+Faabyy@users.noreply.github.com> Date: Sun, 18 Oct 2026 18:57:29 +0000 Subject: [PATCH 1/4] Media: look a clip up on the slide it belongs to (#994) The PPTX import numbers shapes per slide, so two slides can hold different videos under one shape id. find_clip searched the whole deck and took the first match, so the show's media cues, the keep-across-slides filter and the media.* requests all got the first slide's clip: from slide 3 on, video 2 played. find_clip now takes the slide the request comes from and searches it first, then the rest of the deck. Co-Authored-By: Claude # Please enter the commit message for your changes. Lines starting # with '#' will be kept; you may remove them yourself if you want to. # An empty message aborts the commit. # # interactive rebase in progress; onto a31b990 # Last command done (1 command done): # pick 1ab070b Media: look a clip up on the slide it belongs to (#994) # Next commands to do (3 remaining commands): # pick ad4e3f0 Media: stop a clip across slides when the next slide reuses its id (#994) # pick 0957b58 Media: the playing clip decides whether it carries over (#994) # You are currently rebasing. # # Changes to be committed: # modified: ROADMAP.md # modified: crates/ui-egui/src/lib.rs # modified: crates/ui-egui/src/media.rs # modified: crates/ui-egui/src/show.rs # --- ROADMAP.md | 1 + crates/ui-egui/src/lib.rs | 5 +++-- crates/ui-egui/src/media.rs | 26 +++++++++++++++++++++----- crates/ui-egui/src/show.rs | 6 +++--- 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 93bc931e..71dfe098 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| | 2026-10-18 | Native chart sidecar bytes move from JSON arrays into referenced ZIP entries, with shared media byte/item limits and bounded reads; older inline sidecars still load. Synthetic format regressions cover large workbooks and malformed members | +| 2026-10-18 | Slide show: a video or audio clip is looked up on the slide it belongs to, so slides whose shape ids repeat (the PPTX import numbers each slide afresh) no longer play each other's clips (#994) | | 2026-10-17 | Opening a file caps absurd effect sizes (glow, soft edges, shadows, reflections, on shapes, text and table text) at what Format Shape allows: a glow of 10^18 pt in a .pptx made rendering ask for 34 GB and abort | | 2026-10-17 | Opening a .deckcraft file whose shapes repeat an id gives the repeats fresh ids (groups, layouts and masters too), so the deck validates and edits by id find one shape | | 2026-10-17 | Edit Points: turning a shape into a freeform is one undo step, so Undo brings the original shape back (it used to stay a freeform) | diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index eef9b1b3..1de11554 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1347,8 +1347,9 @@ impl SlideApp { } UiRequest::Media { action, shape, ms } => { let owner = if self.show.is_some() { media::Owner::Show } else { media::Owner::Editor }; - if let Some((uid, doc)) = self.session.active().map(|d| (d.uid, d.doc.clone())) { - self.media.request(uid, &doc, &action, shape, ms, owner); + let shown = self.show.as_ref().map(|s| s.state.slide); + if let Some((uid, doc, slide)) = self.session.active().map(|d| (d.uid, d.doc.clone(), shown.unwrap_or(d.selection.slide))) { + self.media.request(uid, &doc, slide, &action, shape, ms, owner); } } } diff --git a/crates/ui-egui/src/media.rs b/crates/ui-egui/src/media.rs index 7fd8fda7..344689b2 100644 --- a/crates/ui-egui/src/media.rs +++ b/crates/ui-egui/src/media.rs @@ -318,9 +318,10 @@ impl MediaHost { } /// Carry out a `media.*` request from the engine for presentation `doc_uid`. - pub fn request(&mut self, doc_uid: u64, doc: &Presentation, action: &str, id: ShapeId, ms: Option, owner: Owner) { + /// `slide` is the slide the request comes from: shape ids are only unique per slide. + pub fn request(&mut self, doc_uid: u64, doc: &Presentation, slide: usize, action: &str, id: ShapeId, ms: Option, owner: Owner) { self.ensure_doc(doc_uid); - let Some(clip) = find_clip(doc, id) else { return }; + let Some(clip) = find_clip(doc, id, Some(slide)) else { return }; let t = ms.map(|v| v as f64 / 1000.0); match action { "play" => self.play(doc_uid, doc, id, &clip, t, owner), @@ -333,8 +334,9 @@ impl MediaHost { } } -/// The clip of media shape `id` anywhere in the deck. -pub fn find_clip(doc: &Presentation, id: ShapeId) -> Option { +/// The clip of media shape `id`. Ids are only unique per slide (the PPTX import numbers each slide +/// afresh), so the `near` slide is searched first, then the rest of the deck. +pub fn find_clip(doc: &Presentation, id: ShapeId, near: Option) -> Option { fn walk(shapes: &[deckcraft_model::Shape], id: ShapeId) -> Option { shapes.iter().find_map(|s| match &s.kind { ShapeKind::Media(m) if s.id == id => Some(m.clone()), @@ -342,7 +344,7 @@ pub fn find_clip(doc: &Presentation, id: ShapeId) -> Option { _ => None, }) } - doc.slides.iter().find_map(|s| walk(&s.shapes, id)) + near.and_then(|i| doc.slides.get(i)).and_then(|s| walk(&s.shapes, id)).or_else(|| doc.slides.iter().find_map(|s| walk(&s.shapes, id))) } /// Media shapes directly on slide `index`: (id, box in slide points, clip). @@ -617,4 +619,18 @@ mod tests { host.set_document(None); assert!(host.gifs.is_empty()); } + + #[test] + fn a_clip_is_looked_up_on_the_slide_it_was_asked_from() { + // The PPTX import numbers shapes per slide, so two slides can hold different clips under one id. + let (mut doc, id, first) = deck(4000, wav_level(0.05, 0.5)); + let other = doc.add_media("two.wav", "audio/wav", wav_level(0.05, 0.25)).expect("media"); + let mut second = (*doc.slides[0]).clone(); + second.shapes.last_mut().expect("shape").kind = ShapeKind::Media(MediaClip { media: other, volume: 1.0, ..Default::default() }); + doc.slides.push(Arc::new(second)); + assert_eq!(find_clip(&doc, id, Some(0)).map(|c| c.media), Some(first.media)); + assert_eq!(find_clip(&doc, id, Some(1)).map(|c| c.media), Some(other), "the second slide's own clip"); + assert_eq!(find_clip(&doc, id, None).map(|c| c.media), Some(first.media), "without a hint: first match"); + assert_eq!(find_clip(&doc, id, Some(9)).map(|c| c.media), Some(first.media), "a slide that doesn't exist"); + } } diff --git a/crates/ui-egui/src/show.rs b/crates/ui-egui/src/show.rs index 51f859af..599e366a 100644 --- a/crates/ui-egui/src/show.rs +++ b/crates/ui-egui/src/show.rs @@ -1070,7 +1070,7 @@ fn show_media(app: &mut SlideApp, show: &mut Show, doc_uid: u64, doc: &Presentat if show.media_slide == cur { return; } - show.media_slide = cur; + let previous = std::mem::replace(&mut show.media_slide, cur); let trans = cur.and_then(|i| doc.slides.get(i)).and_then(|s| s.transition.as_ref()); let sound = trans.and_then(|t| t.sound); let sound_loop = trans.is_some_and(|t| t.sound_loop); @@ -1081,7 +1081,7 @@ fn show_media(app: &mut SlideApp, show: &mut Show, doc_uid: u64, doc: &Presentat .media .ids() .into_iter() - .filter(|id| crate::media::find_clip(doc, *id).is_some_and(|c| c.play_across_slides)) + .filter(|id| crate::media::find_clip(doc, *id, previous).is_some_and(|c| c.play_across_slides)) // A transition sound plays to its end, as in PowerPoint, unless the new slide has its own. .chain((sound.is_none() && !stop_previous).then_some(TRANSITION_SOUND).filter(|id| app.media.is_playing(*id))) .collect(), @@ -1110,7 +1110,7 @@ fn show_media(app: &mut SlideApp, show: &mut Show, doc_uid: u64, doc: &Presentat /// the clip is already playing, Pause toggles, Stop stops. fn run_media_cues(app: &mut SlideApp, show: &mut Show, doc_uid: u64, doc: &Presentation) { for (id, effect) in std::mem::take(&mut show.media_cues) { - let Some(clip) = crate::media::find_clip(doc, id) else { continue }; + let Some(clip) = crate::media::find_clip(doc, id, Some(show.state.slide)) else { continue }; match effect.as_str() { "pause" => app.media.toggle(doc_uid, doc, id, &clip, crate::media::Owner::Show), "stop" => app.media.stop(id), From 2303efe7d5e696703eade575e6c36e61606a7377 Mon Sep 17 00:00:00 2001 From: Faabyy <68284158+Faabyy@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:05:19 +0000 Subject: [PATCH 2/4] Media: stop a clip across slides when the next slide reuses its id (#994) A clip set to play across slides kept its voice even when the next slide holds a different clip under the same shape id, so that slide's own clip never started. Keep the voice only when the next slide has no clip under that id or the same media. Co-Authored-By: Claude --- crates/ui-egui/src/media.rs | 52 +++++++++++++++++++++++++++++++------ crates/ui-egui/src/show.rs | 4 +-- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/media.rs b/crates/ui-egui/src/media.rs index 344689b2..e26b475c 100644 --- a/crates/ui-egui/src/media.rs +++ b/crates/ui-egui/src/media.rs @@ -337,14 +337,27 @@ impl MediaHost { /// The clip of media shape `id`. Ids are only unique per slide (the PPTX import numbers each slide /// afresh), so the `near` slide is searched first, then the rest of the deck. pub fn find_clip(doc: &Presentation, id: ShapeId, near: Option) -> Option { - fn walk(shapes: &[deckcraft_model::Shape], id: ShapeId) -> Option { - shapes.iter().find_map(|s| match &s.kind { - ShapeKind::Media(m) if s.id == id => Some(m.clone()), - ShapeKind::Group { children, .. } => walk(children, id), - _ => None, - }) - } - near.and_then(|i| doc.slides.get(i)).and_then(|s| walk(&s.shapes, id)).or_else(|| doc.slides.iter().find_map(|s| walk(&s.shapes, id))) + near.and_then(|i| clip_on(doc, i, id)).or_else(|| doc.slides.iter().find_map(|s| walk_clip(&s.shapes, id))) +} + +/// The clip `id` on slide `index` only (inside groups too). +pub fn clip_on(doc: &Presentation, index: usize, id: ShapeId) -> Option { + doc.slides.get(index).and_then(|s| walk_clip(&s.shapes, id)) +} + +fn walk_clip(shapes: &[deckcraft_model::Shape], id: ShapeId) -> Option { + shapes.iter().find_map(|s| match &s.kind { + ShapeKind::Media(m) if s.id == id => Some(m.clone()), + ShapeKind::Group { children, .. } => walk_clip(children, id), + _ => None, + }) +} + +/// Whether the voice `id`, playing on slide `from`, keeps playing on slide `to`: its clip plays +/// across slides and `to` doesn't hold another clip under the same id. +pub fn carries_over(doc: &Presentation, id: ShapeId, from: Option, to: usize) -> bool { + let Some(clip) = find_clip(doc, id, from).filter(|c| c.play_across_slides) else { return false }; + clip_on(doc, to, id).is_none_or(|c| c.media == clip.media) } /// Media shapes directly on slide `index`: (id, box in slide points, clip). @@ -633,4 +646,27 @@ mod tests { assert_eq!(find_clip(&doc, id, None).map(|c| c.media), Some(first.media), "without a hint: first match"); assert_eq!(find_clip(&doc, id, Some(9)).map(|c| c.media), Some(first.media), "a slide that doesn't exist"); } + + #[test] + fn a_clip_across_slides_stops_where_the_next_slide_reuses_its_id() { + let (mut doc, id, first) = deck(4000, wav_level(0.05, 0.5)); + if let ShapeKind::Media(m) = &mut Arc::make_mut(&mut doc.slides[0]).shapes.last_mut().expect("shape").kind { + m.play_across_slides = true; + } + let mut plain = (*doc.slides[0]).clone(); + plain.shapes.clear(); + doc.slides.push(Arc::new(plain)); + assert!(carries_over(&doc, id, Some(0), 1), "a slide without that id"); + let mut same = (*doc.slides[0]).clone(); + if let ShapeKind::Media(m) = &mut same.shapes.last_mut().expect("shape").kind { + m.play_across_slides = false; + } + doc.slides.push(Arc::new(same)); + assert!(carries_over(&doc, id, Some(0), 2), "the same media under that id"); + let other = doc.add_media("two.wav", "audio/wav", wav_level(0.05, 0.25)).expect("media"); + let mut reused = (*doc.slides[0]).clone(); + reused.shapes.last_mut().expect("shape").kind = ShapeKind::Media(MediaClip { media: other, volume: 1.0, autoplay: true, ..first.clone() }); + doc.slides.push(Arc::new(reused)); + assert!(!carries_over(&doc, id, Some(0), 3), "another clip under that id"); + } } diff --git a/crates/ui-egui/src/show.rs b/crates/ui-egui/src/show.rs index 599e366a..b959d33f 100644 --- a/crates/ui-egui/src/show.rs +++ b/crates/ui-egui/src/show.rs @@ -1077,11 +1077,11 @@ fn show_media(app: &mut SlideApp, show: &mut Show, doc_uid: u64, doc: &Presentat // [Stop Previous Sound] ends a transition sound still playing. let stop_previous = trans.is_some_and(|t| t.stop_sound); let keep: Vec = match cur { - Some(_) => app + Some(i) => app .media .ids() .into_iter() - .filter(|id| crate::media::find_clip(doc, *id, previous).is_some_and(|c| c.play_across_slides)) + .filter(|id| crate::media::carries_over(doc, *id, previous, i)) // A transition sound plays to its end, as in PowerPoint, unless the new slide has its own. .chain((sound.is_none() && !stop_previous).then_some(TRANSITION_SOUND).filter(|id| app.media.is_playing(*id))) .collect(), From 96697f761256602aa04ce07140821405abc75a29 Mon Sep 17 00:00:00 2001 From: Faabyy <68284158+Faabyy@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:20:29 +0000 Subject: [PATCH 3/4] Media: the playing clip decides whether it carries over (#994) The carry-over check looked the clip up again on the previous slide. If that slide held the same media under the same id without Play Across Slides, the voice stopped one slide later. A slide without the id fell back to the first match in the deck. Use the clip the voice was started with; the next slide is only checked for another clip under that id. Co-Authored-By: Claude --- crates/ui-egui/src/media.rs | 35 +++++++++++++++++++---------------- crates/ui-egui/src/show.rs | 4 ++-- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/crates/ui-egui/src/media.rs b/crates/ui-egui/src/media.rs index e26b475c..edf9413a 100644 --- a/crates/ui-egui/src/media.rs +++ b/crates/ui-egui/src/media.rs @@ -176,6 +176,11 @@ impl MediaHost { self.owners.get(&id).map(|o| o.owner) } + /// The clip voice `id` was started with. + pub fn clip(&self, id: ShapeId) -> Option<&MediaClip> { + self.owners.get(&id).map(|o| &o.clip) + } + /// Shapes with a voice (playing, paused or ended). pub fn ids(&self) -> Vec { self.owners.keys().copied().collect() @@ -353,11 +358,10 @@ fn walk_clip(shapes: &[deckcraft_model::Shape], id: ShapeId) -> Option, to: usize) -> bool { - let Some(clip) = find_clip(doc, id, from).filter(|c| c.play_across_slides) else { return false }; - clip_on(doc, to, id).is_none_or(|c| c.media == clip.media) +/// Whether the voice `id`, playing `clip` (the clip it was started with), keeps playing on slide +/// `to`: the clip plays across slides and `to` doesn't hold another clip under the same id. +pub fn carries_over(clip: &MediaClip, doc: &Presentation, id: ShapeId, to: usize) -> bool { + clip.play_across_slides && clip_on(doc, to, id).is_none_or(|c| c.media == clip.media) } /// Media shapes directly on slide `index`: (id, box in slide points, clip). @@ -650,23 +654,22 @@ mod tests { #[test] fn a_clip_across_slides_stops_where_the_next_slide_reuses_its_id() { let (mut doc, id, first) = deck(4000, wav_level(0.05, 0.5)); - if let ShapeKind::Media(m) = &mut Arc::make_mut(&mut doc.slides[0]).shapes.last_mut().expect("shape").kind { - m.play_across_slides = true; - } + let playing = MediaClip { play_across_slides: true, ..first.clone() }; let mut plain = (*doc.slides[0]).clone(); plain.shapes.clear(); doc.slides.push(Arc::new(plain)); - assert!(carries_over(&doc, id, Some(0), 1), "a slide without that id"); - let mut same = (*doc.slides[0]).clone(); - if let ShapeKind::Media(m) = &mut same.shapes.last_mut().expect("shape").kind { - m.play_across_slides = false; - } - doc.slides.push(Arc::new(same)); - assert!(carries_over(&doc, id, Some(0), 2), "the same media under that id"); + assert!(carries_over(&playing, &doc, id, 1), "a slide without that id"); + // Slide 0 holds the same media under that id, without Play Across Slides: the voice that + // came from elsewhere goes on (its own clip decides), here and on the next slide. + assert!(carries_over(&playing, &doc, id, 0), "the same media under that id"); + assert!(!carries_over(&first, &doc, id, 1), "a clip that doesn't play across slides"); let other = doc.add_media("two.wav", "audio/wav", wav_level(0.05, 0.25)).expect("media"); let mut reused = (*doc.slides[0]).clone(); reused.shapes.last_mut().expect("shape").kind = ShapeKind::Media(MediaClip { media: other, volume: 1.0, autoplay: true, ..first.clone() }); doc.slides.push(Arc::new(reused)); - assert!(!carries_over(&doc, id, Some(0), 3), "another clip under that id"); + assert!(!carries_over(&playing, &doc, id, 2), "another clip under that id"); + let mut host = MediaHost::new(None); + host.play(1, &doc, id, &playing, None, Owner::Show); + assert_eq!(host.clip(id), Some(&playing), "the voice keeps the clip it was started with"); } } diff --git a/crates/ui-egui/src/show.rs b/crates/ui-egui/src/show.rs index b959d33f..a297530a 100644 --- a/crates/ui-egui/src/show.rs +++ b/crates/ui-egui/src/show.rs @@ -1070,7 +1070,7 @@ fn show_media(app: &mut SlideApp, show: &mut Show, doc_uid: u64, doc: &Presentat if show.media_slide == cur { return; } - let previous = std::mem::replace(&mut show.media_slide, cur); + show.media_slide = cur; let trans = cur.and_then(|i| doc.slides.get(i)).and_then(|s| s.transition.as_ref()); let sound = trans.and_then(|t| t.sound); let sound_loop = trans.is_some_and(|t| t.sound_loop); @@ -1081,7 +1081,7 @@ fn show_media(app: &mut SlideApp, show: &mut Show, doc_uid: u64, doc: &Presentat .media .ids() .into_iter() - .filter(|id| crate::media::carries_over(doc, *id, previous, i)) + .filter(|id| app.media.clip(*id).is_some_and(|c| crate::media::carries_over(c, doc, *id, i))) // A transition sound plays to its end, as in PowerPoint, unless the new slide has its own. .chain((sound.is_none() && !stop_previous).then_some(TRANSITION_SOUND).filter(|id| app.media.is_playing(*id))) .collect(), From e03debfd4b79a83c11528964d0ff59aa3c5e03ba Mon Sep 17 00:00:00 2001 From: Faabyy <68284158+Faabyy@users.noreply.github.com> Date: Sun, 18 Oct 2026 20:35:06 +0000 Subject: [PATCH 4/4] Media: engine requests resolve on the editor's slide (#994) The media.* commands look the shape id up on the editor's current slide, so the request has to be carried out against that slide too. Passing the show's slide instead could play another slide's clip under a repeated id. Co-Authored-By: Claude --- crates/ui-egui/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1de11554..946d80df 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1347,8 +1347,8 @@ impl SlideApp { } UiRequest::Media { action, shape, ms } => { let owner = if self.show.is_some() { media::Owner::Show } else { media::Owner::Editor }; - let shown = self.show.as_ref().map(|s| s.state.slide); - if let Some((uid, doc, slide)) = self.session.active().map(|d| (d.uid, d.doc.clone(), shown.unwrap_or(d.selection.slide))) { + // The command resolved `shape` on the editor's slide, also while a show runs. + if let Some((uid, doc, slide)) = self.session.active().map(|d| (d.uid, d.doc.clone(), d.selection.slide)) { self.media.request(uid, &doc, slide, &action, shape, ms, owner); } }