diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index c35381be..a20d02bf 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -374,6 +374,8 @@ pub struct Session { pub authorize: Option Result<()>>, /// Background jobs (see [`jobs`]). jobs: jobs::Jobs, + /// Select Subject's last result, reused while the pixels are unchanged (#2865). + subject_cache: smartselect_cmds::SubjectCache, /// A finished live stroke offered to the `paint.stroke` / `paint.pencil` that commits it /// ([`Session::offer_live_stroke`]). pub(crate) live_commit: Option, @@ -450,6 +452,7 @@ impl Session { smart_cmds::on_close(self, index); if let Some(id) = self.docs.get(index).map(|d| d.doc.id) { self.cancel_jobs_on(id); + self.subject_cache.forget_doc(id); } let d = self.docs.remove(index); self.active = if self.docs.is_empty() { @@ -568,6 +571,10 @@ impl Session { st.coalesce = key; st.revision += 1; st.last_damage = unchanged.then_some(photocraft_geom::Rect::EMPTY); + if !unchanged { + // Free the mask early; a stale entry would only miss (it is keyed by pixels). + self.subject_cache = Default::default(); + } Ok(r) } diff --git a/crates/engine/src/smartselect_cmds.rs b/crates/engine/src/smartselect_cmds.rs index 5871ad60..8fc04c94 100644 --- a/crates/engine/src/smartselect_cmds.rs +++ b/crates/engine/src/smartselect_cmds.rs @@ -73,15 +73,15 @@ pub(crate) fn with_doc_sampler(doc: &Document, layer: Option, all_la /// caller gets `changed: false`, whatever the mode. (Select Subject and Object Selection run /// heuristics that often find nothing on busy photos; clearing the selection then looks like /// a dead button.) -fn apply(s: &mut Session, label: &str, region: Option, m: SelectionMode) -> Result { +fn apply(s: &mut Session, label: &str, region: Option<&Region>, m: SelectionMode) -> Result { if region.is_none() { // Nothing found: leave the selection (and history) alone. let selected = s.active().ok_or(EngineError::NoDocument)?.doc.selection.is_some(); return Ok(json!({ "selected": selected, "changed": false })); } - let bbox = region.as_ref().map(|r| r.bbox); + let bbox = region.map(|r| r.bbox); let selected = s.edit(label, |doc, _| { - doc.selection = combine_region(doc.selection.as_ref(), region.as_ref(), m); + doc.selection = combine_region(doc.selection.as_ref(), region, m); Ok(doc.selection.is_some()) })?; Ok(json!({ "selected": selected, "changed": true, "bounds": bbox.map(|r| [r.x0, r.y0, r.width() as i32, r.height() as i32]) })) @@ -115,7 +115,7 @@ fn quick_selection(s: &mut Session, p: &Value) -> Result { let r = quick::quick_select(smp, canvas, &pts, size)?; if enhance { matting::refine_mask(smp, &matting::region_reader(&r), r.bbox, canvas, &ENHANCE) } else { Some(r) } })?; - apply(s, "Quick Selection", region, m) + apply(s, "Quick Selection", region.as_ref(), m) } fn rect_param(p: &Value) -> Option { @@ -133,18 +133,65 @@ fn rect_param(p: &Value) -> Option { fn object_selection(s: &mut Session, p: &Value) -> Result { let rect = rect_param(p).ok_or_else(|| bad("select.object", "needs \"rect\": [x, y, w, h]"))?; let region = with_sampler(s, b(p, "sampleAllLayers", false), |smp, doc| grabcut::object_select(smp, doc.bounds(), rect, 160_000))?; - apply(s, "Object Selection", region, mode(p, "replace")) + apply(s, "Object Selection", region.as_ref(), mode(p, "replace")) +} + +/// Select Subject's last result (#2865), so running it again on unchanged pixels answers at +/// once instead of recomputing superpixels, saliency and GrabCut (P29). Keyed by the pixels it +/// was computed from: valid while [`same_pixels`](crate::layer_multi_cmds::same_pixels) (a +/// tile-pointer comparison) holds against that document, so selection-only edits such as +/// Deselect keep it. One entry per session; the document is held weakly, so the cache never +/// keeps a closed or edited document's tiles alive (the session also drops it on pixel edits +/// and when its document closes). Its memory is the mask: one byte per pixel of its bounds. +#[derive(Debug, Default)] +pub struct SubjectCache { + doc: std::sync::Weak, + doc_id: Option, + /// The sampled layer; `None` = the composite. + target: Option, + region: Option>, +} + +impl SubjectCache { + /// Forget the result if it was computed from the document `id` (it is closing). + pub(crate) fn forget_doc(&mut self, id: photocraft_doc::DocId) { + if self.doc_id == Some(id) { + *self = Self::default(); + } + } } fn select_subject(s: &mut Session, p: &Value) -> Result { - let region = with_sampler(s, b(p, "sampleAllLayers", true), |smp, doc| subject::select_subject(smp, doc.bounds()))?; - apply(s, "Select Subject", region, mode(p, "replace")) + let all_layers = b(p, "sampleAllLayers", true); + let d = s.active().ok_or(EngineError::NoDocument)?; + // What `with_doc_sampler` samples: the active layer's pixels, else the composite. + let target = d.active_layer.filter(|id| !all_layers && d.doc.layer(*id).and_then(Layer::surface).is_some()); + let c = &s.subject_cache; + let hit = c.target == target && c.doc.upgrade().is_some_and(|old| crate::layer_multi_cmds::same_pixels(&old, &d.doc)); + let region = if hit { + c.region.clone() + } else { + let doc = std::sync::Arc::clone(&d.doc); + let region = with_doc_sampler(&doc, d.active_layer, all_layers, |smp, doc| subject::select_subject(smp, doc.bounds())).map(std::sync::Arc::new); + s.subject_cache = SubjectCache { doc: std::sync::Arc::downgrade(&doc), doc_id: Some(doc.id), target, region: region.clone() }; + region + }; + let out = apply(s, "Select Subject", region.as_deref(), mode(p, "replace"))?; + // Follow the document past the selection edit just made (same pixels), so a short history + // dropping the older state doesn't turn the next repeat into a miss. + if let Some(d) = s.active() + && s.subject_cache.doc_id == Some(d.doc.id) + && s.subject_cache.doc.upgrade().is_some_and(|old| crate::layer_multi_cmds::same_pixels(&old, &d.doc)) + { + s.subject_cache.doc = std::sync::Arc::downgrade(&d.doc); + } + Ok(out) } fn focus_area(s: &mut Session, p: &Value) -> Result { let (range, noise) = (f(p, "range", 0.5).clamp(0.0, 1.0), f(p, "noise", 0.0).clamp(0.0, 1.0)); let region = with_sampler(s, b(p, "sampleAllLayers", true), |smp, doc| focus::focus_area(smp, doc.bounds(), range, noise))?; - apply(s, "Focus Area", region, mode(p, "replace")) + apply(s, "Focus Area", region.as_ref(), mode(p, "replace")) } fn refine_params(p: &Value) -> RefineParams { @@ -453,6 +500,39 @@ mod tests { assert!(s.active().unwrap().doc.selection.is_none()); } + /// #2865: a repeat on unchanged pixels reuses the last mask (an identical selection, even + /// after Deselect); sampling something else or a pixel edit computes it again. + #[test] + fn select_subject_repeat_reuses_the_mask_until_pixels_change() { + let (mut s, _) = disc(8); + let sel = |s: &Session| { + let mut v = Vec::new(); + s.active().unwrap().doc.selection.as_ref().unwrap().read_region_into(Rect::new(0, 0, 200, 160), &mut v); + v + }; + let cached = |s: &Session| s.subject_cache.region.clone().unwrap(); + s.execute("select.subject", json!({})).unwrap(); + let (first, mask) = (sel(&s), cached(&s)); + s.execute("select.deselect", json!({})).unwrap(); + s.execute("select.subject", json!({})).unwrap(); + assert!(std::sync::Arc::ptr_eq(&mask, &cached(&s)), "the repeat recomputed the mask"); + assert_eq!(sel(&s), first); + // Another sampled target (the layer rather than the composite) is a miss. + s.execute("select.subject", json!({"sampleAllLayers": false})).unwrap(); + assert!(!std::sync::Arc::ptr_eq(&mask, &cached(&s))); + // A pixel edit drops the entry; the next run recomputes it. + s.edit("paint", |doc, _| { + let bg = doc.layers[0].surface_mut().unwrap(); + let n = bg.channels(); + bg.write_region(Rect::new(0, 0, 1, 1), &vec![1.0; n]); + Ok(()) + }) + .unwrap(); + assert!(s.subject_cache.region.is_none()); + s.execute("select.subject", json!({})).unwrap(); + assert_eq!(sel(&s), first); + } + /// A vertical Gaussian-blurred edge at x = 40 (warm left, cool right) with the left half selected. fn blurred_edge_selected() -> Session { let mut s = session_with(80, 40, 8, |x, _| { diff --git a/perf/budgets.toml b/perf/budgets.toml index d91bc9f8..5ff4fb39 100644 --- a/perf/budgets.toml +++ b/perf/budgets.toml @@ -351,7 +351,7 @@ issue = 211 bench = "perf_scenarios" row = "Select Subject 24 MP (repeat)" budget_ms = 100 -enforce = false # over budget in the 2026-10-05 baseline +enforce = false # #2865: met (repeats answer from the last mask, 7-10 ms p95, was 270-370 ms); enforce once the baseline is refreshed [[scenario]] id = "P30"