Take the watershed out of the product path
It does not work on a photograph, so nothing should offer it. `Segmentation` is now one model pass and what it recognised: no region field, no merge tree, no label upload, no granularity slider, and no readback of the whole proxy to build a graph that collapses. A click means "the object under the cursor". The region-selection path went with the hierarchy it indexed — including the shift-click add/subtract, which has no meaning for a whole object and would have been a modifier that silently did nothing. The passes, the hierarchy and the semantic prior stay in `dr-gpu` and `dr-segment`, tested and documented. It is the *merge criterion* that fails — the saddle is the minimum gradient along a boundary, so one weak pixel merges two regions and real gradient noise puts a weak pixel on every boundary. That is one function to replace, and the evidence for replacing it is worth keeping. What is gone is the wiring, the option, and the control that offered a user a choice with no outcome. `MaskSource::Regions` remains in the pipeline: it is tested, it round-trips through the sidecar, and a stored layer that names regions must still load and be reported stale rather than failing to parse.
This commit is contained in:
+20
-91
@@ -26,6 +26,13 @@ use crate::labels;
|
||||
use crate::ParamRow;
|
||||
|
||||
/// A loaded image plus its edit state.
|
||||
/// Longest edge the model and the masks work at.
|
||||
///
|
||||
/// ~1.3 MP at 3:2. Large enough that an outline is within a pixel or two of
|
||||
/// where it belongs, small enough that a distance transform over it is a few
|
||||
/// milliseconds and its field a few megabytes.
|
||||
const SEGMENT_PROXY_EDGE: u32 = 1600;
|
||||
|
||||
pub struct DevelopSession {
|
||||
/// Kept so the session can build GPU resources after construction.
|
||||
///
|
||||
@@ -727,14 +734,16 @@ impl DevelopSession {
|
||||
return false;
|
||||
};
|
||||
let (pw, ph) = seg.proxy_size();
|
||||
let labels = seg.labels();
|
||||
let subjects = self.subjects.as_ref();
|
||||
let Some(pass) = self.masks.as_mut() else {
|
||||
return false;
|
||||
};
|
||||
// No label field: region masks were the watershed's, and nothing
|
||||
// produces one any more. A stored layer that still names regions is
|
||||
// skipped by the rasteriser rather than drawn wrong.
|
||||
pass.render(
|
||||
self.graph.masks(),
|
||||
labels,
|
||||
None,
|
||||
subjects,
|
||||
pw as u32,
|
||||
ph as u32,
|
||||
@@ -758,8 +767,8 @@ impl DevelopSession {
|
||||
// The model reads the photograph as captured, not as edited: the
|
||||
// segmentation must survive an exposure change, or every slider would
|
||||
// invalidate the masks that depend on it (docs/segmentation.md §3).
|
||||
let (rgb, rw, rh) = self.neutral_proxy(ctx, options.segment.max_edge)?;
|
||||
let seg = segmentation::compute(ctx, &self.demosaiced, &rgb, rw, rh, options)?;
|
||||
let (rgb, rw, rh) = self.neutral_proxy(ctx, SEGMENT_PROXY_EDGE)?;
|
||||
let seg = segmentation::compute(ctx, &rgb, rw, rh, options)?;
|
||||
|
||||
if self.masks.is_none() {
|
||||
self.masks = MaskPass::new(ctx)
|
||||
@@ -819,26 +828,6 @@ impl DevelopSession {
|
||||
self.segmentation.is_some()
|
||||
}
|
||||
|
||||
pub fn segmentation_level(&self) -> u32 {
|
||||
self.segmentation.as_ref().map_or(0, |s| s.level())
|
||||
}
|
||||
|
||||
/// Whether a region hierarchy exists to click into.
|
||||
pub fn has_regions(&self) -> bool {
|
||||
self.segmentation.as_ref().is_some_and(|s| s.has_regions())
|
||||
}
|
||||
|
||||
pub fn segmentation_region_count(&self) -> usize {
|
||||
self.segmentation.as_ref().map_or(0, |s| s.region_count())
|
||||
}
|
||||
|
||||
/// Move the granularity ladder — the scroll wheel over the canvas.
|
||||
pub fn set_segmentation_level(&mut self, level: u32) {
|
||||
if let Some(seg) = self.segmentation.as_mut() {
|
||||
seg.set_level(level);
|
||||
}
|
||||
}
|
||||
|
||||
/// The subjects the model recognised, as `(label, confidence)`.
|
||||
///
|
||||
/// Confidence is shown rather than hidden because the detector is offered
|
||||
@@ -951,76 +940,16 @@ impl DevelopSession {
|
||||
.map(|id| id.to_string());
|
||||
}
|
||||
|
||||
/// Select the region under a normalised image point.
|
||||
/// Mask the object under a normalised image point.
|
||||
///
|
||||
/// `add` extends the selected layer instead of replacing its selection,
|
||||
/// which is the shift-click every selection tool has. With no layer
|
||||
/// selected a new one is created, because clicking the photograph is how a
|
||||
/// local adjustment begins and requiring "add layer" first would be a step
|
||||
/// with no decision in it.
|
||||
/// Clicking the photograph is how a local adjustment begins, so this
|
||||
/// creates the layer — requiring "add layer" first would be a step with no
|
||||
/// decision in it.
|
||||
///
|
||||
/// Returns the layer that now holds the selection.
|
||||
pub fn select_region_at(&mut self, x: f32, y: f32, add: bool) -> Option<String> {
|
||||
// With no region hierarchy — the default — a click means "the object
|
||||
// under the cursor", which is the interaction the model can actually
|
||||
// support. `add` has no meaning for a whole object, so it is ignored
|
||||
// rather than quietly doing something else.
|
||||
if self.segmentation.as_ref().is_some_and(|s| !s.has_regions()) {
|
||||
let index = self.segmentation.as_ref()?.instance_at(x, y)?;
|
||||
return self.add_subject_mask(index);
|
||||
}
|
||||
|
||||
let seg = self.segmentation.as_ref()?;
|
||||
let picked = seg.regions_at(x, y);
|
||||
if picked.is_empty() {
|
||||
return None;
|
||||
}
|
||||
let (signature, level) = (seg.signature(), seg.level());
|
||||
|
||||
let id = match self.active_mask.clone() {
|
||||
Some(id) => id,
|
||||
None => {
|
||||
let id = self.graph.masks().next_id();
|
||||
let layer = MaskLayer::new(
|
||||
id.clone(),
|
||||
MaskSource::Regions {
|
||||
signature,
|
||||
level,
|
||||
ids: Vec::new(),
|
||||
},
|
||||
);
|
||||
if !self.graph.masks_mut().push(layer) {
|
||||
return None;
|
||||
}
|
||||
self.active_mask = Some(id.clone());
|
||||
id
|
||||
}
|
||||
};
|
||||
|
||||
let layer = self.graph.masks_mut().get_mut(&id)?;
|
||||
let mut ids = match (&layer.source, add) {
|
||||
(MaskSource::Regions { ids, .. }, true) => ids.clone(),
|
||||
_ => Vec::new(),
|
||||
};
|
||||
|
||||
// Clicking a region already in the selection removes it, so one
|
||||
// gesture both adds and corrects — the alternative is a modifier for
|
||||
// subtract that nobody remembers.
|
||||
if add && picked.iter().all(|r| ids.contains(r)) {
|
||||
ids.retain(|r| !picked.contains(r));
|
||||
} else {
|
||||
ids.extend(picked);
|
||||
}
|
||||
ids.sort_unstable();
|
||||
ids.dedup();
|
||||
|
||||
layer.source = MaskSource::Regions {
|
||||
signature,
|
||||
level,
|
||||
ids,
|
||||
};
|
||||
self.history.record(&self.graph, Edit::Discrete);
|
||||
Some(id)
|
||||
pub fn select_region_at(&mut self, x: f32, y: f32) -> Option<String> {
|
||||
let index = self.segmentation.as_ref()?.instance_at(x, y)?;
|
||||
self.add_subject_mask(index)
|
||||
}
|
||||
|
||||
/// Add a layer selecting one detected subject.
|
||||
|
||||
@@ -30,8 +30,6 @@ pub(crate) fn sync(window: &AppWindow, session: &Rc<RefCell<Option<DevelopSessio
|
||||
window.set_mask_rows(ModelRc::new(VecModel::<MaskRow>::default()));
|
||||
window.set_subject_rows(ModelRc::new(VecModel::<SubjectRow>::default()));
|
||||
window.set_segmented(false);
|
||||
window.set_segmentation_regions(0);
|
||||
window.set_has_regions(false);
|
||||
window.set_editing_mask(false);
|
||||
window.set_overlay_on(false);
|
||||
return;
|
||||
@@ -73,9 +71,6 @@ pub(crate) fn sync(window: &AppWindow, session: &Rc<RefCell<Option<DevelopSessio
|
||||
window.set_subject_rows(ModelRc::new(VecModel::from(subjects)));
|
||||
|
||||
window.set_segmented(s.has_segmentation());
|
||||
window.set_segmentation_level(s.segmentation_level() as i32);
|
||||
window.set_segmentation_regions(s.segmentation_region_count() as i32);
|
||||
window.set_has_regions(s.has_regions());
|
||||
window.set_editing_mask(active.is_some());
|
||||
|
||||
// The overlay is regenerated only when there is one to draw. It is a
|
||||
@@ -168,17 +163,6 @@ pub(crate) fn wire(
|
||||
w.set_region_picking(on);
|
||||
});
|
||||
}
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
let session = session.clone();
|
||||
window.on_segmentation_level_changed(move |level| {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
if let Some(s) = session.borrow_mut().as_mut() {
|
||||
s.set_segmentation_level(level.max(2) as u32);
|
||||
}
|
||||
sync(&w, &session);
|
||||
});
|
||||
}
|
||||
|
||||
// --- selecting on the photograph --------------------------------------
|
||||
{
|
||||
@@ -186,12 +170,12 @@ pub(crate) fn wire(
|
||||
let session = session.clone();
|
||||
let redraw = redraw.clone();
|
||||
let rows = rows.clone();
|
||||
window.on_region_picked(move |x, y, add| {
|
||||
window.on_region_picked(move |x, y, _add| {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let picked = session
|
||||
.borrow_mut()
|
||||
.as_mut()
|
||||
.and_then(|s| s.select_region_at(x, y, add));
|
||||
.and_then(|s| s.select_region_at(x, y));
|
||||
if picked.is_none() {
|
||||
// A click that hit no region is not an error and must not
|
||||
// clear the selection: the most likely cause is the letterbox
|
||||
@@ -387,7 +371,6 @@ pub(crate) fn reset(window: &AppWindow) {
|
||||
window.set_overlay_on(false);
|
||||
window.set_segmenting(false);
|
||||
window.set_segmented(false);
|
||||
window.set_has_regions(false);
|
||||
window.set_mask_rows(ModelRc::new(VecModel::<MaskRow>::default()));
|
||||
window.set_subject_rows(ModelRc::new(VecModel::<SubjectRow>::default()));
|
||||
window.set_editing_mask(false);
|
||||
|
||||
+204
-819
File diff suppressed because it is too large
Load Diff
@@ -819,15 +819,11 @@ export component AppWindow inherits Window {
|
||||
in property <[SubjectRow]> subject-rows;
|
||||
in property <bool> segmented: false;
|
||||
in property <bool> segmenting: false;
|
||||
in property <int> segmentation-level: 300;
|
||||
in property <int> segmentation-regions: 0;
|
||||
in property <bool> has-regions: false;
|
||||
in property <bool> editing-mask: false;
|
||||
|
||||
callback segment-image();
|
||||
callback overlay-toggled(bool);
|
||||
callback region-picking-toggled(bool);
|
||||
callback segmentation-level-changed(int);
|
||||
/// A click on the photograph, in fractions of the shown image, plus
|
||||
/// whether it should extend the selection rather than replace it.
|
||||
callback region-picked(float, float, bool);
|
||||
@@ -1906,9 +1902,6 @@ in property <bool> panel-visible: true;
|
||||
subjects: root.subject-rows;
|
||||
segmented: root.segmented;
|
||||
segmenting: root.segmenting;
|
||||
level: root.segmentation-level;
|
||||
region-count: root.segmentation-regions;
|
||||
has-regions: root.has-regions;
|
||||
overlay: root.overlay-on;
|
||||
picking: root.region-picking;
|
||||
editing-mask: root.editing-mask;
|
||||
@@ -1916,7 +1909,6 @@ in property <bool> panel-visible: true;
|
||||
segment => { root.segment-image(); }
|
||||
overlay-toggled(on) => { root.overlay-toggled(on); }
|
||||
picking-toggled(on) => { root.region-picking-toggled(on); }
|
||||
level-changed(v) => { root.segmentation-level-changed(v); }
|
||||
mask-selected(id) => { root.mask-selected(id); }
|
||||
mask-removed(id) => { root.mask-removed(id); }
|
||||
mask-toggled(id, on) => { root.mask-toggled(id, on); }
|
||||
|
||||
+2
-38
@@ -223,16 +223,6 @@ export component MaskPanel inherits Rectangle {
|
||||
in property <bool> segmented: false;
|
||||
/// One is being computed now.
|
||||
in property <bool> segmenting: false;
|
||||
/// How many regions the ladder is currently cut to.
|
||||
in property <int> level: 300;
|
||||
in property <int> region-count: 0;
|
||||
/// Whether a region hierarchy exists to click into.
|
||||
///
|
||||
/// Off in the ordinary case. The watershed's ladder collapses on a
|
||||
/// photograph, so the controls that drive it are hidden rather than shown
|
||||
/// doing nothing — a slider that changes no outcome is worse than an
|
||||
/// absent one, because it invites the user to blame themselves.
|
||||
in property <bool> has-regions: false;
|
||||
|
||||
/// Draw the false-coloured region map over the photograph.
|
||||
in property <bool> overlay: false;
|
||||
@@ -247,7 +237,6 @@ export component MaskPanel inherits Rectangle {
|
||||
callback segment();
|
||||
callback overlay-toggled(bool);
|
||||
callback picking-toggled(bool);
|
||||
callback level-changed(int);
|
||||
|
||||
callback mask-selected(string);
|
||||
callback mask-removed(string);
|
||||
@@ -279,11 +268,7 @@ export component MaskPanel inherits Rectangle {
|
||||
HorizontalLayout {
|
||||
PanelHeading { text: "LOCAL"; }
|
||||
Rectangle { horizontal-stretch: 1; }
|
||||
if root.segmented && root.has-regions: Value {
|
||||
text: root.level + " / " + root.region-count;
|
||||
}
|
||||
if root.segmented && !root.has-regions: Value {
|
||||
text: root.subjects.length + (root.subjects.length == 1 ? " subject" : " subjects");
|
||||
if root.segmented: Value { text: root.subjects.length + (root.subjects.length == 1 ? " subject" : " subjects");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -327,12 +312,7 @@ export component MaskPanel inherits Rectangle {
|
||||
}
|
||||
}
|
||||
|
||||
if root.enabled && root.segmented && root.picking && root.has-regions: Caption {
|
||||
text: "Click the photograph to select a region. Shift-click to add or remove.";
|
||||
wrap: word-wrap;
|
||||
}
|
||||
|
||||
if root.enabled && root.segmented && root.picking && !root.has-regions: Caption {
|
||||
if root.enabled && root.segmented && root.picking: Caption {
|
||||
text: "Click a subject in the photograph to mask it.";
|
||||
wrap: word-wrap;
|
||||
}
|
||||
@@ -340,22 +320,6 @@ 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 && root.has-regions: 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: 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 {
|
||||
// Says where these came from and how they differ from a region.
|
||||
|
||||
Reference in New Issue
Block a user