Show the photographer the mask they are shaping

Nobody can refine an edge they are not being shown. The only thing drawn on
the canvas was the region overlay — a false-coloured picture of what the model
*detected* — which knows nothing of a layer's feather, its falloff, its
morphology, its invert or its opacity, and nothing at all about a gradient, a
range or a stroke. Every control added for mask editing therefore acted on
something invisible, which is why the whole feature reads as absent rather
than as unfinished.

A layer's finished mask now draws over the photograph in one of three styles:
a tint for whether the right thing is selected, an alpha for where the edge
is, an outline for whether that edge is registered against the detail the
other two hide.

The hard part is not the shader. A selection with no adjustment on it changes
no pixel, so it is not active, so it holds no slice of the mask array and is
never rasterised — and that is exactly the layer somebody wants to look at,
for the whole of the time between choosing a subject and deciding what to do
to it. So `MaskStack::rendered` is `active()` plus the layer being looked at,
and the rasteriser, the composer and the distance-field builder all index by
position in it. Which is also why the design's "two uniforms, no recompile" is
not available: a uniform can select a slot, it cannot conjure one.

The reveal is never on the graph. It reaches the pipeline as an argument to
`compose_revealing`, and `compose_for` — which the exporter, the thumbnail and
the neutral probe all call — has no way to ask for one. A flag on the graph
would have been shorter, would have type-checked, and would have been one
forgotten reset away from a red tint baked into an exported file.

And the tools that shape a mask now arm. `Masking.tool` is an `in` property
only Rust may write, and the handler wrote nothing back, so the strip reported
"Select" however many times Paint was pressed and the paint area was never
enabled — the brush, the parts and the whole of FR-DEV-19b reachable from no
control in the application.

The region overlay stands down while a mask is being shown, and its button now
says what it hides: two overlays that look alike and mean different things is
worse than either.
This commit is contained in:
2026-09-10 20:28:06 +02:00
parent 193b35a249
commit c045702a47
10 changed files with 927 additions and 51 deletions
+28 -2
View File
@@ -708,10 +708,36 @@ impl MaskPass {
source: Option<&DemosaicedImage>, source: Option<&DemosaicedImage>,
width: u32, width: u32,
height: u32, height: u32,
) -> Result<&MaskArray, GpuError> {
self.render_revealing(stack, labels, subjects, source, width, height, None)
}
/// TRACES: FR-DEV-19c
/// [`Self::render`], also drawing the layer being looked at.
///
/// A selection with no adjustment on it changes no pixel, so it is not
/// active and has no slice — which is right until somebody asks to *see*
/// it, and that is the state a photographer is in from choosing a subject
/// until deciding what to do to it.
///
/// `reveal` has to be the same one the shader was composed with and the
/// same one the distance fields were built for: all three index this array
/// by position in [`MaskStack::rendered`], and two of them disagreeing
/// shows as an adjustment applied through another layer's mask.
#[allow(clippy::too_many_arguments)]
pub fn render_revealing(
&mut self,
stack: &MaskStack,
labels: Option<&LabelField>,
subjects: Option<&SubjectMasks>,
source: Option<&DemosaicedImage>,
width: u32,
height: u32,
reveal: Option<&dr_pipeline::mask::Reveal>,
) -> Result<&MaskArray, GpuError> { ) -> Result<&MaskArray, GpuError> {
// At least one layer, because a zero-layer texture array is invalid // At least one layer, because a zero-layer texture array is invalid
// and the shader binds this slot unconditionally. // and the shader binds this slot unconditionally.
let active = stack.active_count().clamp(1, MAX_LAYERS) as u32; let active = stack.rendered_count(reveal).clamp(1, MAX_LAYERS) as u32;
self.ensure_array(width, height, active)?; self.ensure_array(width, height, active)?;
let mut encoder = self let mut encoder = self
@@ -721,7 +747,7 @@ impl MaskPass {
label: Some("mask-encoder"), label: Some("mask-encoder"),
}); });
for (slot, layer) in stack.active().enumerate().take(MAX_LAYERS) { for (slot, layer) in stack.rendered(reveal).enumerate().take(MAX_LAYERS) {
// **The path a mask with one part takes is the path every mask // **The path a mask with one part takes is the path every mask
// took before parts existed**: drawn straight into the layer's // took before parts existed**: drawn straight into the layer's
// slice, cleared by the draw itself. Nothing about an unedited // slice, cleared by the draw itself. Nothing about an unedited
+197 -2
View File
@@ -12,8 +12,8 @@
use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext, LabelField, MaskPass}; use dr_gpu::{AdjustPass, DemosaicedImage, GpuContext, LabelField, MaskPass};
use dr_pipeline::descriptor::ParamId; use dr_pipeline::descriptor::ParamId;
use dr_pipeline::mask::{Join, MaskLayer, MaskPart, MaskSource, MaskStack}; use dr_pipeline::mask::{Join, MaskLayer, MaskPart, MaskSource, MaskStack, Reveal, RevealStyle};
use dr_pipeline::operation::compose_full; use dr_pipeline::operation::{compose_full, compose_full_revealing};
use dr_pipeline::spot::SpotSet; use dr_pipeline::spot::SpotSet;
use dr_pipeline::{ops, EditGraph, Framing}; use dr_pipeline::{ops, EditGraph, Framing};
use dr_types::ColourSpace; use dr_types::ColourSpace;
@@ -789,3 +789,198 @@ fn the_order_parts_are_joined_in_is_the_mask() {
"and subtracting after an addition takes it away again" "and subtracting after an addition takes it away again"
); );
} }
// --- seeing the mask (FR-DEV-19c) ------------------------------------------
/// A radial that covers the middle of the frame and nothing near the corners.
fn middle() -> MaskSource {
MaskSource::Radial {
centre: (0.5, 0.5),
radii: (0.3, 0.3),
angle: 0.0,
feather: 0.05,
}
}
/// [`render`], with one layer's mask drawn over the result.
fn render_revealing(ctx: &GpuContext, stack: &MaskStack, reveal: &Reveal) -> Vec<u8> {
let source = grey(ctx);
let shader = compose_full_revealing(
&ops::chain(),
&Framing::new(),
ColourSpace::Srgb,
stack,
&SpotSet::new(),
&[],
Some(reveal),
);
let mut masks = MaskPass::new(ctx).expect("mask pass");
let array = masks
.render_revealing(stack, None, None, None, SIZE, SIZE, Some(reveal))
.expect("rasterise");
let mut adjust = AdjustPass::new(ctx);
adjust
.render_masked(&source, &shader, SIZE, SIZE, Some(array))
.expect("render");
adjust.export_pixels().expect("readback").0
}
/// TRACES: FR-DEV-19c
/// The state every mask is in for its first few seconds: chosen, and not yet
/// used for anything.
///
/// Such a layer changes no pixel, so it is not active, so it occupied no mask
/// slot and was never rasterised — and the reveal drew nothing. That is the
/// whole of "I clicked the category and nothing happened": there was a mask,
/// and no way to see that there was.
#[test]
fn a_selection_with_no_adjustment_can_still_be_seen() {
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let mut stack = MaskStack::new();
stack.push(MaskLayer::new("m1", middle()));
assert!(
stack.is_neutral(),
"the fixture must be a selection with nothing done to it"
);
let pixels = render_revealing(
&ctx,
&stack,
&Reveal {
layer: "m1".into(),
style: RevealStyle::Alpha,
},
);
assert!(
luma_at(&pixels, SIZE / 2, SIZE / 2) > 200,
"the middle is inside the mask and should read white"
);
assert!(
luma_at(&pixels, 1, 1) < 40,
"the corner is outside it and should read black"
);
}
/// And with nobody looking, the same stack changes nothing at all.
///
/// The other half of the property above: a layer renders *because* it is being
/// revealed, so it must stop when the reveal does — otherwise a selection with
/// no adjustment would leave a slice in the array for ever.
#[test]
fn a_mask_nobody_is_looking_at_draws_nothing() {
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let mut stack = MaskStack::new();
stack.push(MaskLayer::new("m1", middle()));
let pixels = render(&ctx, &stack, None);
assert_eq!(
luma_at(&pixels, SIZE / 2, SIZE / 2),
128,
"flat grey, exactly as it went in"
);
}
/// TRACES: FR-DEV-19c
/// A tint has to leave the photograph visible, or it cannot be judged against
/// it — which is the one thing an overlay exists for.
#[test]
fn a_tint_colours_the_mask_and_leaves_the_rest_alone() {
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let mut stack = MaskStack::new();
stack.push(MaskLayer::new("m1", middle()));
let pixels = render_revealing(
&ctx,
&stack,
&Reveal {
layer: "m1".into(),
style: RevealStyle::Tint,
},
);
let at = |x: u32, y: u32| {
let i = ((y * SIZE + x) * 4) as usize;
(pixels[i], pixels[i + 1], pixels[i + 2])
};
let (r, g, _) = at(SIZE / 2, SIZE / 2);
assert!(r > g + 40, "the mask should read red, got r={r} g={g}");
assert!(
g > 20,
"and not opaque — the photograph under it is what the tint is judged \
against, got g={g}"
);
let (r, g, b) = at(1, 1);
assert!(
(120..=136).contains(&r) && r == g && g == b,
"outside the mask the photograph is untouched, got ({r}, {g}, {b})"
);
}
/// TRACES: FR-DEV-19c
/// An outline draws where the mask stops and nowhere else — which is the
/// point of it, since the other two styles cover the detail the boundary has
/// to be judged against.
#[test]
fn an_outline_draws_the_boundary_and_not_the_interior() {
let Some(ctx) = ctx() else {
eprintln!("no adapter; skipping");
return;
};
let mut stack = MaskStack::new();
stack.push(MaskLayer::new("m1", middle()));
let pixels = render_revealing(
&ctx,
&stack,
&Reveal {
layer: "m1".into(),
style: RevealStyle::Edge,
},
);
// Where the line landed, along the row through the centre. Searched
// rather than sampled at one place: the radial's edge crosses this row
// about 9.6 pixels out from the middle on a 32px frame, and asserting a
// particular pixel would be asserting the rounding.
let (at, brightest) = (SIZE / 2..SIZE)
.map(|x| (x, luma_at(&pixels, x, SIZE / 2)))
.max_by_key(|&(_, v)| v)
.expect("the row is not empty");
assert!(
brightest > 160,
"there should be a line somewhere on this row, brightest was {brightest}"
);
assert!(
(SIZE / 2 + 7..=SIZE / 2 + 12).contains(&at),
"and it should be on the mask's boundary, not somewhere else: x={at}"
);
assert_eq!(
luma_at(&pixels, SIZE / 2, SIZE / 2),
128,
"the picture inside the mask is untouched"
);
assert_eq!(
luma_at(&pixels, 1, 1),
128,
"and so is the picture outside it"
);
}
+29
View File
@@ -849,6 +849,35 @@ impl EditGraph {
) )
} }
/// TRACES: FR-DEV-19c
/// [`Self::compose_for`], with one layer's mask drawn over the picture.
///
/// **The screen's composition, and only the screen's.** The reveal is not
/// on the graph and cannot be: it is how a photographer is looking at an
/// edit, not part of one, so it arrives as an argument to the one call
/// that draws the canvas. Every other path through this type composes
/// without it and could not ask for it if it wanted to.
///
/// The revealed layer renders whether or not it carries an adjustment —
/// which is the whole point, since a fresh selection carries none — so the
/// mask array must be rasterised for the same `reveal`. See
/// [`crate::mask::MaskStack::rendered`] for what the two have to agree on.
pub fn compose_revealing(
&self,
output: dr_types::ColourSpace,
reveal: Option<&crate::mask::Reveal>,
) -> ComposedShader {
crate::operation::compose_full_revealing(
&self.ops,
&self.framing,
output,
&self.masks,
&self.spots,
&self.warps,
reveal,
)
}
/// TRACES: FR-DSP-1 /// TRACES: FR-DSP-1
/// How this render relates to the file it stands for. /// How this render relates to the file it stands for.
/// ///
+231 -10
View File
@@ -1760,6 +1760,49 @@ impl MaskLayer {
} }
} }
/// TRACES: FR-DEV-19c
/// How a mask is drawn when the photographer asks to see it.
///
/// Three, because they answer three different questions and no one of them
/// answers all three — which is the argument for offering a choice rather than
/// picking the best one.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum RevealStyle {
/// The mask over the photograph in a flat colour. What every editor's
/// photographers already expect, and the only style that answers "is this
/// selecting the right thing" while the picture is still visible.
Tint,
/// The mask alone, white on black. For judging an edge, which a tint over
/// a busy photograph cannot be read against.
Alpha,
/// The boundary outlined over the untouched picture. For checking
/// registration against detail the other two hide — the same reasoning the
/// region overlay's white outline already carries.
Edge,
}
impl RevealStyle {
/// In the order the interface offers them.
pub const ALL: [RevealStyle; 3] = [Self::Tint, Self::Alpha, Self::Edge];
}
/// TRACES: FR-DEV-19c
/// The layer whose mask is being shown, and how.
///
/// **Never part of an edit.** It is not stored on [`MaskStack`] and it does
/// not travel with the graph: it is passed to the one composition that draws
/// the screen, so an export, a thumbnail and the neutral probe are
/// structurally unable to reveal anything. A flag on the stack would have been
/// fewer parameters and would have tinted every exported file red.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Reveal {
/// Which layer, by id. By id rather than by slot because the slot is
/// derived from which layers render, and that is decided *by* this — see
/// [`MaskStack::rendered`].
pub layer: String,
pub style: RevealStyle,
}
/// The ordered stack of local adjustments. /// The ordered stack of local adjustments.
#[derive(Debug, Clone, Default, PartialEq)] #[derive(Debug, Clone, Default, PartialEq)]
pub struct MaskStack { pub struct MaskStack {
@@ -1838,6 +1881,34 @@ impl MaskStack {
self.active().count() self.active().count()
} }
/// TRACES: FR-DEV-19c
/// [`Self::active`], plus the layer being looked at.
///
/// A selection with no adjustment on it yet is not active — it changes no
/// pixel, so it occupies no mask slot and the rasteriser never draws it.
/// That is right for rendering and exactly wrong for *showing* the mask,
/// which is the state a photographer is in for the whole of the time
/// between choosing a subject and deciding what to do to it.
///
/// So this is the sequence both halves walk whenever a reveal is in play,
/// and the index within it is the texture-array slot — the same contract
/// [`Self::active`] carries, and the reason the rasteriser, the composer
/// and the field builder must all be given the same `reveal` or none of
/// them. Two of them disagreeing shows as an adjustment applied through
/// another layer's mask.
pub fn rendered<'a>(
&'a self,
reveal: Option<&'a Reveal>,
) -> impl Iterator<Item = &'a MaskLayer> {
self.layers
.iter()
.filter(move |l| l.is_active() || reveal.is_some_and(|r| r.layer == l.id))
}
pub fn rendered_count(&self, reveal: Option<&Reveal>) -> usize {
self.rendered(reveal).count()
}
/// Whether any layer changes any pixel. /// Whether any layer changes any pixel.
pub fn is_neutral(&self) -> bool { pub fn is_neutral(&self) -> bool {
self.active_count() == 0 self.active_count() == 0
@@ -1858,25 +1929,45 @@ pub(crate) struct LayerShader {
pub uniform_values: Vec<f32>, pub uniform_values: Vec<f32>,
pub body: String, pub body: String,
pub helpers: Vec<crate::operation::Helper>, pub helpers: Vec<crate::operation::Helper>,
/// TRACES: FR-DEV-19c
/// The block that draws one layer's mask over the finished picture, empty
/// when nothing is being revealed.
///
/// Kept apart from `body` because it belongs at the other end of the
/// shader. Everything in `body` runs on scene-referred colour in the
/// working space, where a flat tint would then be pushed through the base
/// curve and the camera matrix and arrive as some other colour, and a
/// white-on-black alpha would arrive as neither. This runs after the
/// output transform, so what is written is what is seen.
pub reveal: String,
} }
/// Emit the WGSL for every active layer. /// Emit the WGSL for every layer that renders, and for the mask being looked
/// at.
/// ///
/// `slot` is the layer's index in the mask texture array, matching /// `slot` is the layer's index in the mask texture array, matching
/// [`MaskStack::active`]. /// [`MaskStack::rendered`] — the revealed layer renders whether or not it has
pub(crate) fn compose_layers(stack: &MaskStack) -> LayerShader { /// an adjustment on it, which is why the two are one sequence and why every
/// other half of the pipeline has to be given the same `reveal` for the slots
/// to mean the same thing.
pub(crate) fn compose_layers_revealing(stack: &MaskStack, reveal: Option<&Reveal>) -> LayerShader {
let mut out = LayerShader { let mut out = LayerShader {
uniform_fields: String::new(), uniform_fields: String::new(),
uniform_values: Vec::new(), uniform_values: Vec::new(),
body: String::new(), body: String::new(),
helpers: Vec::new(), helpers: Vec::new(),
reveal: String::new(),
}; };
if stack.active().next().is_some() { if stack.rendered(reveal).next().is_some() {
out.helpers.push(MASK_SAMPLER); out.helpers.push(MASK_SAMPLER);
} }
for (slot, layer) in stack.active().enumerate() { for (slot, layer) in stack.rendered(reveal).enumerate() {
if reveal.is_some_and(|r| r.layer == layer.id) {
out.reveal = reveal_block(slot, layer, reveal.expect("just matched").style);
}
let prefix = format!("mask{slot}"); let prefix = format!("mask{slot}");
let _ = writeln!( let _ = writeln!(
@@ -1973,6 +2064,75 @@ pub(crate) fn compose_layers(stack: &MaskStack) -> LayerShader {
out out
} }
/// TRACES: FR-DEV-19c
/// The WGSL that draws one layer's mask over the finished picture.
///
/// # Why this is not two uniforms
///
/// The slot and the style are written into the source, so turning the reveal
/// on, off, or onto another layer recompiles the fused shader. That is a
/// button press rather than a frame — and the alternative costs more than it
/// saves: a uniform can select a slot, but it cannot conjure one for a layer
/// that is not rendering, and the whole reason this exists is that a selection
/// with no adjustment on it yet is exactly that layer. So the composition
/// changes either way, and a uniform would only have added a branch per pixel
/// on top of it.
///
/// **Everything below runs after the output transform.** `c` is already in the
/// output space's primaries and still linear — the clip and the encode come
/// after — which is what makes a stated colour arrive as itself.
fn reveal_block(slot: usize, layer: &MaskLayer, style: RevealStyle) -> String {
let prefix = format!("mask{slot}");
let mut out = format!(
"\n // ==== showing mask {slot}: {} ====\n //\n // Not part of the\
\n // photograph: this is the mask itself, drawn because someone asked to\
\n // see it. Nothing downstream of the screen composes this shader.\n {{\n",
layer.display_name()
);
// The shaped mask, exactly as the layer above applied it — the same two
// uniforms, in the same order. A reveal that showed the raw slice would
// draw a different mask from the one doing the work, which is worse than
// showing none: it would send a photographer to fix an edge that is
// already where they want it.
let _ = writeln!(out, " var m = sample_mask(uv_src, {slot});");
let _ = writeln!(
out,
" m = select(m, 1.0 - m, u.{prefix}_invert > 0.5);"
);
let _ = writeln!(out, " m = clamp(m * u.{prefix}_opacity, 0.0, 1.0);");
let body = match style {
// Red at a bit over half strength. Half is the strength every editor
// settled on for the same reason: past it the tint is opaque enough to
// hide the thing being judged, and below it a mask over a bright sky
// cannot be seen at all.
RevealStyle::Tint => " c = mix(c, vec3<f32>(0.85, 0.10, 0.15), m * 0.55);",
// Neutral, which means the same thing in every output space that
// shares a white point — so this one style needs no correction for the
// panel the window happens to be on.
RevealStyle::Alpha => " c = vec3<f32>(m);",
// The gradient's magnitude, over the untouched picture. Central
// differences one texel apart in the *mask's* own grid, so the outline
// is one mask texel wide however far the view is zoomed in — the
// boundary's position is the thing being checked, and a line that grew
// with the zoom would hide it.
RevealStyle::Edge => {
" let texel = 1.0 / vec2<f32>(textureDimensions(masks));\n\
\x20 let dx = sample_mask(uv_src + vec2<f32>(texel.x, 0.0), SLOT)\n\
\x20 - sample_mask(uv_src - vec2<f32>(texel.x, 0.0), SLOT);\n\
\x20 let dy = sample_mask(uv_src + vec2<f32>(0.0, texel.y), SLOT)\n\
\x20 - sample_mask(uv_src - vec2<f32>(0.0, texel.y), SLOT);\n\
\x20 // Doubled so a soft edge, whose gradient is spread over many\n\
\x20 // texels and therefore shallow everywhere, still draws a line.\n\
\x20 let edge = clamp(2.0 * sqrt(dx * dx + dy * dy), 0.0, 1.0);\n\
\x20 c = mix(c, vec3<f32>(1.0), edge);"
}
};
let _ = writeln!(out, "{}", body.replace("SLOT", &slot.to_string()));
let _ = writeln!(out, " }}");
out
}
/// A stable fingerprint of a segmentation, for [`MaskSource::Regions`]. /// A stable fingerprint of a segmentation, for [`MaskSource::Regions`].
/// ///
/// Built from the things that change what a region id *means* — the proxy /// Built from the things that change what a region id *means* — the proxy
@@ -2021,7 +2181,7 @@ mod tests {
let mut stack = MaskStack::new(); let mut stack = MaskStack::new();
stack.push(layer); stack.push(layer);
assert!(stack.is_neutral()); assert!(stack.is_neutral());
assert_eq!(compose_layers(&stack).body, ""); assert_eq!(compose_layers_revealing(&stack, None).body, "");
} }
#[test] #[test]
@@ -2107,7 +2267,7 @@ mod tests {
stack.push(lit_layer("m1", 1.0)); stack.push(lit_layer("m1", 1.0));
stack.push(lit_layer("m2", -1.0)); stack.push(lit_layer("m2", -1.0));
let shader = compose_layers(&stack); let shader = compose_layers_revealing(&stack, None);
assert!(shader.body.contains("sample_mask(uv_src, 0)")); assert!(shader.body.contains("sample_mask(uv_src, 0)"));
assert!(shader.body.contains("sample_mask(uv_src, 1)")); assert!(shader.body.contains("sample_mask(uv_src, 1)"));
assert!(shader.body.contains("u.mask0_opacity")); assert!(shader.body.contains("u.mask0_opacity"));
@@ -2124,7 +2284,7 @@ mod tests {
stack.push(off); stack.push(off);
stack.push(lit_layer("m2", -1.0)); stack.push(lit_layer("m2", -1.0));
let shader = compose_layers(&stack); let shader = compose_layers_revealing(&stack, None);
assert!( assert!(
shader.body.contains("sample_mask(uv_src, 0)"), shader.body.contains("sample_mask(uv_src, 0)"),
"the one active layer must use slot 0, not slot 1" "the one active layer must use slot 0, not slot 1"
@@ -2132,13 +2292,74 @@ mod tests {
assert!(!shader.body.contains("sample_mask(uv_src, 1)")); assert!(!shader.body.contains("sample_mask(uv_src, 1)"));
} }
/// TRACES: FR-DEV-19c
/// The slot the reveal is given has to be the slot the layer renders
/// through, and a revealed layer renders even with nothing done to it.
///
/// Both halves in one assertion because the failure is the pair coming
/// apart: a reveal pointed at a slot the rasteriser did not draw shows
/// whatever was last in that slice, which reads as the mask being wrong
/// rather than as the reveal being wrong.
#[test]
fn a_revealed_layer_takes_a_slot_of_its_own() {
let mut stack = MaskStack::new();
stack.push(lit_layer("m1", 1.0));
// No adjustment, so this changes no pixel and would ordinarily render
// through no slot at all.
stack.push(MaskLayer::new("m2", MaskSource::brush()));
let reveal = Reveal {
layer: "m2".into(),
style: RevealStyle::Alpha,
};
let shader = compose_layers_revealing(&stack, Some(&reveal));
assert_eq!(
stack.rendered_count(Some(&reveal)),
2,
"the layer being looked at renders alongside the active one"
);
assert!(
shader.reveal.contains("sample_mask(uv_src, 1)"),
"the reveal must read slot 1, which is where m2 renders"
);
assert!(
shader.reveal.contains("u.mask1_opacity"),
"and shape it with that layer's own uniforms, not another's"
);
}
/// A composition nobody asked to see a mask through draws none.
#[test]
fn nothing_is_revealed_unless_it_was_asked_for() {
let mut stack = MaskStack::new();
stack.push(lit_layer("m1", 1.0));
assert!(compose_layers_revealing(&stack, None).reveal.is_empty());
}
/// A reveal aimed at a layer that is not in the stack is not a slot, and
/// must not become one.
#[test]
fn a_reveal_naming_no_layer_reveals_nothing() {
let mut stack = MaskStack::new();
stack.push(lit_layer("m1", 1.0));
let reveal = Reveal {
layer: "gone".into(),
style: RevealStyle::Tint,
};
assert_eq!(stack.rendered_count(Some(&reveal)), 1);
assert!(compose_layers_revealing(&stack, Some(&reveal))
.reveal
.is_empty());
}
#[test] #[test]
fn each_layer_gets_its_own_uniforms() { fn each_layer_gets_its_own_uniforms() {
let mut stack = MaskStack::new(); let mut stack = MaskStack::new();
stack.push(lit_layer("m1", 1.0)); stack.push(lit_layer("m1", 1.0));
stack.push(lit_layer("m2", -1.0)); stack.push(lit_layer("m2", -1.0));
let shader = compose_layers(&stack); let shader = compose_layers_revealing(&stack, None);
assert!(shader.uniform_fields.contains("mask0_exposure_")); assert!(shader.uniform_fields.contains("mask0_exposure_"));
assert!(shader.uniform_fields.contains("mask1_exposure_")); assert!(shader.uniform_fields.contains("mask1_exposure_"));
assert_eq!( assert_eq!(
@@ -2156,7 +2377,7 @@ mod tests {
fn the_inner_block_shadows_c_and_copies_back() { fn the_inner_block_shadows_c_and_copies_back() {
let mut stack = MaskStack::new(); let mut stack = MaskStack::new();
stack.push(lit_layer("m1", 1.0)); stack.push(lit_layer("m1", 1.0));
let body = compose_layers(&stack).body; let body = compose_layers_revealing(&stack, None).body;
assert!(body.contains("var masked = c;")); assert!(body.contains("var masked = c;"));
assert!(body.contains("var c = masked;")); assert!(body.contains("var c = masked;"));
+80 -2
View File
@@ -551,6 +551,28 @@ pub fn compose_full(
masks: &MaskStack, masks: &MaskStack,
spots: &crate::spot::SpotSet, spots: &crate::spot::SpotSet,
warps: &[Box<dyn crate::lens::Warp>], warps: &[Box<dyn crate::lens::Warp>],
) -> ComposedShader {
compose_full_revealing(ops, framing, output, masks, spots, warps, None)
}
/// TRACES: FR-DEV-19c
/// [`compose_full`], with one layer's mask drawn over the finished picture.
///
/// Separate from [`compose_full`] rather than an argument on it, and that is
/// the safety property rather than a convenience: the reveal is a thing the
/// screen does, and every other consumer of the pipeline — the exporter, the
/// thumbnail, the neutral probe — calls the function that has no way to ask
/// for it. A flag reachable from the graph would have been one forgotten reset
/// away from a red tint baked into an exported file.
#[allow(clippy::too_many_arguments)]
pub fn compose_full_revealing(
ops: &[Box<dyn Operation>],
framing: &Framing,
output: ColourSpace,
masks: &MaskStack,
spots: &crate::spot::SpotSet,
warps: &[Box<dyn crate::lens::Warp>],
reveal: Option<&crate::mask::Reveal>,
) -> ComposedShader { ) -> ComposedShader {
// The lens corrections, composed into one coordinate transform. Beside // The lens corrections, composed into one coordinate transform. Beside
// `framing` because they are the other half of the same stage: framing // `framing` because they are the other half of the same stage: framing
@@ -715,7 +737,13 @@ pub fn compose_full(
// from. Their uniforms follow the global ops' in the block for the same // from. Their uniforms follow the global ops' in the block for the same
// reason those follow framing's — slot order is emission order, and // reason those follow framing's — slot order is emission order, and
// nothing addresses a slot by number. // nothing addresses a slot by number.
let layers = crate::mask::compose_layers(masks); let layers = crate::mask::compose_layers_revealing(masks, reveal);
// TRACES: FR-DEV-19c
// Held apart from the body, because it belongs after the output transform
// rather than among the operations — see `mask::LayerShader::reveal`.
// Empty for every composition nobody is looking at a mask through, which
// is all of them but the screen's.
let reveal_block = layers.reveal.clone();
uniform_fields.push_str(&layers.uniform_fields); uniform_fields.push_str(&layers.uniform_fields);
uniform_values.extend_from_slice(&layers.uniform_values); uniform_values.extend_from_slice(&layers.uniform_values);
body.push_str(&layers.body); body.push_str(&layers.body);
@@ -975,7 +1003,7 @@ fn main(@builtin(global_invocation_id) gid: vec3<u32>) {{
c = mix(c, neutral, clipped); c = mix(c, neutral, clipped);
}} }}
{body} {body}
{rendering_tail}{to_output} {rendering_tail}{to_output}{reveal_block}
{store} {store}
}} }}
", ",
@@ -1569,6 +1597,56 @@ mod tests {
); );
} }
/// TRACES: FR-DEV-19c
/// **The property that keeps a reveal off an exported file.**
///
/// `compose_full` is the entry point the exporter, the thumbnail and the
/// neutral probe all use, and it has no argument that could ask for a
/// mask overlay. Only `compose_full_revealing` does, and only the canvas
/// calls it. Asserted rather than left to the type signature because the
/// tempting simplification — a flag on the graph — would type-check, be
/// shorter, and bake a red tint into every file the photographer sold.
#[test]
fn an_ordinary_composition_cannot_draw_a_mask_over_the_picture() {
use crate::mask::{MaskLayer, MaskSource, Reveal, RevealStyle};
let mut stack = MaskStack::new();
let mut layer = MaskLayer::new("m1", MaskSource::brush());
layer.set_param("exposure", crate::descriptor::ParamId("exposure"), 1.0);
stack.push(layer);
let plain = compose_full(
&crate::ops::chain(),
&Framing::new(),
ColourSpace::Srgb,
&stack,
&crate::spot::SpotSet::new(),
&[],
);
assert!(
!plain.source.contains("==== showing mask"),
"an export must never carry the overlay"
);
let shown = compose_full_revealing(
&crate::ops::chain(),
&Framing::new(),
ColourSpace::Srgb,
&stack,
&crate::spot::SpotSet::new(),
&[],
Some(&Reveal {
layer: "m1".into(),
style: RevealStyle::Tint,
}),
);
assert!(shown.source.contains("==== showing mask"));
assert_ne!(
plain.structure_hash, shown.structure_hash,
"two different shaders must not share a pipeline cache entry"
);
}
/// Distortion alone samples once; chromatic aberration samples three times. /// Distortion alone samples once; chromatic aberration samples three times.
/// ///
/// `splits_channels` is the whole reason for this test. Lateral CA fetches /// `splits_channels` is the whole reason for this test. Lateral CA fetches
+96 -19
View File
@@ -36,12 +36,18 @@ the *whole* boundary. When the model's coverage stops two pixels inside the
shoulder and leaks four pixels into the hair, no global number fixes both, and shoulder and leaks four pixels into the hair, no global number fixes both, and
that is the ordinary case rather than a corner one. that is the ordinary case rather than a corner one.
**And you cannot see the mask.** The overlay on the canvas is **And you cannot see the mask.** *(Built — see §6.)* The overlay on the canvas
[`overlay_rgba`](../ui/dr-ui/src/segmentation.rs) — a CPU-built false-colour was [`overlay_rgba`](../ui/dr-ui/src/segmentation.rs) and nothing else — a
picture of *what the model detected*, at proxy resolution. It is not the CPU-built false-colour picture of *what the model detected*, at proxy
layer's alpha: it knows nothing of the layer's feather, its falloff, its resolution. It is not the layer's alpha: it knows nothing of the layer's
morphology, its invert, or its opacity. Nobody can refine an edge they are not feather, its falloff, its morphology, its invert, or its opacity. Nobody can
being shown. refine an edge they are not being shown.
That one turned out to be load-bearing for the other three rather than the
last of four. With no way to see a mask, choosing a category produced a layer
whose extent was invisible and whose adjustment had not been touched yet — so
the correct behaviour and the broken one look identical, and "the segmentation
does not make masks" is what it reads as from the outside.
Those four are one feature, and this is its specification. Those four are one feature, and this is its specification.
@@ -390,31 +396,89 @@ usable along hair.
## 6. Seeing the mask ## 6. Seeing the mask
The mask array is **already bound to the composed adjust shader** — this is **Status: built.** `MaskStack::rendered`, `RevealStyle`,
almost free, and it is the first thing to build, because every other tool here `EditGraph::compose_revealing`, `MaskPass::render_revealing`, and the "Show
mask" strip in the Local panel.
The mask array is **already bound to the composed adjust shader**, so this is
almost free, and it is the first thing to build because every other tool here
is unusable without it. is unusable without it.
Two uniforms in the composed shader: which layer to reveal (−1 for none) and ### 6.1 One correction to this section, found in the building
which style. The block is always emitted, guarded by the uniform, so switching
the overlay on is a uniform write rather than a shader recompile. The draft said "two uniforms — which layer to reveal (−1 for none) and which
style — always emitted and guarded by the uniform, so switching the overlay on
is a uniform write rather than a shader recompile". The second half of that is
not available, and the reason is the case the feature exists for.
A uniform can *select* a slot. It cannot conjure one. A layer with no
adjustment on it changes no pixel, so it is not `is_active`, so it occupies no
slice of the mask array and the rasteriser never draws it — and that is
precisely the layer a photographer wants to look at, for the whole of the time
between choosing a subject and deciding what to do to it. Revealing it means
*rendering* it, which changes the sequence of layers, which changes the
uniform block. The composition moves either way.
So the slot and the style are written into the source, and turning the reveal
on, off, or onto another layer recompiles the fused shader. That is a button
press rather than a frame, and the uniform would only have added a branch per
pixel on top of a recomposition that was happening anyway.
`MaskStack::rendered(reveal)` is the one sequence this rests on: `active()`
plus the layer being looked at. The rasteriser, the composer and the distance
field builder all index by position in it, so all three must be given the same
`reveal` — two of them disagreeing shows as an adjustment applied through
another layer's mask, which is why they take it as an argument rather than
reading a flag.
### 6.2 Where the block runs, and why not with the others
After the output transform, immediately before the clip and the encode — not
among the layer blocks. Everything there runs on scene-referred colour in the
working space, where a flat tint would be pushed through the base curve and
the camera matrix and arrive as some other colour, and an alpha's white on
black would arrive as neither.
### 6.3 Not on the graph
The reveal is an argument to `EditGraph::compose_revealing`, and
`compose_for` — which the exporter, the thumbnail and the neutral probe all
call — has no way to ask for one. A flag on the graph would have been fewer
parameters, would have type-checked, and would have been one forgotten reset
away from a red tint baked into an exported file.
Three styles, all read from the same alpha: Three styles, all read from the same alpha:
- **Tint** — the mask over the picture in a flat colour at ~50%. The default, - **Tint** — the mask over the picture in a flat colour at ~50%. The default,
and what every editor's photographers already expect. Red by default and and what every editor's photographers already expect. Red, and *not yet*
configurable, since a red tint over a red dress shows nothing. configurable: the case for choosing the colour is a red tint over a red
dress, and the answer to that is the Alpha style beside it until somebody
finds a picture where neither works.
- **Alpha** — the mask alone, white on black. For judging an edge, where a - **Alpha** — the mask alone, white on black. For judging an edge, where a
tint over a busy picture cannot be read. tint over a busy picture cannot be read.
- **Edge** — the boundary outlined over the untouched picture. For checking - **Edge** — the boundary outlined over the untouched picture. For checking
registration against detail the other two hide, and the same reasoning the registration against detail the other two hide, and the same reasoning the
region overlay's white outline already carries. region overlay's white outline already carries.
**When it appears.** Automatically while a mask tool is armed, while a part **When it appears.** From the "Show mask" strip in the Local panel, which sits
row is selected, and for ~1s after a shaping slider is released; manually from beside the tool strip and under the same condition — both describe one
a control in the Local panel. The existing `overlay-hidden` property is the selected mask. Off is the resting state, so entering local mode does not paint
precedent and the trap it documents applies unchanged: the photographer's a photograph red.
switch and the automatic reveal are separate questions, and an automatic
reveal must never silently re-arm a switch the photographer turned off. Automatic in exactly one place: arming Paint or Erase turns the tint on if
nothing was showing the mask, which is the same nudge `on_part_added` makes and
on the same argument — a stroke into an invisible mask is indistinguishable
from a tool that did nothing.
A nudge on an explicit action, never a standing rule. Turning the view off and
then picking the eraser leaves it off. The existing `overlay-hidden` property
is the precedent and the trap it documents applies unchanged: an automatic
reveal that re-arms a switch somebody turned off is worse than no automatic
reveal at all.
The two remaining triggers in the draft — while a part row is selected, and for
~1s after a shaping slider is released — are not built. The second is the one
worth returning to, and it is a timer rather than a decision.
The region overlay stays exactly what it is — a picture of what the model The region overlay stays exactly what it is — a picture of what the model
detected — and gains a name in the interface that says so, because two detected — and gains a name in the interface that says so, because two
@@ -597,6 +661,19 @@ the mask pass beyond the blend variants; no change to distance fields, because
a painted part needs none. *This is the whole of the user-visible ask except a painted part needs none. *This is the whole of the user-visible ask except
push and intersect,* and it is deliberately the milestone that stands alone. push and intersect,* and it is deliberately the milestone that stands alone.
Done: parts with `Union` and `Subtract`, painted parts, the tool strip, the
canvas gesture, and the overlay (§6). Outstanding: the brush HUD and cursor —
radius, hardness and flow are sliders with no ring drawn on the photograph,
so the size of the brush is a number rather than a thing you can see — and
the incremental raster of §5.4, without which a layer's whole stroke history
is redrawn per dab.
One lesson from the order it was actually built in, since §6 said it and the
build did not listen: the overlay is not the last quarter of M1, it is the
first. Parts and painting shipped without it and the result was a feature
nobody could tell was working — the panel listed a mask, the photograph showed
nothing, and every report of it came back as "the masks do not work".
**M2 — Combine properly.** `Intersect`, model and gradient and range parts, **M2 — Combine properly.** `Intersect`, model and gradient and range parts,
per-part distance fields (§5.5), the part list with its join chips, folding per-part distance fields (§5.5), the part list with its join chips, folding
two layers. two layers.
+14
View File
@@ -517,6 +517,20 @@ hardness and flow the photographer sets. Strokes are stored as normalised source
rasterised on the device, so a correction stays on what it was painted on through a crop, a zoom and rasterised on the device, so a correction stays on what it was painted on through a crop, a zoom and
an export at any size. A whole stroke is one step in the history. an export at any size. A whole stroke is one step in the history.
**FR-DEV-19c — Seeing the mask.** The mask a selected layer actually resolves to shall be drawable
over the photograph, in a tint, as an alpha, or as an outline. What is shown is the finished mask —
every part folded, with the layer's feather, falloff, morphology, invert and opacity applied — and
it is shown for a layer that carries no adjustment yet, which is the state every mask is in for its
first few seconds. No rendered output ever carries it: the reveal is a property of looking at an
edit, not of the edit, so it reaches the pipeline through the composition that draws the canvas and
through no other.
Nobody can refine an edge they are not being shown. Before this the only thing drawn on the
photograph was FR-DEV-3's region overlay — a false-coloured picture of what the *model detected*,
which knows nothing of a layer's shaping and nothing at all about a gradient, a range or a stroke —
so choosing a subject or a category produced a layer whose extent was invisible, and every control
in FR-DEV-19a and FR-DEV-19b acted on something the photographer could not see.
### 3.4 Display and interaction ### 3.4 Display and interaction
**FR-DSP-1 — Proxy-resolution rendering.** The develop view renders at the resolution actually **FR-DSP-1 — Proxy-resolution rendering.** The develop view renders at the resolution actually
+109 -7
View File
@@ -865,6 +865,20 @@ pub struct DevelopSession {
selected_spot: Option<String>, selected_spot: Option<String>,
/// Whether to draw the false-coloured region overlay. /// Whether to draw the false-coloured region overlay.
show_overlay: bool, show_overlay: bool,
/// TRACES: FR-DEV-19c
/// How the selected layer's mask is being shown, or `None` for not at all.
///
/// Interface state, like `show_overlay` beside it and `active_masks` above
/// — it changes no pixel of the photograph, it is not in the sidecar and
/// it is not on the undo stack. It reaches the pipeline as an argument to
/// the one composition that draws the canvas, which is what makes an
/// export structurally unable to carry it (`EditGraph::compose_revealing`).
///
/// The *style* is stored and not the layer, because the layer is always
/// the selected one: a reveal that stayed pointed at a mask nobody was
/// working on would be describing the wrong thing every time the selection
/// moved, and there is no gesture that wants it.
reveal_style: Option<dr_pipeline::mask::RevealStyle>,
/// Which attribute the panel is filtered to, or all of them. /// Which attribute the panel is filtered to, or all of them.
/// ///
/// `None` is "show everything" and is what a frontend that ignores /// `None` is "show everything" and is what a frontend that ignores
@@ -1020,6 +1034,7 @@ impl DevelopSession {
), ),
selected_spot: None, selected_spot: None,
show_overlay: false, show_overlay: false,
reveal_style: None,
active_tab: None, active_tab: None,
curve_channel: 0, curve_channel: 0,
display_space: dr_types::ColourSpace::Srgb, display_space: dr_types::ColourSpace::Srgb,
@@ -1958,7 +1973,22 @@ impl DevelopSession {
} }
}; };
for layer in self.graph.masks().active() { // TRACES: FR-DEV-19c
// The reveal is in the key because it is in the *sequence*: revealing
// a layer with no adjustment on it gives that layer a slot, which
// renumbers every field after it. Leaving it out is the bug where
// clicking a subject shows the mask of whichever layer happened to be
// beneath it.
let reveal = self.reveal();
mix(reveal.as_ref().map_or(0, |r| {
let mut h: u64 = 1;
for b in r.layer.as_bytes() {
h = h.wrapping_mul(31).wrapping_add(*b as u64);
}
h
}));
for layer in self.graph.masks().rendered(reveal.as_ref()) {
match &layer.base().source { match &layer.base().source {
MaskSource::Subject { index, .. } => { MaskSource::Subject { index, .. } => {
mix(1); mix(1);
@@ -2147,10 +2177,13 @@ impl DevelopSession {
} }
}; };
// In `active()` order, because that is the order the rasteriser walks // In `rendered()` order, because that is the order the rasteriser
// and the order it indexes these by. // walks and the order it indexes these by — the revealed layer
// included, which is why the reveal is asked for here and folded into
// the key above.
let reveal = self.reveal();
let mut fields: Vec<Vec<f32>> = Vec::new(); let mut fields: Vec<Vec<f32>> = Vec::new();
for layer in self.graph.masks().active() { for layer in self.graph.masks().rendered(reveal.as_ref()) {
let field = match self.layer_coverage(layer, pw, ph) { let field = match self.layer_coverage(layer, pw, ph) {
Some(coverage) => { Some(coverage) => {
dr_segment::Shaped::build( dr_segment::Shaped::build(
@@ -2193,7 +2226,16 @@ impl DevelopSession {
} }
fn rasterise_masks(&mut self) -> bool { fn rasterise_masks(&mut self) -> bool {
if self.graph.masks().is_neutral() { // TRACES: FR-DEV-19c
// A stack that changes no pixel is normally not worth a pass — except
// when one of its layers is being looked at, which is exactly the
// state a fresh selection is in. Asking `rendered_count` rather than
// `is_neutral` is what makes "click a category, see its mask" work at
// all: before it, the array was never rasterised, the slice the reveal
// samples held whatever was last in it, and the answer was a blank
// photograph.
let reveal = self.reveal();
if self.graph.masks().rendered_count(reveal.as_ref()) == 0 {
return false; return false;
} }
// **Source space, at the segmentation's proxy size** — not the // **Source space, at the segmentation's proxy size** — not the
@@ -2239,13 +2281,14 @@ impl DevelopSession {
// No label field: region masks were the watershed's, and nothing // No label field: region masks were the watershed's, and nothing
// produces one any more. A stored layer that still names regions is // produces one any more. A stored layer that still names regions is
// skipped by the rasteriser rather than drawn wrong. // skipped by the rasteriser rather than drawn wrong.
pass.render( pass.render_revealing(
self.graph.masks(), self.graph.masks(),
None, None,
subjects, subjects,
Some(source.as_ref()), Some(source.as_ref()),
pw, pw,
ph, ph,
reveal.as_ref(),
) )
.inspect_err(|e| log::warn!("mask rasterisation failed: {e}")) .inspect_err(|e| log::warn!("mask rasterisation failed: {e}"))
.is_ok() .is_ok()
@@ -2426,6 +2469,60 @@ impl DevelopSession {
.unwrap_or_default() .unwrap_or_default()
} }
// ----------------------------------------------------------------------
// Seeing the mask (FR-DEV-19c)
// ----------------------------------------------------------------------
/// TRACES: FR-DEV-19c
/// What the canvas should draw over the photograph, if anything.
///
/// **Only when exactly one layer is selected**, on the same rule the part
/// list and the brush follow: a mask is one thing, and drawing the union
/// of three because three rows were control-clicked would answer a
/// question nobody asked. `None` there rather than picking the first, so
/// the reveal disappearing is itself the signal that the selection is not
/// what it needs to be.
///
/// Rebuilt per call rather than kept in step with the selection, because
/// it is two fields and a clone of one id — cheaper than the invalidation
/// a cached copy would need every time a layer is added, removed, renamed
/// or reordered.
pub(crate) fn reveal(&self) -> Option<dr_pipeline::mask::Reveal> {
let style = self.reveal_style?;
let [id] = self.active_masks.as_slice() else {
return None;
};
Some(dr_pipeline::mask::Reveal {
layer: id.clone(),
style,
})
}
/// How the selected layer's mask is being shown, as an index into
/// [`dr_pipeline::mask::RevealStyle::ALL`], or `0` for not at all.
///
/// An index because the panel offers it as a strip of chips and an index
/// is what a strip of chips reports. The enum stays the thing that is
/// stored, so a fourth style is a variant and a label rather than a number
/// two files have to agree on.
pub fn mask_view(&self) -> usize {
use dr_pipeline::mask::RevealStyle;
self.reveal_style
.and_then(|s| RevealStyle::ALL.iter().position(|&a| a == s))
.map_or(0, |i| i + 1)
}
/// Show the selected layer's mask, or stop.
///
/// Takes no history step and marks nothing dirty: this is how the
/// photograph is being *looked at*, not an edit to it.
pub fn set_mask_view(&mut self, view: usize) {
use dr_pipeline::mask::RevealStyle;
self.reveal_style = view
.checked_sub(1)
.and_then(|i| RevealStyle::ALL.get(i).copied());
}
// ---------------------------------------------------------------------- // ----------------------------------------------------------------------
// The region overlay // The region overlay
// ---------------------------------------------------------------------- // ----------------------------------------------------------------------
@@ -3738,7 +3835,12 @@ impl DevelopSession {
// always entered the structure hash. Moving the window to a P3 panel // always entered the structure hash. Moving the window to a P3 panel
// therefore costs one shader compile and no pipeline change at all. // therefore costs one shader compile and no pipeline change at all.
let space = self.display_space; let space = self.display_space;
let shader = self.graph.compose_for(space); // TRACES: FR-DEV-19c
// **The one composition that may show a mask.** Every other caller of
// the graph — `render_the_file`, the thumbnail, `sample_as_shot` —
// goes through `compose_for`, which cannot ask for a reveal, so no
// exported file can carry one.
let shader = self.graph.compose_revealing(space, self.reveal().as_ref());
// Rasterise the masks first: the shader addresses array slices by // Rasterise the masks first: the shader addresses array slices by
// index, so the array has to describe *this* stack before it is bound. // index, so the array has to describe *this* stack before it is bound.
+86 -6
View File
@@ -264,6 +264,12 @@ pub(crate) fn sync(window: &AppWindow, session: &Rc<RefCell<Option<DevelopSessio
}; };
masking.set_parts(ModelRc::new(VecModel::from(parts))); masking.set_parts(ModelRc::new(VecModel::from(parts)));
// TRACES: FR-DEV-19c
// Pushed rather than assumed, because the session is where it lives and
// because it can change without the panel having asked: selecting a
// second layer takes the reveal away, since a mask is one thing.
masking.set_mask_view(s.mask_view() as i32);
let (radius, hardness, flow) = s.brush(); let (radius, hardness, flow) = s.brush();
masking.set_brush_radius(radius); masking.set_brush_radius(radius);
masking.set_brush_hardness(hardness); masking.set_brush_hardness(hardness);
@@ -279,7 +285,16 @@ pub(crate) fn sync(window: &AppWindow, session: &Rc<RefCell<Option<DevelopSessio
// proxy-sized RGBA buffer — a megabyte or so — and rebuilding it on every // proxy-sized RGBA buffer — a megabyte or so — and rebuilding it on every
// slider event would be a memcpy per frame for a picture that changes only // slider event would be a memcpy per frame for a picture that changes only
// when the level does. // when the level does.
match s.overlay_image() { //
// TRACES: FR-DEV-19c
// **Stood down while a mask is being shown.** The two overlays answer
// different questions — this one is what the model *detected*, the reveal
// is what a layer resolves to — and both at once is a false-coloured
// picture over a tinted one, through which neither can be read. The one
// describing the layer being worked on wins, because by the time a mask
// has been chosen the detections are what the photographer is choosing
// *between* rather than what they are looking at.
match s.overlay_image().filter(|_| s.mask_view() == 0) {
Some(image) => { Some(image) => {
window.set_region_overlay(image); window.set_region_overlay(image);
window.set_overlay_on(true); window.set_overlay_on(true);
@@ -558,6 +573,7 @@ pub(crate) fn wire(
{ {
let weak = window.as_weak(); let weak = window.as_weak();
let session = session.clone(); let session = session.clone();
let redraw = redraw.clone();
let rows = rows.clone(); let rows = rows.clone();
window window
.global::<Masking>() .global::<Masking>()
@@ -587,6 +603,11 @@ pub(crate) fn wire(
// The scope changed, so the adjust panel below is now describing a // The scope changed, so the adjust panel below is now describing a
// different chain. // different chain.
sync_rows(&w, &rows, &session); sync_rows(&w, &rows, &session);
// TRACES: FR-DEV-19c
// And so has the picture, when a mask is being shown: the
// reveal follows the selection, so choosing another layer
// draws another mask.
redraw(&w);
}); });
} }
{ {
@@ -841,13 +862,66 @@ pub(crate) fn wire(
{ {
let weak = window.as_weak(); let weak = window.as_weak();
let session = session.clone(); let session = session.clone();
window.global::<Masking>().on_tool_picked(move |_tool| { let redraw = redraw.clone();
// The tool itself lives in the interface — it arms a gesture and window.global::<Masking>().on_tool_picked(move |tool| {
// changes no pixel — so nothing is set here. What this does is // **The tool is written back here, and that is the whole of this
// re-sync, because arming the brush is what makes the parts of a // handler.** `Masking.tool` is an `in` property: the panel reads
// mask worth showing. // it to light the right chip and `app.slint` reads it to decide
// whether a drag on the photograph paints, but only Rust may write
// it. So a version of this that recorded nothing left the strip
// reporting "Select" however many times "Paint" was pressed, and
// the paint area was never armed — the brush, the parts, the whole
// of FR-DEV-19b, reachable from no control in the application.
//
// It still changes no pixel, which is what the previous note was
// getting at: the tool arms a gesture and belongs to the
// interface, not to the edit, so it takes no history step and is
// not stored.
let Some(w) = weak.upgrade() else { return }; let Some(w) = weak.upgrade() else { return };
let tool = tool.clamp(0, 2);
w.global::<Masking>().set_tool(tool);
// TRACES: FR-DEV-19c
// Arming a brush shows the mask, if nothing was showing it. The
// same nudge `on_part_added` makes and on the same argument: a
// photographer about to correct an edge by hand needs to see the
// edge, and "the tool did nothing" is what a stroke into an
// invisible mask looks like.
//
// A nudge on an explicit action, never a standing rule. Turning
// the view off and then picking the eraser leaves it off — the
// trap `overlay-hidden` documents is that an automatic reveal
// which re-arms a switch somebody turned off is worse than no
// automatic reveal at all.
if tool > 0 {
if let Some(s) = session.borrow_mut().as_mut() {
if s.mask_view() == 0 {
s.set_mask_view(1);
}
}
}
// Re-synced because arming the brush is what makes the parts of a
// mask worth showing.
sync(&w, &session); sync(&w, &session);
redraw(&w);
});
}
// TRACES: FR-DEV-19c
{
let weak = window.as_weak();
let session = session.clone();
let redraw = redraw.clone();
window.global::<Masking>().on_mask_view_picked(move |view| {
let Some(w) = weak.upgrade() else { return };
if let Some(s) = session.borrow_mut().as_mut() {
s.set_mask_view(view.max(0) as usize);
}
// A redraw and not only a sync: the reveal is in the composed
// shader, so what changed is the picture rather than the
// panel.
sync(&w, &session);
redraw(&w);
}); });
} }
// TRACES: FR-DEV-19a // TRACES: FR-DEV-19a
@@ -1127,6 +1201,12 @@ pub(crate) fn reset(window: &AppWindow) {
masking.set_segmenting(false); masking.set_segmenting(false);
masking.set_refining(false); masking.set_refining(false);
masking.set_segmented(false); masking.set_segmented(false);
// TRACES: FR-DEV-19b | FR-DEV-19c
// The tool and the mask view are both about one selected layer, and the
// next photograph has none. Left standing, they would arm a brush over a
// photograph with nothing to paint into and claim a mask was being shown.
masking.set_tool(0);
masking.set_mask_view(0);
masking.set_masks(ModelRc::new(VecModel::<MaskRow>::default())); masking.set_masks(ModelRc::new(VecModel::<MaskRow>::default()));
masking.set_subjects(ModelRc::new(VecModel::<SubjectRow>::default())); masking.set_subjects(ModelRc::new(VecModel::<SubjectRow>::default()));
masking.set_categories(ModelRc::new(VecModel::<CategoryRow>::default())); masking.set_categories(ModelRc::new(VecModel::<CategoryRow>::default()));
+57 -3
View File
@@ -582,7 +582,23 @@ export global Masking {
in property <[PartRow]> parts; in property <[PartRow]> parts;
/// TRACES: FR-DEV-19b /// TRACES: FR-DEV-19b
/// What a drag on the photograph does: 0 selects, 1 paints, 2 erases. /// What a drag on the photograph does: 0 selects, 1 paints, 2 erases.
///
/// Written by Rust and read here, like every other `in` property on this
/// global: `tool-picked` is what the strip reports, and the answer comes
/// back through this. A version of the handler that recorded nothing left
/// the strip stuck on "Select" and the brush reachable from no control.
in property <int> tool: 0; in property <int> tool: 0;
/// TRACES: FR-DEV-19c
/// How the selected layer's mask is drawn over the photograph: 0 not at
/// all, then tint, alpha and edge — `dr_pipeline::mask::RevealStyle::ALL`
/// shifted by one so that zero can mean off.
///
/// **Not `overlay-hidden`.** That switch belongs to the region overlay,
/// which is a picture of what the *model detected*; this is the mask a
/// layer actually resolves to, feather, morphology, invert and all. Two
/// overlays that look alike and mean different things is worse than
/// either, so they are named apart and switched apart.
in property <int> mask-view: 0;
/// The brush: radius as a fraction of the frame's shorter edge, hardness, /// The brush: radius as a fraction of the frame's shorter edge, hardness,
/// and flow. /// and flow.
in property <float> brush-radius: 0.05; in property <float> brush-radius: 0.05;
@@ -630,6 +646,8 @@ export global Masking {
/// TRACES: FR-DEV-19b /// TRACES: FR-DEV-19b
callback tool-picked(int); callback tool-picked(int);
/// TRACES: FR-DEV-19c
callback mask-view-picked(int);
callback brush-changed(float, float, float); callback brush-changed(float, float, float);
/// TRACES: FR-DEV-19a /// TRACES: FR-DEV-19a
callback part-selected(string, int); callback part-selected(string, int);
@@ -638,6 +656,8 @@ export global Masking {
callback part-added(string, int); callback part-added(string, int);
callback add-gradient(bool); callback add-gradient(bool);
/// TRACES: FR-DEV-19b
callback add-brush();
/// `true` asks for the colour range rather than the brightness one. /// `true` asks for the colour range rather than the brightness one.
callback add-range(bool); callback add-range(bool);
callback add-subject(int); callback add-subject(int);
@@ -680,6 +700,32 @@ export component MaskPanel inherits Rectangle {
picked(i) => { Masking.tool-picked(i); } picked(i) => { Masking.tool-picked(i); }
} }
// TRACES: FR-DEV-19c
// **Seeing the mask, which every tool above is unusable without.**
// Nobody can refine an edge they are not being shown, and until this
// existed the only thing drawn on the photograph was the model's
// detections — which know nothing of a layer's feather, its falloff,
// its morphology, its invert or its opacity, and nothing at all about
// a gradient or a stroke.
//
// Four chips rather than a toggle because the three styles answer
// three different questions and no one of them answers all three: a
// tint says whether the right thing is selected, an alpha says where
// the edge is, an outline says whether that edge is registered against
// detail the other two hide.
//
// Beside the tool strip and under the same condition, because both
// describe one selected mask — and `columns: 2` for the reason
// `ChipGrid` exists: four chips in a row would set the width of the
// develop column and take every other panel's controls off the edge.
if Develop.enabled && Masking.parts.length > 0: Segmented {
label: "Show mask";
options: ["Off", "Tint", "Alpha", "Edge"];
columns: 2;
selected: Masking.mask-view;
picked(i) => { Masking.mask-view-picked(i); }
}
if Develop.enabled && Masking.tool > 0: SliderRow { if Develop.enabled && Masking.tool > 0: SliderRow {
label: "Size"; label: "Size";
value: Masking.brush-radius; value: Masking.brush-radius;
@@ -752,10 +798,18 @@ export component MaskPanel inherits Rectangle {
// a mask is judged against the photograph under it, and you cannot see // a mask is judged against the photograph under it, and you cannot see
// that photograph through the thing describing it. // that photograph through the thing describing it.
// //
// Reads as its own action rather than its state: "Show the mask" is // Reads as its own action rather than its state: "Show what was found"
// what pressing it will do, not what is currently true. // is what pressing it will do, not what is currently true.
//
// TRACES: FR-DEV-19c
// **Named for what it actually hides**, which is not the mask. This
// said "Show the mask" / "Hide the mask" while switching the
// false-coloured picture of what the *model detected* — and now that
// there is a control which really does show a mask ("Show mask",
// above), two things called the same thing and meaning different ones
// would be worse than either.
if Develop.enabled && Masking.segmented: Button { if Develop.enabled && Masking.segmented: Button {
text: Masking.overlay-hidden ? "Show the mask" : "Hide the mask"; text: Masking.overlay-hidden ? "Show what was found" : "Hide what was found";
clicked => { Masking.overlay-hidden = !Masking.overlay-hidden; } clicked => { Masking.overlay-hidden = !Masking.overlay-hidden; }
} }