From 25e29e2583f4ef0b6bf63f9affcde83f494e32bd Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Tue, 13 Oct 2026 08:47:18 +0200 Subject: [PATCH] =?UTF-8?q?Filters:=20=E2=8C=98Z=20steps=20the=20open=20di?= =?UTF-8?q?alog's=20settings=20instead=20of=20the=20document=20(part=20of?= =?UTF-8?q?=20#4697)=20(#4715)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- crates/ui-egui/src/dialogs.rs | 1 + crates/ui-egui/src/edit_history.rs | 241 ++++++++++++++++++++++++++++ crates/ui-egui/src/gallery_ui.rs | 12 ++ crates/ui-egui/src/lib.rs | 4 + crates/ui-egui/src/shortcuts.rs | 2 + crates/ui-egui/src/vp_ui.rs | 30 ++++ crates/ui-egui/src/wide_angle_ui.rs | 31 ++++ 7 files changed, 321 insertions(+) create mode 100644 crates/ui-egui/src/edit_history.rs diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fce28f9e40..550a2ec04a 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -129,6 +129,7 @@ fn show_modal( 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(); diff --git a/crates/ui-egui/src/edit_history.rs b/crates/ui-egui/src/edit_history.rs new file mode 100644 index 0000000000..2012f912c7 --- /dev/null +++ b/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 { + undo: Vec, + redo: Vec, + /// The settings as of the latest step. + settled: T, +} + +impl EditHistory { + 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 { + 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) -> Map { + 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>>; + +/// 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 = 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); + } +} diff --git a/crates/ui-egui/src/gallery_ui.rs b/crates/ui-egui/src/gallery_ui.rs index ccf1a7637e..83928e29b4 100644 --- a/crates/ui-egui/src/gallery_ui.rs +++ b/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>, pub preview_ms: f64, + history: EditHistory>, } 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 { 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); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ce97c240d4..ed70750eda 100644 --- a/crates/ui-egui/src/lib.rs +++ b/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, /// Filter › Adaptive Wide Angle dialog (wide_angle_ui). pub(crate) wide_angle: Option, + /// ⌘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, diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index c8528e1aa5..8119695e88 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/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. diff --git a/crates/ui-egui/src/vp_ui.rs b/crates/ui-egui/src/vp_ui.rs index bdaad601c6..bd0353d678 100644 --- a/crates/ui-egui/src/vp_ui.rs +++ b/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, drag: Option, + history: EditHistory, } +/// What ⌘Z steps through: the planes and where the paste sits on them. +type VpEdit = (Vec, [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 { 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)] diff --git a/crates/ui-egui/src/wide_angle_ui.rs b/crates/ui-egui/src/wide_angle_ui.rs index cd8bf0332c..35b996deea 100644 --- a/crates/ui-egui/src/wide_angle_ui.rs +++ b/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, } 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(¶ms), 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"); + } }