From b1433ad4a9d18bff6527c55778807a424c953bc7 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Fri, 21 Aug 2026 22:46:44 +0200 Subject: [PATCH] Give a mask an edge treatment, and find out the watershed has none worth having MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things, and the second is why the first matters more than expected. Mask layers gain a feather, a falloff curve and a morphology, all defined against a signed distance from the boundary rather than as separate features — one exact distance field answers "how soft" and "how far" at once, so dilation is a threshold at -r, erosion one at +r, and closing and opening are one of each in sequence. The compound pair costs a second distance field, which is why they are named rather than presented as a radius that happens to be signed. Types, defaults and sidecar round-trip only; the field itself is next. `edge-feather` and `edge-falloff`, not `feather` and `falloff`, because a radial mask already writes `feather` for the fraction of its radius it ramps over. Same word, different quantity, different units — sharing the key would have made an existing file ambiguous. The diagnostic that provoked this is committed as an ignored test, because "does the ladder land on things a person means" is the question S15 exists to answer and it should not depend on whoever still has the script. On bus.jpg it answers badly: 35,075 regions at blur 2 over an 810x1080 frame, and cutting that to 400 gives *one* region covering nearly the whole picture plus 399 noise specks. Not over-segmentation — collapse. Almost every saddle is near zero, so the merge order joins everything meaningful before it joins anything spurious, and a global cut spends its entire budget on grain. So the granularity ladder does not currently work on a photograph, and the region masks built on it inherit that. Recorded rather than worked around: the next commits move local masking onto the model's instances, where the edge treatment above is what makes a quarter-resolution mask usable. --- core/dr-pipeline/src/mask.rs | 178 ++++++++++++++++++++++++++++++++ core/dr-pipeline/src/sidecar.rs | 56 +++++++++- ui/dr-ui/src/segmentation.rs | 93 +++++++++++++++++ ui/dr-ui/ui/masks.slint | 17 ++- 4 files changed, 340 insertions(+), 4 deletions(-) diff --git a/core/dr-pipeline/src/mask.rs b/core/dr-pipeline/src/mask.rs index 7ab3de3..13e76b8 100644 --- a/core/dr-pipeline/src/mask.rs +++ b/core/dr-pipeline/src/mask.rs @@ -37,6 +37,13 @@ use crate::descriptor::{OpDescriptor, ParamId}; use crate::operation::Operation; use crate::ops; +/// The feather a new layer starts with, as a fraction of the shorter edge. +/// +/// Named because two places have to agree about it: the constructor sets it, +/// and the sidecar omits it when unchanged. A literal in both would eventually +/// be a literal in one. +pub const DEFAULT_FEATHER: f32 = 0.004; + /// Per-layer uniforms the generated shader reads: `invert`, then `opacity`. pub const LAYER_UNIFORM_FIELDS: usize = 2; @@ -48,6 +55,137 @@ pub const LAYER_UNIFORM_FIELDS: usize = 2; /// Eight is comfortably past what an edit uses in practice and still bounded. pub const MAX_LAYERS: usize = 8; +/// How a mask's coverage falls away from its edge. +/// +/// Applied to the **signed distance** from the mask boundary, which is what +/// makes an arbitrary curve possible: the rasteriser computes one exact +/// Euclidean distance field and the choice below is a function of it, so a +/// new shape costs a line rather than a pass. +/// +/// A watershed boundary is pixel-exact, which is correct and also harsher +/// than any edit wants at a subject's edge — an exposure change that stops +/// dead at a hairline reads as a cut-out. So the useful default is a soft one. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum Falloff { + /// No transition. The boundary as the segmentation drew it. + /// + /// Worth keeping rather than approximating with a tiny feather: it is what + /// you want when checking *where* a boundary actually fell, and a feather + /// hides exactly that. + Hard, + /// Straight ramp. Predictable, and visibly banded on a gradient. + Linear, + /// Smoothstep — zero derivative at both ends. + /// + /// The default. The ends are where a ramp shows: a linear falloff leaves a + /// visible crease where the effect starts and where it stops, because the + /// eye finds discontinuities in the *slope*, not in the value. + #[default] + Smooth, + /// Gaussian-shaped. Softest, and reaches further than its radius suggests. + Gaussian, + /// Sharp near the edge, long tail. For blending an adjustment out over a + /// large area without moving the boundary itself. + Exponential, +} + +impl Falloff { + pub fn name(self) -> &'static str { + match self { + Self::Hard => "hard", + Self::Linear => "linear", + Self::Smooth => "smooth", + Self::Gaussian => "gaussian", + Self::Exponential => "exponential", + } + } + + pub fn from_name(name: &str) -> Option { + Some(match name { + "hard" => Self::Hard, + "linear" => Self::Linear, + "smooth" => Self::Smooth, + "gaussian" => Self::Gaussian, + "exponential" => Self::Exponential, + _ => return None, + }) + } + + /// Every variant, for a UI building a choice control. + pub const ALL: [Falloff; 5] = [ + Falloff::Hard, + Falloff::Linear, + Falloff::Smooth, + Falloff::Gaussian, + Falloff::Exponential, + ]; +} + +/// Growing, shrinking and tidying a mask's extent. +/// +/// All four are thresholds of the same distance field, which is why they +/// arrive together rather than one at a time: dilation is "distance ≥ −r", +/// erosion is "distance ≥ +r", and the two compound operations are one of +/// those followed by the other. +/// +/// The compound pair costs a **second** distance field, because after the +/// first threshold the shape has changed and the old distances no longer +/// describe it. That is a real cost and the reason they are named separately +/// rather than presented as a radius that happens to be signed. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum Morphology { + #[default] + None, + /// Grow. The everyday fix for a selection that stops just inside a + /// subject's edge, which is what an under-segmented boundary produces. + Dilate, + /// Shrink. Pulls a selection back off a halo it caught. + Erode, + /// Dilate then erode: fills pinholes and closes narrow gaps without + /// growing the outline. What to reach for when a mask is speckled with + /// missed pixels inside an area that is plainly one thing. + Close, + /// Erode then dilate: removes specks and thin spurs without shrinking the + /// outline. The complement, for a selection that leaked along an edge. + Open, +} + +impl Morphology { + pub fn name(self) -> &'static str { + match self { + Self::None => "none", + Self::Dilate => "dilate", + Self::Erode => "erode", + Self::Close => "close", + Self::Open => "open", + } + } + + pub fn from_name(name: &str) -> Option { + Some(match name { + "none" => Self::None, + "dilate" => Self::Dilate, + "erode" => Self::Erode, + "close" => Self::Close, + "open" => Self::Open, + _ => return None, + }) + } + + /// Whether this needs a second distance field. + pub fn is_compound(self) -> bool { + matches!(self, Self::Close | Self::Open) + } + + pub const ALL: [Morphology; 5] = [ + Morphology::None, + Morphology::Dilate, + Morphology::Erode, + Morphology::Close, + Morphology::Open, + ]; +} + /// Where a mask layer applies. #[derive(Debug, Clone, PartialEq)] pub enum MaskSource { @@ -128,6 +266,27 @@ pub struct MaskLayer { pub opacity: f32, /// Off without being deleted — the A/B a local edit is always wanting. pub enabled: bool, + + /// Half-width of the edge transition, as a fraction of the frame's + /// **shorter edge**. + /// + /// Normalised rather than in pixels for the same reason the gradient + /// geometry is: the same edit renders to a viewport and to a 24 MP export, + /// and a feather measured in pixels would be a different edge in each. + /// + /// Zero means no transition regardless of [`Self::falloff`]. + pub feather: f32, + pub falloff: Falloff, + + /// Grow, shrink or tidy the mask before the feather is applied. + /// + /// Before, and it matters: dilating a *feathered* mask would push the + /// half-way point outward and soften it further, so the two controls would + /// not be independent. Morphology moves the boundary; feather describes + /// how the boundary is crossed. + pub morphology: Morphology, + /// How far, in the same units as [`Self::feather`]. + pub morph_radius: f32, /// This layer's adjustments. /// /// A full chain, the same one [`crate::EditGraph`] holds. That is the @@ -155,6 +314,10 @@ impl Clone for MaskLayer { invert: self.invert, opacity: self.opacity, enabled: self.enabled, + feather: self.feather, + falloff: self.falloff, + morphology: self.morphology, + morph_radius: self.morph_radius, ops, } } @@ -169,6 +332,9 @@ impl std::fmt::Debug for MaskLayer { .field("invert", &self.invert) .field("opacity", &self.opacity) .field("enabled", &self.enabled) + .field("feather", &self.feather) + .field("falloff", &self.falloff) + .field("morphology", &self.morphology) .field("active_ops", &self.active_ops().count()) .finish() } @@ -182,6 +348,10 @@ impl PartialEq for MaskLayer { && self.invert == other.invert && self.opacity == other.opacity && self.enabled == other.enabled + && self.feather == other.feather + && self.falloff == other.falloff + && self.morphology == other.morphology + && self.morph_radius == other.morph_radius && self.params().eq(other.params()) } } @@ -196,6 +366,14 @@ impl MaskLayer { invert: false, opacity: 1.0, enabled: true, + // A small default rather than zero. A watershed boundary is exact + // to the pixel, and an adjustment that stops dead on one looks + // pasted on — the first thing anyone would reach for, so it is + // where the control starts. + feather: DEFAULT_FEATHER, + falloff: Falloff::default(), + morphology: Morphology::default(), + morph_radius: 0.0, ops: ops::chain(), } } diff --git a/core/dr-pipeline/src/sidecar.rs b/core/dr-pipeline/src/sidecar.rs index 16a50d4..4ea41cc 100644 --- a/core/dr-pipeline/src/sidecar.rs +++ b/core/dr-pipeline/src/sidecar.rs @@ -67,7 +67,7 @@ use std::fmt; use std::fmt::Write as _; use crate::graph::EditGraph; -use crate::mask::{MaskLayer, MaskSource, MaskStack}; +use crate::mask::{Falloff, MaskLayer, MaskSource, MaskStack, Morphology, DEFAULT_FEATHER}; use crate::preset::{resolve, Preset}; /// Format version of the document itself. @@ -735,6 +735,19 @@ fn write_mask(out: &mut String, version: &str, layer: &MaskLayer) { if !layer.enabled { let _ = writeln!(out, "enabled = 0"); } + // The edge treatment, written only when it is not the default — the same + // rule the parameters follow, so a file stays readable and a mask nobody + // has fiddled with contributes four fewer lines. + if layer.feather != DEFAULT_FEATHER { + let _ = writeln!(out, "edge-feather = {}", format_value(layer.feather)); + } + if layer.falloff != Falloff::default() { + let _ = writeln!(out, "edge-falloff = {}", layer.falloff.name()); + } + if layer.morphology != Morphology::default() { + let _ = writeln!(out, "morphology = {}", layer.morphology.name()); + let _ = writeln!(out, "morph-radius = {}", format_value(layer.morph_radius)); + } for (op, param, value) in layer.params() { let _ = writeln!(out, "{op}.{param} = {}", format_value(value)); } @@ -761,6 +774,12 @@ struct PartialMask { invert: bool, opacity: f32, enabled: bool, + /// The *layer's* edge transition, distinct from the radial source's own + /// `feather` above — different quantity, different units, different key. + edge_feather: f32, + falloff: Falloff, + morphology: Morphology, + morph_radius: f32, params: Vec<(String, String, f32)>, } @@ -782,6 +801,10 @@ impl PartialMask { invert: false, opacity: 1.0, enabled: true, + edge_feather: DEFAULT_FEATHER, + falloff: Falloff::default(), + morphology: Morphology::default(), + morph_radius: 0.0, params: Vec::new(), } } @@ -812,6 +835,33 @@ impl PartialMask { "invert" => self.invert = value != "0", "opacity" => self.opacity = value.parse::().unwrap_or(1.0).clamp(0.0, 1.0), "enabled" => self.enabled = value != "0", + // Clamped, not trusted: a feather wider than the frame is not a + // mask, and a negative one is a distance field read backwards. + "edge-feather" => { + self.edge_feather = value + .parse::() + .unwrap_or(DEFAULT_FEATHER) + .clamp(0.0, 1.0) + } + "morph-radius" => { + self.morph_radius = value.parse::().unwrap_or(0.0).clamp(0.0, 1.0) + } + // An unrecognised name falls back to the default rather than + // dropping the layer. A newer build's falloff curve is a cosmetic + // difference in the edge; losing the selection under it would not + // be cosmetic. + "edge-falloff" => { + self.falloff = Falloff::from_name(value).unwrap_or_else(|| { + log::warn!("sidecar: unknown falloff '{value}'; using the default"); + Falloff::default() + }) + } + "morphology" => { + self.morphology = Morphology::from_name(value).unwrap_or_else(|| { + log::warn!("sidecar: unknown morphology '{value}'; using none"); + Morphology::default() + }) + } _ => 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)); @@ -856,6 +906,10 @@ impl PartialMask { layer.invert = self.invert; layer.opacity = self.opacity; layer.enabled = self.enabled; + layer.feather = self.edge_feather; + layer.falloff = self.falloff; + layer.morphology = self.morphology; + layer.morph_radius = self.morph_radius; 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 diff --git a/ui/dr-ui/src/segmentation.rs b/ui/dr-ui/src/segmentation.rs index be9a434..e1ad3ef 100644 --- a/ui/dr-ui/src/segmentation.rs +++ b/ui/dr-ui/src/segmentation.rs @@ -713,4 +713,97 @@ mod tests { assert_eq!(a.region_count(), b.region_count()); assert_eq!(a.regions_at(0.5, 0.5), b.regions_at(0.5, 0.5)); } + + /// Dump the granularity ladder for a real photograph, to look at. + /// + /// Ignored, because it needs a file and writes several megabytes. It is + /// committed rather than kept in a scratch directory because "does the + /// ladder land on things a person means" is the question the whole spike + /// exists to answer (docs/segmentation.md §11 step 2), and it should be + /// answerable by anyone who picks this up rather than by whoever still has + /// the script. + /// + /// ```sh + /// # A P6 PPM, which needs no decoder here: + /// # magick photo.jpg -colorspace sRGB photo.ppm + /// DARKROOM_TEST_PPM=photo.ppm DARKROOM_OUT=/tmp/ws \ + /// cargo test -p dr-ui --release ladder_for_a_photograph -- --ignored --nocapture + /// ``` + #[test] + #[ignore = "needs a photograph; run by hand when judging the segmentation"] + fn ladder_for_a_photograph() { + let Ok(path) = std::env::var("DARKROOM_TEST_PPM") else { + eprintln!("set DARKROOM_TEST_PPM to a P6 .ppm"); + return; + }; + let prefix = std::env::var("DARKROOM_OUT").unwrap_or_else(|_| "ladder".into()); + let Some(ctx) = context() else { + eprintln!("no adapter"); + return; + }; + + let (rgb8, w, h) = read_ppm(&path); + println!("image {w} × {h}"); + + let rgba: Vec = rgb8 + .chunks_exact(3) + .flat_map(|p| [p[0], p[1], p[2], 255]) + .collect(); + let source = dr_gpu::DemosaicedImage::from_rgba8(&ctx, &rgba, w as u32, h as u32) + .expect("upload"); + let rgb: Vec = rgb8.iter().map(|&v| v as f32 / 255.0).collect(); + + for blur in [2, 5, 9] { + let mut options = Options { + semantic: false, + ..Options::default() + }; + options.segment.blur_radius = blur; + + let t = std::time::Instant::now(); + let mut seg = compute(&ctx, &source, &rgb, w, h, &options).expect("segmentation"); + println!( + "blur {blur:>2} {} regions in {:.0} ms", + seg.region_count(), + t.elapsed().as_secs_f32() * 1000.0 + ); + + for level in [1200, 400, 120, 40, 12] { + seg.set_level(level); + let (px, ow, oh) = seg.overlay_rgba(); + let rgb: Vec = px.chunks_exact(4).flat_map(|p| [p[0], p[1], p[2]]).collect(); + write_ppm(&format!("{prefix}-b{blur}-l{level}.ppm"), &rgb, ow, oh); + } + } + println!("wrote {prefix}-b*-l*.ppm"); + } + + fn read_ppm(path: &str) -> (Vec, usize, usize) { + let bytes = std::fs::read(path).expect("read ppm"); + // P6\n \n\n. Tokens are whitespace-separated, and + // comments are not handled because the writer above never emits them. + let mut fields = Vec::new(); + let mut i = 0; + while fields.len() < 4 { + while i < bytes.len() && bytes[i].is_ascii_whitespace() { + i += 1; + } + let start = i; + while i < bytes.len() && !bytes[i].is_ascii_whitespace() { + i += 1; + } + fields.push(String::from_utf8_lossy(&bytes[start..i]).to_string()); + } + assert_eq!(fields[0], "P6", "expected a binary PPM"); + let w: usize = fields[1].parse().expect("width"); + let h: usize = fields[2].parse().expect("height"); + (bytes[i + 1..].to_vec(), w, h) + } + + fn write_ppm(path: &str, rgb: &[u8], w: u32, h: u32) { + use std::io::Write as _; + let mut f = std::io::BufWriter::new(std::fs::File::create(path).expect("create")); + write!(f, "P6\n{w} {h}\n255\n").expect("header"); + f.write_all(rgb).expect("body"); + } } diff --git a/ui/dr-ui/ui/masks.slint b/ui/dr-ui/ui/masks.slint index b29f2b6..b0cdb9b 100644 --- a/ui/dr-ui/ui/masks.slint +++ b/ui/dr-ui/ui/masks.slint @@ -213,7 +213,7 @@ export component MaskPanel inherits Rectangle { HorizontalLayout { PanelHeading { text: "LOCAL"; } Rectangle { horizontal-stretch: 1; } - if root.segmented: Value { text: root.region-count + " regions"; } + if root.segmented: Value { text: root.level + " / " + root.region-count; } } if !root.enabled: Caption { text: "No image"; } @@ -264,19 +264,30 @@ export component MaskPanel inherits Rectangle { // Granularity, labelled by what it does rather than by its number: // "detail" is what a photographer is choosing between, where "300 // regions" is an implementation detail they would have to learn. + // **The ceiling is the region count, not a constant.** It was 2000, + // and a photograph that segments into more than that had the finest + // part of its own ladder unreachable — the slider simply stopped + // before the regions did. if root.enabled && root.segmented: SliderRow { label: "Detail"; + hint: "How finely a click divides the picture, out of " + + root.region-count + " regions the watershed found."; value: root.level; default-value: 300; minimum: 8; - maximum: 2000; + maximum: max(root.region-count, 8); changed(v) => { root.level-changed(v); } reset => { root.level-changed(300); } } // --- what the model found ---------------------------------------- if root.enabled && root.segmented && root.subjects.length > 0: Caption { - text: "Recognised"; + // Says where these came from and how they differ from a region. + // A list of four beside a count of three thousand invites exactly + // one question, and the panel should answer it rather than + // provoke it. + text: "Subjects the model recognised. Click one to select the whole thing."; + wrap: word-wrap; } if root.enabled && root.segmented: VerticalLayout {