dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Let one mask be built from more than one selection, and paint into it

Browse Source 
A mask the model draws arrives approximately right — stopping inside a
shoulder, leaking into the hair — and FR-DEV-3's edge controls move the
*whole* boundary, so no value of feather or dilation fixes two errors that
go opposite ways. What fixes them is a second selection joined to the first,
and a layer that held exactly one source had nowhere to put one. The brush
the core has had all along was reachable from no control in the application.

A layer is now an ordered list of parts. Each names a source and how it
joins the mask before it — added to it, or taken out of it — and carries its
own edge treatment, because a model's soft coverage and a stroke painted
where it stopped short do not want the same feather. Invert and opacity stay
on the layer, where the composed shader already reads them.

The sidecar grows `[part]` blocks and nothing else. A layer of one part
writes exactly the bytes it always did; a mask block with no part blocks
after it reads back as one part; and a stroke, a join or a source this build
cannot read costs that part rather than the layer. So every sidecar in every
library still parses to the edit it always was.

On the device the parts fold into the layer's one slice, so eight layers
still cost eight channels: union is a `max` blend and subtraction is the
erase blend the brush already used. A part is drawn into a scratch texture
before it is joined, and that is not incidental — an erase stroke means a
hole in *that part*, not a hole in the mask, and drawn straight onto the
accumulator it would punch through the subject underneath. A layer of one
part skips all of it and takes the path it always took.

In the interface: a part list under the selected layer with a chip saying
which way each joins, Add and Subtract beside it, a Select/Paint/Erase strip
with the brush's size, hardness and flow, and a drag on the photograph that
paints. Pressing Paint on a mask that cannot hold a stroke joins a part that
can, rather than explaining that a subject is not a brush. A whole stroke is
one step in the history.

The edge controls now shape the part that is selected rather than the layer,
which is the one behaviour change to an existing control: with a correction
selected, the feather slider softens the correction and leaves the model's
mask alone.
This commit is contained in:
dtourolle
2026-09-07 20:00:40 +02:00
parent 9ede23073d
commit df741a8a49
20 changed files with 2998 additions and 654 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/dr-pipeline/tests/mask_sidecar.rs
+319 -44
View File
@@ -5,8 +5,8 @@
//! 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::descriptor::{OpId, ParamId};
use dr_pipeline::mask::{Join, MaskLayer, MaskPart, MaskSource, Morphology};
use dr_pipeline::{EditGraph, Sidecar, Version};
fn regions(ids: &[u32]) -> MaskSource {
@@ -55,7 +55,7 @@ fn a_region_mask_survives_a_round_trip() {
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.base().source, regions(&[3, 7, 12]));
assert_eq!(
layer
.ops
@@ -110,8 +110,14 @@ fn gradients_keep_their_geometry() {
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);
assert_eq!(
layers[0].base().source,
graph.masks().layers()[0].base().source
);
assert_eq!(
layers[1].base().source,
graph.masks().layers()[1].base().source
);
}
#[test]
@@ -155,7 +161,7 @@ fn region_order_is_normalised_on_read() {
let parsed = Sidecar::parse(text).expect("parse");
let layer = &parsed.versions["default"].masks.layers()[0];
assert_eq!(
layer.source,
layer.base().source,
MaskSource::Regions {
signature: 5,
level: 10,
@@ -302,11 +308,11 @@ fn brushed(id: &str, gestures: &[Gesture]) -> MaskLayer {
let mut layer = MaskLayer::new(id, MaskSource::brush());
layer.set_param("exposure", ParamId("exposure"), 1.0);
for (erase, radius, path) in gestures {
layer.begin_stroke(*erase, *radius, 0.5, 1.0);
layer.begin_stroke(0, *erase, *radius, 0.5, 1.0);
for &(x, y) in path {
layer.extend_stroke(x, y);
layer.extend_stroke(0, x, y);
}
layer.end_stroke();
layer.end_stroke(0);
}
layer
}
@@ -440,7 +446,10 @@ fn an_unpainted_brush_layer_still_persists() {
let restored = round_trip(&graph);
assert_eq!(restored.masks().len(), 1);
assert_eq!(restored.masks().layers()[0].source, MaskSource::brush());
assert_eq!(
restored.masks().layers()[0].base().source,
MaskSource::brush()
);
}
// ---------------------------------------------------------------------------
@@ -576,10 +585,10 @@ fn show_a_sidecar() {
);
subject.name = "person".into();
subject.invert = true;
subject.feather = 0.02;
subject.falloff = dr_pipeline::mask::Falloff::Gaussian;
subject.morphology = dr_pipeline::mask::Morphology::Dilate;
subject.morph_radius = 0.012;
subject.base_mut().feather = 0.02;
subject.base_mut().falloff = dr_pipeline::mask::Falloff::Gaussian;
subject.base_mut().morphology = dr_pipeline::mask::Morphology::Dilate;
subject.base_mut().morph_radius = 0.012;
subject.set_param("saturation", ParamId("saturation"), -100.0);
subject.set_param("exposure", ParamId("exposure"), -0.4);
graph.masks_mut().push(subject);
@@ -635,7 +644,7 @@ fn category_masks_keep_their_name() {
},
);
sky.name = "sky".into();
sky.feather = 0.03;
sky.base_mut().feather = 0.03;
sky.set_param("exposure", ParamId("exposure"), 0.5);
graph.masks_mut().push(sky);
@@ -655,9 +664,15 @@ fn category_masks_keep_their_name() {
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);
assert!((layers[0].feather - 0.03).abs() < 1e-6);
assert_eq!(
layers[0].base().source,
graph.masks().layers()[0].base().source
);
assert_eq!(
layers[1].base().source,
graph.masks().layers()[1].base().source
);
assert!((layers[0].base().feather - 0.03).abs() < 1e-6);
}
/// The refine strictness survives, and its absence means zero.
@@ -683,7 +698,7 @@ fn a_categorys_refine_strictness_survives_and_defaults_off() {
name: "sky".into(),
},
);
sky.refine = 3.7;
sky.base_mut().refine = 3.7;
graph.masks_mut().push(sky);
// Left at zero, so the writer omits the key entirely — which is the case
@@ -711,12 +726,13 @@ fn a_categorys_refine_strictness_survives_and_defaults_off() {
let layers = round_trip(&graph);
let layers = layers.masks().layers();
assert!(
(layers[0].refine - 3.7).abs() < 1e-6,
(layers[0].base().refine - 3.7).abs() < 1e-6,
"got {}",
layers[0].refine
layers[0].base().refine
);
assert_eq!(
layers[1].refine, 0.0,
layers[1].base().refine,
0.0,
"an absent key is the model's own mask"
);
}
@@ -735,7 +751,7 @@ fn an_out_of_range_refine_is_clamped() {
name: "sky".into(),
},
);
sky.refine = 3.0;
sky.base_mut().refine = 3.0;
graph.masks_mut().push(sky);
// Edited in the written text rather than assembled by hand, so this test
@@ -754,7 +770,7 @@ fn an_out_of_range_refine_is_clamped() {
.apply(&mut restored)
.expect_no_film();
let refine = restored.masks().layers()[0].refine;
let refine = restored.masks().layers()[0].base().refine;
assert!(
refine <= dr_pipeline::mask::MAX_REFINE,
"a file asked for 900 and got {refine}"
@@ -828,7 +844,7 @@ fn inside(values: &[u8]) -> Vec<bool> {
fn with_coverage(id: &str, source: MaskSource, values: &[u8]) -> MaskLayer {
let mut layer = MaskLayer::new(id, source);
layer.set_param("exposure", ParamId("exposure"), 0.75);
layer.coverage = Some(Arc::new(
layer.base_mut().coverage = Some(Arc::new(
Coverage::encode(values, PROXY.0, PROXY.1, RENDERED_LEVELS).expect("a disc should encode"),
));
layer
@@ -849,11 +865,15 @@ fn a_subjects_coverage_survives_a_round_trip() {
let layer = &restored.masks().layers()[0];
assert_eq!(
layer.source,
layer.base().source,
subject(0x1234, 2),
"the identity is still there"
);
let coverage = layer.coverage.as_ref().expect("the pixels came back too");
let coverage = layer
.base()
.coverage
.as_ref()
.expect("the pixels came back too");
assert_eq!((coverage.width(), coverage.height()), PROXY);
assert_eq!(
inside(&coverage.decode()),
@@ -876,9 +896,9 @@ fn a_categorys_coverage_survives_a_round_trip() {
let restored = round_trip(&graph);
let layer = &restored.masks().layers()[0];
assert_eq!(layer.source, source);
assert_eq!(layer.base().source, source);
assert_eq!(
inside(&layer.coverage.as_ref().expect("coverage").decode()),
inside(&layer.base().coverage.as_ref().expect("coverage").decode()),
inside(&values)
);
}
@@ -969,10 +989,10 @@ exposure.exposure = 0.75
.expect_no_film();
let layer = &graph.masks().layers()[0];
assert_eq!(layer.source, subject(4660, 2));
assert_eq!(layer.base().source, subject(4660, 2));
assert_eq!(layer.name, "Dog");
assert!(
layer.coverage.is_none(),
layer.base().coverage.is_none(),
"absent means absent, not an empty mask"
);
}
@@ -1011,9 +1031,9 @@ coverage = 96 64 999 not-a-payload
.expect_no_film();
let layer = &graph.masks().layers()[0];
assert_eq!(layer.source, subject(4660, 2));
assert_eq!(layer.base().source, subject(4660, 2));
assert_eq!(layer.name, "Dog");
assert!(layer.coverage.is_none());
assert!(layer.base().coverage.is_none());
assert_eq!(
layer
.ops
@@ -1048,8 +1068,8 @@ fn coverage_is_not_loaded_onto_a_source_with_no_model_behind_it() {
.expect_no_film();
let layer = &graph.masks().layers()[0];
assert!(matches!(layer.source, MaskSource::Radial { .. }));
assert!(layer.coverage.is_none());
assert!(matches!(layer.base().source, MaskSource::Radial { .. }));
assert!(layer.base().coverage.is_none());
}
/// TRACES: FR-NC-9
@@ -1063,9 +1083,9 @@ fn coverage_is_not_loaded_onto_a_source_with_no_model_behind_it() {
fn a_coverage_one_device_has_and_the_other_lacks_is_not_a_conflict() {
let layer = |with: bool| {
let mut l = lit("m1", &[1], 1.0);
l.source = subject(7, 0);
l.base_mut().source = subject(7, 0);
if with {
l.coverage = Some(Arc::new(
l.base_mut().coverage = Some(Arc::new(
Coverage::encode(&a_disc(), PROXY.0, PROXY.1, RENDERED_LEVELS).expect("encode"),
));
}
@@ -1088,7 +1108,12 @@ fn a_coverage_one_device_has_and_the_other_lacks_is_not_a_conflict() {
"running the model is not an edit: {conflicts:?}"
);
assert!(
ours.masks.get("m1").expect("layer").coverage.is_some(),
ours.masks
.get("m1")
.expect("layer")
.base()
.coverage
.is_some(),
"and the side that has the pixels keeps them"
);
}
@@ -1104,8 +1129,8 @@ fn a_real_edit_is_still_a_conflict_with_coverage_present() {
};
let layer = |ev: f32| {
let mut l = lit("m1", &[1], ev);
l.source = subject(7, 0);
l.coverage = stored();
l.base_mut().source = subject(7, 0);
l.base_mut().coverage = stored();
l
};
@@ -1149,11 +1174,11 @@ fn range_masks_keep_their_bands() {
let layers = restored.masks().layers();
assert_eq!(layers.len(), 2);
assert_eq!(
layers[0].source,
layers[0].base().source,
MaskSource::luminance_range(0.62, 0.91, 0.2)
);
assert_eq!(
layers[1].source,
layers[1].base().source,
MaskSource::colour_range(0.07, 0.04, 0.2, 0.85, 0.11)
);
}
@@ -1200,7 +1225,7 @@ fn a_crossed_band_is_ordered_on_read() {
.expect_no_film();
assert_eq!(
graph.masks().layers()[0].source,
graph.masks().layers()[0].base().source,
MaskSource::luminance_range(0.2, 0.9, 0.1),
"the band came back as the one the numbers describe"
);
@@ -1229,5 +1254,255 @@ fn a_range_mask_with_no_band_falls_back_to_the_default_one() {
.apply(&mut graph)
.expect_no_film();
assert_eq!(graph.masks().layers()[0].source, MaskSource::highlights());
assert_eq!(
graph.masks().layers()[0].base().source,
MaskSource::highlights()
);
}
// ---------------------------------------------------------------------------
// Parts
// ---------------------------------------------------------------------------
/// A subject with a correction painted on top of it — the ordinary shape of a
/// mask that has been edited by hand.
fn corrected(id: &str, join: Join) -> MaskLayer {
let mut layer = MaskLayer::new(
id,
MaskSource::Subject {
signature: 0xdead_beef,
index: 0,
class: "dog".into(),
score: 0.9,
},
);
layer.set_param("exposure", ParamId("exposure"), 0.5);
layer.push_part(MaskPart::painted(layer.next_part_id(), join));
layer.begin_stroke(1, false, 0.05, 0.5, 1.0);
for &(x, y) in &[(0.2, 0.2), (0.3, 0.25), (0.4, 0.3)] {
layer.extend_stroke(1, x, y);
}
layer.end_stroke(1);
layer
}
/// The property that makes this format change safe: a mask nobody has
/// corrected writes what it has always written, so every sidecar in every
/// library keeps parsing to the same edit and no file grows a block it does
/// not need.
#[test]
fn a_mask_of_one_part_writes_no_part_block() {
let mut sidecar = Sidecar::new();
sidecar.put(Version::from_graph(
"default",
"Default",
&graph_with_mask(),
));
let text = sidecar.to_text();
assert!(text.contains("[mask default m1]"), "{text}");
assert!(
!text.contains("[part "),
"a layer of one part invented a block for it:\n{text}"
);
assert!(
!text.contains("join ="),
"a layer of one part wrote a join nothing joins to:\n{text}"
);
}
/// And the other half of it: a file written before parts existed reads back as
/// a layer of exactly one, rather than as one with an empty correction on it.
#[test]
fn a_mask_block_with_no_part_blocks_is_one_part() {
let layers = round_trip(&graph_with_mask());
let layers = layers.masks().layers();
assert_eq!(layers[0].parts().len(), 1);
assert_eq!(layers[0].parts()[0].id, "p1", "the implied part is named");
assert_eq!(layers[0].parts()[0].join, Join::Union);
}
#[test]
fn a_correction_painted_onto_a_subject_survives_a_round_trip() {
let mut graph = EditGraph::default_chain();
let layer = corrected("m1", Join::Subtract);
let expected = layer.parts()[1].strokes().to_vec();
graph.masks_mut().push(layer);
let restored = round_trip(&graph);
let layer = &restored.masks().layers()[0];
assert_eq!(layer.parts().len(), 2, "both parts came back");
assert_eq!(layer.parts()[1].id, "p2");
assert_eq!(layer.parts()[1].join, Join::Subtract);
assert_eq!(layer.parts()[1].strokes(), expected.as_slice());
assert_eq!(
layer.parts()[0].source.kind(),
"subject",
"and the base is still what it was"
);
}
/// A part carries its own edge, which is the whole reason it is a part rather
/// than a second source on the layer: a model's soft coverage and a stroke
/// painted where it stopped short want different boundaries.
#[test]
fn each_part_keeps_its_own_shaping() {
let mut graph = EditGraph::default_chain();
let mut layer = corrected("m1", Join::Union);
layer.base_mut().feather = 0.02;
layer.base_mut().morphology = Morphology::Dilate;
layer.base_mut().morph_radius = 0.01;
layer.parts_mut()[1].feather = 0.0;
layer.parts_mut()[1].invert = true;
graph.masks_mut().push(layer);
let restored = round_trip(&graph);
let layer = &restored.masks().layers()[0];
assert_eq!(layer.parts()[0].feather, 0.02);
assert_eq!(layer.parts()[0].morphology, Morphology::Dilate);
assert_eq!(layer.parts()[1].feather, 0.0);
assert!(layer.parts()[1].invert);
assert!(
!layer.invert,
"the layer's own inversion is a different one"
);
}
#[test]
fn writing_a_corrected_mask_twice_is_byte_identical() {
let mut graph = EditGraph::default_chain();
graph.masks_mut().push(corrected("m1", Join::Subtract));
let mut sidecar = Sidecar::new();
sidecar.put(Version::from_graph("default", "Default", &graph));
let first = sidecar.to_text();
let second = Sidecar::parse(&first).expect("reparse").to_text();
assert_eq!(first, second);
}
/// A part block that names another mask is somebody's correction attached to
/// the wrong photograph if it is honoured. Dropped — and, more importantly,
/// its keys must not fall through to the version, where a `stroke` line would
/// be a global edit nobody made.
#[test]
fn a_part_naming_a_different_mask_is_dropped() {
let text = "drsc 1\n\
\n[version default]\n\
name = Default\n\
exposure.exposure = 0.25\n\
\n[mask default m1]\n\
source = brush\n\
exposure.exposure = 1\n\
\n[part default m9 p2]\n\
join = subtract\n\
source = brush\n\
stroke = add 0.05 0.5 1 0.2,0.2 0.3,0.3\n";
let parsed = Sidecar::parse(text).expect("parse");
let mut graph = EditGraph::default_chain();
parsed
.versions
.get("default")
.expect("version")
.apply(&mut graph)
.expect_no_film();
let layer = &graph.masks().layers()[0];
assert_eq!(layer.parts().len(), 1, "the orphan part was not attached");
assert_eq!(
graph.param(OpId("exposure"), ParamId("exposure")),
Some(0.25),
"and its keys did not land on the version"
);
}
/// The rule a malformed stroke already follows, one level up: a part this
/// build cannot read costs that part, and the rest of the mask — which is work
/// somebody did — survives.
#[test]
fn a_part_with_an_unknown_source_costs_only_that_part() {
let text = "drsc 1\n\
\n[version default]\n\
name = Default\n\
\n[mask default m1]\n\
source = brush\n\
stroke = add 0.05 0.5 1 0.1,0.1 0.2,0.2\n\
exposure.exposure = 1\n\
\n[part default m1 p2]\n\
join = union\n\
source = hologram\n";
let parsed = Sidecar::parse(text).expect("parse");
let mut graph = EditGraph::default_chain();
parsed
.versions
.get("default")
.expect("version")
.apply(&mut graph)
.expect_no_film();
let layer = &graph.masks().layers()[0];
assert_eq!(layer.parts().len(), 1);
assert_eq!(layer.strokes().len(), 1, "the base kept its stroke");
}
/// An unknown join adds, rather than dropping the part. A selection that is
/// visible can be fixed by the photographer; one that quietly took pixels away
/// under a name this build could not read cannot be.
#[test]
fn an_unknown_join_adds_the_part() {
let text = "drsc 1\n\
\n[version default]\n\
name = Default\n\
\n[mask default m1]\n\
source = brush\n\
exposure.exposure = 1\n\
\n[part default m1 p2]\n\
join = xor\n\
source = brush\n\
stroke = add 0.05 0.5 1 0.2,0.2 0.3,0.3\n";
let parsed = Sidecar::parse(text).expect("parse");
let mut graph = EditGraph::default_chain();
parsed
.versions
.get("default")
.expect("version")
.apply(&mut graph)
.expect_no_film();
let layer = &graph.masks().layers()[0];
assert_eq!(layer.parts().len(), 2);
assert_eq!(layer.parts()[1].join, Join::Union);
}
/// Two devices, one correcting a mask and one leaving it alone: the correction
/// arrives, because a layer merges as a whole and the part travelled with it.
#[test]
fn a_correction_made_on_one_device_reaches_the_other() {
let base = version_with("default", 1, |g| {
g.masks_mut().push(lit("m1", &[1, 2], 0.5));
});
let mut ours = base.clone();
ours.revision = 2;
let mut theirs = base.clone();
theirs.revision = 3;
if let Some(layer) = theirs.masks.get_mut("m1") {
layer.push_part(MaskPart::painted("p2", Join::Subtract));
layer.begin_stroke(1, false, 0.05, 0.5, 1.0);
layer.extend_stroke(1, 0.2, 0.2);
layer.extend_stroke(1, 0.4, 0.4);
layer.end_stroke(1);
}
let conflicts = ours.merge(&theirs, Some(&base));
assert!(conflicts.is_empty(), "only one side changed the layer");
let layer = ours.masks.get("m1").expect("layer");
assert_eq!(layer.parts().len(), 2);
assert_eq!(layer.parts()[1].join, Join::Subtract);
}