Filters: ⌘Z steps the open dialog's settings instead of the document (part of #4697) (#4715)

Filter Gallery, Adaptive Wide Angle, Vanishing Point and every parameter dialog (Lens Correction, Image › Adjustments, …) now keep a history of their own settings: ⌘Z / ⇧⌘Z step through it, one drag is one step, and the document's History beneath is left alone. Camera Raw is left to #4136.

Co-authored-by: Brandon Thomas <[email protected]>

crates/ui-egui/src/dialogs.rs

@@ -129,6 +129,7 @@ fn show_modal<T>(
 pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) {
     crate::layer_style::color_picker::prune(app);
     crate::type_panels_ui::color_picker::prune(app);
+    crate::edit_history::track_dialogs(app, ctx);
     let dialogs = app.ui.dialogs.clone();
     let top = dialogs.last().map(|d| d.id);
     let mut shown = Vec::new();

crates/ui-egui/src/edit_history.rs

@@ -0,0 +1,241 @@
+//! Undo inside a dialog (#4697): ⌘Z / ⇧⌘Z step through the open dialog's own settings, as in
+//! Photoshop, instead of reaching the document's History beneath it (which the dialog previews
+//! and is about to change), or doing nothing.
+//!
+//! A dialog keeps an [`EditHistory`] of its settings and calls [`EditHistory::settle`] every frame
+//! and after a control-channel edit. A change becomes one step once no edit is in progress, so a
+//! slider drag is one step rather than one per frame. Liquify, Puppet Warp and Camera Raw keep
+//! their own histories.
+
+use std::collections::HashMap;
+
+use serde_json::{Map, Value};
+
+use crate::PhotocraftApp;
+use crate::state::DialogKind;
+
+/// Most steps a dialog keeps.
+const LIMIT: usize = 100;
+
+pub struct EditHistory<T> {
+    undo: Vec<T>,
+    redo: Vec<T>,
+    /// The settings as of the latest step.
+    settled: T,
+}
+
+impl<T: Clone + PartialEq> EditHistory<T> {
+    pub fn new(now: &T) -> Self {
+        Self { undo: Vec::new(), redo: Vec::new(), settled: now.clone() }
+    }
+
+    /// Records a change since the latest step as a new step, unless `busy` (a drag in progress).
+    pub fn settle(&mut self, now: &T, busy: bool) {
+        if busy || *now == self.settled {
+            return;
+        }
+        if self.undo.len() >= LIMIT {
+            self.undo.remove(0);
+        }
+        self.undo.push(std::mem::replace(&mut self.settled, now.clone()));
+        self.redo.clear();
+    }
+
+    /// Steps `now` back (`Undo`) or forward (`Redo`); an unfinished edit is the latest step.
+    /// False when there is nowhere to go.
+    pub fn step(&mut self, now: &mut T, step: Step) -> bool {
+        self.settle(now, false);
+        let (from, to) = match step {
+            Step::Undo => (&mut self.undo, &mut self.redo),
+            Step::Redo => (&mut self.redo, &mut self.undo),
+        };
+        let Some(next) = from.pop() else { return false };
+        to.push(std::mem::replace(now, next.clone()));
+        self.settled = next;
+        true
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum Step {
+    Undo,
+    Redo,
+}
+
+/// ⌘Z or ⇧⌘Z pressed this frame, consumed.
+fn take_step(ctx: &egui::Context) -> Option<Step> {
+    let cmd_shift = egui::Modifiers::COMMAND | egui::Modifiers::SHIFT;
+    if ctx.input_mut(|i| i.consume_key(cmd_shift, egui::Key::Z)) {
+        Some(Step::Redo)
+    } else if ctx.input_mut(|i| i.consume_key(egui::Modifiers::COMMAND, egui::Key::Z)) {
+        Some(Step::Undo)
+    } else {
+        None
+    }
+}
+
+/// A parameter dialog's settings: its fields without the private `__` keys (preview toggle,
+/// command, label…).
+fn settings(fields: &Map<String, Value>) -> Map<String, Value> {
+    fields.iter().filter(|(k, _)| !k.starts_with("__")).map(|(k, v)| (k.clone(), v.clone())).collect()
+}
+
+/// The parameter dialogs' histories, by dialog id.
+pub type DialogEdits = HashMap<u64, EditHistory<Map<String, Value>>>;
+
+/// Once a frame: records each open parameter dialog's settings and forgets closed dialogs.
+pub fn track_dialogs(app: &mut PhotocraftApp, ctx: &egui::Context) {
+    let busy = ctx.input(|i| i.pointer.any_down());
+    let dialogs = &app.ui.dialogs;
+    app.dialog_edits.retain(|id, _| dialogs.iter().any(|d| d.id == *id));
+    for d in dialogs.iter().filter(|d| d.kind == DialogKind::Command) {
+        let now = settings(&d.fields);
+        app.dialog_edits.entry(d.id).or_insert_with(|| EditHistory::new(&now)).settle(&now, busy);
+    }
+}
+
+/// ⌘Z / ⇧⌘Z while a dialog with its own history is in front: steps that dialog, and the keys
+/// never reach the document's History beneath it. A focused text field keeps them for its typing.
+pub fn keys(app: &mut PhotocraftApp, ctx: &egui::Context) {
+    if ctx.text_edit_focused() {
+        return;
+    }
+    if let Some(top) = app.ui.dialogs.last() {
+        if top.kind != DialogKind::Command {
+            return;
+        }
+        let id = top.id;
+        let mut now = settings(&top.fields);
+        if let Some(step) = take_step(ctx)
+            && app.dialog_edits.entry(id).or_insert_with(|| EditHistory::new(&now)).step(&mut now, step)
+            && let Some(d) = app.ui.dialog_mut(id)
+        {
+            d.fields.retain(|k, _| k.starts_with("__"));
+            d.fields.extend(now);
+        }
+    } else if let Some(d) = app.distort.gallery.as_mut() {
+        if let Some(step) = take_step(ctx) {
+            d.step(step);
+        }
+    } else if let Some(d) = app.wide_angle.as_mut() {
+        if let Some(step) = take_step(ctx) {
+            d.step(step);
+        }
+    } else if let Some(s) = app.distort.vanishing.as_mut()
+        && let Some(step) = take_step(ctx)
+    {
+        s.step(step);
+    }
+}
+
+#[cfg(test)]
+pub(crate) mod tests {
+    use serde_json::json;
+
+    use super::*;
+
+    /// One frame of the shell's key handling with ⌘Z (plus `modifiers`) pressed.
+    pub(crate) fn press_z(app: &mut PhotocraftApp, ctx: &egui::Context, modifiers: egui::Modifiers) {
+        let raw = egui::RawInput {
+            events: vec![egui::Event::Key { key: egui::Key::Z, physical_key: None, pressed: true, repeat: false, modifiers }],
+            ..Default::default()
+        };
+        let mut out = ctx.run_ui(raw, |ui| crate::shortcuts::handle(app, ui.ctx()));
+        out.textures_delta.clear();
+    }
+
+    fn app() -> PhotocraftApp {
+        let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
+        app.run("file.new", json!({"width": 120, "height": 80})).unwrap();
+        app.run("layer.new.layer", json!({})).unwrap();
+        app.run("edit.fill", json!({"color": "#808080"})).unwrap();
+        app
+    }
+
+    fn past(app: &PhotocraftApp) -> usize {
+        app.session.active().unwrap().history.past_len()
+    }
+
+    #[test]
+    fn a_drag_is_one_step_and_a_new_edit_drops_redo() {
+        let mut h = EditHistory::new(&0);
+        for v in 1..=5 {
+            h.settle(&v, true);
+        }
+        h.settle(&5, false);
+        h.settle(&7, false);
+        let mut now = 7;
+        assert!(h.step(&mut now, Step::Undo));
+        assert_eq!(now, 5);
+        assert!(h.step(&mut now, Step::Undo));
+        assert_eq!(now, 0, "the whole drag was one step");
+        assert!(!h.step(&mut now, Step::Undo));
+        assert!(h.step(&mut now, Step::Redo));
+        assert_eq!(now, 5);
+        now = 9;
+        assert!(!h.step(&mut now, Step::Redo), "a new edit drops what was undone");
+        assert!(h.step(&mut now, Step::Undo));
+        assert_eq!(now, 5, "an unsettled edit is undone first");
+    }
+
+    #[test]
+    fn command_z_steps_a_parameter_dialog_and_leaves_its_private_keys() {
+        let ctx = egui::Context::default();
+        let mut app = app();
+        let steps = past(&app);
+        let id = crate::dialogs::open_command_dialog(&mut app, "filter.lensCorrection", "Lens Correction");
+        track_dialogs(&mut app, &ctx);
+        let field = |app: &PhotocraftApp, k: &str| app.ui.dialogs.iter().find(|d| d.id == id).unwrap().fields.get(k).cloned();
+        let vertical = field(&app, "vertical");
+        let private: Vec<String> = app.ui.dialog_mut(id).unwrap().fields.keys().filter(|k| k.starts_with("__")).cloned().collect();
+        assert!(!private.is_empty());
+        app.ui.dialog_mut(id).unwrap().fields.insert("vertical".into(), json!(-50.0));
+        track_dialogs(&mut app, &ctx);
+        app.ui.dialog_mut(id).unwrap().fields.insert("horizontal".into(), json!(30.0));
+        track_dialogs(&mut app, &ctx);
+        press_z(&mut app, &ctx, egui::Modifiers::COMMAND);
+        assert_ne!(field(&app, "horizontal"), Some(json!(30.0)));
+        assert_eq!(field(&app, "vertical"), Some(json!(-50.0)), "one step at a time");
+        press_z(&mut app, &ctx, egui::Modifiers::COMMAND);
+        assert_eq!(field(&app, "vertical"), vertical);
+        assert_eq!(past(&app), steps, "the document beneath is untouched");
+        for k in &private {
+            assert!(field(&app, k).is_some(), "{k} survives");
+        }
+        press_z(&mut app, &ctx, egui::Modifiers::COMMAND | egui::Modifiers::SHIFT);
+        assert_eq!(field(&app, "vertical"), Some(json!(-50.0)));
+    }
+
+    #[test]
+    fn command_z_steps_the_filter_gallery_stack() {
+        let ctx = egui::Context::default();
+        let mut app = app();
+        let steps = past(&app);
+        crate::distort_ui::menu(&mut app, &ctx, "filter.filterGallery", &json!({})).unwrap().unwrap();
+        let before = app.distort.gallery.as_ref().unwrap().effects.clone();
+        crate::distort_ui::menu(&mut app, &ctx, "filter.filterGallery", &json!({"ui": {"add": true}})).unwrap().unwrap();
+        assert_eq!(app.distort.gallery.as_ref().unwrap().effects.len(), before.len() + 1);
+        press_z(&mut app, &ctx, egui::Modifiers::COMMAND);
+        let d = app.distort.gallery.as_ref().unwrap();
+        assert_eq!(d.effects, before);
+        assert!(d.selected < d.effects.len());
+        assert_eq!(past(&app), steps);
+    }
+
+    #[test]
+    fn command_z_steps_vanishing_point_planes() {
+        let ctx = egui::Context::default();
+        let mut app = app();
+        let steps = past(&app);
+        crate::distort_ui::menu(&mut app, &ctx, "filter.vanishingPoint", &json!({})).unwrap().unwrap();
+        let plane = json!([{"corners": [[30, 20], [90, 20], [110, 70], [10, 70]]}]);
+        crate::distort_ui::menu(&mut app, &ctx, "filter.vanishingPoint", &json!({"ui": {"planes": plane}})).unwrap().unwrap();
+        press_z(&mut app, &ctx, egui::Modifiers::COMMAND);
+        let s = app.distort.vanishing.as_ref().unwrap();
+        assert!(s.planes.is_empty());
+        assert_eq!(s.selected, None, "no selection left on a plane that is gone");
+        assert_eq!(past(&app), steps);
+        press_z(&mut app, &ctx, egui::Modifiers::COMMAND | egui::Modifiers::SHIFT);
+        assert_eq!(app.distort.vanishing.as_ref().unwrap().planes.len(), 1);
+    }
+}

crates/ui-egui/src/gallery_ui.rs

@@ -21,6 +21,7 @@ use photocraft_raster::{Interrupt, Surface};
 use serde_json::{Map, Value, json};
 
 use crate::PhotocraftApp;
+use crate::edit_history::{EditHistory, Step};
 use crate::filter_dialog::{Kind, parse_spec};
 use crate::theme::Tokens;
 use crate::widgets;
@@ -91,6 +92,7 @@ pub struct GalleryDialog {
     thumb_src: Surface,
     thumbs: Vec<Option<TextureHandle>>,
     pub preview_ms: f64,
+    history: EditHistory<Vec<EffectLayer>>,
 }
 
 impl GalleryDialog {
@@ -145,6 +147,12 @@ impl GalleryDialog {
         }
     }
 
+    pub(crate) fn step(&mut self, step: Step) {
+        if self.history.step(&mut self.effects, step) {
+            self.selected = self.selected.min(self.effects.len().saturating_sub(1));
+        }
+    }
+
     fn move_to(&mut self, from: usize, to: usize) {
         if from < self.effects.len() && to < self.effects.len() && from != to {
             let e = self.effects.remove(from);
@@ -295,6 +303,7 @@ pub fn open(app: &mut PhotocraftApp) -> Result<(), String> {
     let thumb_src = sample(&src, Rect::new(cx - tw / 2, cy - th / 2, cx - tw / 2 + tw, cy - th / 2 + th), k);
     let sel = effects.len() - 1;
     let open_cats = std::array::from_fn(|i| effects[sel].filter.category() == GALLERY_CATEGORIES[i]);
+    let history = EditHistory::new(&effects);
     app.distort.gallery = Some(GalleryDialog {
         layer,
         layer_name,
@@ -316,6 +325,7 @@ pub fn open(app: &mut PhotocraftApp) -> Result<(), String> {
         thumb_src,
         thumbs: vec![None; GalleryFilter::ALL.len()],
         preview_ms: 0.0,
+        history,
     });
     Ok(())
 }
@@ -370,6 +380,7 @@ pub fn control(app: &mut PhotocraftApp, ui: &Value) -> Result<Value, String> {
     if let Some(Value::Object(m)) = ui.get("params") {
         d.cur().params.extend(m.clone());
     }
+    d.history.settle(&d.effects, false);
     match ui.get("zoom") {
         Some(Value::String(s)) if s == "fit" => d.zoom = 0.0,
         // The gallery's own preview zoom, not the canvas (`zoom_levels` doesn't apply): its − / +
@@ -545,6 +556,7 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) {
     let background = app.background_jobs;
     egui::Area::new(egui::Id::new("gallery-dialog")).order(egui::Order::Foreground).fixed_pos(screen.min).show(ctx, |ui| {
         let Some(d) = app.distort.gallery.as_mut() else { return };
+        d.history.settle(&d.effects, ctx.input(|i| i.pointer.any_down()));
         let (full, _) = ui.allocate_exact_size(screen.size(), Sense::hover());
         let painter = ui.painter().clone();
         painter.rect_filled(full, 0.0, t.chrome);

crates/ui-egui/src/lib.rs

@@ -74,6 +74,7 @@ pub mod distort_ui;
 pub mod doc_props_ui;
 pub mod dock;
 pub mod duplicate_layer_ui;
+pub mod edit_history;
 pub mod egui_fonts;
 pub mod enable_rules;
 pub mod eraser_ui;
@@ -667,6 +668,8 @@ pub struct PhotocraftApp {
     pub(crate) raw_redevelop: Option<camera_raw_ui::Redevelop>,
     /// Filter › Adaptive Wide Angle dialog (wide_angle_ui).
     pub(crate) wide_angle: Option<wide_angle_ui::WideAngleDialog>,
+    /// ⌘Z history of each open parameter dialog (edit_history).
+    pub(crate) dialog_edits: edit_history::DialogEdits,
     /// The filter a "Rasterize?" prompt answered for, run after the dialog closes, where the
     /// egui context is available again (rasterize_prompt).
     pub(crate) pending_after_prompt: Option<(String, serde_json::Value)>,
@@ -846,6 +849,7 @@ impl PhotocraftApp {
             pending_raw_open: None,
             raw_redevelop: None,
             wide_angle: None,
+            dialog_edits: Default::default(),
             pending_after_prompt: None,
             tone_hist: None,
             doc_hist: None,

crates/ui-egui/src/shortcuts.rs

@@ -314,6 +314,8 @@ pub fn handle(app: &mut PhotocraftApp, ctx: &egui::Context) {
     if crate::menu_nav::is_open(ctx) || crate::widgets::dropdown_list_open(ctx) {
         return;
     }
+    // ⌘Z in a filter or adjustment dialog steps the dialog's settings, not the document (#4697).
+    crate::edit_history::keys(app, ctx);
     // Stroke Options owns plain navigation/activation keys, including while its child
     // dropdown is open. This runs before Popup::show registers the current pass, so use
     // the editor's explicit state; otherwise Escape deselects or commits a pending Pen path.

crates/ui-egui/src/vp_ui.rs

@@ -19,6 +19,7 @@ use serde_json::{Value, json};
 
 use crate::PhotocraftApp;
 use crate::canvas::{ToolEvent, ViewXform};
+use crate::edit_history::{EditHistory, Step};
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, serde::Serialize)]
 #[serde(rename_all = "camelCase")]
@@ -63,8 +64,12 @@ pub struct VpSession {
     pub new_layer: bool,
     clip: Option<Clip>,
     drag: Option<Drag>,
+    history: EditHistory<VpEdit>,
 }
 
+/// What ⌘Z steps through: the planes and where the paste sits on them.
+type VpEdit = (Vec<VpPlane>, [f64; 2], f64);
+
 /// Most planes a session holds (the control channel can't make it allocate without bound).
 const MAX_PLANES: usize = 64;
 /// Most grid lines drawn per direction.
@@ -152,6 +157,24 @@ impl VpSession {
         }
     }
 
+    fn edit(&self) -> VpEdit {
+        (self.planes.clone(), self.paste_at, self.paste_width)
+    }
+
+    fn settle(&mut self, busy: bool) {
+        let now = self.edit();
+        self.history.settle(&now, busy || self.drag.is_some());
+    }
+
+    pub(crate) fn step(&mut self, step: Step) {
+        let mut now = self.edit();
+        if self.history.step(&mut now, step) {
+            (self.planes, self.paste_at, self.paste_width) = now;
+            self.drag = None;
+            self.selected = self.selected.filter(|i| *i < self.planes.len());
+        }
+    }
+
     fn delete_selected(&mut self) {
         if let Some(i) = self.selected.take().filter(|i| *i < self.planes.len()) {
             self.planes.remove(i);
@@ -300,6 +323,7 @@ pub fn begin(app: &mut PhotocraftApp, ctx: &egui::Context) -> Result<(), String>
         new_layer: false,
         clip,
         drag: None,
+        history: EditHistory::new(&(Vec::new(), [0.25, 0.25], 0.5)),
     });
     app.ui.status = tl!("Vanishing Point: click four corners to create a plane").into();
     Ok(())
@@ -369,6 +393,7 @@ pub fn control(app: &mut PhotocraftApp, ui: &Value) -> Result<Value, String> {
     if let Some(b) = ui.get("newLayer").and_then(Value::as_bool) {
         s.new_layer = b;
     }
+    s.settle(false);
     Ok(s.describe())
 }
 
@@ -383,6 +408,7 @@ pub fn pointer(app: &mut PhotocraftApp, ev: ToolEvent, _mods: egui::Modifiers) {
             s.drag = None;
         }
     }
+    s.settle(false);
 }
 
 /// Keys while the mode is active (↩ and Esc are handled by `distort_ui::keys`).
@@ -400,6 +426,7 @@ pub fn keys(app: &mut PhotocraftApp, ctx: &egui::Context) {
         s.tool = VpTool::CreatePlane;
     } else if key(Key::Delete) || key(Key::Backspace) {
         s.delete_selected();
+        s.settle(false);
     }
 }
 
@@ -551,6 +578,9 @@ pub fn options_bar(app: &mut PhotocraftApp, ui: &mut egui::Ui) {
             app.distort.vanishing = None;
         }
     });
+    if let Some(s) = app.distort.vanishing.as_mut() {
+        s.settle(ui.input(|i| i.pointer.any_down()));
+    }
 }
 
 #[cfg(test)]

crates/ui-egui/src/wide_angle_ui.rs

@@ -18,6 +18,7 @@ use photocraft_raster::Surface;
 use serde_json::{Value, json};
 
 use crate::PhotocraftApp;
+use crate::edit_history::{EditHistory, Step};
 use crate::theme::Tokens;
 use crate::widgets;
 
@@ -41,6 +42,7 @@ pub struct WideAngleDialog {
     drag: Option<([f64; 2], [f64; 2])>,
     pub residual: f64,
     pub render_ms: f64,
+    history: EditHistory<WideAngle>,
 }
 
 fn tex_image(s: &Surface, w: usize, h: usize) -> egui::ColorImage {
@@ -85,6 +87,13 @@ impl WideAngleDialog {
         self.dirty = false;
     }
 
+    pub(crate) fn step(&mut self, step: Step) {
+        if self.history.step(&mut self.params, step) {
+            self.drag = None;
+            self.dirty = true;
+        }
+    }
+
     fn command_params(&self) -> Value {
         let mut v = serde_json::to_value(&self.params).unwrap_or(json!({}));
         v["layer"] = json!(self.layer.0);
@@ -122,6 +131,7 @@ pub fn open(app: &mut PhotocraftApp, ctx: &egui::Context) -> Result<(), String>
     let mut d = WideAngleDialog {
         layer,
         layer_name: name,
+        history: EditHistory::new(&params),
         params,
         frame,
         scale: 1.0 / k as f64,
@@ -188,6 +198,7 @@ pub fn menu(app: &mut PhotocraftApp, ctx: &egui::Context, id: &str, params: &Val
         d.params.constraints.remove(i as usize);
         d.dirty = true;
     }
+    d.history.settle(&d.params, false);
     if d.dirty {
         d.render(ctx);
     }
@@ -216,6 +227,7 @@ pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) {
     let shift = ctx.input(|i| i.modifiers.shift);
     egui::Area::new(egui::Id::new("awa-dialog")).order(egui::Order::Foreground).fixed_pos(screen.min).show(ctx, |ui| {
         let Some(d) = app.wide_angle.as_mut() else { return };
+        d.history.settle(&d.params, ctx.input(|i| i.pointer.any_down()));
         if d.dirty {
             d.render(ctx);
         }
@@ -445,4 +457,23 @@ mod tests {
         menu(&mut app, &ctx, "filter.adaptiveWideAngle", &json!({"ui": {"cancel": true}})).unwrap().unwrap();
         assert!(app.wide_angle.is_none());
     }
+
+    #[test]
+    fn command_z_steps_the_dialog_not_the_document() {
+        let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
+        let ctx = egui::Context::default();
+        app.run("file.new", json!({"width": 160, "height": 100})).unwrap();
+        app.run("layer.new.layer", json!({})).unwrap();
+        app.run("edit.fill", json!({"color": "#808080"})).unwrap();
+        let steps = app.session.active().unwrap().history.past_len();
+        menu(&mut app, &ctx, "filter.adaptiveWideAngle", &json!({})).unwrap().unwrap();
+        for y in [20, 60] {
+            menu(&mut app, &ctx, "filter.adaptiveWideAngle", &json!({"ui": {"add": {"a": [20, y], "b": [140, y]}}})).unwrap().unwrap();
+        }
+        crate::edit_history::tests::press_z(&mut app, &ctx, egui::Modifiers::COMMAND);
+        assert_eq!(app.wide_angle.as_ref().unwrap().params.constraints.len(), 1, "⌘Z removes the last constraint");
+        assert_eq!(app.session.active().unwrap().history.past_len(), steps, "the document beneath is untouched");
+        crate::edit_history::tests::press_z(&mut app, &ctx, egui::Modifiers::COMMAND | egui::Modifiers::SHIFT);
+        assert_eq!(app.wide_angle.as_ref().unwrap().params.constraints.len(), 2, "⇧⌘Z puts it back");
+    }
 }