Snapshot the whole edit in the history, so a drawn mask can be taken back
The undo stack snapshotted a `Preset` — the parameter map — and a mask layer is deliberately not a parameter. So drawing one changed nothing the history could see: `record` returned `false`, no step opened, and the layer the photographer had just painted had no way back. The interface went on calling `record` in good faith, including from the mask controls, and nothing failed. A film stock went missing the same way. The snapshot is an `EditState` now, so the history is complete by construction rather than by anyone keeping a list in their head. `undo` and `redo` return a `Step` rather than a `bool`. Stepping is not the only outcome a caller has to act on — a step across a change of film leaves the graph without its tables, and only the caller can bake them — and a `bool` would let that be dropped by writing nothing at all, which is the shape of mistake this module had already made once. `DevelopSession` settles the debt either way; a step that found nowhere to go is left alone, since clearing the film because undo hit the floor would take the stock off the picture. Five tests, all of which fail against the old snapshot: a drawn layer is undoable and redoable, a layer's own settings are a step of their own, a change of stock is a step and names what it needs baked back, clearing the film is undoable, and an exposure move does not deep-copy the mask stack. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+26
-3
@@ -3163,7 +3163,7 @@ impl DevelopSession {
|
||||
self.history.reset(&self.graph);
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3f
|
||||
/// TRACES: FR-DEV-3f | FR-DEV-5
|
||||
/// Pay what a restored edit owes the picture.
|
||||
///
|
||||
/// `dr-pipeline` restores a stock's *name* and clears its tables, because
|
||||
@@ -3176,6 +3176,12 @@ impl DevelopSession {
|
||||
/// on holding textures that nothing will sample. That is the two halves
|
||||
/// disagreeing, which is the failure `set_film` exists to make
|
||||
/// impossible — and it is silent in this direction, which is worse.
|
||||
///
|
||||
/// Baked unconditionally rather than only when the stock changed: the
|
||||
/// tables come from the film node's own exposure sliders as well as from
|
||||
/// the stock, and restoring an edit replaces those sliders too. A bake is
|
||||
/// milliseconds and this happens on a keypress, so the cheap correct rule
|
||||
/// beats the clever one.
|
||||
fn pay_film_debt(&mut self, rebake: &dr_pipeline::FilmRebake) {
|
||||
match rebake.wanted() {
|
||||
Some(film) => {
|
||||
@@ -3187,6 +3193,19 @@ impl DevelopSession {
|
||||
}
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-5
|
||||
/// [`Self::pay_film_debt`] for a history step, when the step went
|
||||
/// anywhere.
|
||||
///
|
||||
/// The guard is the whole difference between the two: a step that found
|
||||
/// nowhere to go left the graph alone, and clearing the film because
|
||||
/// undo hit the floor would take the picture's stock off it.
|
||||
fn settle(&mut self, step: &dr_pipeline::Step) {
|
||||
if let dr_pipeline::Step::Took(rebake) = step {
|
||||
self.pay_film_debt(rebake);
|
||||
}
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-5
|
||||
/// Step the edit back one, returning whether anything moved.
|
||||
///
|
||||
@@ -3194,13 +3213,17 @@ impl DevelopSession {
|
||||
/// reason a paste must: this moves values the controls are showing and
|
||||
/// nothing here pushes them.
|
||||
pub fn undo(&mut self) -> bool {
|
||||
self.history.undo(&mut self.graph)
|
||||
let step = self.history.undo(&mut self.graph);
|
||||
self.settle(&step);
|
||||
step.moved()
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-5
|
||||
/// Step the edit forward one, returning whether anything moved.
|
||||
pub fn redo(&mut self) -> bool {
|
||||
self.history.redo(&mut self.graph)
|
||||
let step = self.history.redo(&mut self.graph);
|
||||
self.settle(&step);
|
||||
step.moved()
|
||||
}
|
||||
|
||||
pub fn can_undo(&self) -> bool {
|
||||
|
||||
Reference in New Issue
Block a user