Actions: Step by Step runs each step after the window shows the one before

A Step by Step playback now waits after each step until a UI pass that the window shows has drawn it. A logic-only iteration, a pass in a minimized or covered window and a discarded pass do not count, so a hidden window no longer runs the steps unseen and does not ask for frames while it waits.

crates/ui-egui/src/app.rs

@@ -498,6 +498,8 @@ impl VectorcraftApp {
             return;
         }
         crate::state::sync_zoom_stops_display_print_size(self.session.prefs.display_print_size);
+        // Whether this pass draws the last step of an action playing at Step by Step.
+        let step_held = crate::panels::actions::playback_held(self);
         let t0 = now_ms();
         widgets::set_dialog_open(&ctx, self.ui.dialog.is_some());
         // The canvas records this pass's IME caret geometry; a pass that draws no canvas has none.
@@ -562,6 +564,10 @@ impl VectorcraftApp {
         if !ctx.will_discard() {
             self.perf.frame_ms = now_ms() - t0;
             self.issue_screenshots(&ctx);
+            // A hidden window's pass shows nothing, so the step waits for a pass the user sees.
+            if step_held && !self.viewport_hidden(&ctx) {
+                crate::panels::actions::playback_drawn(self);
+            }
         }
         let _ = json!(null);
     }

crates/ui-egui/src/panels/actions.rs

@@ -255,7 +255,7 @@ pub enum PlaybackSpeed {
     /// Every included step at once.
     #[default]
     Accelerated,
-    /// One step per frame, so the window draws each step's result before the next step runs.
+    /// One step at a time, with the window drawn after each step before the next step runs.
     Step,
     /// [`PlaybackOptions::seconds`] between steps.
     Pause,
@@ -296,8 +296,10 @@ pub struct Playing {
     /// With Pause For, when the next step runs (seconds on the app's clock); none until the frame
     /// after each step, which starts the pause before the next one.
     next_at: Option<f64>,
-    /// With Step by Step, set until the first frame after the first step: that frame draws the
-    /// first step's result and runs no step, since Play in the panel can run after the canvas drew.
+    /// With Step by Step, set after each step until the window shows a frame drawn after it
+    /// ([`playback_drawn`]): no step runs while it is set, so the window shows each step's result
+    /// before the next one runs, also when Play in the panel ran after the canvas drew and while the
+    /// window is minimized or covered.
     hold: bool,
     /// The [`uid`](vectorcraft_engine::DocState::uid) of the document that was active after the
     /// last step played, if any.
@@ -309,17 +311,19 @@ fn active_doc(app: &VectorcraftApp) -> Option<u64> {
 }
 
 /// Runs the next step of the paced playback in progress when it is due at `now` (seconds on the
-/// app's clock): with Step by Step at each call after the first, so each frame runs one step and the
-/// frame after Play draws the first step; with Pause For once the pause has passed, which starts in
-/// the frame after the step before, so a slow step or a late frame does not shorten it. A step that fails ends the playback and shows its error. When a step is due and the active
-/// document is not the one that was active after the last step (another document was made active,
-/// or that one was closed), the playback ends with a status and the rest do not run. → the seconds
-/// until the window should draw again while playback goes on
+/// app's clock): with Step by Step once the window has shown a frame drawn after the step before
+/// ([`playback_drawn`]), so it shows each step before the next one runs; with Pause For once
+/// the pause has passed, which starts in the frame after the step before, so a slow step or a late
+/// frame does not shorten it. A step that fails ends the playback and shows its error. When a step
+/// is due and the active document is not the one that was active after the last step (another
+/// document was made active, or that one was closed), the playback ends with a status and the rest
+/// do not run. → the seconds until the window should draw again while playback goes on (none while
+/// Step by Step waits for a UI pass, which asks for the next frame itself)
 pub fn tick_playback(app: &mut VectorcraftApp, now: f64) -> Option<f64> {
     let doc = active_doc(app);
     let playing = app.ui.action_playing.as_mut()?;
-    if std::mem::take(&mut playing.hold) {
-        return Some(0.0);
+    if playing.hold {
+        return None;
     }
     let pause = f64::from(playing.options.seconds);
     if playing.options.speed == PlaybackSpeed::Pause {
@@ -352,9 +356,26 @@ pub fn tick_playback(app: &mut VectorcraftApp, now: f64) -> Option<f64> {
     playing.doc = doc;
     // With Pause For, the next frame starts the next pause.
     playing.next_at = None;
+    playing.hold = playing.options.speed == PlaybackSpeed::Step;
     Some(0.0)
 }
 
+/// Whether a Step by Step playback waits for a frame that shows its last step ([`playback_drawn`]).
+pub fn playback_held(app: &VectorcraftApp) -> bool {
+    app.ui.action_playing.as_ref().is_some_and(|p| p.hold)
+}
+
+/// Called after a UI pass that the window shows, when [`playback_held`] was true as the pass began:
+/// the Step by Step playback runs its next step in the frame after this one, which it asks for.
+pub fn playback_drawn(app: &mut VectorcraftApp) {
+    if let Some(playing) = app.ui.action_playing.as_mut().filter(|p| p.hold) {
+        playing.hold = false;
+        if let Some(ctx) = &app.ui_ctx {
+            ctx.request_repaint();
+        }
+    }
+}
+
 /// The command behind Playback Options… ([`playback_options`]).
 pub const PLAYBACK_OPTIONS: &str = "ui.action.playbackOptions";
 
@@ -2862,6 +2883,25 @@ mod tests {
         app.session.active().unwrap().history.undo.len()
     }
 
+    /// One frame of a shown window at `now`: the logic pass, then a UI pass that draws → what the
+    /// logic pass's [`tick_playback`] returns.
+    fn frame(app: &mut VectorcraftApp, now: f64) -> Option<f64> {
+        let wait = tick_playback(app, now);
+        playback_drawn(app);
+        wait
+    }
+
+    /// Draws frames until the playback in progress ends (at most 20).
+    fn play_out(app: &mut VectorcraftApp) {
+        for _ in 0..20 {
+            if app.ui.action_playing.is_none() {
+                return;
+            }
+            frame(app, 0.0);
+        }
+        panic!("the playback did not end");
+    }
+
     #[test]
     fn playback_options_sets_each_speed_and_refuses_bad_values() {
         let mut app = VectorcraftApp::new(Session::new(), Default::default());
@@ -2926,19 +2966,83 @@ mod tests {
         let before = undo_steps(&app);
         assert_eq!(play(&mut app, 0, 0).unwrap()["pending"], json!(2));
         assert_eq!(x(&app), 1.0, "the first step runs at once");
-        assert_eq!(tick_playback(&mut app, 0.0), Some(0.0), "the frame after Play draws the first step");
+        assert_eq!(frame(&mut app, 0.0), None, "the frame after Play draws the first step");
         assert_eq!(x(&app), 1.0, "and runs no step, even when Play ran after the canvas drew");
-        assert_eq!(tick_playback(&mut app, 0.0), Some(0.0), "the window draws again before the next step");
+        assert_eq!(frame(&mut app, 0.0), Some(0.0), "the window draws again before the next step");
         assert_eq!(x(&app), 3.0, "one more step in this frame");
-        assert_eq!(tick_playback(&mut app, 0.0), None, "the last step ends the playback");
+        assert_eq!(frame(&mut app, 0.0), None, "the last step ends the playback");
         assert_eq!(x(&app), 7.0);
         assert!(app.ui.action_playing.is_none());
-        assert_eq!(tick_playback(&mut app, 0.0), None);
+        assert_eq!(frame(&mut app, 0.0), None);
         assert_eq!(x(&app), 7.0);
         assert_eq!(undo_steps(&app), before + 3, "each step is its own undo step");
         assert_eq!(app.session.active().unwrap().history.undo.last().unwrap().label, "Three Moves");
     }
 
+    #[test]
+    fn step_by_step_runs_no_step_until_the_window_draws_the_one_before() {
+        let (mut app, x) = app_with_three_moves();
+        app.run(PLAYBACK_OPTIONS, json!({"speed": "step"})).unwrap();
+        play(&mut app, 0, 0).unwrap();
+        // A hidden window runs logic passes only, which draw nothing.
+        for _ in 0..3 {
+            assert_eq!(tick_playback(&mut app, 0.0), None, "no step runs and no frame is asked for");
+        }
+        assert_eq!(x(&app), 1.0);
+        playback_drawn(&mut app);
+        assert_eq!(tick_playback(&mut app, 0.0), Some(0.0));
+        assert_eq!(x(&app), 3.0, "the step after a drawn frame runs");
+        for _ in 0..3 {
+            assert_eq!(tick_playback(&mut app, 0.0), None);
+        }
+        assert_eq!(x(&app), 3.0, "the next step waits until the window draws this one");
+        playback_drawn(&mut app);
+        assert_eq!(tick_playback(&mut app, 0.0), None, "the last step ends the playback");
+        assert_eq!(x(&app), 7.0);
+    }
+
+    #[test]
+    fn step_by_step_waits_while_the_window_is_hidden() {
+        let (mut app, x) = app_with_three_moves();
+        app.run(PLAYBACK_OPTIONS, json!({"speed": "step"})).unwrap();
+        let ctx = egui::Context::default();
+        // One host iteration: `tick`, then a UI pass, in a shown or an occluded window.
+        let frame = |app: &mut VectorcraftApp, hidden: bool| {
+            let mut raw =
+                egui::RawInput { screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1200.0, 800.0))), ..Default::default() };
+            if hidden {
+                raw.viewports.entry(egui::ViewportId::ROOT).or_default().occluded = Some(true);
+            }
+            app.begin_host_iteration();
+            let mut out = ctx.run_ui(raw, |ui| {
+                app.tick(ui.ctx());
+                app.ui(ui);
+            });
+            out.textures_delta.clear();
+        };
+        // Fonts install on the first pass and are ready on the second.
+        frame(&mut app, false);
+        frame(&mut app, false);
+        play(&mut app, 0, 0).unwrap();
+        for _ in 0..3 {
+            frame(&mut app, true);
+        }
+        assert_eq!(x(&app), 1.0, "no step runs while the window is hidden, even in a UI pass");
+        frame(&mut app, false);
+        assert_eq!(x(&app), 1.0, "the shown frame draws the first step");
+        frame(&mut app, true);
+        assert_eq!(x(&app), 3.0, "the first step was shown, so the second runs");
+        for _ in 0..3 {
+            frame(&mut app, true);
+        }
+        assert_eq!(x(&app), 3.0, "the third step waits until the window shows the second");
+        frame(&mut app, false);
+        assert_eq!(x(&app), 3.0, "the shown frame draws the second step");
+        frame(&mut app, false);
+        assert_eq!(x(&app), 7.0);
+        assert!(app.ui.action_playing.is_none());
+    }
+
     #[test]
     fn pause_for_waits_the_seconds_between_steps() {
         let (mut app, x) = app_with_three_moves();
@@ -3004,12 +3108,12 @@ mod tests {
         app.run(PLAYBACK_OPTIONS, json!({"speed": "step"})).unwrap();
         play(&mut app, 0, 0).unwrap();
         app.run(START_RECORDING, json!({"set": 0})).unwrap();
-        while tick_playback(&mut app, 0.0).is_some() {}
+        play_out(&mut app);
         app.run(STOP_RECORDING, json!({})).unwrap();
         assert_eq!(x(&app), 21.0);
         assert_eq!(app.ui.action_sets[0].actions[2].steps.len(), 2);
         play(&mut app, 0, 2).unwrap();
-        while tick_playback(&mut app, 0.0).is_some() {}
+        play_out(&mut app);
         assert_eq!(x(&app), 27.0, "paced: the recorded batches play the second and third moves");
         app.run(PLAYBACK_OPTIONS, json!({"speed": "accelerated"})).unwrap();
         play(&mut app, 0, 2).unwrap();
@@ -3043,8 +3147,8 @@ mod tests {
         app.run(PLAYBACK_OPTIONS, json!({"speed": "step"})).unwrap();
         app.ui.status.clear();
         play(&mut app, 0, 0).unwrap();
-        assert_eq!(tick_playback(&mut app, 0.0), Some(0.0));
-        while tick_playback(&mut app, 0.0).is_some() {}
+        assert_eq!(frame(&mut app, 0.0), None);
+        play_out(&mut app);
         assert_ne!(app.ui.status, "the action stopped playing: its document is no longer active");
         assert_eq!(x(&app), 10.0, "the moves after the step ran in the document it made active");
     }
@@ -3055,11 +3159,11 @@ mod tests {
         app.run(PLAYBACK_OPTIONS, json!({"speed": "step"})).unwrap();
         app.ui.action_sets[0].actions[0].steps[1] = ("no.such.command".into(), json!({}));
         play(&mut app, 0, 0).unwrap();
-        assert_eq!(tick_playback(&mut app, 0.0), Some(0.0), "the frame after Play draws the first step");
-        assert_eq!(tick_playback(&mut app, 0.0), None);
+        assert_eq!(frame(&mut app, 0.0), None, "the frame after Play draws the first step");
+        assert_eq!(frame(&mut app, 0.0), None);
         assert!(app.ui.action_playing.is_none());
         assert!(!app.ui.status.is_empty(), "the step's error shows");
-        assert_eq!(tick_playback(&mut app, 0.0), None);
+        assert_eq!(frame(&mut app, 0.0), None);
         assert_eq!(x(&app), 1.0, "the step after the failing one does not run");
     }
 }