History: jump to a state in one step (history.selectState) (#2966) (#3533)

Clicking a History panel state ran Edit > Undo / Redo once per state. Each
step bumped the document revision, so the canvas could not tell what the
jump changed and recomposited the whole document (re-uploading every
changed layer on the GPU path).

history.selectState {state} returns the document to a state (numbered as
the panel lists them, redo states included) in one command: the history
moves by pointer swaps, the document becomes current once, with one
revision and the damage rect between start and target, and the targeted
layers restored as Undo restores them. The panel runs it once per click;
an open Free Transform refuses it (panel guard kept, and the transform
intercept greys and refuses it for every caller). A floating piece goes
back as with Undo instead of being dropped as a new step.

Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>

crates/engine/src/commands.rs

@@ -1310,6 +1310,7 @@ fn build() -> Vec<CommandSpec> {
     v.extend(crate::analysis_cmds::specs());
     v.extend(crate::notes_cmds::specs());
     v.extend(crate::history_cmds::specs());
+    v.extend(crate::history_select_cmds::specs());
     v.extend(crate::proof_sim::specs());
     v.extend(crate::presets::specs());
     v.extend(crate::document_preset_cmds::specs());

crates/engine/src/float_cmds.rs

@@ -8,7 +8,8 @@
 //! - `select.drop` drops the piece into its layer and moves the selection with it, as one history
 //!   step.
 //! - Any other command drops the piece first (so it lands exactly where it was shown), except Undo,
-//!   which puts it back where it was cut (nothing was changed yet).
+//!   which puts it back where it was cut (nothing was changed yet), and `history.selectState`,
+//!   which puts it back and then returns to the chosen state.
 //!
 //! While floating, the document is unchanged; [`displayed`] gives the document with the piece at
 //! its offset, for the canvas. The selection stays where it was until the drop.
@@ -221,6 +222,13 @@ pub(crate) fn before_command(s: &mut Session, id: &str) -> Result<Option<Value>>
             st.floating = None;
             Ok(Some(json!({"floating": "returned"})))
         }
+        // Returning to a History state puts the piece back too (it never reached the document),
+        // and then moves the history: dropping it first would add a step and discard the redo
+        // states the jump may be heading for.
+        "history.selectState" => {
+            st.floating = None;
+            Ok(None)
+        }
         _ if keeps_floating(id) => Ok(None),
         _ => drop_floating(s).map(|_| None),
     }

crates/engine/src/history_select_cmds.rs

@@ -0,0 +1,258 @@
+//! History panel › click a state: `history.selectState` returns the document to that state in one
+//! step (#2966), as Photoshop does. The panel used to run Edit › Undo / Redo once per state: each
+//! bumped the document revision, so the canvas could no longer tell what the jump changed and
+//! recomposited (and re-uploaded) the whole document, plus N rounds of per-command bookkeeping.
+//!
+//! States are numbered as the History panel lists them, like `history.deleteState`: 0 is the
+//! oldest state still held, `past` (the number of undo states) is the current one, and the redo
+//! states (dimmed in the panel) follow it. Jumping back leaves the skipped states redoable;
+//! jumping forward redoes them. Nothing is discarded until the next edit.
+
+use serde_json::{Value, json};
+
+use crate::commands::{CommandSpec, int};
+use crate::{EngineError, Result, Session};
+
+const CMD: &str = "history.selectState";
+
+fn bad(msg: impl Into<String>) -> EngineError {
+    EngineError::BadParams { cmd: CMD.into(), msg: msg.into() }
+}
+
+fn select_state(s: &mut Session, p: &Value) -> Result<Value> {
+    let st = s.active().ok_or(EngineError::NoDocument)?;
+    let past = st.history.past_len();
+    let last = past.saturating_add(st.history.redo_labels().count());
+    let v = match p.get("state") {
+        None | Some(Value::Null) => return Err(bad("missing `state` (the History entry to return to, 0 = oldest)")),
+        Some(_) => int(p, "state").ok_or_else(|| bad("`state` must be a number"))?,
+    };
+    let target = usize::try_from(v).ok().filter(|i| *i <= last).ok_or_else(|| bad(format!("`state` = {v} is not a state (0..={last})")))?;
+    // A background job is computing from the current state: it must not move under it.
+    if s.active_job().is_some() {
+        return Err(EngineError::Other("the history can't move while a command is still running on this document".into()));
+    }
+    let st = s.active_mut().ok_or(EngineError::NoDocument)?;
+    if target == past {
+        return Ok(json!({ "state": target, "changed": false }));
+    }
+    st.coalesce = None;
+    // Step the history itself (pointer moves, no pixels touched), then make the reached state
+    // current once: one revision, one damage rect for the canvas.
+    let start = st.doc.clone();
+    let mut doc = start.clone();
+    let mut layers = None;
+    let mut at = past;
+    while at != target {
+        let step = if target < at { st.history.undo(doc.clone()) } else { st.history.redo(doc.clone()) };
+        let Some((d, l)) = step else { break };
+        doc = d;
+        layers = Some(l);
+        at = if target < at { at - 1 } else { at + 1 };
+    }
+    let Some(layers) = layers else {
+        return Err(EngineError::Other("the history can't step right now".into()));
+    };
+    // Save As changes file identity outside history, as Edit › Undo keeps it.
+    if doc.name != start.name {
+        std::sync::Arc::make_mut(&mut doc).name.clone_from(&start.name);
+    }
+    let damage = crate::layer_multi_cmds::step_damage(&start, &doc);
+    st.doc = doc;
+    crate::restore_target(st, layers);
+    st.revision += 1;
+    st.last_damage = damage;
+    if at != target {
+        // Not reached (the bounds were checked): the document is consistent at the state reached.
+        return Err(EngineError::Other(format!("the history stopped at state {at}")));
+    }
+    Ok(json!({ "state": target, "changed": true, "current": st.history.entries().last() }))
+}
+
+fn can_select(s: &Session) -> std::result::Result<(), String> {
+    let st = s.active().ok_or_else(|| "no document open".to_string())?;
+    if st.history.can_undo() || st.history.can_redo() { Ok(()) } else { Err("the history holds only one state".into()) }
+}
+
+pub fn specs() -> Vec<CommandSpec> {
+    vec![CommandSpec {
+        id: CMD,
+        label: "Select History State",
+        menu: &[],
+        shortcut: None,
+        params: r##"{"state":index} (History entry as the panel lists them: 0 = oldest, then the current state, then the redo states) → {state,changed,current?}: returns the document to that state in one step; the states after it stay redoable"##,
+        enabled: can_select,
+        run: select_state,
+        journal: true,
+    }]
+}
+
+#[cfg(test)]
+mod tests {
+    use std::sync::Arc;
+
+    use super::*;
+
+    /// A document with four steps after Open, each targeting different layers.
+    fn session() -> Session {
+        let mut s = Session::new();
+        s.execute("file.new", json!({"width": 64, "height": 32})).unwrap();
+        s.execute("layer.new.layer", json!({})).unwrap();
+        s.execute("edit.fill", json!({"color": "#ff0000"})).unwrap();
+        s.execute("layer.new.layer", json!({})).unwrap();
+        s.execute("edit.fill", json!({"color": "#00ff00"})).unwrap();
+        s
+    }
+
+    type Look = (Arc<photocraft_doc::Document>, Option<photocraft_doc::LayerId>, Vec<photocraft_doc::LayerId>, Vec<String>, Vec<String>);
+
+    /// What the panel can see of a session: the document, the targeted layers, the entries and
+    /// the redo states.
+    fn look(s: &Session) -> Look {
+        let st = s.active().unwrap();
+        (st.doc.clone(), st.active_layer, st.selected_layers.clone(), st.history.entries(), st.history.redo_labels().map(str::to_string).collect())
+    }
+
+    /// Step from the current state to `target` with Edit › Undo / Redo, note what that gives, and
+    /// step back.
+    fn stepped(s: &mut Session, target: usize) -> Look {
+        let current = s.active().unwrap().history.past_len();
+        let (there, back) = if target < current { ("edit.undo", "edit.redo") } else { ("edit.redo", "edit.undo") };
+        let n = target.abs_diff(current);
+        for _ in 0..n {
+            s.execute(there, json!({})).unwrap();
+        }
+        let seen = look(s);
+        for _ in 0..n {
+            s.execute(back, json!({})).unwrap();
+        }
+        seen
+    }
+
+    fn same(a: &Look, b: &Look) {
+        assert!(Arc::ptr_eq(&a.0, &b.0), "the same document");
+        assert_eq!((&a.1, &a.2, &a.3, &a.4), (&b.1, &b.2, &b.3, &b.4));
+    }
+
+    #[test]
+    fn jumping_back_and_forward_matches_stepping() {
+        for target in 0..=4usize {
+            for from_back in [0usize, 2, 4] {
+                // Start `from_back` states before the end, then jump to `target`.
+                let mut s = session();
+                for _ in 0..from_back {
+                    s.execute("edit.undo", json!({})).unwrap();
+                }
+                let current = 4 - from_back;
+                let expected = stepped(&mut s, target);
+                let rev = s.active().unwrap().revision;
+                let r = s.execute(CMD, json!({"state": target})).unwrap();
+                assert_eq!(r["state"], json!(target));
+                assert_eq!(r["changed"], json!(target != current));
+                same(&look(&s), &expected);
+                assert_eq!(s.active().unwrap().history.past_len(), target);
+                let bumps = s.active().unwrap().revision - rev;
+                assert_eq!(bumps, u64::from(target != current), "{from_back} → {target}: one revision for the jump");
+            }
+        }
+    }
+
+    #[test]
+    fn a_jump_reports_only_the_pixels_it_changed() {
+        let mut s = Session::new();
+        s.execute("file.new", json!({"width": 400, "height": 300})).unwrap();
+        s.execute("layer.new.layer", json!({})).unwrap();
+        s.execute("select.rect", json!({"x": 10, "y": 10, "width": 20, "height": 20})).unwrap();
+        s.execute("edit.fill", json!({"color": "#ff0000"})).unwrap();
+        s.execute("select.rect", json!({"x": 40, "y": 30, "width": 10, "height": 10})).unwrap();
+        s.execute("edit.fill", json!({"color": "#0000ff"})).unwrap();
+        let past = s.active().unwrap().history.past_len();
+        s.execute(CMD, json!({"state": past - 4})).unwrap();
+        let damage = s.active().unwrap().last_damage.expect("a bounded damage rect, not a full recomposite");
+        assert!(damage.width() < 400 && damage.height() < 300, "{damage:?}");
+        // And forward again.
+        s.execute(CMD, json!({"state": past})).unwrap();
+        assert!(s.active().unwrap().last_damage.is_some());
+    }
+
+    #[test]
+    fn redo_states_survive_a_jump_and_go_with_the_next_edit() {
+        let mut s = session();
+        let all = s.active().unwrap().history.entries();
+        s.execute(CMD, json!({"state": 1})).unwrap();
+        let (_, _, _, entries, redo) = look(&s);
+        assert_eq!(entries, all[..2].to_vec());
+        assert_eq!(redo, all[2..].to_vec(), "the later states are dimmed, next one first");
+        assert!(s.is_enabled(CMD));
+        s.execute("edit.fill", json!({"color": "#0000ff"})).unwrap();
+        assert!(!s.active().unwrap().history.can_redo(), "a new step discards them");
+    }
+
+    #[test]
+    fn select_state_restores_the_layers_the_state_targeted() {
+        let mut s = session();
+        let bottom = s.active().unwrap().doc.layers.first().unwrap().id;
+        // Target the bottom layer, then jump back to the first fill: its layer comes back, as
+        // with Undo.
+        s.select_layer(bottom).unwrap();
+        let expected = stepped(&mut s, 2);
+        s.select_layer(bottom).unwrap();
+        s.execute(CMD, json!({"state": 2})).unwrap();
+        same(&look(&s), &expected);
+        assert_ne!(s.active().unwrap().active_layer, Some(bottom));
+    }
+
+    #[test]
+    fn a_floating_piece_goes_back_and_the_jump_still_happens() {
+        let mut s = session();
+        s.execute("select.rect", json!({"x": 2, "y": 2, "width": 8, "height": 8})).unwrap();
+        s.execute("select.float", json!({"dx": 5, "dy": 0})).unwrap();
+        let all = s.active().unwrap().history.entries();
+        let doc = s.active().unwrap().doc.clone();
+        assert!(s.active().unwrap().floating.is_some());
+        s.execute(CMD, json!({"state": 1})).unwrap();
+        let st = s.active().unwrap();
+        assert!(st.floating.is_none(), "the piece went back, it wasn't dropped");
+        assert_eq!(st.history.past_len(), 1);
+        let redo: Vec<String> = st.history.redo_labels().map(str::to_string).collect();
+        assert_eq!(redo, all[2..].to_vec(), "no Drop step was added");
+        // Forward to the state the piece was floated on: the document as it was, without the piece.
+        s.execute(CMD, json!({"state": all.len() - 1})).unwrap();
+        assert!(Arc::ptr_eq(&s.active().unwrap().doc, &doc));
+    }
+
+    #[test]
+    fn select_state_refuses_bad_params_and_changes_nothing() {
+        let mut s = Session::new();
+        assert!(s.execute(CMD, json!({"state": 0})).is_err(), "no document");
+        assert!(!s.is_enabled(CMD));
+        s.execute("file.new", json!({"width": 8, "height": 8})).unwrap();
+        assert!(!s.is_enabled(CMD), "only the opening state");
+        let mut s = session();
+        s.execute("edit.undo", json!({})).unwrap();
+        let before = look(&s);
+        let rev = s.active().unwrap().revision;
+        for p in [
+            json!({}),
+            json!({"state": null}),
+            json!({"state": -1}),
+            json!({"state": 5}),
+            json!({"state": 99}),
+            json!({"state": i64::MAX}),
+            json!({"state": f64::MAX}),
+            json!({"state": "x"}),
+            json!({"state": "1"}),
+            json!({"state": [1]}),
+            json!({"state": {"i": 1}}),
+            json!({"state": true}),
+            json!([1]),
+        ] {
+            assert!(s.execute(CMD, p.clone()).is_err(), "{p}");
+            let now = look(&s);
+            assert!(Arc::ptr_eq(&now.0, &before.0) && now.3 == before.3 && now.4 == before.4, "{p} changed nothing");
+        }
+        assert_eq!(s.active().unwrap().revision, rev);
+        // The last redo state is reachable.
+        assert!(s.execute(CMD, json!({"state": 4})).is_ok());
+    }
+}

crates/engine/src/lib.rs

@@ -54,6 +54,7 @@ pub mod group_view_cmds;
 pub mod guide_layout;
 pub mod hidden_target;
 pub mod history_cmds;
+pub mod history_select_cmds;
 pub mod image_cmds;
 pub mod inspect;
 pub mod installed_profiles;

crates/ui-egui/src/history_panel_select_tests.rs

@@ -0,0 +1,60 @@
+//! #2966: a click on a History panel state returns the document there with one command
+//! (`history.selectState`), not one Undo / Redo per state, so the canvas refreshes once and only
+//! where the jump changed pixels.
+
+use egui::vec2;
+use egui_kittest::Harness;
+use egui_kittest::kittest::Queryable;
+use serde_json::json;
+
+use super::*;
+
+/// Open, then New Layer, Fill, New Layer, Fill.
+fn harness() -> Harness<'static, PhotocraftApp> {
+    let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
+    app.run("file.new", json!({"width": 64, "height": 48})).unwrap();
+    for (cmd, p) in
+        [("layer.new.layer", json!({})), ("edit.fill", json!({"color": "#ff0000"})), ("layer.new.layer", json!({})), ("edit.fill", json!({"color": "#00ff00"}))]
+    {
+        app.run(cmd, p).unwrap();
+    }
+    let mut ready = false;
+    let mut h = Harness::builder().with_size(vec2(300.0, 400.0)).build_ui_state(
+        move |ui, app: &mut PhotocraftApp| {
+            if !ready {
+                PhotocraftApp::setup_context(ui.ctx(), theme::ThemeKind::ProMedium);
+                ready = true;
+                return;
+            }
+            history(app, ui);
+        },
+        app,
+    );
+    h.run_steps(3);
+    h
+}
+
+fn state(h: &Harness<'_, PhotocraftApp>) -> (usize, usize, u64) {
+    let st = h.state().session.active().unwrap();
+    (st.history.past_len(), st.history.redo_labels().count(), st.revision)
+}
+
+#[test]
+fn clicking_a_state_jumps_there_with_one_command() {
+    let mut h = harness();
+    let (past, redo, rev) = state(&h);
+    assert_eq!((past, redo), (4, 0));
+    // Back three states: one command, one revision (the canvas sees one change).
+    h.get_all_by_label("New Layer").next().expect("the first New Layer row").click();
+    h.run_steps(2);
+    assert_eq!(state(&h), (1, 3, rev + 1));
+    assert_eq!(h.state().perf.last_command, "history.selectState");
+    // Forward to the last (dimmed) state, again in one step.
+    h.get_all_by_label("Fill").last().expect("the last Fill row").click();
+    h.run_steps(2);
+    assert_eq!(state(&h), (4, 0, rev + 2));
+    // The current state: nothing to do.
+    h.get_all_by_label("Fill").last().expect("the last Fill row").click();
+    h.run_steps(2);
+    assert_eq!(state(&h), (4, 0, rev + 2));
+}

crates/ui-egui/src/panels.rs

@@ -3102,7 +3102,7 @@ fn history(app: &mut PhotocraftApp, ui: &mut egui::Ui) {
         ui.painter().line_segment([rect.left_bottom(), rect.right_bottom()], Stroke::new(1.0, t.separator));
     }
     let mut snap_cmd = None;
-    let mut target: Option<isize> = None;
+    let mut target: Option<usize> = None;
     let all = entries.iter().map(|e| (e.clone(), false)).chain(redo.iter().map(|e| (e.clone(), true)));
     // The dock gives History a fixed height: the rows scroll above the footer.
     let footer = if t.pro { widgets::footer_height(ui) + ui.spacing().item_spacing.y } else { 0.0 };
@@ -3113,6 +3113,7 @@ fn history(app: &mut PhotocraftApp, ui: &mut egui::Ui) {
         snap_cmd = snapshot_rows(app, ui);
         for (i, (e, is_redo)) in all.enumerate() {
             let (rect, resp) = ui.allocate_exact_size(vec2(ui.available_width(), 24.0), Sense::click());
+            resp.widget_info(|| egui::WidgetInfo::selected(egui::WidgetType::SelectableLabel, true, i == current, &e));
             if i == current {
                 ui.painter().rect_filled(rect, if t.pro { 0.0 } else { t.radius_sm }, if t.pro { t.row_selected } else { t.accent_soft });
             } else if resp.hovered() {
@@ -3135,8 +3136,8 @@ fn history(app: &mut PhotocraftApp, ui: &mut egui::Ui) {
                 if is_redo { t.text_faint } else { t.icon },
             );
             ui.painter().text(pos2(rect.left() + 58.0, rect.center().y), Align2::LEFT_CENTER, &e, egui::FontId::proportional(12.0), color);
-            if resp.clicked() {
-                target = Some(i as isize - current as isize);
+            if resp.clicked() && i != current {
+                target = Some(i);
             }
         }
     });
@@ -3167,13 +3168,13 @@ fn history(app: &mut PhotocraftApp, ui: &mut egui::Ui) {
         app.ui.status = e;
         app.ui.status_error = true;
     }
-    if let Some(delta) = target.filter(|_| app.ui.transform.is_none()) {
-        let (cmd, n) = if delta < 0 { ("edit.undo", -delta) } else { ("edit.redo", delta) };
-        for _ in 0..n {
-            if app.run(cmd, json!({})).is_err() {
-                break;
-            }
-        }
+    // One command per click (#2966): stepping with Undo/Redo once per state made the canvas
+    // recomposite the whole document.
+    if let Some(state) = target.filter(|_| app.ui.transform.is_none())
+        && let Err(e) = app.run("history.selectState", json!({ "state": state }))
+    {
+        app.ui.status = e;
+        app.ui.status_error = true;
     }
 }
 
@@ -4632,8 +4633,9 @@ mod snapshot_row_tests {
         assert_eq!(st.history.entries().len(), steps + 1);
         assert_eq!(st.history.entries().last().map(String::as_str), Some("Snapshot 1"));
         assert_eq!(top_pixel(&h), [1.0, 0.0, 0.0, 1.0], "back to red");
-        // Right-click › Rename…: type a name, Enter.
-        h.get_by_label("Snapshot 1").click_secondary();
+        // Right-click › Rename…: type a name, Enter. The new "Snapshot 1" state is listed under the
+        // snapshot rows, so the first match is the snapshot's row.
+        h.get_all_by_label("Snapshot 1").next().unwrap().click_secondary();
         h.run_steps(2);
         h.get_by_label("Rename…").click();
         h.run_steps(2);
@@ -4659,8 +4661,8 @@ mod snapshot_row_tests {
         h.run_steps(2);
         assert_eq!(snapshot_names(&h), ["Lesson start"], "Escape cancels");
         assert_eq!(h.state().session.active().unwrap().history.entries().len(), steps_now + 1, "the click selected it once");
-        // Right-click › Delete.
-        h.get_by_label("Lesson start").click_secondary();
+        // Right-click › Delete (the snapshot's row, above the state the click made).
+        h.get_all_by_label("Lesson start").next().unwrap().click_secondary();
         h.run_steps(2);
         h.get_by_label("Delete").click();
         h.run_steps(2);
@@ -6045,3 +6047,7 @@ mod properties_target_regressions {
         assert_eq!(after, before, "pending width belongs to the old document/layer");
     }
 }
+
+#[cfg(test)]
+#[path = "history_panel_select_tests.rs"]
+mod history_panel_select_tests;

crates/ui-egui/src/transform_tool.rs

@@ -1206,8 +1206,8 @@ pub fn is_enabled(app: &PhotocraftApp, id: &str) -> Option<bool> {
         i if i.starts_with("edit.transform.") && t.content_aware.is_some() => Some(false),
         "edit.undo" => Some(steps.is_some_and(|s| s.can_undo(&Step::of(t)))),
         "edit.redo" => Some(steps.is_some_and(|s| !s.redo.is_empty())),
-        // No Toggle Last State under the box, and one box at a time.
-        "edit.toggleLastState" | "edit.contentAwareScale" | "edit.puppetWarp" => Some(false),
+        // No Toggle Last State or history jump under the box, and one box at a time.
+        "edit.toggleLastState" | "history.selectState" | "edit.contentAwareScale" | "edit.puppetWarp" => Some(false),
         // Rotate and Flip turn the box (not while warping, where the mesh is the box).
         _ if box_preset(id).is_some() => Some(t.warp.is_none()),
         // Editable type can't be distorted (#2630, as #2701 rules for the menus outside a box).
@@ -1240,9 +1240,10 @@ fn box_preset(id: &str) -> Option<(f64, f64, f64)> {
 }
 
 /// While transforming, the box owns the history for every caller (menus, shortcuts, the control
-/// channel, MCP): Undo and Redo step through it, and Toggle Last State, which would change the
-/// document under the box, is refused. Edit › Transform's Rotate and Flip turn the box itself, as
-/// in Photoshop, instead of the layer under it; committing stays with ↩ and the options bar.
+/// channel, MCP): Undo and Redo step through it, and Toggle Last State and History state jumps,
+/// which would change the document under the box, are refused. Edit › Transform's Rotate and
+/// Flip turn the box itself, as in Photoshop, instead of the layer under it; committing stays
+/// with ↩ and the options bar.
 /// `None` for other commands or when not transforming.
 /// The commands the open box answers itself ([`is_enabled`], [`intercept`]).
 fn box_command(id: &str) -> bool {
@@ -1257,7 +1258,9 @@ pub fn intercept(app: &mut PhotocraftApp, id: &str) -> Option<Result<serde_json:
     match id {
         "edit.undo" => Some(Ok(step(app, false))),
         "edit.redo" => Some(Ok(step(app, true))),
-        "edit.toggleLastState" | "edit.contentAwareScale" | "edit.puppetWarp" => Some(Err("Commit or cancel the transform first".into())),
+        "edit.toggleLastState" | "history.selectState" | "edit.contentAwareScale" | "edit.puppetWarp" => {
+            Some(Err("Commit or cancel the transform first".into()))
+        }
         _ => {
             let (kx, ky, angle) = box_preset(id)?;
             if warping {