Merge pull request #882 from marcoarnulfo/fix/show-anim-steps

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),

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);

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));
@@ -2274,4 +2274,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);
+    }
 }