Merge branch 'worktree-agent-afa2042c919d0d111' into integration
This commit is contained in:
+317
-69
@@ -9,6 +9,9 @@
|
||||
//! declared [`ParamKind`], not from which parameter it is (ARCH §4.3), so a
|
||||
//! new operation appears in the panel with no change here (FR-DEV-3c).
|
||||
|
||||
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering};
|
||||
use std::sync::Arc;
|
||||
|
||||
use dr_decode::RawImage;
|
||||
use dr_gpu::{
|
||||
AdjustPass, DemosaicedImage, Demosaicer, GpuContext, Histogram, HistogramPass, MaskPass,
|
||||
@@ -33,7 +36,185 @@ use crate::ParamRow;
|
||||
/// milliseconds and its field a few megabytes.
|
||||
const SEGMENT_PROXY_EDGE: u32 = 1600;
|
||||
|
||||
/// Which photograph a piece of background work was started for.
|
||||
///
|
||||
/// Minted per session, never reused, and carried by the work rather than
|
||||
/// looked up when it finishes. A segmentation takes most of a second, so the
|
||||
/// user can be two frames further on by the time one lands, and the answer to
|
||||
/// "is this still wanted" has to be decided from what the work *was* rather
|
||||
/// than from what happens to be open.
|
||||
///
|
||||
/// The alternative — a counter beside the session slot, bumped on every open —
|
||||
/// is written from four places in `lib.rs` and would apply one photograph's
|
||||
/// subjects to another the first time somebody added a fifth and forgot.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct SessionId(u64);
|
||||
|
||||
impl SessionId {
|
||||
pub(crate) fn next() -> Self {
|
||||
static NEXT: AtomicU64 = AtomicU64::new(1);
|
||||
Self(NEXT.fetch_add(1, Ordering::Relaxed))
|
||||
}
|
||||
}
|
||||
|
||||
/// Says that nobody is waiting for a job's answer any more.
|
||||
///
|
||||
/// Not a cancellation in the sense of stopping the work: the model is one
|
||||
/// opaque call of about half a second and `ort` offers no way in. This is
|
||||
/// checked at the seams there are — before the job starts, and again between
|
||||
/// the proxy readback and the inference — so a job abandoned while the user
|
||||
/// was still paging usually costs nothing, and one abandoned mid-inference
|
||||
/// costs only the run it was already committed to.
|
||||
///
|
||||
/// What it buys in every case is that the next photograph's segmentation is
|
||||
/// the only one anybody is waiting on.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub struct Abandon(Arc<AtomicBool>);
|
||||
|
||||
impl Abandon {
|
||||
pub fn now(&self) {
|
||||
self.0.store(true, Ordering::Relaxed);
|
||||
}
|
||||
|
||||
pub fn asked(&self) -> bool {
|
||||
self.0.load(Ordering::Relaxed)
|
||||
}
|
||||
}
|
||||
|
||||
/// What a finished [`SegmentationJob`] hands back.
|
||||
///
|
||||
/// The rasteriser travels with the subjects because it is needed the instant
|
||||
/// they arrive and nowhere before. Building it is a shader compile — 23 ms on
|
||||
/// a desktop, and compiling shaders is among the slowest things a mobile
|
||||
/// driver does — so building it on adoption put a dropped frame on the one
|
||||
/// redraw the user is waiting for. Here it is on the thread that was waiting
|
||||
/// anyway.
|
||||
///
|
||||
/// `None` where the device has no mask rasteriser at all: the session keeps
|
||||
/// the photograph and loses local adjustments, which is the same bargain the
|
||||
/// histogram makes.
|
||||
pub struct Segmented {
|
||||
seg: Segmentation,
|
||||
masks: Option<MaskPass>,
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// A segmentation lifted out of the session that asked for it.
|
||||
///
|
||||
/// [`DevelopSession`] cannot go to a worker. Not because of what it holds —
|
||||
/// the device, the source texture and the passes are all `Send` — but because
|
||||
/// it lives behind one `Rc<RefCell<Option<…>>>` that every callback in the
|
||||
/// window reaches through, and the window has to keep reaching through it
|
||||
/// while the work runs. Handing the session over would freeze the interface
|
||||
/// exactly as thoroughly as blocking on it did.
|
||||
///
|
||||
/// So the work takes a copy of the two things it needs. The device is `Arc`s,
|
||||
/// the source is shared rather than copied, and the answer comes back as plain
|
||||
/// data.
|
||||
pub struct SegmentationJob {
|
||||
ctx: GpuContext,
|
||||
source: Arc<DemosaicedImage>,
|
||||
session: SessionId,
|
||||
abandon: Abandon,
|
||||
}
|
||||
|
||||
impl SegmentationJob {
|
||||
/// The photograph this was started for.
|
||||
pub fn session(&self) -> SessionId {
|
||||
self.session
|
||||
}
|
||||
|
||||
/// The handle that tells this job its answer is no longer wanted.
|
||||
pub fn abandon(&self) -> Abandon {
|
||||
self.abandon.clone()
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// Find the subjects. **Blocking, and roughly two thirds of a second.**
|
||||
///
|
||||
/// `Ok(None)` means abandoned rather than found-nothing: an image with no
|
||||
/// recognisable subject in it still comes back as `Ok(Some(_))` with an
|
||||
/// empty instance list, and the panel says so.
|
||||
///
|
||||
/// There is deliberately no `&mut DevelopSession` in scope here. That is
|
||||
/// the whole point of the split — a caller cannot accidentally hold the
|
||||
/// session across the half second, because it was never given one.
|
||||
pub fn run(&self, options: &segmentation::Options) -> Result<Option<Segmented>, String> {
|
||||
if self.abandon.asked() {
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
// Timed and logged because this is the feature's largest cost and the
|
||||
// split between the two halves decides where any further work goes. A
|
||||
// number from the device it actually runs on beats an estimate from
|
||||
// the desktop.
|
||||
let started = std::time::Instant::now();
|
||||
let (rgb, rw, rh) = self.neutral_proxy(SEGMENT_PROXY_EDGE)?;
|
||||
let proxied = started.elapsed();
|
||||
|
||||
// The one seam inside the run. Past here the model owns the thread
|
||||
// until it is done.
|
||||
if self.abandon.asked() {
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
let seg = segmentation::compute(&self.ctx, &rgb, rw, rh, options)?;
|
||||
log::info!(
|
||||
"segmented {rw}×{rh}: {} subject(s), proxy {:.0} ms, total {:.0} ms",
|
||||
seg.instances().len(),
|
||||
proxied.as_secs_f32() * 1000.0,
|
||||
started.elapsed().as_secs_f32() * 1000.0,
|
||||
);
|
||||
|
||||
let masks = MaskPass::new(&self.ctx)
|
||||
.inspect_err(|e| log::warn!("no mask rasteriser on this device: {e}"))
|
||||
.ok();
|
||||
Ok(Some(Segmented { seg, masks }))
|
||||
}
|
||||
|
||||
/// Render the *unedited* image to a CPU buffer at proxy size.
|
||||
///
|
||||
/// 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).
|
||||
///
|
||||
/// A throwaway [`AdjustPass`] with a neutral graph rather than the
|
||||
/// session's own — which this could not reach from here in any case, and
|
||||
/// must not: reusing it would overwrite the frame the histogram reads and
|
||||
/// leave the view showing an unedited image until the next redraw.
|
||||
///
|
||||
/// This is `export_pixels`, which is ungated: an export is not the display
|
||||
/// round-trip AC-8 forbids, and neither is this.
|
||||
fn neutral_proxy(&self, max_edge: u32) -> Result<(Vec<f32>, usize, usize), String> {
|
||||
let (sw, sh) = self.source.size();
|
||||
let scale = (max_edge as f32 / sw.max(sh) as f32).min(1.0);
|
||||
let (w, h) = (
|
||||
((sw as f32 * scale) as u32).max(1),
|
||||
((sh as f32 * scale) as u32).max(1),
|
||||
);
|
||||
|
||||
let neutral = EditGraph::default_chain();
|
||||
let mut pass = AdjustPass::new(&self.ctx);
|
||||
pass.render(&self.source, &neutral.compose(), w, h)
|
||||
.map_err(|e| format!("could not render the segmentation proxy: {e}"))?;
|
||||
let (rgba, pw, ph) = pass
|
||||
.export_pixels()
|
||||
.map_err(|e| format!("could not read the segmentation proxy: {e}"))?;
|
||||
|
||||
// Straight to float RGB, dropping alpha. The values stay display-
|
||||
// encoded because that is what the model was trained on — one of the
|
||||
// few places in this codebase where not linearising is correct.
|
||||
let rgb = rgba
|
||||
.chunks_exact(4)
|
||||
.flat_map(|p| [p[0] as f32 / 255.0, p[1] as f32 / 255.0, p[2] as f32 / 255.0])
|
||||
.collect();
|
||||
Ok((rgb, pw as usize, ph as usize))
|
||||
}
|
||||
}
|
||||
|
||||
pub struct DevelopSession {
|
||||
/// This session's name, for work that outlives the frame it started on.
|
||||
id: SessionId,
|
||||
/// Kept so the session can build GPU resources after construction.
|
||||
///
|
||||
/// The distance fields behind a subject mask are made when a layer is
|
||||
@@ -50,7 +231,7 @@ pub struct DevelopSession {
|
||||
/// a history the *call sites* had to remember would be one press of undo
|
||||
/// away from wrong every time a control is added.
|
||||
history: History,
|
||||
demosaiced: DemosaicedImage,
|
||||
demosaiced: Arc<DemosaicedImage>,
|
||||
adjust: AdjustPass,
|
||||
/// TRACES: FR-DSP-7
|
||||
/// Optional, because a session that cannot count its frames is still a
|
||||
@@ -142,10 +323,11 @@ impl DevelopSession {
|
||||
graph.set_orientation(orientation);
|
||||
let history = History::new(&graph);
|
||||
Self {
|
||||
id: SessionId::next(),
|
||||
ctx: ctx.clone(),
|
||||
graph,
|
||||
history,
|
||||
demosaiced,
|
||||
demosaiced: Arc::new(demosaiced),
|
||||
adjust: AdjustPass::new(ctx),
|
||||
histogram: HistogramPass::new(ctx)
|
||||
.inspect_err(|e| log::warn!("no histogram on this device: {e}"))
|
||||
@@ -864,84 +1046,60 @@ impl DevelopSession {
|
||||
// Segmentation (S15, docs/segmentation.md)
|
||||
// ----------------------------------------------------------------------
|
||||
|
||||
/// This session's name, carried by any work started against it.
|
||||
pub fn id(&self) -> SessionId {
|
||||
self.id
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// Compute the region map this image's local masks select from.
|
||||
/// Everything a segmentation needs, so it can be run somewhere else.
|
||||
///
|
||||
/// **Blocking, and roughly half a second.** The caller is responsible for
|
||||
/// running it off the UI thread — see the worker in `lib.rs`. It is
|
||||
/// exposed as a plain blocking call rather than something async because
|
||||
/// what it needs is a GPU context and a CPU core, not a runtime.
|
||||
pub fn segment(&mut self, ctx: &GpuContext, options: &segmentation::Options) -> Result<(), String> {
|
||||
// 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).
|
||||
// Timed and logged, because this blocks the interface and the size of
|
||||
// that stall is the feature's largest open risk. A number from the
|
||||
// device it actually runs on beats an estimate from the desktop.
|
||||
let started = std::time::Instant::now();
|
||||
let (rgb, rw, rh) = self.neutral_proxy(ctx, SEGMENT_PROXY_EDGE)?;
|
||||
let proxied = started.elapsed();
|
||||
|
||||
let seg = segmentation::compute(ctx, &rgb, rw, rh, options)?;
|
||||
log::info!(
|
||||
"segmented {rw}×{rh}: {} subject(s), proxy {:.0} ms, total {:.0} ms",
|
||||
seg.instances().len(),
|
||||
proxied.as_secs_f32() * 1000.0,
|
||||
started.elapsed().as_secs_f32() * 1000.0,
|
||||
);
|
||||
|
||||
if self.masks.is_none() {
|
||||
self.masks = MaskPass::new(ctx)
|
||||
.inspect_err(|e| log::warn!("no mask rasteriser on this device: {e}"))
|
||||
.ok();
|
||||
/// Taking the job is cheap — two `Arc` bumps and a texture handle — and
|
||||
/// nothing about the session is borrowed past the call, which is what
|
||||
/// lets the window go on drawing while the answer is being found.
|
||||
pub fn segmentation_job(&self) -> SegmentationJob {
|
||||
SegmentationJob {
|
||||
ctx: self.ctx.clone(),
|
||||
source: self.demosaiced.clone(),
|
||||
session: self.id,
|
||||
abandon: Abandon::default(),
|
||||
}
|
||||
}
|
||||
|
||||
/// TRACES: FR-DEV-3
|
||||
/// Take on a segmentation found elsewhere.
|
||||
///
|
||||
/// The caller is responsible for checking that this result was computed
|
||||
/// for *this* session — see [`SegmentationJob::session`]. Nothing here can
|
||||
/// tell one photograph's subjects from another's, and a mismatch is
|
||||
/// silent: the masks would rasterise, the overlay would draw, and the
|
||||
/// outlines would simply follow a subject that is not in the picture.
|
||||
pub fn adopt_segmentation(&mut self, found: Segmented) {
|
||||
// Kept rather than replaced where there is one already: a second
|
||||
// segmentation of the same photograph would otherwise throw away a
|
||||
// working rasteriser for an identical one.
|
||||
self.masks = self.masks.take().or(found.masks);
|
||||
|
||||
// The fields themselves are built per *layer*, on demand — there are
|
||||
// none yet, and building one per detected object would transform
|
||||
// several megapixels for masks the user may never make.
|
||||
self.segmentation = Some(seg);
|
||||
self.segmentation = Some(found.seg);
|
||||
self.subjects = None;
|
||||
self.subject_key = 0;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Render the *unedited* image to a CPU buffer at proxy size.
|
||||
/// Find the subjects and take them on, blocking until both are done.
|
||||
///
|
||||
/// Goes through a throwaway [`AdjustPass`] with a neutral graph rather
|
||||
/// than the session's own. Reusing `self.adjust` would overwrite the frame
|
||||
/// the histogram reads and leave the view showing an unedited image until
|
||||
/// the next redraw — a visible flicker for the sake of not allocating.
|
||||
///
|
||||
/// This is `export_pixels`, which is ungated: an export is not the display
|
||||
/// round-trip AC-8 forbids, and neither is this.
|
||||
fn neutral_proxy(
|
||||
&self,
|
||||
ctx: &GpuContext,
|
||||
max_edge: u32,
|
||||
) -> Result<(Vec<f32>, usize, usize), String> {
|
||||
let (sw, sh) = self.demosaiced.size();
|
||||
let scale = (max_edge as f32 / sw.max(sh) as f32).min(1.0);
|
||||
let (w, h) = (
|
||||
((sw as f32 * scale) as u32).max(1),
|
||||
((sh as f32 * scale) as u32).max(1),
|
||||
);
|
||||
|
||||
let neutral = EditGraph::default_chain();
|
||||
let mut pass = AdjustPass::new(ctx);
|
||||
pass.render(&self.demosaiced, &neutral.compose(), w, h)
|
||||
.map_err(|e| format!("could not render the segmentation proxy: {e}"))?;
|
||||
let (rgba, pw, ph) = pass
|
||||
.export_pixels()
|
||||
.map_err(|e| format!("could not read the segmentation proxy: {e}"))?;
|
||||
|
||||
// Straight to float RGB, dropping alpha. The values stay display-
|
||||
// encoded because that is what the model was trained on — one of the
|
||||
// few places in this codebase where not linearising is correct.
|
||||
let rgb = rgba
|
||||
.chunks_exact(4)
|
||||
.flat_map(|p| [p[0] as f32 / 255.0, p[1] as f32 / 255.0, p[2] as f32 / 255.0])
|
||||
.collect();
|
||||
Ok((rgb, pw as usize, ph as usize))
|
||||
/// Test-only, and deliberately: a session-shaped blocking call is exactly
|
||||
/// the shape that put two thirds of a second on the UI thread in the first
|
||||
/// place, and leaving it public would invite the next caller to reach for
|
||||
/// it. A test has nothing else to be doing.
|
||||
#[cfg(test)]
|
||||
fn segment(&mut self, options: &segmentation::Options) -> Result<(), String> {
|
||||
if let Some(found) = self.segmentation_job().run(options)? {
|
||||
self.adopt_segmentation(found);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn has_segmentation(&self) -> bool {
|
||||
@@ -2005,6 +2163,96 @@ mod tests {
|
||||
out
|
||||
}
|
||||
|
||||
// ----------------------------------------------------------------------
|
||||
// Segmentation off the UI thread
|
||||
// ----------------------------------------------------------------------
|
||||
|
||||
/// The property the whole arrangement rests on.
|
||||
///
|
||||
/// If someone puts an `Rc`, a `Cell` or a raw pipeline handle into
|
||||
/// `SegmentationJob`, this stops compiling — which is the only warning
|
||||
/// there would be, since the call site in `masks_ui` would then fail with
|
||||
/// a lifetime error a long way from the cause.
|
||||
#[test]
|
||||
fn a_job_and_its_answer_can_cross_a_thread() {
|
||||
fn is_send<T: Send>() {}
|
||||
is_send::<SegmentationJob>();
|
||||
is_send::<Segmented>();
|
||||
is_send::<Abandon>();
|
||||
}
|
||||
|
||||
/// Two sessions over the same file are still two photographs as far as a
|
||||
/// late result is concerned, because opening one twice is opening it
|
||||
/// twice.
|
||||
#[test]
|
||||
fn every_session_has_its_own_identity() {
|
||||
let Some(ctx) = headless() else { return };
|
||||
let rgba: Vec<u8> = (0..16 * 16).flat_map(|_| [128, 128, 128, 255]).collect();
|
||||
let open = || {
|
||||
DevelopSession::open_rgb(&ctx, &rgba, 16, 16, dr_types::Orientation::NORMAL)
|
||||
.expect("session")
|
||||
};
|
||||
let (a, b) = (open(), open());
|
||||
assert_ne!(a.id(), b.id());
|
||||
assert_eq!(a.id(), a.id(), "and stable within one session");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_job_carries_the_session_it_was_taken_from() {
|
||||
let Some(ctx) = headless() else { return };
|
||||
let rgba: Vec<u8> = (0..16 * 16).flat_map(|_| [128, 128, 128, 255]).collect();
|
||||
let session =
|
||||
DevelopSession::open_rgb(&ctx, &rgba, 16, 16, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
assert_eq!(session.segmentation_job().session(), session.id());
|
||||
}
|
||||
|
||||
/// Abandoning before the run reaches the proxy must cost nothing at all —
|
||||
/// this is the case that fires when the user pages on while a job is still
|
||||
/// waiting for a thread.
|
||||
#[test]
|
||||
fn an_abandoned_job_does_no_work() {
|
||||
let Some(ctx) = headless() else { return };
|
||||
let rgba: Vec<u8> = (0..16 * 16).flat_map(|_| [128, 128, 128, 255]).collect();
|
||||
let session =
|
||||
DevelopSession::open_rgb(&ctx, &rgba, 16, 16, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
|
||||
let job = session.segmentation_job();
|
||||
job.abandon().now();
|
||||
|
||||
let started = std::time::Instant::now();
|
||||
let out = job.run(&crate::segmentation::Options::default());
|
||||
assert!(
|
||||
matches!(out, Ok(None)),
|
||||
"abandoned is not an error and not an empty answer: {:?}",
|
||||
out.map(|o| o.is_some())
|
||||
);
|
||||
assert!(
|
||||
started.elapsed() < std::time::Duration::from_millis(50),
|
||||
"it returned without loading the model"
|
||||
);
|
||||
}
|
||||
|
||||
/// A finished segmentation is adopted whole, and the session says so.
|
||||
#[test]
|
||||
fn adopting_a_result_gives_the_session_its_subjects() {
|
||||
let Some(ctx) = headless() else { return };
|
||||
let rgba: Vec<u8> = (0..100 * 100).flat_map(|_| [128, 128, 128, 255]).collect();
|
||||
let mut session =
|
||||
DevelopSession::open_rgb(&ctx, &rgba, 100, 100, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
assert!(!session.has_segmentation());
|
||||
|
||||
let job = session.segmentation_job();
|
||||
let Ok(Some(found)) = job.run(&crate::segmentation::Options::default()) else {
|
||||
eprintln!("no model; skipping");
|
||||
return;
|
||||
};
|
||||
session.adopt_segmentation(found);
|
||||
assert!(session.has_segmentation());
|
||||
}
|
||||
|
||||
// ----------------------------------------------------------------------
|
||||
// The overlay's clip rectangle
|
||||
// ----------------------------------------------------------------------
|
||||
@@ -2026,7 +2274,7 @@ mod tests {
|
||||
DevelopSession::open_rgb(ctx, &rgba, 100, 100, dr_types::Orientation::NORMAL)
|
||||
.expect("session");
|
||||
session
|
||||
.segment(ctx, &crate::segmentation::Options::default())
|
||||
.segment(&crate::segmentation::Options::default())
|
||||
.ok()?;
|
||||
Some(session)
|
||||
}
|
||||
|
||||
+1
-1
@@ -1769,7 +1769,7 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
});
|
||||
}
|
||||
|
||||
masks_ui::wire(&window, &session, &rows, &redraw, gpu.clone());
|
||||
masks_ui::wire(&window, &session, &rows, &redraw);
|
||||
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
|
||||
+289
-37
@@ -5,7 +5,7 @@
|
||||
//! [`crate::segmentation`], which is the same split every other `*_ui` module
|
||||
//! in this crate draws.
|
||||
//!
|
||||
//! # The one thing this module does decide
|
||||
//! # The things this module does decide
|
||||
//!
|
||||
//! Selecting a mask layer re-scopes the adjust panel to that layer's chain.
|
||||
//! That happens because [`crate::develop::DevelopSession::rows`] answers
|
||||
@@ -13,16 +13,109 @@
|
||||
//! `sync_rows` afterwards — there is no second panel and no duplicated
|
||||
//! control-building code. It is worth stating plainly because the absence of
|
||||
//! code is easy to mistake for an omission.
|
||||
//!
|
||||
//! And whether a segmentation that has finished is still wanted. Finding the
|
||||
//! subjects takes most of a second on a 22 MP frame, so it runs on a worker
|
||||
//! and the window polls for the answer — which means the answer can arrive
|
||||
//! for a photograph the user has left. [`delivery`] is that rule, kept as a
|
||||
//! named function with tests because both of its ways of being wrong are
|
||||
//! silent: applied to the wrong image it draws outlines that follow a subject
|
||||
//! which is not in the picture, and discarded too eagerly it throws away work
|
||||
//! the user waited for.
|
||||
|
||||
use std::cell::RefCell;
|
||||
use std::rc::Rc;
|
||||
|
||||
use slint::{ComponentHandle as _, ModelRc, VecModel};
|
||||
|
||||
use crate::develop::DevelopSession;
|
||||
use crate::develop::{Abandon, DevelopSession, Segmented, SessionId};
|
||||
use crate::segmentation;
|
||||
use crate::{sync_rows, AppWindow, MaskRow, ParamRow, SubjectRow};
|
||||
|
||||
/// How often the window looks to see whether the model has finished.
|
||||
///
|
||||
/// The interval `apply_when_ready` polls a sidecar fetch at, for the same
|
||||
/// reason: a tick that finds nothing costs a `try_recv` on an empty channel,
|
||||
/// and twenty a second is imperceptible against a result that took most of a
|
||||
/// second to produce.
|
||||
const POLL: std::time::Duration = std::time::Duration::from_millis(50);
|
||||
|
||||
/// What to do with a segmentation that has finished.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum Delivery {
|
||||
Apply,
|
||||
Discard,
|
||||
}
|
||||
|
||||
/// Does this answer still belong to the photograph on screen?
|
||||
///
|
||||
/// The session a job was taken from names the photograph it is about, and
|
||||
/// sessions are never reused — so re-opening the same file is a different
|
||||
/// answer to this, correctly: the second session has no segmentation and its
|
||||
/// panel says so.
|
||||
fn delivery(computed_for: SessionId, open: Option<SessionId>) -> Delivery {
|
||||
match open {
|
||||
Some(id) if id == computed_for => Delivery::Apply,
|
||||
_ => Delivery::Discard,
|
||||
}
|
||||
}
|
||||
|
||||
fn open_session(session: &Rc<RefCell<Option<DevelopSession>>>) -> Option<SessionId> {
|
||||
session.borrow().as_ref().map(|s| s.id())
|
||||
}
|
||||
|
||||
/// The one segmentation that may be in flight.
|
||||
///
|
||||
/// One at a time. Two runs would be two model loads, two proxy readbacks and
|
||||
/// two cores for a single answer, and the second to land would overwrite the
|
||||
/// first — so the cost buys nothing. The button is already insensitive while
|
||||
/// `segmenting` is set, but that is a property of the window; this is the
|
||||
/// invariant, so it lives where it can be tested.
|
||||
#[derive(Default)]
|
||||
struct Running {
|
||||
/// The photograph the running job was started for.
|
||||
started_for: Option<SessionId>,
|
||||
/// Tells that job nobody wants its answer.
|
||||
abandon: Abandon,
|
||||
/// Held here rather than by its own closure. A timer that kept itself
|
||||
/// alive to be able to stop itself would be an `Rc` cycle — one leaked
|
||||
/// timer and one leaked channel per photograph segmented — and owning it
|
||||
/// here also means starting the next job drops the previous timer instead
|
||||
/// of leaving it polling a channel nothing will send on.
|
||||
poll: Option<Rc<slint::Timer>>,
|
||||
}
|
||||
|
||||
impl Running {
|
||||
/// Claim the slot for `id`, or refuse because a run for that same
|
||||
/// photograph is already under way.
|
||||
///
|
||||
/// A job left over from a photograph the user has since left is *not* a
|
||||
/// reason to refuse: it is abandoned and displaced. Refusing would leave
|
||||
/// the next photograph's "Find subjects" doing nothing for as long as a
|
||||
/// run nobody wants takes to finish, which is exactly the wait this whole
|
||||
/// change exists to remove.
|
||||
fn start(&mut self, id: SessionId, abandon: Abandon) -> bool {
|
||||
if self.started_for == Some(id) {
|
||||
return false;
|
||||
}
|
||||
self.abandon.now();
|
||||
self.started_for = Some(id);
|
||||
self.abandon = abandon;
|
||||
true
|
||||
}
|
||||
|
||||
/// Give the slot up, if `id` still holds it.
|
||||
///
|
||||
/// Abandoning on the way out covers the discard case and costs nothing in
|
||||
/// the success case, where the run it names has already finished.
|
||||
fn finish(&mut self, id: SessionId) {
|
||||
if self.started_for == Some(id) {
|
||||
self.abandon.now();
|
||||
self.started_for = None;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Push every mask-related property from the session into the window.
|
||||
pub(crate) fn sync(window: &AppWindow, session: &Rc<RefCell<Option<DevelopSession>>>) {
|
||||
let slot = session.borrow();
|
||||
@@ -106,60 +199,50 @@ pub(crate) fn sync_overlay_view(window: &AppWindow, session: &DevelopSession) {
|
||||
}
|
||||
|
||||
/// Install the panel's callbacks.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) fn wire(
|
||||
window: &AppWindow,
|
||||
session: &Rc<RefCell<Option<DevelopSession>>>,
|
||||
rows: &Rc<VecModel<ParamRow>>,
|
||||
redraw: &Rc<dyn Fn(&AppWindow)>,
|
||||
gpu: Option<dr_gpu::GpuContext>,
|
||||
) {
|
||||
let running: Rc<RefCell<Running>> = Rc::default();
|
||||
|
||||
// --- computing the region map -----------------------------------------
|
||||
{
|
||||
let weak = window.as_weak();
|
||||
let session = session.clone();
|
||||
let redraw = redraw.clone();
|
||||
let rows = rows.clone();
|
||||
let running = running.clone();
|
||||
window.on_segment_image(move || {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let Some(ctx) = gpu.clone() else {
|
||||
log::warn!("no GPU context; cannot segment");
|
||||
// Both the photograph and the device come out of the session, so
|
||||
// no session is nothing to look at and nothing to look with.
|
||||
let Some(job) = session.borrow().as_ref().map(|s| s.segmentation_job()) else {
|
||||
return;
|
||||
};
|
||||
let id = job.session();
|
||||
|
||||
// **Blocking, on the UI thread, and flagged rather than hidden.**
|
||||
//
|
||||
// Half a second of watershed and inference. Moving it to a worker
|
||||
// needs the session — which owns GPU resources and is behind a
|
||||
// `RefCell` shared with every callback here — to be reachable from
|
||||
// another thread, and that is a restructuring of the develop
|
||||
// session rather than a change to this call.
|
||||
//
|
||||
// So it blocks, and the button says "Finding regions…" first: a
|
||||
// frozen window with a stale label is the version of this that
|
||||
// reads as a crash. See the note in `segmentation.rs` — this is
|
||||
// the largest rough edge in the feature.
|
||||
let claimed = running.borrow_mut().start(id, job.abandon());
|
||||
if !claimed {
|
||||
return;
|
||||
}
|
||||
|
||||
// Set before the thread rather than by it, so there is no moment
|
||||
// in which the press has been taken and nothing on screen says so.
|
||||
// The button reads "Looking…" and goes insensitive off this.
|
||||
w.set_segmenting(true);
|
||||
// Let the label reach the screen before the stall begins.
|
||||
slint::Timer::single_shot(std::time::Duration::from_millis(16), {
|
||||
let weak = w.as_weak();
|
||||
let session = session.clone();
|
||||
let rows = rows.clone();
|
||||
let redraw = redraw.clone();
|
||||
move || {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
let options = segmentation::Options::default();
|
||||
if let Some(s) = session.borrow_mut().as_mut() {
|
||||
if let Err(e) = s.segment(&ctx, &options) {
|
||||
log::warn!("segmentation failed: {e}");
|
||||
}
|
||||
}
|
||||
w.set_segmenting(false);
|
||||
sync(&w, &session);
|
||||
sync_rows(&w, &rows, &session);
|
||||
redraw(&w);
|
||||
}
|
||||
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
let options = 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:
|
||||
// the answer was unwanted before it existed.
|
||||
let _ = tx.send(job.run(&options));
|
||||
});
|
||||
|
||||
watch(&w, id, rx, &session, &rows, &redraw, &running);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -379,6 +462,77 @@ pub(crate) fn wire(
|
||||
}
|
||||
}
|
||||
|
||||
/// Wait for a segmentation without the window waiting with it.
|
||||
///
|
||||
/// A repeating timer rather than a callback from the worker, because Slint
|
||||
/// properties may only be touched from the thread that owns the event loop and
|
||||
/// this is the shape `apply_when_ready` already uses for the sidecar fetch.
|
||||
///
|
||||
/// Two things end the wait, and only one of them is the answer arriving. The
|
||||
/// other is the photograph changing underneath: that is checked *first*, on
|
||||
/// every tick, so the abandonment reaches the worker while it may still be in
|
||||
/// the proxy — and so the next photograph's "Find subjects" is available
|
||||
/// within a tick rather than at the end of a run nobody wants.
|
||||
fn watch(
|
||||
window: &AppWindow,
|
||||
id: SessionId,
|
||||
rx: std::sync::mpsc::Receiver<Result<Option<Segmented>, String>>,
|
||||
session: &Rc<RefCell<Option<DevelopSession>>>,
|
||||
rows: &Rc<VecModel<ParamRow>>,
|
||||
redraw: &Rc<dyn Fn(&AppWindow)>,
|
||||
running: &Rc<RefCell<Running>>,
|
||||
) {
|
||||
let weak = window.as_weak();
|
||||
let session = session.clone();
|
||||
let rows = rows.clone();
|
||||
let redraw = redraw.clone();
|
||||
let slot = running.clone();
|
||||
|
||||
let timer = Rc::new(slint::Timer::default());
|
||||
let stop = Rc::downgrade(&timer);
|
||||
timer.start(slint::TimerMode::Repeated, POLL, move || {
|
||||
let done = || {
|
||||
if let Some(t) = stop.upgrade() {
|
||||
t.stop();
|
||||
}
|
||||
};
|
||||
|
||||
if delivery(id, open_session(&session)) == Delivery::Discard {
|
||||
// Nothing is touched on the way out. `segmenting` belongs to the
|
||||
// photograph now open — which may well have a run of its own going
|
||||
// — and the only route to here is through `reset`, which cleared
|
||||
// it for that image already.
|
||||
slot.borrow_mut().finish(id);
|
||||
done();
|
||||
return;
|
||||
}
|
||||
|
||||
let Ok(answer) = rx.try_recv() else { return };
|
||||
slot.borrow_mut().finish(id);
|
||||
done();
|
||||
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
match answer {
|
||||
Ok(Some(found)) => {
|
||||
if let Some(s) = session.borrow_mut().as_mut() {
|
||||
s.adopt_segmentation(found);
|
||||
}
|
||||
}
|
||||
// Abandoned. Unreachable from here in practice — the check above
|
||||
// catches every case that raises the flag — and handled rather
|
||||
// than asserted, because the cost of being wrong is one wasted
|
||||
// sync against a panic in a photographer's hands.
|
||||
Ok(None) => {}
|
||||
Err(e) => log::warn!("segmentation failed: {e}"),
|
||||
}
|
||||
w.set_segmenting(false);
|
||||
sync(&w, &session);
|
||||
sync_rows(&w, &rows, &session);
|
||||
redraw(&w);
|
||||
});
|
||||
running.borrow_mut().poll = Some(timer);
|
||||
}
|
||||
|
||||
/// Clear the panel when the open image changes.
|
||||
///
|
||||
/// Its own function rather than a call to [`sync`] with an empty session,
|
||||
@@ -394,3 +548,101 @@ pub(crate) fn reset(window: &AppWindow) {
|
||||
window.set_subject_rows(ModelRc::new(VecModel::<SubjectRow>::default()));
|
||||
window.set_editing_mask(false);
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// The ordinary case: the answer comes back to the photograph that asked.
|
||||
#[test]
|
||||
fn a_result_for_the_open_photograph_is_applied() {
|
||||
let open = SessionId::next();
|
||||
assert_eq!(delivery(open, Some(open)), Delivery::Apply);
|
||||
}
|
||||
|
||||
/// The bug this rule exists for. Two thirds of a second is long enough to
|
||||
/// press "Find subjects", think better of it and swipe to the next frame —
|
||||
/// and the result that lands then describes a picture nobody is looking at.
|
||||
#[test]
|
||||
fn a_result_for_a_photograph_the_user_has_left_is_discarded() {
|
||||
let asked = SessionId::next();
|
||||
let now_open = SessionId::next();
|
||||
assert_eq!(delivery(asked, Some(now_open)), Delivery::Discard);
|
||||
}
|
||||
|
||||
/// Back to the library, or a frame that failed to decode: there is no
|
||||
/// session to apply anything to.
|
||||
#[test]
|
||||
fn a_result_arriving_with_nothing_open_is_discarded() {
|
||||
assert_eq!(delivery(SessionId::next(), None), Delivery::Discard);
|
||||
}
|
||||
|
||||
/// Re-opening the same file is a new session, so a result outstanding from
|
||||
/// the previous visit does not land in it. Conservative on purpose: the
|
||||
/// alternative is keying on the path, and the second visit's panel would
|
||||
/// then be filled from a proxy rendered before the first visit's edits.
|
||||
#[test]
|
||||
fn re_opening_the_same_file_does_not_inherit_a_result() {
|
||||
let first_visit = SessionId::next();
|
||||
let second_visit = SessionId::next();
|
||||
assert_ne!(first_visit, second_visit);
|
||||
assert_eq!(delivery(first_visit, Some(second_visit)), Delivery::Discard);
|
||||
}
|
||||
|
||||
/// Pressing the button twice must not put two model runs on two cores for
|
||||
/// one answer.
|
||||
#[test]
|
||||
fn one_photograph_cannot_start_two_segmentations() {
|
||||
let mut running = Running::default();
|
||||
let id = SessionId::next();
|
||||
assert!(running.start(id, Abandon::default()));
|
||||
assert!(!running.start(id, Abandon::default()), "already looking");
|
||||
}
|
||||
|
||||
/// And the other half of that rule: a run left over from a photograph the
|
||||
/// user has left must not hold the slot against the one now on screen.
|
||||
#[test]
|
||||
fn a_new_photograph_displaces_a_run_nobody_is_waiting_for() {
|
||||
let mut running = Running::default();
|
||||
let stale = Abandon::default();
|
||||
assert!(running.start(SessionId::next(), stale.clone()));
|
||||
|
||||
assert!(running.start(SessionId::next(), Abandon::default()));
|
||||
assert!(
|
||||
stale.asked(),
|
||||
"the displaced run is told its answer is unwanted"
|
||||
);
|
||||
}
|
||||
|
||||
/// A run that finishes releases the slot, so the same photograph can be
|
||||
/// segmented again — after a failed attempt worth retrying, say.
|
||||
#[test]
|
||||
fn finishing_frees_the_slot() {
|
||||
let mut running = Running::default();
|
||||
let id = SessionId::next();
|
||||
assert!(running.start(id, Abandon::default()));
|
||||
running.finish(id);
|
||||
assert!(running.start(id, Abandon::default()));
|
||||
}
|
||||
|
||||
/// A timer left over from an earlier job reports in after the slot has
|
||||
/// moved on. It must not cancel the run that now holds it, or the user
|
||||
/// would be able to start a third while the second is still going.
|
||||
#[test]
|
||||
fn a_late_finish_does_not_release_someone_elses_slot() {
|
||||
let mut running = Running::default();
|
||||
let departed = SessionId::next();
|
||||
assert!(running.start(departed, Abandon::default()));
|
||||
|
||||
let now_open = SessionId::next();
|
||||
let live = Abandon::default();
|
||||
assert!(running.start(now_open, live.clone()));
|
||||
|
||||
running.finish(departed);
|
||||
assert!(!live.asked(), "the live run is left alone");
|
||||
assert!(
|
||||
!running.start(now_open, Abandon::default()),
|
||||
"and it still holds the slot"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,6 +24,11 @@
|
||||
//! runs **once per image, when the user asks**, and never on the frame path.
|
||||
//! Every interaction it enables — click a subject, grow a mask, change a
|
||||
//! falloff — reads its cached output.
|
||||
//!
|
||||
//! All of it is on a worker. [`compute`] takes an owned buffer and a
|
||||
//! `GpuContext`, which is what makes that possible — nothing here touches the
|
||||
//! develop session, and [`crate::develop::SegmentationJob`] is the piece that
|
||||
//! carries the proxy render across with it.
|
||||
|
||||
use std::sync::Arc;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user