Let a photographer name the state they liked, and go back to it or look at it

FR-DEV-5 asked for named snapshots of an edit state and FR-DEV-7 for a
comparison against a chosen one, and neither existed. The history stack
is per sitting and forgotten with it, on purpose — the gap that mattered
was an automatically saved mis-drag with no way back, and that was closed
first. What was left was the other half: a state the photographer wants
to keep *because* it is worth keeping, which is a different thing from a
step and is not served by making the steps last longer.

A snapshot is an edit state, and an edit state is exactly what a sidecar
version stores, so it is stored as one: a `[version]` block carrying
`snapshot-of = <uuid>`. The parameters, the masks and their parts, the
repairs and the film all arrive through the blocks that already carry
them, a merge keys on the uuid as it does for any version, and a build
that predates the key reads the block as a named version and keeps it —
the right failure. Only the pointer is new. The one reader that has to
know is `default_version`, which must never answer with a snapshot: a
file whose edit is missing is not a file whose edit is one of its saved
moments. The snapshots of an edit are listed by that pointer, oldest
first, the same on every device.

Writing them back removes what this sitting deleted and puts in what it
holds, and leaves standing whatever it never saw — a snapshot the other
device took since the photograph was opened here is not this device's to
remove by not knowing about it. That is the rule the version merge
already keeps, applied one level down, and it is why the save carries
the deleted ids rather than replacing the list wholesale as the masks
are. Each is re-pointed at the uuid the save settled on, because the
default may have been fused onto its canonical identity since the
snapshot was taken.

Restoring is one history step, so undo takes it back whole, as a paste
is. Taking and deleting are not steps: they change nothing about the
photograph, and an undo that removed a snapshot would be undoing a
decision to remember. Holding the eye beside one renders the snapshot
and hands the edit straight back — the same suspension "Before" uses,
against a point the photographer chose rather than the file. Two
sessions on the same photograph get ids that cannot collide, stamped
with the second and a random word, because the merge folds equal ids
into one.
This commit is contained in:
2026-09-12 01:08:10 +02:00
parent 369eb8fbf0
commit d3b6127db6
11 changed files with 903 additions and 68 deletions
+159 -1
View File
@@ -126,6 +126,23 @@ pub struct Version {
pub uuid: String, pub uuid: String,
pub name: String, pub name: String,
pub is_default: bool, pub is_default: bool,
/// TRACES: FR-DEV-5
/// The version this is a named snapshot of, if it is one.
///
/// A snapshot *is* an edit state, which is exactly what a version stores,
/// so it is stored as one: the parameters, the masks and their parts, the
/// repairs and the film all arrive through the blocks that already carry
/// them, and a merge keys on the uuid as it does for any other version.
/// What sets a snapshot apart is only this pointer — it belongs to
/// another version's history rather than standing beside it as a variant
/// (FR-CAT-12), so a reader listing what a photograph *is* skips it, and
/// a reader listing what one edit *was* finds it here.
///
/// Written as `snapshot-of`. A build that predates the key reads the
/// block as an ordinary named version and keeps it, which is the right
/// failure: nothing is lost, and the newer build finds it as a snapshot
/// again.
pub snapshot_of: Option<String>,
/// Monotonic per-edit counter (FR-NC-8). /// Monotonic per-edit counter (FR-NC-8).
/// ///
/// The primary merge discriminator, ahead of [`Self::modified`]: a device /// The primary merge discriminator, ahead of [`Self::modified`]: a device
@@ -222,6 +239,7 @@ impl Version {
uuid: uuid.into(), uuid: uuid.into(),
name: name.into(), name: name.into(),
is_default: false, is_default: false,
snapshot_of: None,
revision: 1, revision: 1,
device: String::new(), device: String::new(),
modified: 0, modified: 0,
@@ -267,6 +285,12 @@ impl Version {
}) })
} }
/// TRACES: FR-DEV-5
/// Whether this version is a named snapshot of another one.
pub fn is_snapshot(&self) -> bool {
self.snapshot_of.is_some()
}
/// Record `graph` into this version, bumping the revision. /// Record `graph` into this version, bumping the revision.
/// ///
/// The revision bump is what makes this the write path rather than a /// The revision bump is what makes this the write path rather than a
@@ -645,7 +669,52 @@ impl Sidecar {
.values() .values()
.filter(|v| v.is_default) .filter(|v| v.is_default)
.max_by_key(|v| (v.revision, v.modified)) .max_by_key(|v| (v.revision, v.modified))
.or_else(|| self.versions.values().next()) // Never a snapshot: a file with no default and a snapshot in it
// is a file whose edit is *missing*, and answering with a saved
// state of it would open the photograph at a moment the
// photographer deliberately stepped away from.
.or_else(|| self.versions.values().find(|v| !v.is_snapshot()))
}
/// TRACES: FR-DEV-5
/// The named snapshots of one version, oldest first.
///
/// Taken time is `modified`, so the order is the order they were taken in
/// whichever device took them; the uuid breaks a tie so the list reads
/// the same on every device.
pub fn snapshots_of(&self, uuid: &str) -> Vec<&Version> {
let mut out: Vec<&Version> = self
.versions
.values()
.filter(|v| v.snapshot_of.as_deref() == Some(uuid))
.collect();
out.sort_by(|a, b| (a.modified, &a.uuid).cmp(&(b.modified, &b.uuid)));
out
}
/// TRACES: FR-DEV-5 | FR-NC-9
/// Write a session's snapshots of `uuid` into the file.
///
/// `removed` are the ones the session deleted, taken out by id;
/// `snapshots` are the ones it holds, put in. A snapshot in the file that
/// is in neither — one another device took since this session opened
/// the photograph — is left standing, which is the same rule
/// [`Version::merge`] keeps for a version only one side has: never treat
/// "I did not see it" as "I removed it".
///
/// Each snapshot is re-pointed at `uuid` on the way in, because the
/// default version may have been fused onto a canonical identity since
/// the snapshot was taken, and a snapshot of a uuid nothing carries is
/// a snapshot of nothing.
pub fn replace_snapshots(&mut self, uuid: &str, snapshots: Vec<Version>, removed: &[String]) {
for id in removed {
self.versions.remove(id);
}
for mut snapshot in snapshots {
snapshot.snapshot_of = Some(uuid.to_string());
snapshot.is_default = false;
self.put(snapshot);
}
} }
/// TRACES: FR-NC-8 | FR-NC-9 /// TRACES: FR-NC-8 | FR-NC-9
@@ -763,6 +832,10 @@ impl Sidecar {
if v.is_default { if v.is_default {
let _ = writeln!(out, "default = 1"); let _ = writeln!(out, "default = 1");
} }
// TRACES: FR-DEV-5
if let Some(of) = &v.snapshot_of {
let _ = writeln!(out, "snapshot-of = {of}");
}
let _ = writeln!(out, "revision = {}", v.revision); let _ = writeln!(out, "revision = {}", v.revision);
if !v.device.is_empty() { if !v.device.is_empty() {
let _ = writeln!(out, "device = {}", v.device); let _ = writeln!(out, "device = {}", v.device);
@@ -942,6 +1015,8 @@ impl Sidecar {
match key { match key {
"name" => version.name = value.to_string(), "name" => version.name = value.to_string(),
"default" => version.is_default = value != "0", "default" => version.is_default = value != "0",
// TRACES: FR-DEV-5
"snapshot-of" => version.snapshot_of = Some(value.to_string()),
"revision" => version.revision = value.parse().unwrap_or(0), "revision" => version.revision = value.parse().unwrap_or(0),
"device" => version.device = value.to_string(), "device" => version.device = value.to_string(),
"modified" => version.modified = value.parse().unwrap_or(0), "modified" => version.modified = value.parse().unwrap_or(0),
@@ -2347,6 +2422,89 @@ mod tests {
assert_eq!(g.param(exposure::ID, exposure::EXPOSURE), Some(0.0)); assert_eq!(g.param(exposure::ID, exposure::EXPOSURE), Some(0.0));
} }
/// TRACES: FR-DEV-5
/// A snapshot is a version with a pointer, and the pointer survives the
/// file: it comes back as a snapshot of the edit it was taken from, with
/// the edit it stored, and the photograph still opens at its default.
#[test]
fn a_snapshot_round_trips_as_a_snapshot_of_its_edit() {
let mut sidecar = Sidecar::new();
let mut current = Version::from_graph("u1", "Default", &edited());
current.is_default = true;
sidecar.put(current);
let mut mono = EditGraph::default_chain();
mono.set_param(saturation::ID, saturation::SATURATION, -100.0);
let mut snapshot = Version::from_graph("snap-1", "Black and white", &mono);
snapshot.snapshot_of = Some("u1".into());
snapshot.modified = 7;
sidecar.put(snapshot);
let text = sidecar.to_text();
assert!(text.contains("snapshot-of = u1"), "{text}");
let parsed = Sidecar::parse(&text).expect("valid");
assert_eq!(parsed.default_version().expect("default").uuid, "u1");
let snapshots = parsed.snapshots_of("u1");
assert_eq!(snapshots.len(), 1);
assert_eq!(snapshots[0].name, "Black and white");
assert!(snapshots[0].is_snapshot());
let mut g = EditGraph::default_chain();
snapshots[0].apply(&mut g).expect_no_film();
assert_eq!(
g.param(saturation::ID, saturation::SATURATION),
Some(-100.0)
);
}
/// TRACES: FR-DEV-5
/// A file whose only versions are snapshots has no edit to open, and
/// must not answer with one of the snapshots as if it were.
#[test]
fn a_snapshot_is_never_the_default() {
let mut sidecar = Sidecar::new();
let mut snapshot = Version::from_graph("snap-1", "Earlier", &edited());
snapshot.snapshot_of = Some("gone".into());
sidecar.put(snapshot);
assert!(sidecar.default_version().is_none());
}
/// TRACES: FR-DEV-5 | FR-NC-9
/// Writing a session's snapshots back removes what it deleted and keeps
/// what it never saw — another device's snapshot is not this device's to
/// remove by not knowing about it.
#[test]
fn replacing_snapshots_removes_only_what_was_deleted() {
let mut sidecar = Sidecar::new();
let mut current = Version::from_graph("u1", "Default", &edited());
current.is_default = true;
sidecar.put(current);
for id in ["mine-1", "mine-2", "theirs-1"] {
let mut v = Version::from_graph(id, id, &edited());
v.snapshot_of = Some("u1".into());
sidecar.put(v);
}
// This session loaded mine-1 and mine-2, deleted mine-2, took mine-3.
let mut kept = Version::from_graph("mine-1", "mine-1", &edited());
kept.snapshot_of = Some("u1".into());
let taken = Version::from_graph("mine-3", "mine-3", &edited());
sidecar.replace_snapshots("u1", vec![kept, taken], &["mine-2".to_string()]);
let ids: Vec<&str> = sidecar
.snapshots_of("u1")
.iter()
.map(|v| v.uuid.as_str())
.collect();
assert_eq!(ids, ["mine-1", "mine-3", "theirs-1"]);
assert_eq!(
sidecar.versions["mine-3"].snapshot_of.as_deref(),
Some("u1"),
"a snapshot taken without a pointer is pointed at the edit"
);
}
#[test] #[test]
fn update_bumps_the_revision() { fn update_bumps_the_revision() {
// FR-NC-9 resolves by revision; a write that did not bump it would // FR-NC-9 resolves by revision; a write that did not bump it would
+19 -1
View File
@@ -5,7 +5,7 @@
Every entry here is extracted from the comment beside the code that implements it, so this file cannot describe a gesture the application does not have. Add one by writing a `GESTURE:` block next to the implementation; there is nowhere else to write it. Every entry here is extracted from the comment beside the code that implements it, so this file cannot describe a gesture the application does not have. Add one by writing a `GESTURE:` block next to the implementation; there is nowhere else to write it.
38 gestures, in 4 places. 40 gestures, in 4 places.
## Develop ## Develop
@@ -133,6 +133,24 @@ The column is 280px wide and the colour mixer alone puts thirty-six of these in
<sub>`ui/dr-ui/ui/controls.slint:294`</sub> <sub>`ui/dr-ui/ui/controls.slint:294`</sub>
### See the photograph as a snapshot had it
- **Touch** — Press and hold the eye beside the snapshot
- **Pointer** — Press and hold the eye beside the snapshot
The same hold as "Before", against a point the photographer chose rather than the file: "the version I liked twenty minutes ago" is how a choice between two treatments is actually made. It takes no history step and changes nothing; letting go puts the edit back.
<sub>`ui/dr-ui/ui/history.slint:92`</sub>
### Keep the photograph as it is now, under a name
- **Touch** — Type a name in the History panel and press Snapshot
- **Pointer** — Type a name in the History panel and press Snapshot
The history is forgotten with the sitting, on purpose; a snapshot is the photographer saying this one should not be. It is written into the sidecar as a version of the edit, so it survives a restart and reaches the other device. Pressing a snapshot puts the photograph back to it, as one step that undo takes back whole.
<sub>`ui/dr-ui/ui/history.slint:307`</sub>
### Show or hide one mask layer ### Show or hide one mask layer
- **Touch** — Tap the ring at the head of its row - **Touch** — Tap the ring at the head of its row
+61 -61
View File
File diff suppressed because one or more lines are too long
+283
View File
@@ -778,6 +778,27 @@ pub struct DevelopSession {
/// a history the *call sites* had to remember would be one press of undo /// a history the *call sites* had to remember would be one press of undo
/// away from wrong every time a control is added. /// away from wrong every time a control is added.
history: History, history: History,
/// TRACES: FR-DEV-5
/// The named snapshots of this edit, as the sidecar had them plus what
/// this sitting took, minus what it deleted.
///
/// Beside the history rather than inside it, because they answer a
/// different question. The history is what was done in this sitting and
/// is deliberately forgotten with it; a snapshot is a state the
/// photographer *named*, which is the act of saying it should outlive
/// the sitting. It is persisted as a version of the sidecar pointing at
/// this one (`Version::snapshot_of`), which is what makes it survive a
/// restart and reach the other device.
snapshots: Vec<dr_pipeline::Version>,
/// The ids of snapshots deleted this sitting, so the save can remove
/// them from the file without removing what another device added since
/// — see `Sidecar::replace_snapshots`.
removed_snapshots: Vec<String>,
/// TRACES: FR-DEV-7
/// The snapshot the canvas is showing instead of the edit, while a
/// comparison is held. Viewing state: nothing about the edit changes,
/// and it goes down with the session.
compared_snapshot: Option<String>,
demosaiced: Arc<DemosaicedImage>, demosaiced: Arc<DemosaicedImage>,
adjust: AdjustPass, adjust: AdjustPass,
/// TRACES: FR-DSP-7 /// TRACES: FR-DSP-7
@@ -1041,6 +1062,9 @@ impl DevelopSession {
ctx: ctx.clone(), ctx: ctx.clone(),
graph, graph,
history, history,
snapshots: Vec::new(),
removed_snapshots: Vec::new(),
compared_snapshot: None,
demosaiced: Arc::new(demosaiced), demosaiced: Arc::new(demosaiced),
adjust: AdjustPass::new(ctx), adjust: AdjustPass::new(ctx),
histogram: HistogramPass::new(ctx) histogram: HistogramPass::new(ctx)
@@ -5446,6 +5470,162 @@ impl DevelopSession {
self.history.reset(&self.graph); self.history.reset(&self.graph);
} }
// ---- named snapshots ---------------------------------------------------
/// TRACES: FR-DEV-5
/// Hand the session the snapshots its sidecar holds. Called once, on
/// open, beside [`Self::apply_version`].
pub fn set_snapshots(&mut self, snapshots: Vec<dr_pipeline::Version>) {
self.snapshots = snapshots;
self.removed_snapshots.clear();
}
/// The snapshots as they stand, oldest first.
pub fn snapshots(&self) -> &[dr_pipeline::Version] {
&self.snapshots
}
/// The ids deleted this sitting, for the save.
pub fn removed_snapshots(&self) -> &[String] {
&self.removed_snapshots
}
/// TRACES: FR-DEV-5
/// Name the state the photograph is in, and keep it. Returns the id.
///
/// Not a history step: taking a snapshot changes nothing about the edit,
/// and an undo that removed one would be undoing a decision to remember
/// rather than a change to the photograph. Deleting one is the same.
///
/// The id is stamped with the second and a per-process random word
/// rather than counted, because two devices can each take a snapshot of
/// the same photograph and both have to survive the merge — which keys
/// on this id, and would fold two `snap-3`s into one.
pub fn take_snapshot(&mut self, name: &str) -> String {
use std::hash::{BuildHasher, Hasher};
let now = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|d| d.as_secs() as i64)
.unwrap_or(0);
let salt = std::collections::hash_map::RandomState::new()
.build_hasher()
.finish();
let id = format!("snap-{now}-{:08x}", salt as u32);
let name = name.trim();
let name = if name.is_empty() {
format!("Snapshot {}", self.snapshots.len() + 1)
} else {
name.to_string()
};
let mut version = dr_pipeline::Version::from_graph(id.clone(), name, &self.graph);
// The stack with the model's coverage folded in, for the reason the
// save uses it: a subject layer stored by identity alone renders as
// nothing until a model is run, and a snapshot restored on the other
// device, or in a batch export, never gets one.
version.masks = self.masks_for_storage();
version.modified = now;
self.snapshots.push(version);
id
}
/// TRACES: FR-DEV-5
/// Put the photograph back the way a snapshot has it. One history step,
/// so it is undoable as a whole, exactly as a paste is.
pub fn restore_snapshot(&mut self, id: &str) -> bool {
let Some(version) = self.snapshots.iter().find(|v| v.uuid == id).cloned() else {
return false;
};
let rebake = version.apply(&mut self.graph);
self.pay_film_debt(&rebake);
self.history
.record(&self.graph, Edit::Action(labels::step::SNAPSHOT_RESTORED));
true
}
/// TRACES: FR-DEV-5
pub fn rename_snapshot(&mut self, id: &str, name: &str) {
let name = name.trim();
if name.is_empty() {
return;
}
if let Some(v) = self.snapshots.iter_mut().find(|v| v.uuid == id) {
v.name = name.to_string();
}
}
/// TRACES: FR-DEV-5
/// Forget a snapshot. Remembered as a deletion so the save takes it out
/// of the file rather than merely not putting it back.
pub fn delete_snapshot(&mut self, id: &str) {
let before = self.snapshots.len();
self.snapshots.retain(|v| v.uuid != id);
if self.snapshots.len() != before {
self.removed_snapshots.push(id.to_string());
}
if self.compared_snapshot.as_deref() == Some(id) {
self.compared_snapshot = None;
}
}
/// TRACES: FR-DEV-7
/// Hold a comparison against a snapshot, or let it go. Returns whether
/// anything changed, so a repeat costs no render.
pub fn compare_snapshot(&mut self, id: Option<&str>) -> bool {
let id = id.filter(|id| self.snapshots.iter().any(|v| v.uuid == *id));
if self.compared_snapshot.as_deref() == id {
return false;
}
self.compared_snapshot = id.map(str::to_string);
true
}
/// The snapshot being held against the edit, if one is.
pub fn compared_snapshot(&self) -> Option<&str> {
self.compared_snapshot.as_deref()
}
/// TRACES: FR-DEV-7
/// Render the photograph as a snapshot has it, without becoming it.
///
/// The same suspension [`Self::render_original`] uses — borrow the graph
/// for one render and hand it back — because it is the same question
/// about a different reference point: "the version I liked twenty
/// minutes ago" instead of the file. Nothing is recorded and nothing is
/// marked modified. A held comparison against a snapshot that has since
/// been deleted falls back to the edit itself, which is what is on
/// screen anyway.
pub fn render_compared(&mut self, width: u32, height: u32) -> Result<slint::Image, String> {
let Some(version) = self
.compared_snapshot
.as_deref()
.and_then(|id| self.snapshots.iter().find(|v| v.uuid == id))
.cloned()
else {
return self.render(width, height);
};
let saved = self.graph.state();
let debt = version.apply(&mut self.graph);
self.pay_film_debt(&debt);
let rendered = self.render(width, height);
let debt = self.graph.set_state(&saved);
self.pay_film_debt(&debt);
rendered
}
/// TRACES: FR-DEV-5
/// The snapshot list as the panel draws it, oldest first.
pub fn snapshot_rows(&self) -> Vec<crate::SnapshotRow> {
self.snapshots
.iter()
.map(|v| crate::SnapshotRow {
id: v.uuid.as_str().into(),
name: v.name.as_str().into(),
comparing: self.compared_snapshot.as_deref() == Some(v.uuid.as_str()),
})
.collect()
}
/// TRACES: FR-DEV-3f | FR-DEV-5 /// TRACES: FR-DEV-3f | FR-DEV-5
/// Pay what a restored edit owes the picture. /// Pay what a restored edit owes the picture.
/// ///
@@ -6538,6 +6718,109 @@ mod tests {
); );
} }
/// TRACES: FR-DEV-5
/// A snapshot is a state the photographer named: taking one changes
/// nothing, going back to it is one step, and undo takes the whole of
/// that step back.
#[test]
fn a_snapshot_is_restored_as_one_step_and_undone_as_one() {
let Some(ctx) = headless() else { return };
let (mut session, _) = grey_session(&ctx);
let rows = session.rows();
let row = rows
.iter()
.find(|row| {
session.set_param(row.op_index, row.param_index, row.maximum);
!session.is_neutral()
})
.expect("some control in the panel moves the picture")
.clone();
let liked = session.copy_settings();
let steps_before = session.history_rows().len();
let id = session.take_snapshot("Liked this");
assert_eq!(session.snapshots().len(), 1);
assert_eq!(session.snapshots()[0].name, "Liked this");
assert_eq!(
session.history_rows().len(),
steps_before,
"naming a state is not a change to the photograph"
);
// Move on, then go back.
session.set_param(row.op_index, row.param_index, row.minimum);
let moved_on = session.copy_settings();
assert_ne!(moved_on, liked, "the premise: the edit has moved");
let steps_moved = session.history_rows().len();
assert!(session.restore_snapshot(&id));
assert_eq!(session.copy_settings(), liked, "back to the named state");
assert_eq!(
session.history_rows().len(),
steps_moved + 1,
"restoring is one step"
);
assert!(session.undo());
assert_eq!(
session.copy_settings(),
moved_on,
"and undo takes the whole restore back"
);
// A name nobody typed is numbered rather than blank.
session.take_snapshot(" ");
assert_eq!(session.snapshots()[1].name, "Snapshot 2");
session.delete_snapshot(&id);
assert_eq!(session.snapshots().len(), 1);
assert_eq!(session.removed_snapshots(), [id.as_str()]);
}
/// TRACES: FR-DEV-7 | FR-DEV-5
/// Holding a snapshot against the edit is the same bargain as holding
/// the original: the picture changes, and nothing else does.
#[test]
fn comparing_against_a_snapshot_leaves_the_edit_exactly_as_it_was() {
let Some(ctx) = headless() else { return };
let (mut session, _) = grey_session(&ctx);
let rows = session.rows();
let row = rows
.iter()
.find(|row| {
session.set_param(row.op_index, row.param_index, row.maximum);
!session.is_neutral()
})
.expect("some control in the panel moves the picture")
.clone();
let id = session.take_snapshot("Bright");
session.set_param(row.op_index, row.param_index, row.minimum);
let edit = session.copy_settings();
let steps = session.history_rows().len();
assert!(session.compare_snapshot(Some(&id)), "the hold began");
assert!(
!session.compare_snapshot(Some(&id)),
"a repeat of the same hold is not a change"
);
assert_eq!(session.compared_snapshot(), Some(id.as_str()));
session
.render_compared(64, 64)
.expect("render the snapshot");
assert_eq!(session.copy_settings(), edit, "every parameter comes back");
assert_eq!(session.history_rows().len(), steps, "looking is not a step");
assert!(session.compare_snapshot(None), "and letting go is one");
assert!(session.compared_snapshot().is_none());
assert!(
!session.compare_snapshot(Some("nothing-by-this-name")),
"a snapshot that does not exist cannot be held"
);
}
/// TRACES: FR-DEV-3 | FR-CAT-8 /// TRACES: FR-DEV-3 | FR-CAT-8
/// Reopening an edited photograph renders its subject mask, with no model. /// Reopening an edited photograph renders its subject mask, with no model.
/// ///
+14
View File
@@ -113,6 +113,20 @@ pub const GESTURES: &[Gesture] = &[
pointer: "Double-click its track, or right-click it", pointer: "Double-click its track, or right-click it",
keys: "R, for the control last moved", keys: "R, for the control last moved",
}, },
Gesture {
title: "See the photograph as a snapshot had it",
section: "Develop",
touch: "Press and hold the eye beside the snapshot",
pointer: "Press and hold the eye beside the snapshot",
keys: "",
},
Gesture {
title: "Keep the photograph as it is now, under a name",
section: "Develop",
touch: "Type a name in the History panel and press Snapshot",
pointer: "Type a name in the History panel and press Snapshot",
keys: "",
},
Gesture { Gesture {
title: "Show or hide one mask layer", title: "Show or hide one mask layer",
section: "Develop", section: "Develop",
+5
View File
@@ -28,6 +28,9 @@ pub mod step {
pub const PASTE: LocalizedKey = LocalizedKey("history.paste"); pub const PASTE: LocalizedKey = LocalizedKey("history.paste");
pub const FILM: LocalizedKey = LocalizedKey("history.film"); pub const FILM: LocalizedKey = LocalizedKey("history.film");
/// TRACES: FR-DEV-5
/// The photograph put back to a named snapshot, as one step.
pub const SNAPSHOT_RESTORED: LocalizedKey = LocalizedKey("history.snapshot_restored");
/// TRACES: FR-DEV-3 /// TRACES: FR-DEV-3
/// A neutral was picked off the photograph, setting the white balance. /// A neutral was picked off the photograph, setting the white balance.
@@ -97,6 +100,7 @@ pub mod step {
dr_pipeline::history::OPENED, dr_pipeline::history::OPENED,
dr_pipeline::history::UNNAMED, dr_pipeline::history::UNNAMED,
PASTE, PASTE,
SNAPSHOT_RESTORED,
FILM, FILM,
SAMPLED_NEUTRAL, SAMPLED_NEUTRAL,
RESET_ALL, RESET_ALL,
@@ -289,6 +293,7 @@ fn catalogued(key: &str) -> Option<&'static str> {
"history.opened" => "Opened", "history.opened" => "Opened",
"history.edit" => "Edit", "history.edit" => "Edit",
"history.paste" => "Paste Settings", "history.paste" => "Paste Settings",
"history.snapshot_restored" => "Restore A Snapshot",
"history.film" => "Film Stock", "history.film" => "Film Stock",
"history.sampled_neutral" => "Sample Neutral", "history.sampled_neutral" => "Sample Neutral",
+94
View File
@@ -1838,6 +1838,11 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
// the list is rebuilt when it would read differently, not when the picture // the list is rebuilt when it would read differently, not when the picture
// is redrawn, and those are very different rates. // is redrawn, and those are very different rates.
let drawn_history: Rc<Cell<Option<u64>>> = Rc::new(Cell::new(None)); let drawn_history: Rc<Cell<Option<u64>>> = Rc::new(Cell::new(None));
// TRACES: FR-DEV-5
// And the snapshot list, compared by value rather than by a revision: it
// is a handful of rows, and a held comparison changes one of them
// without a step being taken.
let drawn_snapshots: Rc<RefCell<Option<Vec<SnapshotRow>>>> = Rc::new(RefCell::new(None));
// TRACES: FR-CULL-3 // TRACES: FR-CULL-3
// How the photographer wants focus peaking drawn, or `None` for off. // How the photographer wants focus peaking drawn, or `None` for off.
@@ -1874,6 +1879,7 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
let session = session.clone(); let session = session.clone();
let viewport = viewport.clone(); let viewport = viewport.clone();
let drawn_history = drawn_history.clone(); let drawn_history = drawn_history.clone();
let drawn_snapshots = drawn_snapshots.clone();
let display = display.clone(); let display = display.clone();
let chosen_peaking = chosen_peaking.clone(); let chosen_peaking = chosen_peaking.clone();
Rc::new(move |window: &AppWindow, draft: bool| { Rc::new(move |window: &AppWindow, draft: bool| {
@@ -1916,6 +1922,16 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
steps.set_rows(slint::ModelRc::new(slint::VecModel::from(s.history_rows()))); steps.set_rows(slint::ModelRc::new(slint::VecModel::from(s.history_rows())));
steps.set_undo_label(s.undo_label().into()); steps.set_undo_label(s.undo_label().into());
} }
// TRACES: FR-DEV-5
// The snapshots, on the same path. Rebuilt only when the list
// would read differently, for the reason the steps are: a held
// comparison redraws, and it is the one thing here that changes
// a row without changing the history.
let snapshots = s.snapshot_rows();
if drawn_snapshots.borrow().as_ref() != Some(&snapshots) {
*drawn_snapshots.borrow_mut() = Some(snapshots.clone());
steps.set_snapshots(slint::ModelRc::new(slint::VecModel::from(snapshots)));
}
// TRACES: FR-DEV-3 // TRACES: FR-DEV-3
// Which part of the region overlay the view is showing. Here // Which part of the region overlay the view is showing. Here
@@ -1979,6 +1995,10 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
s.render_uncropped(w, h).map(|(image, _, _)| image) s.render_uncropped(w, h).map(|(image, _, _)| image)
} else if window.get_showing_original() { } else if window.get_showing_original() {
s.render_original(w, h) s.render_original(w, h)
} else if s.compared_snapshot().is_some() {
// TRACES: FR-DEV-7
// The same hold, against a snapshot rather than the file.
s.render_compared(w, h)
} else { } else {
s.render(w, h) s.render(w, h)
}; };
@@ -3043,6 +3063,80 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
} }
}); });
} }
// TRACES: FR-DEV-5
// Named snapshots. Taking and deleting one change the list and nothing
// else — no rows, no history — so they redraw only to push the list,
// through the same path everything else pushes it on. Restoring one is
// the paste's shape: the whole edit moves, so the rows resync and the
// picture is redrawn.
{
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
window.global::<Steps>().on_snapshot_taken(move |name| {
let Some(w) = weak.upgrade() else { return };
if let Some(s) = session.borrow_mut().as_mut() {
s.take_snapshot(&name);
}
redraw(&w);
});
}
{
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
let rows = rows.clone();
window.global::<Steps>().on_snapshot_restored(move |id| {
let Some(w) = weak.upgrade() else { return };
let restored = session
.borrow_mut()
.as_mut()
.is_some_and(|s| s.restore_snapshot(&id));
if restored {
sync_rows(&w, &rows, &session);
// The mask panel too: a snapshot carries its layers, and
// the redraw syncs the repairs and the overlay but not the
// rows the layers are listed in.
masks_ui::sync(&w, &session);
redraw(&w);
}
});
}
{
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
window.global::<Steps>().on_snapshot_removed(move |id| {
let Some(w) = weak.upgrade() else { return };
if let Some(s) = session.borrow_mut().as_mut() {
s.delete_snapshot(&id);
}
redraw(&w);
});
}
{
// TRACES: FR-DEV-7
// Holding a snapshot against the edit, and letting it go — the
// Before button's shape, and its rule: no rows are synced and no
// history is touched. A full frame rather than a draft, for the
// reason the original is: a soft comparison shows a difference the
// edit does not have.
let weak = window.as_weak();
let session = session.clone();
let render_now = render_now.clone();
window
.global::<Steps>()
.on_snapshot_compared(move |id, down| {
let Some(w) = weak.upgrade() else { return };
let changed = session
.borrow_mut()
.as_mut()
.is_some_and(|s| s.compare_snapshot(down.then_some(id.as_str())));
if changed {
render_now(&w, false);
}
});
}
// ---- zoom, pan and crop --------------------------------------------- // ---- zoom, pan and crop ---------------------------------------------
// //
+17
View File
@@ -544,6 +544,11 @@ pub enum Amendment {
/// `Some(None)` is a real edit: "develop this normally again". Without /// `Some(None)` is a real edit: "develop this normally again". Without
/// the distinction, clearing a film could never be saved. /// the distinction, clearing a film could never be saved.
film: Option<Option<dr_pipeline::sidecar::FilmRef>>, film: Option<Option<dr_pipeline::sidecar::FilmRef>>,
/// TRACES: FR-DEV-5
/// The named snapshots, when this is an image's own edit being
/// written back: the ones the session holds, and the ids it deleted.
/// A paste carries none — a snapshot is a state of one photograph.
snapshots: Option<(Vec<dr_pipeline::Version>, Vec<String>)>,
/// TRACES: FR-DEV-3 | FR-CAT-8 /// TRACES: FR-DEV-3 | FR-CAT-8
/// The local adjustments, when this is an image's own edit being /// The local adjustments, when this is an image's own edit being
/// written back rather than a paste onto someone else's. /// written back rather than a paste onto someone else's.
@@ -795,6 +800,7 @@ fn amend(base: dr_pipeline::Sidecar, w: &SidecarWrite) -> dr_pipeline::Sidecar {
scope, scope,
masks, masks,
film, film,
..
} => { } => {
preset.amend(&mut version.params, *scope); preset.amend(&mut version.params, *scope);
// TRACES: FR-DEV-3f // TRACES: FR-DEV-3f
@@ -823,6 +829,17 @@ fn amend(base: dr_pipeline::Sidecar, w: &SidecarWrite) -> dr_pipeline::Sidecar {
version.modified = now_secs(); version.modified = now_secs();
sidecar.put(version); sidecar.put(version);
// TRACES: FR-DEV-5 | FR-NC-9
// After the version, and against the uuid the fuse settled on: a
// snapshot points at its edit by uuid, and the edit may have just been
// renamed onto the canonical one.
if let Amendment::Settings {
snapshots: Some((kept, removed)),
..
} = &w.amendment
{
sidecar.replace_snapshots(&w.version_uuid, kept.clone(), removed);
}
sidecar sidecar
} }
+3
View File
@@ -3321,6 +3321,9 @@ fn collect_settings_writes(
// not a parameter. Pasting one would be pasting a choice the // not a parameter. Pasting one would be pasting a choice the
// clipboard never captured. // clipboard never captured.
film: None, film: None,
// Nor snapshots: they are states of the photograph they were
// taken on, and mean nothing on another.
snapshots: None,
}, },
}) })
}); });
+96 -4
View File
@@ -315,6 +315,7 @@ pub fn save_local(
preset: &Preset, preset: &Preset,
scope: Scope, scope: Scope,
masks: Option<&dr_pipeline::mask::MaskStack>, masks: Option<&dr_pipeline::mask::MaskStack>,
snapshots: Option<(&[dr_pipeline::Version], &[String])>,
) -> Result<(), String> { ) -> Result<(), String> {
let path = local_sidecar_path(image); let path = local_sidecar_path(image);
@@ -369,6 +370,10 @@ pub fn save_local(
version.revision = version.revision.saturating_add(1); version.revision = version.revision.saturating_add(1);
version.modified = now_secs(); version.modified = now_secs();
sidecar.put(version); sidecar.put(version);
// TRACES: FR-DEV-5
if let Some((kept, removed)) = snapshots {
sidecar.replace_snapshots(&uuid, kept.to_vec(), removed);
}
let text = sidecar.to_text(); let text = sidecar.to_text();
@@ -401,9 +406,9 @@ pub fn save_open_edit(
session: &Rc<RefCell<Option<DevelopSession>>>, session: &Rc<RefCell<Option<DevelopSession>>>,
library: &Rc<library_ui::LibraryController>, library: &Rc<library_ui::LibraryController>,
) { ) {
// Both taken in one borrow: they are one edit, and a mask stack captured // All taken in one borrow: they are one edit, and a mask stack captured
// from a session that had already moved on would be a different image's. // from a session that had already moved on would be a different image's.
let Some((preset, masks, film)) = session.borrow().as_ref().map(|s| { let Some((preset, masks, film, snapshots, removed)) = session.borrow().as_ref().map(|s| {
( (
s.copy_settings(), s.copy_settings(),
// TRACES: FR-DEV-3 // TRACES: FR-DEV-3
@@ -429,6 +434,12 @@ pub fn save_open_edit(
.unwrap_or_default() .unwrap_or_default()
}), }),
}), }),
// TRACES: FR-DEV-5
// The snapshots as they stand and the ids deleted this sitting,
// so the file gains what was taken and loses what was removed —
// and keeps what another device added meanwhile.
s.snapshots().to_vec(),
s.removed_snapshots().to_vec(),
) )
}) else { }) else {
return; return;
@@ -443,7 +454,13 @@ pub fn save_open_edit(
// `Everything`: this is the image's *own* edit being written back, // `Everything`: this is the image's *own* edit being written back,
// not a paste onto someone else's. Excluding framing here would // not a paste onto someone else's. Excluding framing here would
// make a crop the one adjustment that never survived a restart. // make a crop the one adjustment that never survived a restart.
if let Err(e) = save_local(path, &preset, Scope::everything(), Some(&masks)) { if let Err(e) = save_local(
path,
&preset,
Scope::everything(),
Some(&masks),
Some((&snapshots, &removed)),
) {
log::warn!("saving {}: {e}", path.display()); log::warn!("saving {}: {e}", path.display());
} }
} }
@@ -461,6 +478,7 @@ pub fn save_open_edit(
// sliders were written and every mask was dropped. // sliders were written and every mask was dropped.
masks: Some(masks), masks: Some(masks),
film: Some(film), film: Some(film),
snapshots: Some((snapshots, removed)),
}, },
}; };
library_ui::start_sidecar_writes(window, library, vec![write]); library_ui::start_sidecar_writes(window, library, vec![write]);
@@ -536,6 +554,15 @@ pub fn apply_stored_edit(
let mut slot = session.borrow_mut(); let mut slot = session.borrow_mut();
let Some(s) = slot.as_mut() else { return false }; let Some(s) = slot.as_mut() else { return false };
s.apply_version(version); s.apply_version(version);
// TRACES: FR-DEV-5
// And the snapshots taken of it, on this device or another.
s.set_snapshots(
sidecar
.snapshots_of(&version.uuid)
.into_iter()
.cloned()
.collect(),
);
} }
crate::sync_rows(window, rows, session); crate::sync_rows(window, rows, session);
@@ -1162,6 +1189,7 @@ mod tests {
&Preset::capture(&graph), &Preset::capture(&graph),
Scope::everything(), Scope::everything(),
Some(graph.masks()), Some(graph.masks()),
None,
) )
.unwrap(); .unwrap();
@@ -1209,6 +1237,7 @@ mod tests {
&Preset::capture(&edited()), &Preset::capture(&edited()),
Scope::everything(), Scope::everything(),
None, None,
None,
) )
.unwrap(); .unwrap();
@@ -1242,6 +1271,7 @@ mod tests {
&Preset::capture(&edited()), &Preset::capture(&edited()),
Scope::everything(), Scope::everything(),
None, None,
None,
) )
.unwrap(); .unwrap();
save_local( save_local(
@@ -1249,6 +1279,7 @@ mod tests {
&Preset::capture(&edited()), &Preset::capture(&edited()),
Scope::everything(), Scope::everything(),
None, None,
None,
) )
.unwrap(); .unwrap();
@@ -1267,6 +1298,7 @@ mod tests {
&Preset::capture(&edited()), &Preset::capture(&edited()),
Scope::everything(), Scope::everything(),
None, None,
None,
) )
.unwrap(); .unwrap();
let first = load_local(&image) let first = load_local(&image)
@@ -1279,6 +1311,7 @@ mod tests {
&Preset::capture(&edited()), &Preset::capture(&edited()),
Scope::everything(), Scope::everything(),
None, None,
None,
) )
.unwrap(); .unwrap();
let second = load_local(&image) let second = load_local(&image)
@@ -1313,6 +1346,7 @@ mod tests {
&Preset::capture(&edited()), &Preset::capture(&edited()),
Scope::everything(), Scope::everything(),
None, None,
None,
) )
.unwrap(); .unwrap();
@@ -1335,7 +1369,8 @@ mod tests {
&image, &image,
&Preset::capture(&edited()), &Preset::capture(&edited()),
Scope::everything(), Scope::everything(),
None None,
None,
) )
.is_err()); .is_err());
assert_eq!( assert_eq!(
@@ -1353,6 +1388,7 @@ mod tests {
&Preset::capture(&edited()), &Preset::capture(&edited()),
Scope::everything(), Scope::everything(),
None, None,
None,
) )
.unwrap(); .unwrap();
@@ -1638,4 +1674,60 @@ mod tests {
"the failed overwrite kept the new value" "the failed overwrite kept the new value"
); );
} }
/// TRACES: FR-DEV-5
/// A snapshot goes into the file beside the edit and comes back out as a
/// snapshot of it; deleting one takes it out of the file; and neither
/// touches the edit the photograph opens at.
#[test]
fn snapshots_survive_the_local_sidecar_and_leave_it_when_deleted() {
let dir = tempdir("snapshots");
let image = dir.join("IMG_0001.CR3");
std::fs::write(&image, b"").unwrap();
let graph = edited();
let mut liked = EditGraph::default_chain();
liked.set_param(
dr_pipeline::ops::exposure::ID,
dr_pipeline::ops::exposure::EXPOSURE,
1.5,
);
let snapshot = dr_pipeline::Version::from_graph("snap-1", "Brighter", &liked);
save_local(
&image,
&Preset::capture(&graph),
Scope::everything(),
None,
Some((&[snapshot], &[])),
)
.unwrap();
let text = std::fs::read_to_string(local_sidecar_path(&image)).unwrap();
let sidecar = Sidecar::parse(&text).unwrap();
let edit = sidecar.default_version().expect("the edit");
assert!(!edit.is_snapshot(), "the photograph opens at its edit");
let back = sidecar.snapshots_of(&edit.uuid);
assert_eq!(back.len(), 1);
assert_eq!(back[0].name, "Brighter");
assert_eq!(
back[0]
.params
.get(&("exposure".to_string(), "exposure".to_string())),
Some(&1.5)
);
save_local(
&image,
&Preset::capture(&graph),
Scope::everything(),
None,
Some((&[], &["snap-1".to_string()])),
)
.unwrap();
let text = std::fs::read_to_string(local_sidecar_path(&image)).unwrap();
let sidecar = Sidecar::parse(&text).unwrap();
let edit = sidecar.default_version().expect("the edit is still there");
assert!(sidecar.snapshots_of(&edit.uuid).is_empty(), "{text}");
}
} }
+152 -1
View File
@@ -19,7 +19,7 @@
import { Theme } from "theme.slint"; import { Theme } from "theme.slint";
import { Develop } from "session.slint"; import { Develop } from "session.slint";
import { PanelHeading, Caption, Label, Value, Button } from "widgets.slint"; import { PanelHeading, Caption, Label, Value, Button, Field } from "widgets.slint";
// One step, flattened for Slint's model system. // One step, flattened for Slint's model system.
export struct HistoryRow { export struct HistoryRow {
@@ -40,6 +40,105 @@ export struct HistoryRow {
undone: bool, undone: bool,
} }
/// TRACES: FR-DEV-5
/// One named snapshot of the open edit.
export struct SnapshotRow {
/// Routing back to the core. Opaque to this file.
id: string,
name: string,
/// The canvas is showing this one instead of the edit, while held.
comparing: bool,
}
/// TRACES: FR-DEV-5 | FR-DEV-7
/// A snapshot: press it to go back to it, hold the eye beside it to look.
component SnapshotItem inherits Rectangle {
in property <SnapshotRow> data;
in property <bool> enabled: true;
callback restored();
/// Both edges, like "Before": down shows the snapshot, up shows the edit.
callback compared(bool);
callback removed();
height: Theme.row-height;
background: root.data.comparing
? Theme.selected
: (touch.has-hover ? Theme.hover : transparent);
// Behind the two buttons, so a press on either does its own job and a
// press anywhere else on the row is the restore.
touch := TouchArea {
width: 100%;
height: max(parent.height, Theme.touch-target);
y: (parent.height - self.height) / 2;
enabled: root.enabled;
mouse-cursor: root.enabled ? MouseCursor.pointer : MouseCursor.default;
clicked => { root.restored(); }
}
HorizontalLayout {
padding-left: Theme.gap-sm;
padding-right: Theme.gap-sm;
spacing: Theme.gap-sm;
Label {
text: root.data.name;
emphasised: root.data.comparing || touch.has-hover;
horizontal-stretch: 1;
overflow: elide;
vertical-alignment: center;
}
// GESTURE: See the photograph as a snapshot had it
// where: Develop
// touch: Press and hold the eye beside the snapshot
// pointer: Press and hold the eye beside the snapshot
// why: The same hold as "Before", against a point the
// photographer chose rather than the file: "the version
// I liked twenty minutes ago" is how a choice between two
// treatments is actually made. It takes no history step
// and changes nothing; letting go puts the edit back.
//
// TRACES: FR-DEV-7
eye := TouchArea {
width: Theme.touch-target;
mouse-cursor: pointer;
enabled: root.enabled;
pointer-event(event) => {
if event.kind == PointerEventKind.down
&& event.button == PointerEventButton.left {
root.compared(true);
}
if event.kind == PointerEventKind.up
|| event.kind == PointerEventKind.cancel {
root.compared(false);
}
}
Label {
text: "◐";
emphasised: eye.has-hover || root.data.comparing;
horizontal-alignment: center;
vertical-alignment: center;
}
}
cut := TouchArea {
width: Theme.touch-target;
mouse-cursor: pointer;
enabled: root.enabled;
clicked => { root.removed(); }
Label {
text: "×";
emphasised: cut.has-hover;
horizontal-alignment: center;
vertical-alignment: center;
}
}
}
}
component StepRow inherits Rectangle { component StepRow inherits Rectangle {
in property <HistoryRow> data; in property <HistoryRow> data;
in property <bool> enabled: true; in property <bool> enabled: true;
@@ -115,6 +214,18 @@ export global Steps {
callback redo(); callback redo();
/// A row's own `index`, not its position in `rows`. /// A row's own `index`, not its position in `rows`.
callback picked(int); callback picked(int);
/// TRACES: FR-DEV-5
/// The named snapshots of the open edit, oldest first. Rust owns the
/// list; the panel only names, restores, deletes and holds them.
in property <[SnapshotRow]> snapshots;
/// The name typed for the next one. Empty is allowed: Rust numbers it.
callback snapshot-taken(string);
callback snapshot-restored(string);
callback snapshot-removed(string);
/// TRACES: FR-DEV-7
/// `true` on the way down, `false` on the way up.
callback snapshot-compared(string, bool);
} }
export component HistoryPanel inherits Rectangle { export component HistoryPanel inherits Rectangle {
@@ -189,6 +300,46 @@ export component HistoryPanel inherits Rectangle {
overflow: elide; overflow: elide;
} }
// TRACES: FR-DEV-5
// Snapshots above the steps: a snapshot is the state worth keeping
// out of a run of steps, so it sits where the steps lead to.
//
// GESTURE: Keep the photograph as it is now, under a name
// where: Develop
// touch: Type a name in the History panel and press Snapshot
// pointer: Type a name in the History panel and press Snapshot
// why: The history is forgotten with the sitting, on purpose;
// a snapshot is the photographer saying this one should
// not be. It is written into the sidecar as a version
// of the edit, so it survives a restart and reaches the
// other device. Pressing a snapshot puts the photograph
// back to it, as one step that undo takes back whole.
if Develop.enabled: HorizontalLayout {
spacing: Theme.gap-sm;
name := Field {
label: "Snapshot name";
placeholder: "Name this state";
horizontal-stretch: 1;
}
Button {
text: "Snapshot";
clicked => {
Steps.snapshot-taken(name.text);
name.text = "";
}
}
}
for snapshot in Steps.snapshots: SnapshotItem {
data: snapshot;
enabled: Develop.enabled;
restored => { Steps.snapshot-restored(snapshot.id); }
compared(down) => { Steps.snapshot-compared(snapshot.id, down); }
removed => { Steps.snapshot-removed(snapshot.id); }
}
if Develop.enabled && Steps.rows.length > 0: Rectangle { if Develop.enabled && Steps.rows.length > 0: Rectangle {
height: 1px; height: 1px;
background: Theme.rule; background: Theme.rule;