Give a mis-drag a way back
Develop edits now save themselves to a sidecar the moment you leave the image, so until this there was no way to undo one — the mistake was persisted and the only recourse was to remember the old number. The history is a stack of snapshots, because the edit graph is already plain data: `Preset::capture` reduces it to what differs from default and `Preset::apply` puts it back, so undo is those two calls and nothing else. A command object per action, with an inverse beside it, would have been a second thing every operation had to register — and operations are declared in YAML precisely so that a new one needs no code written for it. A snapshot cannot fall behind them. The interesting part is coalescing. A slider drag emits an event per frame and must be one step, not forty. Nothing in the interface reports a gesture boundary — the same wall the render coalescing hit, and it is answered the same way rather than by threading a "finger is down" out of every slider, curve point and crop handle. What stands in for the boundary is the control plus recency: changes to the same control within 700 ms amend one step. Which control is "the same" is asked of the graph, not listed: an operation whose declared presentation claims a parameter is one where a single gesture moves several — a curve point carries an x and a y — so those coalesce as one widget. Nothing in the history names the tone curve. The compromise, and it is a real one: a control let go of and picked up again within the window is one step rather than two. Buying the other answer costs a gesture-boundary signal on every control, which is more surface than the difference is worth. The stack is bounded at 64 states for NFR-RES-1 — a develop session stays open for hours. Sixty-four rather than a byte cap: what is being bounded is steps a photographer would want back, and a byte cap would give the elaborate edit the shallowest history, which is exactly backwards. The session owns its history and every mutator records into it, so the callbacks in `lib.rs` cannot change the edit and forget to — with a dozen generic callbacks that would have been one press of undo away from wrong every time a control was added. Opening a photograph makes its stored edit the floor rather than a step: it is not work done in this sitting, and an undo reaching behind it would discard a previous session's edit and then save that on the way out. Not yet done, from FR-DEV-5: history is per-session and in memory, and there are no named snapshots. What mattered was that a saved mis-drag had no way back at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+65
-2
@@ -13,8 +13,8 @@ use dr_decode::RawImage;
|
||||
use dr_gpu::{AdjustPass, DemosaicedImage, Demosaicer, GpuContext};
|
||||
use dr_pipeline::ops::curve;
|
||||
use dr_pipeline::{
|
||||
CropRect, EditGraph, OpCapability, OpId, ParamId, ParamKind, Presentation, Preset, Scope, Unit,
|
||||
WidgetKind,
|
||||
CropRect, Edit, EditGraph, History, OpCapability, OpId, ParamId, ParamKind, Presentation,
|
||||
Preset, Scope, Unit, WidgetKind,
|
||||
};
|
||||
|
||||
use crate::labels;
|
||||
@@ -23,6 +23,15 @@ use crate::ParamRow;
|
||||
/// A loaded image plus its edit state.
|
||||
pub struct DevelopSession {
|
||||
graph: EditGraph,
|
||||
/// TRACES: FR-DEV-5
|
||||
/// Undo, kept beside the graph rather than in the window.
|
||||
///
|
||||
/// Every mutator below records into it, so a caller cannot change the edit
|
||||
/// and forget to. That is the whole reason it lives here: the callbacks in
|
||||
/// `lib.rs` are generic by construction and there are a dozen of them, and
|
||||
/// a history the *call sites* had to remember would be one press of undo
|
||||
/// away from wrong every time a control is added.
|
||||
history: History,
|
||||
demosaiced: DemosaicedImage,
|
||||
adjust: AdjustPass,
|
||||
}
|
||||
@@ -73,8 +82,10 @@ impl DevelopSession {
|
||||
) -> Self {
|
||||
let mut graph = EditGraph::default_chain();
|
||||
graph.set_orientation(orientation);
|
||||
let history = History::new(&graph);
|
||||
Self {
|
||||
graph,
|
||||
history,
|
||||
demosaiced,
|
||||
adjust: AdjustPass::new(ctx),
|
||||
}
|
||||
@@ -450,6 +461,9 @@ impl DevelopSession {
|
||||
for p in &cap.params {
|
||||
self.graph.set_param(cap.id, p.id, p.default);
|
||||
}
|
||||
// One step, though it moved every parameter the operation has: the
|
||||
// user pressed one button.
|
||||
self.history.record(&self.graph, Edit::Discrete);
|
||||
}
|
||||
|
||||
/// Reset a curve, which is to reset its operation.
|
||||
@@ -471,6 +485,8 @@ impl DevelopSession {
|
||||
return;
|
||||
};
|
||||
self.graph.set_param(op, param, value);
|
||||
let edit = Edit::for_param(&self.graph, op, param);
|
||||
self.history.record(&self.graph, edit);
|
||||
}
|
||||
|
||||
/// Return one parameter to its default.
|
||||
@@ -487,10 +503,12 @@ impl DevelopSession {
|
||||
.map(|p| p.default)
|
||||
.unwrap_or(0.0);
|
||||
self.graph.set_param(op, param, default);
|
||||
self.history.record(&self.graph, Edit::Discrete);
|
||||
}
|
||||
|
||||
pub fn reset_all(&mut self) {
|
||||
self.graph.reset();
|
||||
self.history.record(&self.graph, Edit::Discrete);
|
||||
}
|
||||
|
||||
fn lookup(&self, op_index: i32, param_index: i32) -> Option<(OpId, ParamId)> {
|
||||
@@ -654,6 +672,10 @@ impl DevelopSession {
|
||||
/// Set the crop rectangle, in fractions of the source.
|
||||
pub fn set_crop(&mut self, rect: CropRect) {
|
||||
self.graph.set_crop(rect);
|
||||
// Keyed on the operation, not on a parameter: one drag of one handle
|
||||
// moves the origin and the extent together.
|
||||
self.history
|
||||
.record(&self.graph, Edit::Op(dr_pipeline::framing::ID));
|
||||
}
|
||||
|
||||
pub fn crop(&self) -> CropRect {
|
||||
@@ -673,6 +695,7 @@ impl DevelopSession {
|
||||
self.graph.set_crop(rotate_crop(crop, turns));
|
||||
}
|
||||
self.graph.rotate_quarters(turns);
|
||||
self.history.record(&self.graph, Edit::Discrete);
|
||||
}
|
||||
|
||||
/// Straightening, in degrees. Positive turns the image clockwise.
|
||||
@@ -697,6 +720,7 @@ impl DevelopSession {
|
||||
dr_pipeline::framing::FLIP_H,
|
||||
f32::from(u8::from(!h)),
|
||||
);
|
||||
self.history.record(&self.graph, Edit::Discrete);
|
||||
}
|
||||
|
||||
pub fn toggle_flip_v(&mut self) {
|
||||
@@ -706,6 +730,7 @@ impl DevelopSession {
|
||||
dr_pipeline::framing::FLIP_V,
|
||||
f32::from(u8::from(!v)),
|
||||
);
|
||||
self.history.record(&self.graph, Edit::Discrete);
|
||||
}
|
||||
|
||||
/// Set the straightening angle, in degrees.
|
||||
@@ -715,6 +740,10 @@ impl DevelopSession {
|
||||
dr_pipeline::framing::ANGLE,
|
||||
degrees,
|
||||
);
|
||||
self.history.record(
|
||||
&self.graph,
|
||||
Edit::Param(dr_pipeline::framing::ID, dr_pipeline::framing::ANGLE),
|
||||
);
|
||||
}
|
||||
|
||||
/// Whether the framing currently changes the image — what lights the
|
||||
@@ -737,6 +766,7 @@ impl DevelopSession {
|
||||
let view = self.graph.framing().view();
|
||||
self.graph.framing_mut().reset();
|
||||
self.graph.framing_mut().set_view(view);
|
||||
self.history.record(&self.graph, Edit::Discrete);
|
||||
}
|
||||
|
||||
/// How far the viewport is zoomed in: 1.0 fits the frame, 4.0 is 4×.
|
||||
@@ -859,6 +889,10 @@ impl DevelopSession {
|
||||
/// values the sliders are showing, and nothing here pushes them.
|
||||
pub fn apply_settings(&mut self, preset: &Preset, scope: Scope) {
|
||||
preset.apply(&mut self.graph, scope);
|
||||
// A paste is undoable, and is the action most in need of it: it
|
||||
// replaces everything in scope at once, so getting it wrong costs more
|
||||
// than any single control can.
|
||||
self.history.record(&self.graph, Edit::Discrete);
|
||||
}
|
||||
|
||||
/// TRACES: FR-CAT-8
|
||||
@@ -870,6 +904,35 @@ impl DevelopSession {
|
||||
/// orientation survives it, since that was never an edit.
|
||||
pub fn apply_version(&mut self, version: &dr_pipeline::Version) {
|
||||
version.apply(&mut self.graph);
|
||||
// The stored edit becomes the floor rather than a step. It is not
|
||||
// something the user did in this sitting, and an undo that reached
|
||||
// behind it would discard a previous session's work in one press —
|
||||
// then persist that on the way out, since saving is automatic.
|
||||
self.history.reset(&self.graph);
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-5
|
||||
/// Step the edit back one, returning whether anything moved.
|
||||
///
|
||||
/// The panel must be rebuilt from [`Self::rows`] afterwards, for the same
|
||||
/// 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)
|
||||
}
|
||||
|
||||
/// 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)
|
||||
}
|
||||
|
||||
pub fn can_undo(&self) -> bool {
|
||||
self.history.can_undo()
|
||||
}
|
||||
|
||||
pub fn can_redo(&self) -> bool {
|
||||
self.history.can_redo()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user