Give the develop column room, and the mask a way out of the way
Three faults reported together, and they share a shape: each is something the panel decided on the photographer's behalf. **The column was 280px on every screen.** That width was chosen for a tablet, where the column is a large fraction of the display and every pixel of it is taken from the photograph. On a desktop window the mode strip alone — two modes, a separator, "All", and a chip per attribute the operation set declares — does not fit, so it scrolled sideways. A control you have to pan to reach is one you do not know is there. 380px when the window is classed expanded, 280px when it is not; driven off `layout-class` because reading the window width inside the layout that sets it is a binding loop. **The overlay could not be hidden.** An earlier "Overlay" button was removed for a good reason — it *armed* the overlay, so local mode could be entered and still show nothing. Hiding is the opposite need and was never served: a mask is judged against the photograph beneath it, and that photograph is exactly what the overlay covers. `overlay-hidden` is kept separate from `overlay-on` so a recompute cannot switch the overlay back on under someone who just turned it off. **The masks were coarse because the model saw the subject small.** The graph's input is a fixed 640x640 and every frame is letterboxed into it, so a bird 200px across in a 1600px proxy reaches the model at 80px. `Tiling::Grid` has been implemented and tested since the segmentation spike and defaulted off, because it costs one inference per tile — 2.8s for a 3x2 grid against 470ms. Now offered as "Look closer (slower)", which says what it costs, rather than spending it on every image or on none. The tiling choice enters the segmentation signature. A mask stores the signature its region ids index into, and a tiled run finds different instances in a different order; sharing a signature would silently reinterpret a layer built against the coarse pass — a wrong mask rather than a stale one, and nothing announces it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -309,7 +309,7 @@ pub(crate) fn wire(
|
||||
let redraw = redraw.clone();
|
||||
let rows = rows.clone();
|
||||
let running = running.clone();
|
||||
window.on_segment_image(move || {
|
||||
window.on_segment_image(move |fine| {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
// Both the photograph and the device come out of the session, so
|
||||
// no session is nothing to look at and nothing to look with.
|
||||
@@ -329,7 +329,13 @@ pub(crate) fn wire(
|
||||
w.set_segmenting(true);
|
||||
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
let options = segmentation::Options::default();
|
||||
// The only thing the button decides. Everything else about the
|
||||
// run is the same, which is what makes the second pass a genuine
|
||||
// re-run of the first rather than a different feature.
|
||||
let options = segmentation::Options {
|
||||
fine,
|
||||
..segmentation::Options::default()
|
||||
};
|
||||
std::thread::spawn(move || {
|
||||
// A failed send means the window stopped waiting — the user
|
||||
// moved on, or the app is closing. Neither is worth reporting:
|
||||
|
||||
@@ -197,11 +197,29 @@ pub struct Options {
|
||||
/// costs a subject that cannot be selected at all, which is the worse
|
||||
/// failure for a selection tool.
|
||||
pub confidence: f32,
|
||||
/// TRACES: FR-DEV-3
|
||||
/// Run the model over overlapping tiles instead of the whole frame once.
|
||||
///
|
||||
/// Off by default and deliberately so. The graph's input is fixed at
|
||||
/// 640x640 (docs/segmentation.md F6), so every image is letterboxed into
|
||||
/// it and a subject 200px across in a 1600px proxy reaches the model at
|
||||
/// 80px — which is where a coarse outline comes from. Tiling is the only
|
||||
/// route to more resolution with a fixed window, and it costs one
|
||||
/// inference per tile: about 2.8s for a 3x2 grid against 470ms whole-frame.
|
||||
///
|
||||
/// Six times the wait is the wrong default for the common case, where the
|
||||
/// subject is large in frame and whole-frame inference is already the best
|
||||
/// answer. It is the right answer for a bird against sky, so it is offered
|
||||
/// per-image rather than chosen once for all of them.
|
||||
pub fine: bool,
|
||||
}
|
||||
|
||||
impl Default for Options {
|
||||
fn default() -> Self {
|
||||
Self { confidence: 0.30 }
|
||||
Self {
|
||||
confidence: 0.30,
|
||||
fine: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -218,7 +236,7 @@ pub fn compute(
|
||||
height: usize,
|
||||
options: &Options,
|
||||
) -> Result<Segmentation, String> {
|
||||
let found = detect(rgb, width, height)?;
|
||||
let found = detect(rgb, width, height, options.fine)?;
|
||||
|
||||
let instances: Vec<InstanceSummary> = found
|
||||
.iter()
|
||||
@@ -234,7 +252,19 @@ pub fn compute(
|
||||
width as u32,
|
||||
height as u32,
|
||||
instances.len() as u32,
|
||||
options.confidence.to_bits() as u64,
|
||||
// The tiling choice belongs in the signature as much as the
|
||||
// confidence does. A mask stores the signature of the segmentation its
|
||||
// region ids index into (`MaskSource::Regions`), and a tiled run finds
|
||||
// different instances in a different order — so if the two runs shared
|
||||
// a signature, a layer built against the coarse pass would be silently
|
||||
// reinterpreted against the fine one. That is a *wrong* mask, which is
|
||||
// far worse than a stale one, because nothing announces it.
|
||||
options.confidence.to_bits() as u64
|
||||
^ if options.fine {
|
||||
0x9E37_79B9_7F4A_7C15
|
||||
} else {
|
||||
0
|
||||
},
|
||||
);
|
||||
|
||||
Ok(Segmentation {
|
||||
@@ -249,10 +279,27 @@ pub fn compute(
|
||||
/// Loading is ~24 ms against the ~470 ms of inference that follows, and this
|
||||
/// runs once per image — so caching the session would keep 11 MB of weights
|
||||
/// resident for the life of the app to save five percent of a background task.
|
||||
fn detect(rgb: &[f32], width: usize, height: usize) -> Result<Vec<dr_segment::Instance>, String> {
|
||||
fn detect(
|
||||
rgb: &[f32],
|
||||
width: usize,
|
||||
height: usize,
|
||||
fine: bool,
|
||||
) -> Result<Vec<dr_segment::Instance>, String> {
|
||||
let mut model = dr_segment::SemanticModel::embedded().map_err(|e| e.to_string())?;
|
||||
let options = dr_segment::SemanticOptions {
|
||||
// A quarter shared with each neighbour. It has to exceed zero at all,
|
||||
// or a subject sitting on a seam is cut in half by both tiles and
|
||||
// recognised by neither; a quarter is enough to carry a whole subject
|
||||
// inside one tile at the sizes tiling is reached for.
|
||||
tiling: if fine {
|
||||
dr_segment::Tiling::Grid { overlap: 0.25 }
|
||||
} else {
|
||||
dr_segment::Tiling::Whole
|
||||
},
|
||||
..dr_segment::SemanticOptions::default()
|
||||
};
|
||||
model
|
||||
.detect(rgb, width, height, &dr_segment::SemanticOptions::default())
|
||||
.detect(rgb, width, height, &options)
|
||||
.map_err(|e| e.to_string())
|
||||
}
|
||||
|
||||
|
||||
+35
-4
@@ -909,6 +909,16 @@ export component AppWindow inherits Window {
|
||||
/// opaque map hides the thing being judged.
|
||||
in property <float> overlay-strength: 0.55;
|
||||
in property <bool> overlay-on: false;
|
||||
/// TRACES: FR-DEV-3
|
||||
/// The photographer's switch for the same overlay, kept separate from
|
||||
/// `overlay-on` because the two answer different questions: `overlay-on`
|
||||
/// is "is there a mask to draw", which Rust decides, and this is "do I
|
||||
/// want to look at it right now", which only they can.
|
||||
///
|
||||
/// Folding them into one property would mean the next recompute — any
|
||||
/// mask edit — silently switched the overlay back on under someone who had
|
||||
/// just turned it off to check their work against the photograph.
|
||||
in-out property <bool> overlay-hidden: false;
|
||||
/// Which part of the source-space overlay the view is showing, in overlay
|
||||
/// pixels. Without it the overlay stays frame-sized while the photograph
|
||||
/// moves under it.
|
||||
@@ -957,7 +967,7 @@ export component AppWindow inherits Window {
|
||||
/// rather than as one per frame of the gesture.
|
||||
callback gradient-handle-released();
|
||||
|
||||
callback segment-image();
|
||||
callback segment-image(bool);
|
||||
/// 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);
|
||||
@@ -1519,7 +1529,7 @@ in property <bool> panel-visible: true;
|
||||
// it is a diagnostic and not an edit: it must not reach
|
||||
// the histogram, an export, or the texture the develop
|
||||
// pass hands the compositor.
|
||||
if root.overlay-on && root.total > 0: Image {
|
||||
if root.overlay-on && !root.overlay-hidden && root.total > 0: Image {
|
||||
x: parent.shown-x;
|
||||
y: parent.shown-y;
|
||||
width: parent.shown-w;
|
||||
@@ -2106,7 +2116,24 @@ in property <bool> panel-visible: true;
|
||||
// width, which the layout then influences. Slint flags it, and
|
||||
// it can panic at runtime.
|
||||
Rectangle {
|
||||
width: root.panel-visible ? 280px : 0px;
|
||||
// TRACES: FR-UI-2
|
||||
// 280px was chosen for a tablet, where the column is a
|
||||
// large fraction of the screen and every pixel of it is
|
||||
// taken from the photograph. On a desktop window it is
|
||||
// simply too narrow: the mode strip alone is two modes,
|
||||
// a separator, "All", and one chip per attribute the
|
||||
// operation set declares, which is more words than 280px
|
||||
// holds — so it scrolled sideways, and a control you have
|
||||
// to pan to reach is one you do not know is there.
|
||||
//
|
||||
// Driven off `layout-class` rather than `root.width`
|
||||
// because reading the window width inside the layout that
|
||||
// sets it is the binding loop the comment on `expanded`
|
||||
// above describes. Rust already classifies the window; this
|
||||
// just spends the extra width where there is some.
|
||||
width: root.panel-visible
|
||||
? (root.layout-class == "expanded" ? 380px : 280px)
|
||||
: 0px;
|
||||
visible: root.panel-visible;
|
||||
background: Theme.surface;
|
||||
clip: true;
|
||||
@@ -2255,8 +2282,12 @@ in property <bool> panel-visible: true;
|
||||
subjects: root.subject-rows;
|
||||
segmented: root.segmented;
|
||||
segmenting: root.segmenting;
|
||||
// Two-way: the panel is where it is toggled and
|
||||
// the canvas is what obeys it, so neither owns
|
||||
// the value alone.
|
||||
overlay-hidden <=> root.overlay-hidden;
|
||||
|
||||
segment => { root.segment-image(); }
|
||||
segment(fine) => { root.segment-image(fine); }
|
||||
mask-selected(id) => { root.mask-selected(id); }
|
||||
mask-removed(id) => { root.mask-removed(id); }
|
||||
mask-toggled(id, on) => { root.mask-toggled(id, on); }
|
||||
|
||||
+44
-2
@@ -243,8 +243,14 @@ export component MaskPanel inherits Rectangle {
|
||||
/// One is being computed now.
|
||||
in property <bool> segmenting: false;
|
||||
|
||||
/// Whether the photographer has hidden the overlay to see the photograph.
|
||||
/// Two-way bound to the window's own, so the canvas and this control never
|
||||
/// disagree about it.
|
||||
in-out property <bool> overlay-hidden: false;
|
||||
|
||||
callback segment();
|
||||
|
||||
/// `true` asks for the slower, tiled pass.
|
||||
callback segment(bool);
|
||||
|
||||
callback mask-selected(string);
|
||||
callback mask-removed(string);
|
||||
@@ -299,7 +305,7 @@ export component MaskPanel inherits Rectangle {
|
||||
text: root.segmenting ? "Looking…" : "Find subjects";
|
||||
enabled: !root.segmenting;
|
||||
primary: true;
|
||||
clicked => { root.segment(); }
|
||||
clicked => { root.segment(false); }
|
||||
}
|
||||
|
||||
// No "Overlay" button and no "Select" button. Both switched on things
|
||||
@@ -312,6 +318,42 @@ export component MaskPanel inherits Rectangle {
|
||||
wrap: word-wrap;
|
||||
}
|
||||
|
||||
// Hiding the overlay is not the "Overlay" button this panel used to
|
||||
// have, and the distinction is the reason it is back. That one *armed*
|
||||
// the overlay: local mode could be entered and still show nothing,
|
||||
// which is the fault the comment below records. This one only takes an
|
||||
// overlay that is already there out of the way for a moment, which is
|
||||
// the one thing a photographer needs constantly and had no way to do —
|
||||
// a mask is judged against the photograph under it, and you cannot see
|
||||
// that photograph through the thing describing it.
|
||||
//
|
||||
// Reads as its own action rather than its state: "Show the mask" is
|
||||
// what pressing it will do, not what is currently true.
|
||||
if root.enabled && root.segmented: Button {
|
||||
text: root.overlay-hidden ? "Show the mask" : "Hide the mask";
|
||||
clicked => { root.overlay-hidden = !root.overlay-hidden; }
|
||||
}
|
||||
|
||||
// The second pass, offered rather than taken automatically.
|
||||
//
|
||||
// The model's input is a fixed 640x640 square and every frame is
|
||||
// letterboxed into it, so a subject that is small in the frame reaches
|
||||
// the model small — a bird at 200px in a 1600px proxy arrives at 80px,
|
||||
// and an outline traced at 80px is what a coarse mask is. Tiling runs
|
||||
// the model over overlapping windows instead, so that bird arrives at
|
||||
// its own size.
|
||||
//
|
||||
// It costs one inference per tile: about 2.8s for a 3x2 grid against
|
||||
// 470ms for the whole frame. That is the wrong trade for the usual
|
||||
// photograph, where the subject fills much of the frame and the first
|
||||
// pass is already the best answer available — so it is a button and
|
||||
// not a default, and it says what it costs.
|
||||
if root.enabled && root.segmented: Button {
|
||||
text: root.segmenting ? "Looking closer…" : "Look closer (slower)";
|
||||
enabled: !root.segmenting;
|
||||
clicked => { root.segment(true); }
|
||||
}
|
||||
|
||||
// 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.
|
||||
|
||||
Reference in New Issue
Block a user