Files
dtourolleandClaude Opus 5 93efdf27a6 Keep the film stock when an edit is saved
`Version::update` is the write path an automatic save goes through. It copied
the parameters and the masks and said nothing about the film, so a photograph
developed on a stock was written back without it and opened the next time
without its emulsion. Nothing reported a failure — the line was simply not
there.

It is the third of the three routines that captured "the edit" and the only
one that got it wrong, which is the argument for not having three. All of them
now destructure one `EditState`, so `from_graph`, `update` and `apply` cannot
disagree about what an edit consists of, and the next part of one cannot be
lost by anybody writing a line too few.

Two tests, both of which fail without the fix: the field survives `update`,
and the stock survives the round trip through the file. `apply` returns the
`FilmRebake` it always implicitly owed, so `apply_version` now reads the debt
off the call rather than off `version.film` — and pays it in both directions,
since a version with no film has to clear the adjust pass too or it keeps
textures bound that nothing will sample.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-26 22:53:00 +02:00

299 lines
9.6 KiB
Rust

//! TRACES: FR-DEV-8 | FR-NC-9
//! Repairs must survive the sidecar, and survive two devices.
//!
//! The sidecar is the authoritative store (ARCH §6.12), so a spot that does not
//! round-trip is not a persistence bug but lost work — and a spot that
//! round-trips *nearly* is worse than one that fails, because a disc copied
//! from slightly the wrong place looks like a damaged file rather than like a
//! feature that did not run.
use dr_pipeline::spot::{Spot, SpotMode, DEFAULT_RADIUS};
use dr_pipeline::{EditGraph, Sidecar, Version};
fn spot_at(centre: (f32, f32), offset: (f32, f32)) -> Spot {
Spot::new(centre, offset, DEFAULT_RADIUS)
}
/// A graph carrying three repairs: the default kind, a clone, and one switched
/// off — which between them cover every field the line format carries.
fn graph_with_spots() -> EditGraph {
let mut graph = EditGraph::default_chain();
graph
.spots_mut()
.place(spot_at((0.25, 0.75), (0.08, -0.02)));
let mut cloned = spot_at((0.6, 0.4), (-0.12, 0.05));
cloned.mode = SpotMode::Clone;
cloned.set_feather(0.0);
cloned.set_opacity(0.5);
graph.spots_mut().place(cloned);
let mut off = spot_at((0.1, 0.1), (0.2, 0.2));
off.enabled = false;
graph.spots_mut().place(off);
graph
}
fn round_trip(graph: &EditGraph) -> EditGraph {
let mut sidecar = Sidecar::new();
sidecar.put(Version::from_graph("default", "Default", graph));
let text = sidecar.to_text();
let parsed = Sidecar::parse(&text).expect("reparse");
let mut restored = EditGraph::default_chain();
parsed
.versions
.get("default")
.expect("version survived")
.apply(&mut restored)
.expect_no_film();
restored
}
#[test]
fn every_field_of_every_repair_comes_back() {
let graph = graph_with_spots();
let restored = round_trip(&graph);
assert_eq!(
restored.spots().spots(),
graph.spots().spots(),
"a repair is eight numbers and every one of them matters"
);
}
/// The order is the order they were made in, which decides which repair lands
/// on top where two overlap and which of them may share a pass. Sorting by id
/// on the way out would look tidier and would reorder the photograph.
#[test]
fn the_order_they_were_made_in_survives() {
let graph = graph_with_spots();
let ids: Vec<&str> = graph
.spots()
.spots()
.iter()
.map(|s| s.id.as_str())
.collect();
let restored = round_trip(&graph);
let back: Vec<&str> = restored
.spots()
.spots()
.iter()
.map(|s| s.id.as_str())
.collect();
assert_eq!(ids, back);
}
/// What lets a caller skip an upload by comparing content: the same edit must
/// produce the same bytes, or every save looks like a change to sync.
#[test]
fn writing_the_same_repairs_twice_is_byte_identical() {
let graph = graph_with_spots();
let mut a = Sidecar::new();
a.put(Version::from_graph("default", "Default", &graph));
let mut b = Sidecar::new();
b.put(Version::from_graph("default", "Default", &graph));
assert_eq!(a.to_text(), b.to_text());
}
/// A frame nobody has repaired must not grow a line, in the same way an
/// operation at neutral contributes nothing.
#[test]
fn an_unrepaired_frame_writes_no_spot_lines() {
let mut sidecar = Sidecar::new();
sidecar.put(Version::from_graph(
"default",
"Default",
&EditGraph::default_chain(),
));
assert!(!sidecar.to_text().contains("spot."));
}
/// The file is meant to be readable by a human debugging an edit that went
/// wrong, and hand-editable by one who knows what they are doing.
#[test]
fn a_hand_written_line_loads() {
let text = "drsc 1\n\
\n\
[version default]\n\
name = Default\n\
default = 1\n\
revision = 1\n\
spot.abc123 = 0.4 0.6 0.02 0.5 0.1 -0.05 1 heal\n";
let sidecar = Sidecar::parse(text).expect("parse");
let spots = &sidecar.versions["default"].spots;
assert_eq!(spots.len(), 1);
let spot = &spots.spots()[0];
assert_eq!(spot.id, "abc123");
assert_eq!(spot.centre, (0.4, 0.6));
assert_eq!(spot.radius, 0.02);
assert_eq!(spot.feather, 0.5);
assert_eq!(spot.offset, (0.1, -0.05));
assert_eq!(spot.mode, SpotMode::Heal);
assert!(spot.enabled);
}
/// A truncated or hand-mangled line costs that repair and not the file. The
/// alternative — refusing the version — throws away every other repair, the
/// crop and the exposure over one bad line.
#[test]
fn a_malformed_line_costs_one_repair() {
let text = "drsc 1\n\
\n\
[version default]\n\
revision = 1\n\
exposure.exposure = 0.75\n\
spot.good11 = 0.4 0.6 0.02 0.5 0.1 -0.05 1 heal\n\
spot.short1 = 0.4 0.6 0.02\n\
spot.nomode = 0.4 0.6 0.02 0.5 0.1 -0.05 1 smudge\n\
spot.notnum = x y 0.02 0.5 0.1 -0.05 1 heal\n";
let sidecar = Sidecar::parse(text).expect("parse");
let version = &sidecar.versions["default"];
assert_eq!(version.spots.len(), 1);
assert_eq!(version.spots.spots()[0].id, "good11");
assert_eq!(
version.params.get(&("exposure".into(), "exposure".into())),
Some(&0.75),
"the rest of the edit still loaded"
);
}
/// A field a newer build added must not cost the repair. The disc is complete
/// without it, and refusing would be a device running behind deleting work it
/// merely does not understand.
#[test]
fn a_trailing_field_from_a_newer_build_is_ignored_not_refused() {
let text = "drsc 1\n\
\n\
[version default]\n\
revision = 1\n\
spot.abc123 = 0.4 0.6 0.02 0.5 0.1 -0.05 1 heal rotation=0.5\n";
let sidecar = Sidecar::parse(text).expect("parse");
assert_eq!(sidecar.versions["default"].spots.len(), 1);
}
/// A spot switched off is state the photographer set, not an absence.
#[test]
fn a_disabled_repair_stays_disabled() {
let graph = graph_with_spots();
let restored = round_trip(&graph);
let off = restored.spots().spots().last().expect("three repairs");
assert!(!off.enabled);
}
// ---------------------------------------------------------------------------
// Sync merge (FR-NC-9)
// ---------------------------------------------------------------------------
fn version_with(uuid: &str, revision: u64, build: impl FnOnce(&mut EditGraph)) -> Version {
let mut graph = EditGraph::default_chain();
build(&mut graph);
let mut v = Version::from_graph(uuid, "Default", &graph);
v.revision = revision;
v
}
/// The case the id derivation exists for. Two devices, offline, each removing a
/// different mark: with counted ids both would be `spot3` and one would be
/// lost here without a word.
#[test]
fn repairs_from_two_devices_both_survive() {
let base = version_with("default", 1, |_| {});
let mut ours = version_with("default", 2, |g| {
g.spots_mut().place(spot_at((0.2, 0.2), (0.05, 0.0)));
});
let theirs = version_with("default", 2, |g| {
g.spots_mut().place(spot_at((0.8, 0.8), (-0.05, 0.0)));
});
let conflicts = ours.merge(&theirs, Some(&base));
assert!(conflicts.is_empty(), "different marks are not a conflict");
assert_eq!(ours.spots.len(), 2);
}
/// And the other half of that: two devices that removed *the same* piece of
/// dust agree on the id, so the merge sees one repair — which is right, because
/// it is one repair, and the alternative is the same disc drawn twice.
#[test]
fn the_same_mark_removed_on_both_devices_stays_one_repair() {
let base = version_with("default", 1, |_| {});
let mut ours = version_with("default", 2, |g| {
g.spots_mut().place(spot_at((0.5, 0.5), (0.06, 0.0)));
});
let theirs = version_with("default", 3, |g| {
g.spots_mut().place(spot_at((0.5, 0.5), (0.06, 0.0)));
});
let conflicts = ours.merge(&theirs, Some(&base));
assert!(
conflicts.is_empty(),
"the same repair is not a disagreement"
);
assert_eq!(ours.spots.len(), 1);
}
/// Both devices dragging one repair's source is genuinely ambiguous: the two
/// offsets cannot be averaged into a third that either photographer wanted, so
/// the higher revision takes the spot whole.
#[test]
fn both_moving_one_repair_is_a_conflict_resolved_by_revision() {
let placed = spot_at((0.5, 0.5), (0.06, 0.0));
let id = placed.id.clone();
let base = version_with("default", 1, |g| {
g.spots_mut().place(placed.clone());
});
let mut ours = version_with("default", 2, |g| {
g.spots_mut().place(placed.clone());
g.spots_mut().get_mut(&id).unwrap().set_offset((0.2, 0.0));
});
let theirs = version_with("default", 5, |g| {
g.spots_mut().place(placed.clone());
g.spots_mut().get_mut(&id).unwrap().set_offset((0.0, -0.2));
});
let conflicts = ours.merge(&theirs, Some(&base));
assert_eq!(conflicts, vec![("spot".to_string(), id.clone())]);
assert_eq!(
ours.spots.get(&id).map(|s| s.offset),
Some((0.0, -0.2)),
"the higher revision wins the repair whole"
);
}
/// A repair deleted on one device and untouched on the other stays deleted —
/// the disjoint case again, in the direction that is easy to get backwards.
#[test]
fn a_deletion_propagates() {
let placed = spot_at((0.5, 0.5), (0.06, 0.0));
let id = placed.id.clone();
let base = version_with("default", 1, |g| {
g.spots_mut().place(placed.clone());
});
let mut ours = version_with("default", 2, |g| {
g.spots_mut().place(placed.clone());
});
let theirs = version_with("default", 3, |_| {});
assert!(ours.merge(&theirs, Some(&base)).is_empty());
assert!(ours.spots.get(&id).is_none());
}