graphic design tool
git clone https://git.lucas.co/cce-designer.git
feat: parameter edits are undoable
The params pane's write-back, the row menu and set_param record into
the parameter history Reset Parameters began. A step holds the
parameters that changed, by name, so it restores those and nothing else
on the node; a drag is one step, ended by a press, a release, Enter,
Tab, Escape or a second's idle.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CLAUDE.md | 53 +++++++++++++++++++---------
src/app.rs | 99 +++++++++++++++++++++++++++++++++++++++++++++-------
src/main.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/param_history.rs | 83 ++++++++++++++++++++++++++++++++++++-------
src/window.rs | 3 ++
5 files changed, 292 insertions(+), 43 deletions(-)
diff --git a/CLAUDE.md b/CLAUDE.md
index f08eddd..a2284dc 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -130,23 +130,42 @@ template's defaults with their numbers scaled by half again, a stand-in
that stored nothing and read nothing of the node's own values.
`the_cameras_and_the_parameter_reset_are_commands` is the test.
-**Reset Parameters is undoable** (`src/param_history.rs`, the same day),
-which makes it the first thing outside a text box or a viewer state that
-is. `State::param_history` holds one kind of step: a node's whole
-parameter list as it stood before a command rewrote it, by node ID, so a
-rename in between does not lose it and a deleted node drops its step with
-a line on the status bar. A snapshot of ONE node, not of the tree:
-restoring the tree would take back every edit made anywhere since, none of
-which are recorded. Undo and Redo consult it LAST — a code row, then a
-viewer state, then this (`Application::undo` for the chord,
-`Action::Undo` for the palette) — so the order across the three is by
-owner and not by time. New Project and Open clear it; the sync channel's
-reload does not, the nodes being the same ones. It is not cce-ui's
-`History` because that has no way to look at a step before taking it, and
-what is filed for redo is the current state of the node the step names.
-Any other command that rewrites a node's parameters at once records the
-same way: a `ParamSnapshot` to `param_history.record` before the write.
-`reset_parameters_is_undone_and_redone` is the test.
+**Parameter edits are undoable** (`src/param_history.rs`, the same day),
+the first thing outside a text box or a viewer state that is.
+`State::param_history` holds one kind of step: the parameters of one node
+that CHANGED, as they stood before. Four writers record: the params pane's
+write-back (`sync_parameters_to_project`), the row menu
+(`run_param_action`, whose work is `run_param_action_unrecorded`), MCP's
+`set_param`, and Reset Parameters; the middle two through
+`State::record_param_edit`. What writes a parameter without passing one
+of them is not recorded: a viewer state's handles (which have their own
+history while the state lasts), a camera node under an orbit, View 1:1
+and the image commands, the template merge.
+
+- **By node ID and parameter NAME.** A rename in between does not lose a
+ step, and a deleted node drops its step with a line on the status bar.
+ A step restores the parameters it names and nothing else — not the
+ node's whole list, which would take back a camera's orbit or a curve's
+ handles along with a slider, and not the tree.
+- **One step per gesture.** The pane writes back on every motion of a
+ drag, so records of one group (the node and the parameters changed) are
+ one step until the group is broken: by any press or release, by Enter,
+ Tab or Escape (all at the top of `handle_event`), or by
+ `GROUP_IDLE` (a second) with nothing recorded, which is what ends a run
+ of wheel notches. A write-back that changes nothing records nothing, so
+ a button and the Open dropdown, which end as they began, are no edit.
+- **Undo and Redo consult it LAST** — a code row, then a viewer state,
+ then this (`Application::undo` for the chord, `Action::Undo` for the
+ palette) — so the order across the three is by owner and not by time.
+ A focused text box is ahead of all of them, in the toolkit's runner.
+- **New Project and Open clear it**; the sync channel's reload does not,
+ the nodes being the same ones.
+- It is not cce-ui's `History` because that has no way to look at a step
+ before taking it, and what is filed for redo is the current state of the
+ node the step names.
+
+`a_parameter_edit_is_undone_a_gesture_at_a_time` and
+`reset_parameters_is_undone_and_redone` are the tests.
(The former bespoke HTTP API on port 3000 was retired in favor of this;
app-internal threads like the cce-files choosers now return results via
diff --git a/src/app.rs b/src/app.rs
index bca10b2..9b661db 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -3125,18 +3125,26 @@ impl State {
return false;
}
let node = &mut self.param_editor_dir_mut().children[slot];
- let before = crate::param_history::ParamSnapshot {
- node_id: node.id.clone(),
- params: node.params.clone(),
- what: "Reset Parameters",
- };
+ let was = node.params.clone();
for (pname, text, is_expr) in defaults {
if let Some(p) = node.params.iter_mut().find(|p| p.name == pname) {
p.set_text(text);
p.set_expr(is_expr);
}
}
- self.param_history.record(before);
+ let before = crate::param_history::ParamSnapshot {
+ node_id: node.id.clone(),
+ params: was
+ .into_iter()
+ .zip(node.params.iter())
+ .filter(|(a, b)| !crate::param_history::same(a, b))
+ .map(|(a, _)| a)
+ .collect(),
+ what: "Reset Parameters".to_string(),
+ };
+ if !before.params.is_empty() {
+ self.param_history.record(before);
+ }
self.sync_nodes();
self.rebuild_scene_geometry();
self.sync_parameters_pane();
@@ -3155,20 +3163,43 @@ impl State {
self.update_status_text(&format!("{verb} {}: the node is gone", step.what));
return false;
};
- let replaced = crate::param_history::ParamSnapshot {
- node_id: step.node_id.clone(),
- params: std::mem::replace(&mut node.params, step.params),
- what: step.what,
- };
+ // By name: the step holds the parameters that changed, and the rest
+ // of the node is as whatever wrote it last left it.
+ let mut replaced = Vec::new();
+ for was in step.params {
+ if let Some(p) = node.params.iter_mut().find(|p| p.name == was.name) {
+ replaced.push(std::mem::replace(p, was));
+ }
+ }
let name = node.name.clone();
- self.param_history.file(undo, replaced);
+ let what = step.what.clone();
+ self.param_history.file(
+ undo,
+ crate::param_history::ParamSnapshot { node_id: step.node_id, params: replaced, what: step.what },
+ );
+ self.sync_grid_settings();
self.sync_nodes();
self.rebuild_scene_geometry();
self.sync_parameters_pane();
- self.update_status_text(&format!("{verb} {}: {name}", step.what));
+ self.update_status_text(&format!("{verb} {what}: {name}"));
true
}
+ /// Record that `pname` of a node was `before` until just now, if it is
+ /// not still. For the writers that change one parameter in one go: the
+ /// row menu and `set_param`.
+ pub fn record_param_edit(&mut self, node_id: &str, before: ParamDef) {
+ let now = crate::viewer_state::find_node_by_id(&self.fs_root, node_id)
+ .and_then(|n| n.params.iter().find(|p| p.name == before.name));
+ if now.is_some_and(|p| !crate::param_history::same(p, &before)) {
+ self.param_history.record(crate::param_history::ParamSnapshot {
+ node_id: node_id.to_string(),
+ what: before.name.clone(),
+ params: vec![before],
+ });
+ }
+ }
+
pub fn cursor_in_viewport(&self) -> bool {
if self.network_overlay() {
// The complement of the overlay: everything in the body the
@@ -3713,6 +3744,8 @@ impl State {
// text again and the status line says why.
let mut rejected: Vec<(String, String, String)> = Vec::new();
let mut pane_actions = Vec::new();
+ // What each changed parameter was, for undo.
+ let mut was: Vec<ParamDef> = Vec::new();
for (u_name, u_val, _) in &updated_params {
// The params pane reports its display key (label when
// set, else name), so resolve back to the param by that
@@ -3735,6 +3768,7 @@ impl State {
continue;
}
}
+ was.push(p.clone());
p.set_text(u_val.clone());
param_changed = true;
if as_expr {
@@ -3774,6 +3808,21 @@ impl State {
}
}
+ // One step per gesture: a drag writes back on every
+ // motion. A button or the Open dropdown ends as it
+ // began, and is no edit.
+ was.retain(|w| {
+ child.params.iter().find(|p| p.name == w.name).is_some_and(|p| !crate::param_history::same(p, w))
+ });
+ let edit = (!was.is_empty()).then(|| crate::param_history::ParamSnapshot {
+ node_id: child.id.clone(),
+ what: was.iter().map(|p| p.name.as_str()).collect::<Vec<_>>().join(", "),
+ params: was,
+ });
+ if let Some(edit) = edit {
+ self.param_history.record_grouped(edit);
+ }
+
if !triggered_buttons.is_empty() || !display_resets.is_empty() || !rejected.is_empty() {
let mut disp_params = self.param().node_params();
for btn_name in triggered_buttons.iter().chain(display_resets.iter()) {
@@ -5089,6 +5138,16 @@ impl State {
/// One row-menu action on one parameter, by node id and name — the
/// entry the menu, a test and any future command share.
pub fn run_param_action(&mut self, node_id: &str, pname: &str, action: ParamMenuAction) {
+ let before = crate::viewer_state::find_node_by_id(&self.fs_root, node_id)
+ .and_then(|n| n.params.iter().find(|p| p.name == pname))
+ .cloned();
+ self.run_param_action_unrecorded(node_id, pname, action);
+ if let Some(before) = before {
+ self.record_param_edit(node_id, before);
+ }
+ }
+
+ fn run_param_action_unrecorded(&mut self, node_id: &str, pname: &str, action: ParamMenuAction) {
let node_label = crate::geometry::node_path_names(&self.fs_root, node_id)
.map(|n| format!("/{}", n.join("/")))
.unwrap_or_else(|| node_id.to_string());
@@ -8845,6 +8904,20 @@ pub(crate) fn geometry_to_spreadsheet_data(geom: &Detail) -> (Vec<String>, Vec<V
}
pub fn handle_event(&mut self, event: &WindowEvent) -> bool {
+ // A press, a release, or a key that ends an entry ends the gesture
+ // an undo step is: what the params pane writes next is a new one.
+ match event {
+ WindowEvent::MouseInput { .. } => self.param_history.break_group(),
+ WindowEvent::KeyboardInput { event } if event.state == ElementState::Pressed => {
+ if matches!(
+ event.logical_key,
+ Key::Named(NamedKey::Enter | NamedKey::Tab | NamedKey::Escape)
+ ) {
+ self.param_history.break_group();
+ }
+ }
+ _ => {}
+ }
match event {
WindowEvent::MouseWheel { delta } => {
// The dialog is modal: a wheel over it scrolls it, and a wheel
diff --git a/src/main.rs b/src/main.rs
index 5d3d114..d31b1bf 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -6774,6 +6774,103 @@ mod tests {
assert!(!state.param_history_step(true), "New Project kept the old project's undo");
}
+ /// An edit to a parameter can be taken back however it was made: a row
+ /// of the pane, `set_param`, the row menu. A drag writes back on every
+ /// motion and is one step; a press between two drags makes them two.
+ #[test]
+ fn a_parameter_edit_is_undone_a_gesture_at_a_time() {
+ use crate::app::{McpAction, ParamMenuAction};
+ let mut state = State::new(false);
+ let mut redraw = false;
+ let slot = state
+ .current_dir()
+ .children
+ .iter()
+ .position(|c| c.node_type == "sphere")
+ .expect("the bundled project has a sphere");
+ state.apply_action(McpAction::Select { slot }, &mut redraw).unwrap();
+ let node_id = state.current_dir().children[slot].id.clone();
+ let param = |state: &State, name: &str| -> (String, bool) {
+ let p = state.current_dir().children[slot].params.iter().find(|p| p.name == name).unwrap();
+ (p.text().to_string(), p.is_expr())
+ };
+ // The pane reporting a row at a value, as a drag does per motion.
+ let pane = |state: &mut State, name: &str, value: &str| {
+ let rows: Vec<(String, String, String)> = state
+ .param()
+ .node_params()
+ .into_iter()
+ .map(|(n, v, t)| if n == name { (n, value.to_string(), t) } else { (n, v, t) })
+ .collect();
+ state.param_mut().set_display_params(&rows);
+ state.sync_parameters_to_project();
+ };
+ let radius = param(&state, "Radius");
+ let rows = param(&state, "Rows");
+
+ // One drag: three motions, one step.
+ for v in ["1.10", "1.20", "1.30"] {
+ pane(&mut state, "Radius", v);
+ }
+ assert_eq!(state.param_history.undo_len(), 1, "a drag is one step");
+ // A write-back that changes nothing records nothing.
+ state.sync_parameters_to_project();
+ assert_eq!(state.param_history.undo_len(), 1);
+ // A release and a press, then a second drag of the same row.
+ state.param_history.break_group();
+ for v in ["1.40", "1.50"] {
+ pane(&mut state, "Radius", v);
+ }
+ assert_eq!(state.param_history.undo_len(), 2, "a second drag is a second step");
+ // Another row, with no press between: its own step all the same.
+ pane(&mut state, "Rows", "9");
+ assert_eq!(state.param_history.undo_len(), 3);
+
+ assert!(state.run_command("undo"));
+ assert_eq!(param(&state, "Rows"), rows);
+ assert_eq!(param(&state, "Radius").0, "1.50", "undoing Rows left Radius alone");
+ assert!(state.run_command("undo"));
+ assert_eq!(param(&state, "Radius").0, "1.30");
+ assert!(state.run_command("undo"));
+ assert_eq!(param(&state, "Radius"), radius);
+ assert!(!state.param_history_step(true), "three steps were recorded");
+ let shown = state.param().node_params().into_iter().find(|r| r.0 == "Radius").unwrap().1;
+ assert_eq!(shown, radius.0, "the pane shows the restored value");
+ for want in ["1.30", "1.50"] {
+ assert!(state.run_command("redo"));
+ assert_eq!(param(&state, "Radius").0, want);
+ }
+ // An edit after an undo forks: what was undone is not redone over it.
+ assert!(state.run_command("undo"));
+ state.param_history.break_group();
+ pane(&mut state, "Radius", "2.00");
+ assert!(!state.param_history_step(false), "a new edit left the redo branch standing");
+ assert!(state.run_command("undo"));
+ assert_eq!(param(&state, "Radius").0, "1.30");
+
+ // A step restores what it changed and nothing else on the node.
+ pane(&mut state, "Radius", "2.50");
+ state.current_dir_mut().children[slot].params.iter_mut().find(|p| p.name == "Rows").unwrap().set_text("21".to_string());
+ assert!(state.run_command("undo"));
+ assert_eq!(param(&state, "Radius").0, "1.30");
+ assert_eq!(param(&state, "Rows").0, "21", "undoing Radius took back an edit to Rows");
+
+ // set_param, and one that is refused.
+ let before = state.param_history.undo_len();
+ state.apply_action(McpAction::SetParam { slot, name: "Radius".into(), value: "3.00".into() }, &mut redraw).unwrap();
+ assert!(state.apply_action(McpAction::SetParam { slot, name: "Radius".into(), value: "abc".into() }, &mut redraw).is_err());
+ assert_eq!(state.param_history.undo_len(), before + 1, "a refused value recorded a step");
+ assert!(state.run_command("undo"));
+ assert_eq!(param(&state, "Radius").0, "1.30");
+
+ // The row menu: the expression flag is part of what comes back.
+ state.run_param_action(&node_id, "Radius", ParamMenuAction::EditExpression);
+ assert!(param(&state, "Radius").1);
+ state.run_param_action(&node_id, "Radius", ParamMenuAction::CopyParameter);
+ assert!(state.run_command("undo"));
+ assert_eq!(param(&state, "Radius"), ("1.30".to_string(), false), "Copy Parameter is no edit, and Edit Expression is one");
+ }
+
/// New Project from the palette starts a project. The command named a
/// label no arm dispatched, so the row ran and nothing happened.
#[test]
diff --git a/src/param_history.rs b/src/param_history.rs
index 438a6b1..9713d8f 100644
--- a/src/param_history.rs
+++ b/src/param_history.rs
@@ -1,42 +1,93 @@
-//! Undo for edits to a node's parameters that are made all at once.
+//! Undo for edits to a node's parameters.
//!
//! The app has no project-wide history: a text box undoes its own typing
//! and a viewer state its own handles. This is the third, and it holds one
-//! kind of step — a node's whole parameter list as it stood before a command
-//! rewrote it. Reset Parameters is the first to record here.
+//! kind of step — parameters of one node as they stood before something
+//! changed them: a row of the params pane, the row menu, `set_param`, or a
+//! command that rewrites the lot (Reset Parameters).
//!
-//! A snapshot is of ONE node, by id. Restoring the whole tree would be less
-//! to write and would take back every edit made anywhere since, none of
-//! which are recorded; restoring one node's parameters takes back what the
-//! command did to that node and leaves the rest of the project alone.
+//! A step holds the parameters that CHANGED, by name, of ONE node, by id.
+//! Restoring the whole tree would be less to write and would take back
+//! every edit made anywhere since; restoring a node's whole list would take
+//! back what is written to it without passing here — a camera node's
+//! Rotation under an orbit, a curve's Points under its handles. A step puts
+//! back what it replaced and nothing else.
+//!
+//! **One step per gesture.** The pane writes back on every motion of a
+//! drag, so records that share a group — the node and the parameters
+//! changed — are one step until the group is broken: by a press or a
+//! release, by Enter, Tab or Escape, or by [`GROUP_IDLE`] without a record,
+//! which is what ends a run of wheel notches.
//!
//! It is not `cce_ui::history::History` because a step has to be looked at
//! before it is taken: the snapshot filed for redo is the CURRENT state of
//! the node the step names, and which node that is is in the step.
use crate::param::ParamDef;
+use std::time::{Duration, Instant};
-/// A node's parameters as they stood, and what changed them.
+/// Parameters of a node as they stood, and what changed them.
#[derive(Clone)]
pub struct ParamSnapshot {
pub node_id: String,
pub params: Vec<ParamDef>,
- /// What the step was, for the status line: "Reset Parameters".
- pub what: &'static str,
+ /// What the step was, for the status line: "Reset Parameters",
+ /// "Radius".
+ pub what: String,
+}
+
+impl ParamSnapshot {
+ /// The group an edit of these parameters belongs to.
+ pub fn group(&self) -> String {
+ let names: Vec<&str> = self.params.iter().map(|p| p.name.as_str()).collect();
+ format!("{}\u{0}{}", self.node_id, names.join("\u{0}"))
+ }
}
-pub const LIMIT: usize = 64;
+/// Whether two states of a parameter are one: what a step would restore.
+pub fn same(a: &ParamDef, b: &ParamDef) -> bool {
+ a.text() == b.text() && a.is_expr() == b.is_expr() && a.view == b.view
+}
+
+pub const LIMIT: usize = 256;
+/// How long a group stands with nothing recorded into it.
+pub const GROUP_IDLE: Duration = Duration::from_millis(1000);
#[derive(Default)]
pub struct ParamHistory {
undo: Vec<ParamSnapshot>,
redo: Vec<ParamSnapshot>,
+ /// The group of the last record and when it was made.
+ group: Option<(String, Instant)>,
}
impl ParamHistory {
/// File `before` as what the next undo returns to. A new edit forks:
/// what had been undone cannot be redone over it.
pub fn record(&mut self, before: ParamSnapshot) {
+ self.group = None;
+ self.push(before);
+ }
+
+ /// [`Self::record`], unless the last record was of the same group and
+ /// the group still stands: the earlier snapshot already holds what this
+ /// gesture began from.
+ pub fn record_grouped(&mut self, before: ParamSnapshot) {
+ let group = before.group();
+ let now = Instant::now();
+ let standing = matches!(&self.group, Some((g, at)) if *g == group && now.duration_since(*at) < GROUP_IDLE);
+ if !(standing && !self.undo.is_empty()) {
+ self.push(before);
+ }
+ self.group = Some((group, now));
+ }
+
+ /// The gesture is over: the next grouped record is a step of its own.
+ pub fn break_group(&mut self) {
+ self.group = None;
+ }
+
+ fn push(&mut self, before: ParamSnapshot) {
self.redo.clear();
self.undo.push(before);
if self.undo.len() > LIMIT {
@@ -44,9 +95,10 @@ impl ParamHistory {
}
}
- /// Take a step off one stack. The caller restores it and files the
- /// node's state from before the restore with [`Self::file`].
+ /// Take a step off one stack. The caller restores it and files what
+ /// the restore replaced with [`Self::file`].
pub fn take(&mut self, undo: bool) -> Option<ParamSnapshot> {
+ self.group = None;
if undo { self.undo.pop() } else { self.redo.pop() }
}
@@ -63,9 +115,14 @@ impl ParamHistory {
!self.redo.is_empty()
}
+ pub fn undo_len(&self) -> usize {
+ self.undo.len()
+ }
+
/// Another document: its nodes are not these.
pub fn clear(&mut self) {
self.undo.clear();
self.redo.clear();
+ self.group = None;
}
}
diff --git a/src/window.rs b/src/window.rs
index 87a195c..1807c3e 100644
--- a/src/window.rs
+++ b/src/window.rs
@@ -347,7 +347,9 @@ impl State {
McpAction::SetParam { slot, name, value } => {
let dir = state.current_dir_mut();
if let Some(child) = dir.children.get_mut(slot) {
+ let node_id = child.id.clone();
if let Some(p) = child.params.iter_mut().find(|p| p.name == name) {
+ let before = p.clone();
// A value that reads as a reference becomes an
// expression, as one typed into the pane does; an
// expression is checked when it evaluates. Anything
@@ -362,6 +364,7 @@ impl State {
if as_expr {
p.set_expr(true);
}
+ state.record_param_edit(&node_id, before);
// Same sequence as the interactive param-pane
// path, so settings params (viewport flags,
// grid) actually take effect via automation.