diff --git a/core/dr-pipeline/src/graph.rs b/core/dr-pipeline/src/graph.rs index b4b68fa..09779e5 100644 --- a/core/dr-pipeline/src/graph.rs +++ b/core/dr-pipeline/src/graph.rs @@ -11,7 +11,8 @@ use crate::descriptor::{ Facet, LocalizedKey, OpDescriptor, OpId, ParamId, ParamKind, Presentation, }; use crate::framing::{CropRect, Framing}; -use crate::operation::{compose_with_framing, ComposedShader, Operation}; +use crate::mask::MaskStack; +use crate::operation::{compose_full, ComposedShader, Operation}; use crate::ops; /// TRACES: FR-DEV-3a @@ -74,6 +75,13 @@ pub struct EditGraph { /// pixel that colour is read from — and changes the output's dimensions, /// which no colour operation can do. See [`crate::framing`]. framing: Framing, + /// The local adjustments (FR-DEV-3). + /// + /// Also apart from `ops`, and for a sharper reason than framing's: each + /// layer *contains* a chain of its own. Folding the stack into the global + /// list would make the list recursive and every consumer that walks it + /// have to know that some entries are really sub-graphs. + masks: MaskStack, } impl EditGraph { @@ -94,9 +102,19 @@ impl EditGraph { Self { ops: ops::chain(), framing: Framing::new(), + masks: MaskStack::new(), } } + /// The local adjustment stack. + pub fn masks(&self) -> &MaskStack { + &self.masks + } + + pub fn masks_mut(&mut self) -> &mut MaskStack { + &mut self.masks + } + /// The framing — crop, straighten, rotation and flips. /// /// Reached directly rather than through `set_param` because the crop is a @@ -251,6 +269,10 @@ impl EditGraph { } } self.framing.reset(); + // Masks go too, and this is why `apply` can be a replacement rather + // than an overlay: a sidecar with no mask blocks means an edit with no + // local adjustments, not an edit that keeps whatever was on screen. + self.masks = MaskStack::new(); } /// Set the crop rectangle. Clamped to keep it inside the frame. @@ -282,7 +304,9 @@ impl EditGraph { /// makes the framing active without changing the image, and reporting a /// merely-zoomed image as edited would mark a clean file dirty. pub fn is_neutral(&self) -> bool { - !self.ops.iter().any(|o| o.is_active()) && !self.framing.edits_image() + !self.ops.iter().any(|o| o.is_active()) + && !self.framing.edits_image() + && self.masks.is_neutral() } /// Generate the fused shader for the current state, encoded to sRGB. @@ -302,7 +326,7 @@ impl EditGraph { /// graph renders to the screen and to a file in the same breath, and the /// two want different answers. pub fn compose_for(&self, output: dr_types::ColourSpace) -> ComposedShader { - compose_with_framing(&self.ops, &self.framing, output) + compose_full(&self.ops, &self.framing, output, &self.masks) } } diff --git a/core/dr-pipeline/src/sidecar.rs b/core/dr-pipeline/src/sidecar.rs index 3350848..16a50d4 100644 --- a/core/dr-pipeline/src/sidecar.rs +++ b/core/dr-pipeline/src/sidecar.rs @@ -67,6 +67,7 @@ use std::fmt; use std::fmt::Write as _; use crate::graph::EditGraph; +use crate::mask::{MaskLayer, MaskSource, MaskStack}; use crate::preset::{resolve, Preset}; /// Format version of the document itself. @@ -143,6 +144,17 @@ pub struct Version { pub flag: u8, /// The edit itself: `(op, param) -> value`, non-default values only. pub params: BTreeMap<(String, String), f32>, + /// TRACES: FR-DEV-3 | FR-NC-9 + /// The local adjustments. + /// + /// Written as its own `[mask]` blocks rather than folded into + /// [`Self::params`], because a layer is not a scalar: it carries a + /// selection, a geometry, and a chain of its own. Flattening it into + /// dotted keys would encode a list of region ids as something like + /// `m1.region.0 = 12`, which is neither readable nor mergeable — and + /// per-field merge under FR-NC-9 is most of the reason the ids are stored + /// as ids at all. + pub masks: MaskStack, /// Keys this build did not recognise, kept verbatim. /// /// An operation this build lacks would otherwise be deleted the moment an @@ -168,6 +180,7 @@ impl Version { rating: 0, flag: 0, params: capture(graph), + masks: graph.masks().clone(), unknown: BTreeMap::new(), } } @@ -195,6 +208,7 @@ impl Version { }; graph.set_param(op, param, *value); } + *graph.masks_mut() = self.masks.clone(); } /// Record `graph` into this version, bumping the revision. @@ -204,11 +218,86 @@ impl Version { /// did not bump it is a local edit a remote one will silently win. pub fn update(&mut self, graph: &EditGraph, device: &str, now: i64) { self.params = capture(graph); + self.masks = graph.masks().clone(); self.revision = self.revision.saturating_add(1); self.device = device.to_string(); self.modified = now; } + /// Merge the mask stacks, returning the layers that genuinely conflicted. + /// + /// Reported as `("mask", id)` so a caller surfacing conflicts can show + /// them in the same list as contested parameters without needing a second + /// channel for them. + fn merge_masks( + &mut self, + remote: &Version, + base: Option<&Version>, + remote_wins: bool, + ) -> Vec<(String, String)> { + let empty = MaskStack::new(); + let base_masks = base.map(|b| &b.masks).unwrap_or(&empty); + let mut conflicts = Vec::new(); + + let ids: Vec = self + .masks + .layers() + .iter() + .chain(remote.masks.layers()) + .map(|l| l.id.clone()) + .collect::>() + .into_iter() + .collect(); + + for id in ids { + let ours = self.masks.get(&id); + let theirs = remote.masks.get(&id); + let was = base_masks.get(&id); + + let we_changed = ours != was; + let they_changed = theirs != was; + + match (we_changed, they_changed) { + // Only they touched it: take theirs, including a deletion. + (false, true) => match theirs { + Some(layer) => self.put_mask(layer.clone()), + None => { + self.masks.remove(&id); + } + }, + (true, true) if ours != theirs => { + conflicts.push(("mask".to_string(), id.clone())); + if remote_wins { + match theirs { + Some(layer) => self.put_mask(layer.clone()), + None => { + self.masks.remove(&id); + } + } + } + } + _ => {} + } + } + + conflicts + } + + /// Replace a layer of the same id, or append it. + /// + /// Position is not merged. Two devices that reordered the same stack have + /// no combined order that is either one's, and layer order only decides + /// which of two *overlapping* masks composites last — a much smaller + /// wrong than losing a layer. + fn put_mask(&mut self, layer: MaskLayer) { + match self.masks.get_mut(&layer.id) { + Some(existing) => *existing = layer, + None => { + self.masks.push(layer); + } + } + } + /// TRACES: FR-NC-9 /// Merge a remote version into this one at the node level. /// @@ -292,6 +381,18 @@ impl Version { self.rating = merge_judgement(self.rating, remote.rating, remote_wins); self.flag = merge_judgement(self.flag, remote.flag, remote_wins); + // Masks merge by layer id, which is the same disjoint-survives rule + // the parameters follow one level up: a layer added on the phone and + // a layer added on the desktop are different ids, so both survive and + // neither is a conflict. + // + // A layer *both* sides edited resolves wholesale to the higher + // revision rather than field by field. Two people's versions of one + // mask cannot be interleaved into a third — half of one selection + // plus half of another's opacity is a layer neither of them made — + // so the layer is the unit, exactly as the value is for a parameter. + conflicts.extend(self.merge_masks(remote, base, remote_wins)); + // Unknown keys follow the same rule, so an operation neither side // understands is not dropped by the merge either. for (k, v) in &remote.unknown { @@ -411,6 +512,9 @@ impl Sidecar { for (k, raw) in &v.unknown { let _ = writeln!(out, "{k} = {raw}"); } + for layer in v.masks.layers() { + write_mask(&mut out, &v.uuid, layer); + } } out } @@ -435,6 +539,13 @@ impl Sidecar { let mut sidecar = Sidecar::new(); let mut current: Option = None; + // Mask blocks are collected rather than attached as they are read. + // They name their version explicitly, so they need not follow it in + // the file — and a `[mask]` block for a version that never appears is + // then simply dropped instead of corrupting whichever version + // happened to be open. + let mut mask: Option = None; + let mut masks: Vec<(String, MaskLayer)> = Vec::new(); for raw in lines { let line = raw.trim(); @@ -446,6 +557,7 @@ impl Sidecar { .strip_prefix("[version ") .and_then(|s| s.strip_suffix(']')) { + masks.extend(mask.take().and_then(PartialMask::finish)); if let Some(v) = current.take() { sidecar.put(v); } @@ -456,6 +568,24 @@ impl Sidecar { continue; } + if let Some(head) = line + .strip_prefix("[mask ") + .and_then(|s| s.strip_suffix(']')) + { + masks.extend(mask.take().and_then(PartialMask::finish)); + match head.split_once(char::is_whitespace) { + Some((version, id)) => { + mask = Some(PartialMask::new(version.trim(), id.trim())); + } + // A header missing one of its two names cannot be + // attached to anything. Dropped with a warning rather + // than guessed at, since guessing would put someone + // else's adjustment on this photograph. + None => log::warn!("sidecar: malformed mask header '{line}'; ignoring"), + } + continue; + } + let Some((key, value)) = line.split_once('=') else { // Not a key-value line and not a block header. Keep it so a // newer format's construct survives a round trip here. @@ -469,6 +599,11 @@ impl Sidecar { }; let (key, value) = (key.trim(), value.trim()); + if let Some(m) = mask.as_mut() { + m.set(key, value); + continue; + } + let Some(version) = current.as_mut() else { sidecar.unknown_blocks.push(line.to_string()); continue; @@ -504,13 +639,253 @@ impl Sidecar { }, } } + masks.extend(mask.take().and_then(PartialMask::finish)); if let Some(v) = current.take() { sidecar.put(v); } + + for (uuid, layer) in masks { + match sidecar.versions.get_mut(&uuid) { + Some(v) => { + v.masks.push(layer); + } + None => log::warn!( + "sidecar: mask {} names version {uuid}, which is not in this file; ignoring", + layer.id + ), + } + } + Ok(sidecar) } } + +/// Write one mask layer as its own block. +/// +/// The version uuid is repeated in the header rather than relying on the +/// block's position in the file. A sidecar is edited by hand, merged by two +/// devices, and round-tripped by builds that do not know what a mask is — +/// under all three, "belongs to whichever version appeared above me" is a +/// relationship that quietly breaks. Naming it costs one field. +fn write_mask(out: &mut String, version: &str, layer: &MaskLayer) { + let _ = write!(out, "\n[mask {version} {}]\n", layer.id); + if !layer.name.is_empty() { + let _ = writeln!(out, "name = {}", layer.name); + } + let _ = writeln!(out, "source = {}", layer.source.kind()); + + match &layer.source { + MaskSource::Regions { + signature, + level, + ids, + } => { + let _ = writeln!(out, "signature = {signature}"); + let _ = writeln!(out, "level = {level}"); + // One space-separated line rather than a key per id: a selection + // is a few hundred numbers, and three hundred lines of + // `region.7 = 1` would bury the rest of the file. + let list: Vec = ids.iter().map(|i| i.to_string()).collect(); + let _ = writeln!(out, "regions = {}", list.join(" ")); + } + MaskSource::Linear { + centre, + angle, + width, + } => { + let _ = writeln!( + out, + "centre = {} {}", + format_value(centre.0), + format_value(centre.1) + ); + let _ = writeln!(out, "angle = {}", format_value(*angle)); + let _ = writeln!(out, "width = {}", format_value(*width)); + } + MaskSource::Radial { + centre, + radii, + angle, + feather, + } => { + let _ = writeln!( + out, + "centre = {} {}", + format_value(centre.0), + format_value(centre.1) + ); + let _ = writeln!( + out, + "radii = {} {}", + format_value(radii.0), + format_value(radii.1) + ); + let _ = writeln!(out, "angle = {}", format_value(*angle)); + let _ = writeln!(out, "feather = {}", format_value(*feather)); + } + } + + if layer.invert { + let _ = writeln!(out, "invert = 1"); + } + if layer.opacity != 1.0 { + let _ = writeln!(out, "opacity = {}", format_value(layer.opacity)); + } + if !layer.enabled { + let _ = writeln!(out, "enabled = 0"); + } + for (op, param, value) in layer.params() { + let _ = writeln!(out, "{op}.{param} = {}", format_value(value)); + } +} + +/// A mask block being read, before it is complete enough to be a layer. +/// +/// Separate from [`MaskLayer`] because the source cannot be built until every +/// one of its fields has been seen, and the fields arrive one line at a time +/// in whatever order the writer chose. +struct PartialMask { + version: String, + id: String, + name: String, + kind: String, + signature: u64, + level: u32, + ids: Vec, + centre: (f32, f32), + radii: (f32, f32), + angle: f32, + width: f32, + feather: f32, + invert: bool, + opacity: f32, + enabled: bool, + params: Vec<(String, String, f32)>, +} + +impl PartialMask { + fn new(version: &str, id: &str) -> Self { + Self { + version: version.to_string(), + id: id.to_string(), + name: String::new(), + kind: String::new(), + signature: 0, + level: 0, + ids: Vec::new(), + centre: (0.5, 0.5), + radii: (0.25, 0.25), + angle: 0.0, + width: 0.0, + feather: 0.0, + invert: false, + opacity: 1.0, + enabled: true, + params: Vec::new(), + } + } + + fn set(&mut self, key: &str, value: &str) { + match key { + "name" => self.name = value.to_string(), + "source" => self.kind = value.to_string(), + "signature" => self.signature = value.parse().unwrap_or(0), + "level" => self.level = value.parse().unwrap_or(0), + "regions" => { + self.ids = value + .split_whitespace() + .filter_map(|t| t.parse().ok()) + .collect(); + // Sorted and deduplicated on the way in rather than trusted + // from the file: the mask's identity is the *set*, and a + // hand-edited or merged line arriving out of order would + // otherwise be a different cache key for the same selection. + self.ids.sort_unstable(); + self.ids.dedup(); + } + "centre" => self.centre = pair(value).unwrap_or(self.centre), + "radii" => self.radii = pair(value).unwrap_or(self.radii), + "angle" => self.angle = value.parse().unwrap_or(0.0), + "width" => self.width = value.parse().unwrap_or(0.0), + "feather" => self.feather = value.parse().unwrap_or(0.0), + "invert" => self.invert = value != "0", + "opacity" => self.opacity = value.parse::().unwrap_or(1.0).clamp(0.0, 1.0), + "enabled" => self.enabled = value != "0", + _ => match (key.split_once('.'), value.parse::()) { + (Some((op, param)), Ok(v)) if v.is_finite() => { + self.params.push((op.to_string(), param.to_string(), v)); + } + _ => log::warn!("sidecar: unreadable mask key {key}; ignoring"), + }, + } + } + + /// Build the layer, or `None` if the source kind is one this build has + /// never heard of — a newer format's mask type, which is skipped rather + /// than guessed at. + fn finish(self) -> Option<(String, MaskLayer)> { + let source = match self.kind.as_str() { + "regions" => MaskSource::Regions { + signature: self.signature, + level: self.level, + ids: self.ids, + }, + "linear" => MaskSource::Linear { + centre: self.centre, + angle: self.angle, + width: self.width, + }, + "radial" => MaskSource::Radial { + centre: self.centre, + radii: self.radii, + angle: self.angle, + feather: self.feather, + }, + other => { + log::warn!( + "sidecar: unknown mask source '{other}'; skipping layer {}", + self.id + ); + return None; + } + }; + + let mut layer = MaskLayer::new(self.id, source); + layer.name = self.name; + layer.invert = self.invert; + layer.opacity = self.opacity; + layer.enabled = self.enabled; + for (op, param, value) in &self.params { + // `ParamId` holds a `&'static str` and this one came off disk, so + // it is matched against the descriptors and the *static* id is + // what reaches the operation — exactly what `resolve` does for the + // global chain. + let Some(id) = layer + .ops + .iter() + .find(|o| o.descriptor().id.0 == op) + .and_then(|o| o.descriptor().params.iter().find(|p| p.id.0 == param)) + .map(|p| p.id) + else { + log::warn!("sidecar: unknown mask parameter {op}.{param}; ignoring"); + continue; + }; + layer.set_param(op, id, *value); + } + + Some((self.version, layer)) + } +} + +/// Two whitespace-separated floats. +fn pair(value: &str) -> Option<(f32, f32)> { + let mut it = value.split_whitespace(); + let a = it.next()?.parse().ok()?; + let b = it.next()?.parse().ok()?; + Some((a, b)) +} + /// Format a value without a trailing `.0` on whole numbers, and without /// exponent notation — both so the file stays diffable and hand-readable. fn format_value(v: f32) -> String { diff --git a/core/dr-pipeline/tests/mask_sidecar.rs b/core/dr-pipeline/tests/mask_sidecar.rs new file mode 100644 index 0000000..d3af680 --- /dev/null +++ b/core/dr-pipeline/tests/mask_sidecar.rs @@ -0,0 +1,385 @@ +//! Local adjustments must survive the sidecar. +//! +//! The sidecar is the authoritative store (ARCH §6.1) — the catalog is a +//! disposable index and the RAW is never written. So a mask that does not +//! round-trip is not a persistence bug, it is lost work, and the tests here +//! are about the ways that happens quietly rather than loudly. + +use dr_pipeline::descriptor::ParamId; +use dr_pipeline::mask::{MaskLayer, MaskSource}; +use dr_pipeline::{EditGraph, Sidecar, Version}; + +fn regions(ids: &[u32]) -> MaskSource { + MaskSource::Regions { + signature: 0xdead_beef, + level: 300, + ids: ids.to_vec(), + } +} + +/// A graph with one masked adjustment on it. +fn graph_with_mask() -> EditGraph { + let mut graph = EditGraph::default_chain(); + let mut layer = MaskLayer::new("m1", regions(&[3, 7, 12])); + layer.name = "Subject".into(); + layer.set_param("exposure", ParamId("exposure"), 0.75); + graph.masks_mut().push(layer); + 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); + restored +} + +#[test] +fn a_region_mask_survives_a_round_trip() { + let graph = graph_with_mask(); + let restored = round_trip(&graph); + + let layers = restored.masks().layers(); + assert_eq!(layers.len(), 1, "the layer came back"); + + let layer = &layers[0]; + assert_eq!(layer.id, "m1"); + assert_eq!(layer.name, "Subject"); + assert_eq!(layer.source, regions(&[3, 7, 12])); + assert_eq!(layer.ops.iter().find(|o| o.descriptor().id.0 == "exposure").map(|o| o.param(ParamId("exposure"))), Some(0.75)); +} + +#[test] +fn writing_the_same_state_twice_is_byte_identical() { + // What lets a caller skip an upload by comparing content rather than + // trusting a dirty flag — the property the whole `versions` ordering + // exists for, now that masks are in the file too. + let graph = graph_with_mask(); + let mut a = Sidecar::new(); + a.put(Version::from_graph("default", "Default", &graph)); + + let once = a.to_text(); + let twice = Sidecar::parse(&once).expect("reparse").to_text(); + assert_eq!(once, twice); +} + +#[test] +fn gradients_keep_their_geometry() { + let mut graph = EditGraph::default_chain(); + + let mut linear = MaskLayer::new( + "m1", + MaskSource::Linear { + centre: (0.25, 0.75), + angle: 1.25, + width: 0.4, + }, + ); + linear.set_param("exposure", ParamId("exposure"), -1.0); + graph.masks_mut().push(linear); + + let mut radial = MaskLayer::new( + "m2", + MaskSource::Radial { + centre: (0.6, 0.4), + radii: (0.3, 0.15), + angle: -0.5, + feather: 0.65, + }, + ); + radial.set_param("exposure", ParamId("exposure"), 0.5); + graph.masks_mut().push(radial); + + let restored = round_trip(&graph); + let layers = restored.masks().layers(); + assert_eq!(layers.len(), 2); + assert_eq!(layers[0].source, graph.masks().layers()[0].source); + assert_eq!(layers[1].source, graph.masks().layers()[1].source); +} + +#[test] +fn flags_and_opacity_survive() { + let mut graph = EditGraph::default_chain(); + let mut layer = MaskLayer::new("m1", regions(&[1])); + layer.set_param("exposure", ParamId("exposure"), 1.0); + layer.invert = true; + layer.opacity = 0.35; + layer.enabled = false; + graph.masks_mut().push(layer); + + let restored = round_trip(&graph); + let layer = &restored.masks().layers()[0]; + assert!(layer.invert); + assert!((layer.opacity - 0.35).abs() < 1e-6); + assert!(!layer.enabled, "a disabled layer must stay disabled, not vanish"); +} + +/// A selection's identity is the *set*. Two files naming the same regions in +/// different orders describe one mask and must compare equal, or the two +/// devices that wrote them will fight forever over a difference that is not +/// one. +#[test] +fn region_order_is_normalised_on_read() { + let text = "drsc 1\n\ + \n[version default]\n\ + name = Default\n\ + revision = 1\n\ + modified = 0\n\ + \n[mask default m1]\n\ + source = regions\n\ + signature = 5\n\ + level = 10\n\ + regions = 12 3 7 3\n\ + exposure.exposure = 1\n"; + + let parsed = Sidecar::parse(text).expect("parse"); + let layer = &parsed.versions["default"].masks.layers()[0]; + assert_eq!( + layer.source, + MaskSource::Regions { + signature: 5, + level: 10, + ids: vec![3, 7, 12], + }, + "ids sort and deduplicate on the way in" + ); +} + +/// The version skew case. An older build must not delete a mask type it has +/// never heard of *silently* — but neither may it apply one it cannot read. +#[test] +fn an_unknown_mask_source_is_skipped_not_guessed() { + let text = "drsc 1\n\ + \n[version default]\n\ + name = Default\n\ + revision = 1\n\ + modified = 0\n\ + \n[mask default m1]\n\ + source = luminosity\n\ + threshold = 0.5\n\ + exposure.exposure = 1\n"; + + let parsed = Sidecar::parse(text).expect("parse"); + assert!( + parsed.versions["default"].masks.is_empty(), + "an unreadable mask must not become a wrong one" + ); +} + +#[test] +fn a_mask_naming_no_known_version_is_dropped() { + let text = "drsc 1\n\ + \n[version default]\n\ + name = Default\n\ + revision = 1\n\ + modified = 0\n\ + \n[mask ghost m1]\n\ + source = regions\n\ + signature = 1\n\ + level = 1\n\ + regions = 1\n"; + + let parsed = Sidecar::parse(text).expect("parse"); + assert!(parsed.versions["default"].masks.is_empty()); +} + +/// Masks name their version rather than relying on file order, so a block +/// that appears before its version still lands on it. +#[test] +fn a_mask_block_need_not_follow_its_version() { + let text = "drsc 1\n\ + \n[mask second m1]\n\ + source = regions\n\ + signature = 1\n\ + level = 1\n\ + regions = 4 5\n\ + exposure.exposure = 1\n\ + \n[version first]\n\ + name = First\n\ + revision = 1\n\ + modified = 0\n\ + \n[version second]\n\ + name = Second\n\ + revision = 1\n\ + modified = 0\n"; + + let parsed = Sidecar::parse(text).expect("parse"); + assert!(parsed.versions["first"].masks.is_empty()); + assert_eq!( + parsed.versions["second"].masks.len(), + 1, + "the block named its version and should have reached it" + ); +} + +/// Loading is a replacement, not an overlay: opening an unedited image after +/// an edited one must not leave the previous image's masks on screen. +#[test] +fn applying_a_maskless_version_clears_existing_masks() { + let mut graph = graph_with_mask(); + assert_eq!(graph.masks().len(), 1); + + let plain = Version::from_graph("clean", "Clean", &EditGraph::default_chain()); + plain.apply(&mut graph); + assert!(graph.masks().is_empty()); +} + +#[test] +fn a_graph_with_only_a_masked_edit_is_not_neutral() { + let graph = graph_with_mask(); + assert!(!graph.is_neutral(), "a local adjustment is still an edit"); +} + +/// A bare selection with nothing applied to it changes no pixel, so the image +/// is unedited — but the layer must still be written, or the selection the +/// user made is lost on reload. +#[test] +fn a_selection_with_no_adjustment_still_persists() { + let mut graph = EditGraph::default_chain(); + graph.masks_mut().push(MaskLayer::new("m1", regions(&[9]))); + assert!(graph.is_neutral(), "no adjustment means no pixel changes"); + + let restored = round_trip(&graph); + assert_eq!( + restored.masks().len(), + 1, + "the selection is work and must survive even though it renders nothing" + ); +} + +#[test] +fn unknown_top_level_keys_still_round_trip_alongside_masks() { + let text = "drsc 1\n\ + \n[version default]\n\ + name = Default\n\ + revision = 1\n\ + modified = 0\n\ + future.thing = 3\n\ + \n[mask default m1]\n\ + source = regions\n\ + signature = 1\n\ + level = 1\n\ + regions = 2\n\ + exposure.exposure = 1\n"; + + let out = Sidecar::parse(text).expect("parse").to_text(); + assert!(out.contains("future.thing"), "unknown keys are still preserved"); + assert!(out.contains("[mask default m1]")); +} + +// --------------------------------------------------------------------------- +// 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 +} + +fn lit(id: &str, ids: &[u32], ev: f32) -> MaskLayer { + let mut layer = MaskLayer::new(id, regions(ids)); + layer.set_param("exposure", ParamId("exposure"), ev); + layer +} + +/// The case the whole key-wise merge exists for, one level up: a layer added +/// on the phone and a layer added on the desktop are not a conflict. +#[test] +fn disjoint_layers_from_two_devices_both_survive() { + let base = version_with("default", 1, |_| {}); + + let mut ours = version_with("default", 2, |g| { + g.masks_mut().push(lit("m1", &[1], 1.0)); + }); + let theirs = version_with("default", 2, |g| { + g.masks_mut().push(lit("m2", &[2], -1.0)); + }); + + let conflicts = ours.merge(&theirs, Some(&base)); + assert!(conflicts.is_empty(), "different layers are not a conflict"); + assert_eq!(ours.masks.len(), 2); + assert!(ours.masks.get("m1").is_some()); + assert!(ours.masks.get("m2").is_some()); +} + +#[test] +fn a_layer_only_the_remote_added_arrives() { + let base = version_with("default", 1, |_| {}); + let mut ours = version_with("default", 2, |_| {}); + let theirs = version_with("default", 3, |g| { + g.masks_mut().push(lit("m1", &[5], 2.0)); + }); + + assert!(ours.merge(&theirs, Some(&base)).is_empty()); + assert_eq!(ours.masks.len(), 1); +} + +#[test] +fn both_editing_one_layer_is_a_conflict_resolved_by_revision() { + let base = version_with("default", 1, |g| { + g.masks_mut().push(lit("m1", &[1], 0.5)); + }); + + let mut ours = version_with("default", 2, |g| { + g.masks_mut().push(lit("m1", &[1], 1.0)); + }); + let theirs = version_with("default", 9, |g| { + g.masks_mut().push(lit("m1", &[1], -1.0)); + }); + + let conflicts = ours.merge(&theirs, Some(&base)); + assert_eq!(conflicts, vec![("mask".to_string(), "m1".to_string())]); + + let layer = ours.masks.get("m1").expect("layer survived"); + let ev = layer + .ops + .iter() + .find(|o| o.descriptor().id.0 == "exposure") + .map(|o| o.param(ParamId("exposure"))); + assert_eq!(ev, Some(-1.0), "the higher revision wins the layer whole"); +} + +/// A layer deleted on one device and untouched on the other must stay +/// deleted, or a mask the user removed reappears on every sync. +#[test] +fn a_remote_deletion_is_honoured() { + let base = version_with("default", 1, |g| { + g.masks_mut().push(lit("m1", &[1], 1.0)); + }); + let mut ours = version_with("default", 2, |g| { + g.masks_mut().push(lit("m1", &[1], 1.0)); + }); + let theirs = version_with("default", 3, |_| {}); + + assert!(ours.merge(&theirs, Some(&base)).is_empty()); + assert!(ours.masks.is_empty(), "the deletion should not be undone"); +} + +#[test] +fn an_untouched_layer_is_left_alone() { + let base = version_with("default", 1, |g| { + g.masks_mut().push(lit("m1", &[1], 1.0)); + }); + let mut ours = version_with("default", 2, |g| { + g.masks_mut().push(lit("m1", &[1], 1.0)); + g.masks_mut().push(lit("m2", &[2], 0.5)); + }); + let theirs = version_with("default", 3, |g| { + g.masks_mut().push(lit("m1", &[1], 1.0)); + }); + + assert!(ours.merge(&theirs, Some(&base)).is_empty()); + assert_eq!(ours.masks.len(), 2, "our new layer is not a conflict"); +}