Merge pull request #4076 from dotCooCoo/fix/trace-eye-button-hold

Image Trace: the eye button's hold ends on release, and the panel uses any selected trace or image

crates/ui-egui/src/canvas.rs

@@ -277,6 +277,11 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
     // that does not draw the overlay drops them, for example with the option off, with the edges
     // hidden, on the Home screen or with no document open.
     let hidden_chars = std::mem::take(&mut app.canvas.hidden_chars);
+    // A release ends the Image Trace panel's eye-button hold, also when the panel is no longer
+    // drawn, so a later press elsewhere does not show the source image.
+    if app.ui.trace_hold_source.is_some() && !ui.input(|i| i.pointer.any_down()) {
+        app.ui.trace_hold_source = None;
+    }
     // The IME geometry this pass, if the Type tool gets one; `ime::end_of_pass` consumes it after
     // the dialogs and panels draw.
     app.ime_caret_rect = None;
@@ -424,8 +429,8 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
         rich_black: app.session.prefs.black_on_screen == "rich",
         // A tool's drag on the canvas, during an interaction (not a dialog's preview).
         dragging: app.session.in_interaction() && ui.data(|d| d.get_temp::<Drag>(drag_id())) == Some(Drag::Tool),
-        // The Image Trace panel's eye button, only while a pointer button is down.
-        trace_source: app.ui.trace_hold_source.clone().filter(|_| ui.input(|i| i.pointer.any_down())),
+        // The Image Trace panel's eye button, while it is held.
+        trace_source: app.ui.trace_hold_source.clone(),
     };
     // Placed documents' bitmaps are being made: draw again when they are ready.
     if vectorcraft_render::placed_document::busy() {
@@ -4353,6 +4358,29 @@ mod tests {
         out.textures_delta.clear();
     }
 
+    /// The Image Trace panel's eye-button hold ends when the pointer is released, even when the
+    /// panel stopped drawing while the button was held, so a later press on the canvas does not
+    /// show the source image.
+    #[test]
+    fn a_release_ends_the_image_trace_hold_without_the_panel() {
+        let mut app = VectorcraftApp::new(Session::new(), Default::default());
+        app.run("file.new", json!({"width": 200, "height": 200})).unwrap();
+        let made = app.run("shape.rectangle", json!({"x": 10, "y": 10, "width": 50, "height": 50})).unwrap();
+        let id = vectorcraft_doc::NodeId(made["id"].as_u64().unwrap());
+        let ctx = egui::Context::default();
+        let at = pos2(400.0, 300.0);
+        let press = |pressed| egui::Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() };
+        frame(&mut app, &ctx, vec![egui::Event::PointerMoved(at), press(true)]);
+        app.ui.trace_hold_source = Some(std::sync::Arc::new(vec![id]));
+        frame(&mut app, &ctx, vec![]);
+        assert!(app.ui.trace_hold_source.is_some(), "held while the button is down");
+        frame(&mut app, &ctx, vec![press(false)]);
+        assert_eq!(app.ui.trace_hold_source, None, "the release ends the hold");
+        frame(&mut app, &ctx, vec![press(true)]);
+        assert_eq!(app.ui.trace_hold_source, None, "a later press does not show the source image");
+        frame(&mut app, &ctx, vec![press(false)]);
+    }
+
     /// The baseline marker of selected point type runs under the line, wherever its alignment
     /// puts the anchor: under centred and right-aligned type too, not from the anchor rightwards
     /// (#2879).

crates/ui-egui/src/panels/image_trace.rs

@@ -92,17 +92,30 @@ pub(crate) fn is_trace(n: &Node) -> bool {
         && n.children().is_some_and(|c| c.first().is_some_and(|i| matches!(i.kind, NodeKind::Image(_))))
 }
 
-/// The active document and the first selected object.
-fn target_key(app: &VectorcraftApp) -> Option<(u64, NodeId)> {
-    Some((app.session.active()?.uid, first_selected(app)?.id))
+/// The object Trace acts on: the first selected image or Image Trace object in paint order, as
+/// `imageTrace.make` takes it, else the first selected object.
+fn target_node(app: &VectorcraftApp) -> Option<Node> {
+    let st = app.session.active()?;
+    let traceable = |n: &&Node| is_trace(n) || matches!(n.kind, NodeKind::Image(_));
+    st.selection.in_paint_order(&st.doc).into_iter().find_map(|id| st.doc.node(id).filter(traceable).cloned()).or_else(|| first_selected(app))
 }
 
-/// What the selection is: an Image Trace object (with its stored settings), a plain image, or neither.
+/// The active document and the object Trace acts on.
+fn target_key(app: &VectorcraftApp) -> Option<(u64, NodeId)> {
+    Some((app.session.active()?.uid, target_node(app)?.id))
+}
+
+/// What Trace acts on: an Image Trace object (with its stored settings), a plain image, or neither.
 fn target(app: &VectorcraftApp) -> (bool, bool, Option<Value>) {
-    let Some(n) = first_selected(app) else { return (false, false, None) };
+    let Some(n) = target_node(app) else { return (false, false, None) };
     (is_trace(&n), matches!(n.kind, NodeKind::Image(_)), n.extras().trace.as_deref().cloned())
 }
 
+/// Whether any selected object is an Image Trace object, wherever it is in the selection.
+fn any_trace(app: &VectorcraftApp) -> bool {
+    app.session.active().is_some_and(|st| st.selection.objects.iter().any(|id| st.doc.node(*id).is_some_and(is_trace)))
+}
+
 /// The one selected Image Trace object's preset ("Custom" once its settings were changed) and view.
 pub(crate) fn selected_trace(app: &VectorcraftApp) -> Option<(String, TraceView)> {
     let st = app.session.active()?;
@@ -281,6 +294,7 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
         st.params = all.first().map(|p| p.1.clone()).unwrap_or_default();
     }
     let (is_trace, is_image, stored) = target(app);
+    let any_trace = any_trace(app);
     let object = target_key(app);
     if let Some(t) = stored.filter(|t| *t != st.synced || object != st.object) {
         // A change of the object's view alone (here or in the Control bar) keeps the settings that
@@ -317,16 +331,16 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
         let eye = 24.0;
         if let Some(v) = view_dropdown(ui, st.view, (ui.available_width() - eye - ui.spacing().item_spacing.x).min(170.0)) {
             st.view = v;
-            if is_trace {
+            if any_trace {
                 set_view(app, v);
             }
         }
         let held = app.ui.trace_hold_source.is_some();
-        let r = widgets::icon_button_enabled(ui, "eye", tl!("Press and hold to view the source image"), held, is_trace, eye);
+        let r = widgets::icon_button_enabled(ui, "eye", tl!("Press and hold to view the source image"), held, any_trace, eye);
         if cfg!(test) {
             ui.ctx().data_mut(|d| d.insert_temp(egui::Id::unique("trace-hold-source"), r.rect));
         }
-        hold_source(app, ui.ctx(), is_trace && r.is_pointer_button_down_on());
+        hold_source(app, ui.ctx(), any_trace && r.is_pointer_button_down_on());
     });
     ui.horizontal(|ui| {
         widgets::dim_label(ui, tl!("Mode:"));
@@ -501,7 +515,7 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
         if r.on_disabled_hover_text(tl!("Select an image to trace")).clicked() {
             trace(app, &mut st);
         }
-        if ui.add_enabled_ui(is_trace, |ui| widgets::flat_button(ui, tl!("Expand"), 80.0)).inner.clicked() {
+        if ui.add_enabled_ui(any_trace, |ui| widgets::flat_button(ui, tl!("Expand"), 80.0)).inner.clicked() {
             app.run("imageTrace.expand", json!({})).ok();
         }
     });
@@ -519,11 +533,11 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
 }
 
 pub fn menu(app: &mut VectorcraftApp, ui: &mut Ui) {
-    let (is_trace, _, _) = target(app);
-    if menu_item(ui, tl!("Release"), is_trace, false) {
+    let any_trace = any_trace(app);
+    if menu_item(ui, tl!("Release"), any_trace, false) {
         app.run("imageTrace.release", json!({})).ok();
     }
-    if menu_item(ui, tl!("Expand"), is_trace, false) {
+    if menu_item(ui, tl!("Expand"), any_trace, false) {
         app.run("imageTrace.expand", json!({})).ok();
     }
     ui.separator();
@@ -910,6 +924,97 @@ mod tests {
         frame(&mut app, &ctx, vec![press(false)]);
     }
 
+    /// With another object first in the selection, the eye button and the panel menu's Release
+    /// act on the selected Image Trace object.
+    #[test]
+    fn the_trace_controls_work_when_another_object_is_selected_first() {
+        let mut app = traced_app();
+        let trace_id = first_selected(&app).unwrap().id;
+        let made = app.run("shape.rectangle", json!({"x": 80, "y": 80, "width": 10, "height": 10})).unwrap();
+        let rect = made["id"].as_u64().unwrap();
+        app.run("select.set", json!({"ids": [rect, trace_id.0]})).unwrap();
+        assert_eq!(first_selected(&app).unwrap().id.0, rect, "the rectangle comes first");
+        let ctx = egui::Context::default();
+        crate::theme::install_fonts(&ctx);
+        frame(&mut app, &ctx, vec![]);
+        let at = ctx.data(|d| d.get_temp::<Rect>(egui::Id::unique("trace-hold-source"))).unwrap().center();
+        let press = |pressed| Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers: Default::default() };
+        frame(&mut app, &ctx, vec![Event::PointerMoved(at), press(true)]);
+        assert_eq!(app.ui.trace_hold_source.as_deref(), Some(&vec![trace_id]), "the eye button shows the source image");
+        frame(&mut app, &ctx, vec![press(false)]);
+        assert_eq!(app.ui.trace_hold_source, None);
+
+        let menu_frame = |app: &mut VectorcraftApp, events: Vec<Event>| {
+            let raw = egui::RawInput { screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(280.0, 700.0))), events, ..Default::default() };
+            let mut out = ctx.run_ui(raw, |ui| menu(app, ui));
+            out.textures_delta.clear();
+            crate::tests_removeanchors::shapes_text(&out.shapes.iter().map(|c| c.shape.clone()).collect::<Vec<_>>())
+        };
+        let texts = menu_frame(&mut app, vec![]);
+        let at = texts.iter().find(|(t, _)| t.trim() == "Release").map(|(_, r)| r.center()).unwrap_or_else(|| panic!("no Release in {texts:?}"));
+        let press = |pressed| Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers: Default::default() };
+        menu_frame(&mut app, vec![Event::PointerMoved(at), press(true)]);
+        menu_frame(&mut app, vec![press(false)]);
+        let st = app.session.active().unwrap();
+        assert!(st.doc.node(trace_id).is_none(), "Release puts the source image back");
+    }
+
+    /// With another object first in the selection, the panel's View menu and Expand act on the
+    /// selected Image Trace object.
+    #[test]
+    fn view_and_expand_work_when_another_object_is_selected_first() {
+        let mut app = traced_app();
+        let trace_id = first_selected(&app).unwrap().id;
+        let made = app.run("shape.rectangle", json!({"x": 80, "y": 80, "width": 10, "height": 10})).unwrap();
+        let rect = made["id"].as_u64().unwrap();
+        app.run("select.set", json!({"ids": [rect, trace_id.0]})).unwrap();
+        let ctx = egui::Context::default();
+        crate::theme::install_fonts(&ctx);
+        click(&mut app, &ctx, "Tracing Result");
+        click(&mut app, &ctx, "Outlines");
+        let view = |app: &VectorcraftApp| app.session.active().unwrap().doc.node(trace_id).unwrap().trace_view();
+        assert_eq!(view(&app), TraceView::Outlines, "the View menu sets the Image Trace object's view");
+        click(&mut app, &ctx, "Expand");
+        let st = app.session.active().unwrap();
+        assert!(!is_trace(st.doc.node(trace_id).unwrap()), "Expand keeps only the traced paths");
+    }
+
+    /// With another object first in the selection, Trace traces the selected image, the View
+    /// dropdown shows the selected Image Trace object's view, and Preview traces it again.
+    #[test]
+    fn trace_view_and_preview_work_when_another_object_is_selected_first() {
+        let mut app = traced_app();
+        app.run("imageTrace.release", json!({})).unwrap();
+        let image = first_selected(&app).unwrap().id;
+        let made = app.run("shape.rectangle", json!({"x": 80, "y": 80, "width": 10, "height": 10})).unwrap();
+        let rect = made["id"].as_u64().unwrap();
+        app.run("select.set", json!({"ids": [rect, image.0]})).unwrap();
+        assert_eq!(first_selected(&app).unwrap().id.0, rect, "the rectangle comes first");
+        let ctx = egui::Context::default();
+        crate::theme::install_fonts(&ctx);
+        click(&mut app, &ctx, "Trace");
+        let st = app.session.active().unwrap();
+        let trace_id = st.selection.objects.iter().copied().find(|id| st.doc.node(*id).is_some_and(is_trace));
+        let trace_id = trace_id.unwrap_or_else(|| panic!("Trace makes an Image Trace object: {}", app.ui.status));
+
+        // The panel last showed Tracing Result; the object is drawn as Outlines.
+        frame(&mut app, &ctx, vec![]);
+        app.run("imageTrace.setView", json!({"view": "outlines"})).unwrap();
+        app.run("select.set", json!({"ids": [rect, trace_id.0]})).unwrap();
+        let texts = frame(&mut app, &ctx, vec![]);
+        assert!(texts.iter().any(|(t, _)| t == "Outlines") && !texts.iter().any(|(t, _)| t == "Tracing Result"), "{texts:?}");
+
+        // Tracing again replaces the object, so look it up in the selection.
+        let strokes = |app: &VectorcraftApp| {
+            let st = app.session.active().unwrap();
+            let n = st.selection.objects.iter().find_map(|id| st.doc.node(*id).filter(|n| is_trace(n)));
+            n.and_then(|n| n.extras().trace.as_deref().cloned()).map(|t| t["params"]["strokes"].clone())
+        };
+        assert_eq!(strokes(&app), Some(json!(false)));
+        click(&mut app, &ctx, "Strokes");
+        assert_eq!(strokes(&app), Some(json!(true)), "Preview traces the object again");
+    }
+
     #[test]
     fn properties_can_reopen_the_trace_controls() {
         let mut app = traced_app();