Let undo reach the repairs before the tool can make one

A history state was a Preset — a map of scalars — which was right while
every edit in the graph was a parameter. A spot is not one, and undo is the
first thing anyone does with a repair: place it, dislike it, take it back.
Left as it was, that press would have stepped some unrelated slider and
left the spot on the photograph, which reads as undo being broken rather
than as undo being absent.

So a state is now the pair, params and spot set. The same door is the one
the mask stack will come through: mask edits are outside undo today for
exactly this reason, and FR-DEV-5 is not finished until they are not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-26 20:27:34 +02:00
co-authored by Claude Opus 5
parent f00b3ae924
commit 2958444835
3 changed files with 104 additions and 8 deletions
+56
View File
@@ -239,3 +239,59 @@ fn modes_survive_their_own_names() {
}
assert_eq!(SpotMode::from_name("smudge"), None);
}
// ---------------------------------------------------------------------------
// Undo (FR-DEV-5)
// ---------------------------------------------------------------------------
/// The press a photographer reaches for first: place a repair, dislike it,
/// take it back. Before the history carried the spot set this stepped some
/// unrelated slider and left the repair on the photograph, which reads as undo
/// being broken rather than absent.
#[test]
fn undo_takes_a_repair_back() {
use dr_pipeline::history::{Edit, History};
let mut graph = EditGraph::default_chain();
let mut history = History::new(&graph);
graph.spots_mut().place(spot_at((0.5, 0.5), (0.08, 0.0)));
assert!(history.record(&graph, Edit::Discrete));
assert_eq!(graph.spots().len(), 1);
assert!(history.undo(&mut graph));
assert_eq!(graph.spots().len(), 0, "the repair is still on the frame");
assert!(history.redo(&mut graph));
assert_eq!(graph.spots().len(), 1, "and redo could not put it back");
}
/// Moving a repair is undoable too, and separately from placing it: the two
/// are different decisions and a photographer who nudges a source expects one
/// press to return it, not to lose the spot entirely.
#[test]
fn undo_steps_back_through_a_moved_source() {
use dr_pipeline::history::{Edit, History};
let mut graph = EditGraph::default_chain();
let id = graph
.spots_mut()
.place(spot_at((0.5, 0.5), (0.08, 0.0)))
.unwrap();
let mut history = History::new(&graph);
graph
.spots_mut()
.get_mut(&id)
.unwrap()
.set_offset((0.2, 0.1));
history.record(&graph, Edit::Discrete);
assert!(history.undo(&mut graph));
assert_eq!(
graph.spots().get(&id).map(|s| s.offset),
Some((0.08, 0.0)),
"the source did not go back where it was"
);
assert_eq!(graph.spots().len(), 1, "and the repair itself survived");
}