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 <noreply@anthropic.com>
This commit is contained in:
Faabyy
2026-10-18 18:57:29 +00:00
co-authored by Claude
parent 010941ec60
commit 1ab070bbf4
4 changed files with 28 additions and 10 deletions
+3 -2
View File
@@ -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);
}
}
}
+21 -5
View File
@@ -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<u64>, 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<u64>, 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<MediaClip> {
/// 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<usize>) -> Option<MediaClip> {
fn walk(shapes: &[deckcraft_model::Shape], id: ShapeId) -> Option<MediaClip> {
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<MediaClip> {
_ => 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");
}
}
+3 -3
View File
@@ -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),