Remove: cancel queued strokes when the paint target changes (#7380) (#7464)

* fix(remove): cancel waiting strokes when their target changes

* fix(remove): store pending target identity with waiting strokes

* test(remove): cover queued strokes across layer and channel switches

* Tests: format retouch_ui.rs and drop the unreachable! arm clippy denies

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/ui-egui/src/lib.rs

@@ -605,9 +605,9 @@ pub struct PhotocraftApp {
     /// Footprint trail of a retouching drag (see `stroke_trail`).
     trail: Option<stroke_trail::Trail>,
     /// Remove tool strokes waiting to be applied together (Remove after each stroke off), the
-    /// document they belong to, and their tint on the canvas (`retouch_ui`).
+    /// document, layer and channel they belong to, and their tint on the canvas (`retouch_ui`).
     pub(crate) remove_pending: Vec<Vec<[f64; 3]>>,
-    pub(crate) remove_pending_doc: Option<photocraft_doc::DocId>,
+    pub(crate) remove_pending_target: Option<retouch_ui::RemoveTarget>,
     pub(crate) remove_trail: Option<stroke_trail::Trail>,
     /// The open menus' items, reused between frames (`menus::ItemCache`).
     pub(crate) menu_cache: Option<menus::ItemCache>,
@@ -879,7 +879,7 @@ impl PhotocraftApp {
             committed_stroke: None,
             trail: None,
             remove_pending: Vec::new(),
-            remove_pending_doc: None,
+            remove_pending_target: None,
             remove_trail: None,
             menu_cache: None,
             move_preview: None,

crates/ui-egui/src/retouch_ui.rs

@@ -166,15 +166,31 @@ fn remove_params(o: &crate::state::ToolOptions) -> Value {
     json!({"sampleAllLayers": o.sample_all_layers, "newLayer": o.remove_new_layer})
 }
 
+/// The selection a queued Remove stroke was drawn against. A later selection must not
+/// redirect the queued strokes to a different layer or colour component.
+#[derive(Clone, PartialEq)]
+pub(crate) struct RemoveTarget {
+    doc: photocraft_doc::DocId,
+    layer: Option<photocraft_doc::LayerId>,
+    channel: photocraft_engine::channel_cmds::ChannelTarget,
+    paint: Value,
+}
+
+fn remove_target(app: &PhotocraftApp) -> Option<RemoveTarget> {
+    let st = app.session.active()?;
+    Some(RemoveTarget { doc: st.doc.id, layer: st.active_layer, channel: st.channel_view.target, paint: crate::canvas::paint_target(app) })
+}
+
 /// Remove after each stroke off: a finished stroke waits with the others, tinted on the canvas,
 /// until ↩ or the options bar's ✓ applies them all as one removal (Esc or ⊘ drops them).
 pub fn remove_pending_add(app: &mut PhotocraftApp, points: &[[f64; 3]]) {
+    let Some(target) = remove_target(app) else { return };
     let Some(st) = app.session.active() else { return };
-    let (doc, size) = (st.doc.id, [st.doc.size.width, st.doc.size.height]);
-    if app.remove_pending_doc != Some(doc) {
+    let size = [st.doc.size.width, st.doc.size.height];
+    if app.remove_pending_target.as_ref() != Some(&target) {
         app.remove_pending.clear();
         app.remove_trail = None;
-        app.remove_pending_doc = Some(doc);
+        app.remove_pending_target = Some(target);
     }
     let diameter = app.session.tools.brush.size;
     app.remove_trail.get_or_insert_with(|| crate::stroke_trail::Trail::new(size)).feed_stroke(points, diameter);
@@ -185,12 +201,13 @@ pub fn remove_pending_add(app: &mut PhotocraftApp, points: &[[f64; 3]]) {
 
 /// Apply the waiting strokes as one removal (one job, one undo step). False when none wait.
 pub fn remove_pending_commit(app: &mut PhotocraftApp) -> bool {
-    if app.remove_pending.is_empty() {
+    // Enter can arrive before the next canvas frame checks pending-stroke lifetime.
+    if !remove_pending_live(app) {
         return false;
     }
     let strokes = std::mem::take(&mut app.remove_pending);
     app.remove_trail = None;
-    app.remove_pending_doc = None;
+    app.remove_pending_target = None;
     let mut p = remove_params(&app.ui.tool_options);
     p["strokes"] = json!(strokes);
     p["target"] = crate::canvas::paint_target(app);
@@ -209,7 +226,7 @@ pub fn remove_pending_cancel(app: &mut PhotocraftApp) -> bool {
     }
     app.remove_pending.clear();
     app.remove_trail = None;
-    app.remove_pending_doc = None;
+    app.remove_pending_target = None;
     app.ui.status.clear();
     app.ui.status_error = false;
     true
@@ -221,8 +238,7 @@ pub fn remove_pending_live(app: &mut PhotocraftApp) -> bool {
     if app.remove_pending.is_empty() {
         return false;
     }
-    let doc = app.session.active().map(|st| st.doc.id);
-    if app.ui.tool == Tool::Remove && doc.is_some() && app.remove_pending_doc == doc {
+    if app.ui.tool == Tool::Remove && remove_target(app).as_ref() == app.remove_pending_target.as_ref() {
         return true;
     }
     remove_pending_cancel(app);
@@ -1181,6 +1197,55 @@ mod tests {
         assert!(!remove_pending_live(&mut app) && app.remove_trail.is_none());
     }
 
+    #[test]
+    fn queued_remove_cannot_commit_after_layer_or_channel_target_changes() {
+        for change in ["layer", "color", "mask"] {
+            let mut app = app();
+            app.run("layer.new.layer", json!({"name": "Original"})).unwrap();
+            if change == "color" {
+                app.run("channel.target", json!({"channel": "red"})).unwrap();
+            }
+            app.ui.tool = Tool::Remove;
+            app.ui.tool_options.remove_after_each_stroke = false;
+            remove_pending_add(&mut app, &[[12.0, 10.0, 1.0], [16.0, 12.0, 1.0]]);
+            assert_eq!(app.remove_pending.len(), 1);
+            assert!(remove_pending_live(&mut app));
+
+            match change {
+                "layer" => {
+                    app.run("layer.new.layer", json!({"name": "Other"})).unwrap();
+                }
+                "color" => {
+                    app.run("channel.target", json!({"channel": "blue"})).unwrap();
+                }
+                _ => {
+                    app.run("layer.layerMask.revealAll", json!({})).unwrap();
+                    app.ui.mask_target = true;
+                }
+            }
+
+            let journal = app.session.journal.len();
+            assert!(!remove_pending_commit(&mut app), "{change}: Enter must not retarget a queued stroke");
+            assert_eq!(app.session.journal.len(), journal, "{change}: no Remove command ran");
+            assert!(app.remove_pending.is_empty() && app.remove_trail.is_none(), "{change}: queue cleared");
+            assert!(app.remove_pending_target.is_none());
+        }
+    }
+
+    #[test]
+    fn queued_remove_starts_a_new_queue_after_target_switch() {
+        let mut app = app();
+        app.run("layer.new.layer", json!({"name": "Original"})).unwrap();
+        app.ui.tool = Tool::Remove;
+        app.ui.tool_options.remove_after_each_stroke = false;
+        remove_pending_add(&mut app, &[[12.0, 10.0, 1.0], [16.0, 12.0, 1.0]]);
+        app.run("layer.new.layer", json!({"name": "Other"})).unwrap();
+        remove_pending_add(&mut app, &[[40.0, 10.0, 1.0], [44.0, 12.0, 1.0]]);
+        assert_eq!(app.remove_pending.len(), 1, "strokes from different layers must not be combined");
+        assert_eq!(app.remove_pending[0][0][0], 40.0);
+        assert!(remove_pending_live(&mut app));
+    }
+
     #[test]
     fn remove_strokes_waiting_are_dropped_when_the_tool_changes() {
         let mut app = app();