Apply background jobs to the layers they started on (#7620)

Fixes #7366

Co-authored-by: Claude Haiku 5.5 <noreply@anthropic.com>

crates/engine/src/jobs.rs

@@ -219,12 +219,36 @@ struct Running {
     started_ms: f64,
     coalesce: Option<String>,
     color_restrict: Option<usize>,
+    /// The document's layer selection when the job started: applying targets it, not whatever
+    /// got selected meanwhile (a Type click does that without editing the document, #7366).
+    layers: Option<LayerSelection>,
     rx: std::sync::mpsc::Receiver<Result<Payload>>,
     apply: Option<Apply>,
     #[cfg(not(target_arch = "wasm32"))]
     handle: Option<std::thread::JoinHandle<()>>,
 }
 
+/// A document's active layer, selected layers and selection anchor.
+#[derive(Clone, PartialEq)]
+struct LayerSelection {
+    active: Option<LayerId>,
+    selected: Vec<LayerId>,
+    anchor: Option<LayerId>,
+}
+
+impl LayerSelection {
+    fn of(st: &crate::DocState) -> Self {
+        Self { active: st.active_layer, selected: st.selected_layers.clone(), anchor: st.layer_anchor }
+    }
+
+    fn set(self, st: &mut crate::DocState) {
+        st.active_layer = self.active;
+        st.selected_layers = self.selected;
+        st.layer_anchor = self.anchor;
+        crate::channel_cmds::fix_view(st);
+    }
+}
+
 impl Running {
     fn info(&self) -> JobInfo {
         JobInfo {
@@ -333,10 +357,10 @@ pub fn run<T: Send + 'static>(
     if s.jobs.pending.is_some() {
         return Err(EngineError::Other(format!("`{label}` started a second background job in one command")));
     }
-    let (doc, snapshot) = match (lock_document, s.active()) {
-        (true, Some(st)) => (Some(st.doc.id), Some(Arc::downgrade(&st.doc))),
+    let (doc, snapshot, layers) = match (lock_document, s.active()) {
+        (true, Some(st)) => (Some(st.doc.id), Some(Arc::downgrade(&st.doc)), Some(LayerSelection::of(st))),
         (true, None) => return Err(EngineError::NoDocument),
-        (false, _) => (None, None),
+        (false, _) => (None, None, None),
     };
     let ctx = JobCtx::new();
     let (tx, rx) = std::sync::mpsc::channel::<Result<Payload>>();
@@ -382,6 +406,7 @@ pub fn run<T: Send + 'static>(
         started_ms: now_ms(),
         coalesce: s.coalesce_request.clone(),
         color_restrict: s.color_restrict,
+        layers,
         rx,
         apply: Some(erased),
         #[cfg(not(target_arch = "wasm32"))]
@@ -843,7 +868,7 @@ impl Session {
     }
 
     fn apply_job(&mut self, job: &Running, apply: Option<Apply>, payload: Payload) -> Result<Value> {
-        let Running { label, doc, snapshot, coalesce, color_restrict, command, params, journal, .. } = job;
+        let Running { label, doc, snapshot, coalesce, color_restrict, layers, command, params, journal, .. } = job;
         let prev_active = self.active;
         let target = match doc {
             Some(d) => {
@@ -858,6 +883,19 @@ impl Session {
             }
             None => None,
         };
+        // Selecting a layer doesn't change the document, so the snapshot check above lets it
+        // through. Apply against the layers the job started on: the colour restriction and any
+        // `active`-based edit land on them (#7366). The user's selection is put back below.
+        let user_layers = match (target, layers) {
+            (Some(i), Some(started)) => self.docs.get_mut(i).and_then(|st| {
+                let now = LayerSelection::of(st);
+                (now != *started).then(|| {
+                    started.clone().set(st);
+                    now
+                })
+            }),
+            _ => None,
+        };
         self.coalesce_request = coalesce.clone();
         self.color_restrict = *color_restrict;
         // Apply-time hooks can dispatch commands too. Keep the same single-step recording
@@ -880,6 +918,15 @@ impl Session {
         if r.is_ok() {
             self.after_command(command, params.clone(), *journal, Some(nested_from));
         }
+        // Give the user back the layers they selected while the job ran, unless the job chose a
+        // selection itself (a new layer, say) or one of their layers is gone.
+        if let (Some(started), Some(user), Some(st)) = (layers, user_layers, target.and_then(|i| self.docs.get_mut(i))) {
+            let kept = LayerSelection::of(st) == *started;
+            let exist = user.active.iter().chain(&user.selected).all(|id| st.doc.layer(*id).is_some());
+            if (r.is_err() || kept) && exist {
+                user.set(st);
+            }
+        }
         // Keep the user's active document unless the job opened a new one.
         // This also restores the selection when the apply step returned an error.
         if target.is_some() && self.active == target {

crates/ui-egui/tests/remove_channel_target.rs

@@ -0,0 +1,100 @@
+//! #7366: a background Remove on a Red-only target keeps G/B/alpha intact when a Type click
+//! selects another layer while the job is pending.
+
+use photocraft_doc::LayerId;
+use photocraft_engine::Session;
+use photocraft_engine::jobs::Started;
+use photocraft_ui_egui::{PhotocraftApp, Services, control, type_tool};
+
+const REMOVE: &str = r#"{"points":[[12,10],[16,12]],"size":6}"#;
+
+fn run(s: &mut Session, id: &str, p: &str) {
+    s.execute(id, p.parse().unwrap()).unwrap();
+}
+
+fn pixels(s: &Session, doc: usize, id: LayerId) -> Vec<f32> {
+    let d = &s.documents()[doc].doc;
+    d.layer(id).unwrap().surface().unwrap().read_region(d.bounds())
+}
+
+#[derive(Clone, Copy)]
+enum Input {
+    None,
+    TypeTool,
+    ControlPointer,
+}
+
+fn probe(input: Input) {
+    let mut a = PhotocraftApp::new(Session::new(), Services::default());
+    run(&mut a.session, "file.new", r#"{"name":"A","width":32,"height":24,"background":"transparent"}"#);
+    run(&mut a.session, "type.create", r#"{"text":"X","x":2,"y":18,"size":14}"#);
+    let text = a.session.active().unwrap().active_layer.unwrap();
+    run(&mut a.session, "layer.new.layer", r#"{"name":"Target"}"#);
+    let target = a.session.active().unwrap().active_layer.unwrap();
+    a.session
+        .edit("fixture", |d, l| {
+            let q = d.layer_mut(l.unwrap()).unwrap().surface_mut().unwrap();
+            for y in 0..24 {
+                for x in 0..32 {
+                    q.write_pixel(x, y, &[((x * 7 + y * 13) % 31) as f32 / 30., (x % 5) as f32 / 4., (y % 7) as f32 / 6., 1.]);
+                }
+            }
+            Ok(())
+        })
+        .unwrap();
+    run(&mut a.session, "file.new", r##"{"name":"B","width":32,"height":24,"background":"#6a219c"}"##);
+    a.session.set_active(0);
+    a.sync_views();
+    run(&mut a.session, "channel.target", r#"{"channel":"red"}"#);
+    let b = a.session.documents()[1].doc.clone();
+    let text_before = a.session.documents()[0].doc.layer(text).unwrap().clone();
+
+    // The inline Red-only Remove is the oracle.
+    let mut inline = Session::new();
+    inline.add_document(a.session.documents()[0].doc.as_ref().clone(), None);
+    inline.select_layer(target).unwrap();
+    run(&mut inline, "channel.target", r#"{"channel":"red"}"#);
+    run(&mut inline, "paint.remove", REMOVE);
+    let expected = pixels(&inline, 0, target);
+
+    let job = match a.session.start("paint.remove", REMOVE.parse().unwrap()).unwrap() {
+        Started::Job(id) => id,
+        v => panic!("must start a background job: {v:?}"),
+    };
+    match input {
+        Input::None => {}
+        Input::TypeTool => assert!(type_tool::pointer_down(&mut a, 4., 14., Default::default()), "the click must hit the text"),
+        Input::ControlPointer => {
+            let (req, _rx) = control::ControlRequest::new("ui.pointer", r#"{"tool":"type","events":[{"kind":"down","x":4,"y":14}]}"#.parse().unwrap());
+            let _ = control::handle(&mut a, &Default::default(), &req);
+        }
+    }
+    let selected = if matches!(input, Input::None) { target } else { text };
+    assert_eq!(a.session.active().unwrap().active_layer, Some(selected), "the input selects the text layer");
+
+    a.session.set_active(1);
+    a.session.wait_job(job).unwrap();
+    assert_eq!(a.session.documents()[1].doc, b, "B is unchanged");
+    let st = &a.session.documents()[0];
+    assert_eq!(st.doc.layer(text).unwrap(), &text_before, "the text layer is unchanged");
+    assert_eq!(st.active_layer, Some(selected), "the user's selection survives the job");
+    let actual = pixels(&a.session, 0, target);
+    let wrong = expected.as_chunks::<4>().0.iter().zip(actual.as_chunks::<4>().0).filter(|(e, r)| e[1..] != r[1..]).count();
+    assert_eq!(wrong, 0, "pixels with G/B/alpha changed outside the Red target");
+    assert_eq!(actual, expected);
+}
+
+#[test]
+fn no_input_keeps_red_restriction() {
+    probe(Input::None);
+}
+
+#[test]
+fn type_tool_click_keeps_red_restriction() {
+    probe(Input::TypeTool);
+}
+
+#[test]
+fn control_pointer_click_keeps_red_restriction() {
+    probe(Input::ControlPointer);
+}