From c7423aa98654a96f788ade3f9c772bdb1ddea1f2 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 15:44:28 +0200 Subject: [PATCH] Show: Back leaves a slide whose first animation plays by itself (part of #844) --- crates/anim/src/show.rs | 18 ++++++++++------- crates/anim/src/tests.rs | 24 +++++++++++++++++++++++ crates/ui-egui/src/show.rs | 40 +++++++++++++++++++++++++++++++++++--- 3 files changed, 72 insertions(+), 10 deletions(-) diff --git a/crates/anim/src/show.rs b/crates/anim/src/show.rs index 3d9c241f..a1117c62 100644 --- a/crates/anim/src/show.rs +++ b/crates/anim/src/show.rs @@ -299,7 +299,8 @@ impl ShowState { ShowAction::FinishStep(n) } - /// ← / Backspace: undo the last step, else go to the previous slide fully built. + /// ← / Backspace: undo the last step, else go to the previous slide fully built. An automatic + /// first step (With/After Previous) is part of the slide appearing, so it is never undone. pub fn prev(&mut self, pres: &Presentation) -> ShowAction { if self.ended { self.ended = false; @@ -310,13 +311,16 @@ impl ShowState { self.playing = false; return ShowAction::Slide(self.slide); } - if self.playing { + // #844: undoing the automatic first step left the slide as before it appeared, and the show + // played the step again at once, so Back never got past the slide. + let undo = if self.playing { Some(self.step) } else { self.step.checked_sub(1) }; + let auto_first = undo == Some(0) && !pres.show.without_animation && ShowState::timeline(pres, self.slide).step_is_auto(0); + if let Some(k) = undo + && !auto_first + { self.playing = false; - return ShowAction::StepBack(self.step); - } - if self.step > 0 { - self.step -= 1; - return ShowAction::StepBack(self.step); + self.step = k; + return ShowAction::StepBack(k); } let target = match self.prev_index(pres) { Some(i) => Some(i), diff --git a/crates/anim/src/tests.rs b/crates/anim/src/tests.rs index 1ac69104..da158ae9 100644 --- a/crates/anim/src/tests.rs +++ b/crates/anim/src/tests.rs @@ -808,6 +808,30 @@ fn show_steps_through_builds() { assert_eq!(s.step, 1); } +#[test] +fn show_back_leaves_a_slide_whose_first_step_is_automatic() { + // #844: undoing the automatic first step left the slide as before it appeared, and the show + // played that step again at once, so Back never reached the slide before. + for start in [AnimStart::WithPrevious, AnimStart::AfterPrevious] { + let mut p = pres(2); + Arc::make_mut(&mut p.slides[1]).shapes = vec![shape(1, "A"), shape(2, "B")]; + Arc::make_mut(&mut p.slides[1]).animations = + vec![anim(1, AnimClass::Entrance, "fade", start), anim(2, AnimClass::Entrance, "fade", AnimStart::OnClick)]; + let mut s = ShowState::new(&p); + assert_eq!(s.next(&p), ShowAction::Slide(1)); + assert!(s.playing, "{start:?}"); + // While the automatic step plays. + assert_eq!(s.prev(&p), ShowAction::Slide(0), "{start:?}"); + assert_eq!(s.next(&p), ShowAction::Slide(1)); + // Once it has played, the click step is undone first, then Back leaves the slide. + assert_eq!(s.next(&p), ShowAction::PlayStep(1)); + assert_eq!(s.step_done(&p), ShowAction::FinishStep(1)); + assert_eq!(s.prev(&p), ShowAction::StepBack(1), "{start:?}"); + assert_eq!(s.prev(&p), ShowAction::Slide(0), "{start:?}"); + assert_eq!((s.step, s.playing), (0, false)); + } +} + #[test] fn show_auto_advance() { let mut p = pres(2); diff --git a/crates/ui-egui/src/show.rs b/crates/ui-egui/src/show.rs index a875773c..02e9d4cc 100644 --- a/crates/ui-egui/src/show.rs +++ b/crates/ui-egui/src/show.rs @@ -650,12 +650,12 @@ pub fn ui(app: &mut SlideApp, ui: &mut Ui) { show.step_end = now; } } else if show.trans.is_none() { - // Auto-advance (After: timings) or autoplay preview. + // Auto-advance (After: timings) or autoplay preview. An automatic first step is started + // by `ShowState` as the slide appears; starting it here too replayed it after Back (#844). let idle = now - show.step_end.max(show.slide_start); let cur_step = show.state.step; - let next_auto = show.timeline(&doc).step_is_auto(cur_step); let n_steps = show.timeline(&doc).steps(); - let step_due = cur_step < n_steps && ((show.autoplay && idle > 0.3) || next_auto); + let step_due = show.autoplay && cur_step < n_steps && idle > 0.3; let holding = show.grid || show.zoom.is_some() || show.zoom_arm; if !holding && (step_due || (!show.rehearse && show.state.auto_advance_due(&doc, idle) && !show.ended)) { actions.push(show.state.next(&doc)); @@ -2246,4 +2246,38 @@ mod tests { let doc = s.active().unwrap().doc.clone(); assert_eq!(next_rect(right, &doc).width(), 400.0); } + + #[test] + fn back_leaves_a_slide_whose_first_animation_is_with_previous() { + // #844: Back on such a slide undid its first step, and the next frame played that step + // again by itself, so Back never reached the slide before. + let mut session = deckcraft_engine::Session::with_new(); + session.execute("slide.new", &json!({})).unwrap(); + let id = session.active().unwrap().doc.slides[1].shapes[0].id; + session + .execute("animation.add", &json!({"effect": "fade", "class": "entrance", "ids": [id.0], "start": "withPrevious", "duration": 500})) + .unwrap(); + let app = SlideApp::new(session, crate::Services::default()); + let mut harness = egui_kittest::Harness::builder().with_size(vec2(1000.0, 700.0)).build_ui_state( + |ui, app: &mut SlideApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + app, + ); + harness.run_steps(3); + harness.state_mut().start_show(0, false); + harness.run_steps(2); + harness.key_press(egui::Key::ArrowRight); + // The first step plays by itself and ends (0.5 s; a frame is 0.25 s). + harness.run_steps(8); + let st = &harness.state().show.as_ref().unwrap().state; + assert_eq!((st.slide, st.step, st.playing), (1, 1, false), "on slide 2, fully built"); + for _ in 0..5 { + harness.key_press(egui::Key::ArrowLeft); + harness.run_steps(2); + } + let st = &harness.state().show.as_ref().unwrap().state; + assert_eq!(st.slide, 0, "five Backs leave slide 2 (step {}, playing {})", st.step, st.playing); + } }