Merge pull request #3791 from dotCooCoo/fix/opening-waits-only-for-a-drag

Open and Place: a file read in the background waits for a busy tool only during a drag

crates/engine/src/lib.rs

@@ -1425,6 +1425,9 @@ pub struct Session {
     pub(crate) liquify_stroke: Option<Box<cmd::distortcmds::LiquifyStroke>>,
     /// A press on the Plane Switching Widget is under way: its drag and release are the widget's.
     pub(crate) plane_widget_press: bool,
+    /// The pointer button is held: the last pointer event was a press or a drag, not a release or
+    /// a move ([`Session::pointer_held`]).
+    pub(crate) pointer_held: bool,
     /// A guide being dragged out of a ruler ([`Session::ruler_guide`]).
     pub(crate) ruler_guide: Option<vectorcraft_tools::rulerguide::NewGuide>,
     /// Which selected type overflows its frame, for the markers every tool but the Type tools shows.
@@ -1520,6 +1523,7 @@ impl Session {
             envelope_defaults: None,
             liquify_stroke: None,
             plane_widget_press: false,
+            pointer_held: false,
             ruler_guide: None,
             overflow_marks: Default::default(),
             font_search: None,

crates/engine/src/tests.rs

@@ -825,6 +825,26 @@ fn selection_tool_drag_is_one_undo_step() {
     assert_eq!(s.doc().unwrap().doc.node(a).unwrap().geometric_bounds().unwrap().x0, 100.0);
 }
 
+/// The pointer button is held from a press or a drag until a release or a move. The Dimension
+/// tool stays busy after its first click with the button released, waiting for the second one.
+#[test]
+fn the_pointer_is_held_from_a_press_until_a_release_or_a_move() {
+    let mut s = session();
+    let v = ViewInfo::default();
+    s.select_tool("dimension", v).unwrap();
+    let mut held = vec![];
+    for (kind, x) in [(PointerKind::Down, 100.0), (PointerKind::Drag, 100.0), (PointerKind::Up, 100.0)] {
+        s.pointer(&PointerEvent::new(kind, x, 100.0), v).unwrap();
+        held.push(s.pointer_held());
+    }
+    assert!(s.tool_busy(), "the first point waits for the second");
+    for (kind, x) in [(PointerKind::Move, 200.0), (PointerKind::Down, 300.0), (PointerKind::Drag, 310.0), (PointerKind::Move, 320.0)] {
+        s.pointer(&PointerEvent::new(kind, x, 100.0), v).unwrap();
+        held.push(s.pointer_held());
+    }
+    assert_eq!(held, [true, true, false, false, true, true, false]);
+}
+
 /// How many steps there are to undo and to redo, and whether the document has unsaved changes.
 pub(crate) fn history(s: &Session) -> (usize, usize, bool) {
     let st = s.doc().unwrap();

crates/engine/src/tooling.rs

@@ -313,6 +313,11 @@ impl Session {
     /// Feed a pointer event to the active tool. Returns requests for the UI.
     pub fn pointer(&mut self, ev: &PointerEvent, view: ViewInfo) -> Result<Vec<UiRequest>> {
         self.last_view = view;
+        match ev.kind {
+            PointerKind::Down | PointerKind::Drag => self.pointer_held = true,
+            PointerKind::Up | PointerKind::Move => self.pointer_held = false,
+            PointerKind::DoubleClick => {}
+        }
         // The Plane Switching Widget takes its clicks whatever the tool.
         if let Some(r) = self.plane_widget_pointer(ev, view) {
             return r;
@@ -437,6 +442,13 @@ impl Session {
         self.tool.busy()
     }
 
+    /// Is the pointer button held: was the last pointer event a press or a drag, not a release or a
+    /// move? A tool that is busy while the button is not held is waiting between its clicks (the
+    /// Dimension tool after its first point).
+    pub fn pointer_held(&self) -> bool {
+        self.pointer_held
+    }
+
     /// Is the active tool moving, scaling or rotating the selection with a drag?
     pub fn tool_transforming(&self) -> bool {
         self.tool.transforming()

crates/ui-egui/src/opening.rs

@@ -82,10 +82,14 @@ pub fn start(app: &mut VectorcraftApp, path: &str, bytes: Vec<u8>) -> Result<Val
 /// becomes the active one, and the dialog's OK acts on the active document (#1694: Save As
 /// options confirmed after another file had finished opening saved that file's art under the
 /// first one's name). The files wait, their status line too, until the dialog is closed. Nor while
-/// a drag (a tool's interaction, or a tool busy with a drag of its own) is under way: a document
-/// that opens cancels it.
+/// an interaction is open (a drag, a slider's preview, or a tool's preview between its clicks) or
+/// a tool is busy with a drag of its own while the pointer button is held: a document that opens
+/// cancels it. A tool that is busy while the button is not held and no interaction is open is
+/// waiting between its clicks (the Dimension tool after its first point): the file opens, and the
+/// tool starts afresh, as when the user switches documents.
 pub fn poll(app: &mut VectorcraftApp) {
-    if app.ui.dialog.is_some() || app.session.in_interaction() || app.session.tool_busy() {
+    let s = &app.session;
+    if app.ui.dialog.is_some() || s.in_interaction() || (s.tool_busy() && s.pointer_held()) {
         return;
     }
     let mut i = 0;
@@ -253,6 +257,94 @@ mod tests {
         assert_eq!(names, ["a.png"], "the drag placed its file");
     }
 
+    /// A file that finishes opening while a preview runs waits for the preview to end, so the
+    /// preview is not canceled: a slider's preview with the tool idle, a slider's preview while the
+    /// Dimension tool waits after its first point, and the Dimension tool's line following the
+    /// pointer after its second point.
+    #[test]
+    fn a_file_ready_during_a_preview_waits_for_it_to_end() {
+        use vectorcraft_tools::{PointerEvent, PointerKind};
+        let view = vectorcraft_engine::ViewInfo::default();
+        let click = |app: &mut VectorcraftApp, x: f64| {
+            for kind in [PointerKind::Down, PointerKind::Up] {
+                crate::canvas::dispatch(app, &PointerEvent::new(kind, x, 20.0), view);
+            }
+        };
+        for case in ["slider", "slider between clicks", "line"] {
+            let mut app = VectorcraftApp::new(Session::new(), Services::default());
+            app.run("file.new", json!({"width": 400, "height": 300})).unwrap();
+            if case != "slider" {
+                app.select_tool("dimension");
+                click(&mut app, 20.0);
+            }
+            if case == "line" {
+                click(&mut app, 80.0);
+                crate::canvas::dispatch(&mut app, &PointerEvent::new(PointerKind::Move, 50.0, 60.0), view);
+            } else {
+                app.session.begin_interaction("Preview").unwrap();
+            }
+            assert_eq!(app.session.tool_busy(), case != "slider", "{case}");
+            start(&mut app, "/art/Second.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false)).unwrap();
+            let t0 = std::time::Instant::now();
+            while t0.elapsed().as_millis() < 400 {
+                poll(&mut app);
+                std::thread::yield_now();
+            }
+            assert!(app.session.documents().len() == 1 && app.session.in_interaction(), "the file waits for the preview: {case}");
+            if case == "line" {
+                click(&mut app, 50.0);
+            } else {
+                app.session.cancel_interaction().unwrap();
+            }
+            let t0 = std::time::Instant::now();
+            while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 {
+                poll(&mut app);
+                std::thread::yield_now();
+            }
+            assert_eq!(app.session.documents().len(), 2, "{case}");
+        }
+    }
+
+    /// A file that finishes opening while a tool waits between its clicks with no preview open (the
+    /// Dimension tool after its first point, then after its second before the pointer moves) opens
+    /// without waiting for the next click, and the tool starts afresh in it, as when the user
+    /// switches documents.
+    #[test]
+    fn a_file_ready_between_a_tools_clicks_opens() {
+        use vectorcraft_tools::{PointerEvent, PointerKind};
+        let mut app = VectorcraftApp::new(Session::new(), Services::default());
+        app.run("file.new", json!({"width": 400, "height": 300})).unwrap();
+        app.select_tool("dimension");
+        let view = vectorcraft_engine::ViewInfo::default();
+        let click = |app: &mut VectorcraftApp, x: f64| {
+            for kind in [PointerKind::Down, PointerKind::Up] {
+                crate::canvas::dispatch(app, &PointerEvent::new(kind, x, 20.0), view);
+            }
+        };
+        let opened = |app: &mut VectorcraftApp| {
+            let t0 = std::time::Instant::now();
+            while !app.opening.pending.is_empty() && t0.elapsed().as_secs() < 30 {
+                poll(app);
+                std::thread::yield_now();
+            }
+        };
+        click(&mut app, 20.0);
+        assert!(app.session.tool_busy() && !app.session.in_interaction(), "the first point waits for the second");
+        start(&mut app, "/art/Second.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(400.0, 300.0), false)).unwrap();
+        opened(&mut app);
+        assert_eq!(app.session.documents().len(), 2, "the file opens after one click");
+        assert!(!app.session.tool_busy(), "the tool starts afresh");
+        click(&mut app, 20.0);
+        click(&mut app, 80.0);
+        assert!(app.session.tool_busy() && !app.session.in_interaction(), "the second point waits for the line's place");
+        start(&mut app, "/art/Third.vectorcraft", vectorcraft_format::save(&vectorcraft_doc::Document::new(120.0, 80.0), false)).unwrap();
+        opened(&mut app);
+        assert_eq!(app.session.documents().len(), 3, "the file opens after two clicks");
+        assert!(!app.session.tool_busy() && !app.session.in_interaction(), "the tool starts afresh");
+        let second = &app.session.documents()[1].doc;
+        assert!(second.layers.iter().all(|l| l.children().is_none_or(|c| c.is_empty())), "the unfinished dimension is not made");
+    }
+
     #[test]
     fn a_file_read_off_the_ui_thread_opens_as_one_read_at_once() {
         let mut app = VectorcraftApp::new(Session::new(), Services::default());

crates/ui-egui/src/placing.rs

@@ -153,16 +153,18 @@ fn spawn(
 }
 
 /// Place the files that are ready (each frame), in the order they were started: a file read
-/// sooner than one started before it waits for that one. Not while a dialog is open or a drag (a
-/// tool's interaction, or a tool busy with a drag of its own, such as the place cursor's) is under
-/// way, as [`crate::opening::poll`]: a placed file's document becomes the active one and the
-/// dialog's OK acts on the active document; a drag in another document is canceled when the
-/// active document changes, and a drag in the same one previews from the document as it was
-/// before the drag, without the placed art.
+/// sooner than one started before it waits for that one. Not while a dialog is open, an
+/// interaction is open, or a tool is busy with a drag of its own (the place cursor's) while the
+/// pointer button is held, as [`crate::opening::poll`]: a placed file's document becomes the
+/// active one and the dialog's OK acts on the active document; a drag in another document is
+/// canceled when the active document changes, and a drag or preview in the same one previews from
+/// the document as it was before it began, without the placed art. A tool that is busy while the
+/// button is not held is waiting between its clicks (the Dimension tool after its first point):
+/// the file is placed unless the tool has a preview open (the Flare tool after its center drag).
 pub fn poll(app: &mut VectorcraftApp) {
     while app.ui.dialog.is_none()
         && !app.session.in_interaction()
-        && !app.session.tool_busy()
+        && !(app.session.tool_busy() && app.session.pointer_held())
         && let Some(p) = app.placing.pending.first()
     {
         let r = match p.done.try_recv() {
@@ -606,4 +608,30 @@ mod tests {
         assert_eq!(placed_in(0).iter().map(|(n, _)| n.as_str()).collect::<Vec<_>>(), ["photo.tif"]);
         assert_eq!(placed_in(1), [("a.png".to_string(), Some(vectorcraft_geom::Rect::new(20.0, 20.0, 60.0, 60.0)))], "the drag placed its file");
     }
+
+    /// A file read while a tool waits between its clicks (the Dimension tool after its first point),
+    /// in the file's document or in another one, is placed without waiting for the next click. In
+    /// the same document the tool keeps its point; in another one it starts afresh, as when the user
+    /// switches documents.
+    #[test]
+    fn a_file_read_between_a_tools_clicks_is_placed() {
+        use vectorcraft_tools::{PointerEvent, PointerKind};
+        let path = saved("clicks", "photo.tif", &picture(Tiff, 60, 40));
+        for other_document in [false, true] {
+            let mut app = app(true);
+            start(&mut app, &json!({"path": path, "at": [300, 200]}), false).unwrap();
+            if other_document {
+                app.run("file.new", json!({"width": 400, "height": 300})).unwrap();
+            }
+            app.select_tool("dimension");
+            let view = vectorcraft_engine::ViewInfo::default();
+            for kind in [PointerKind::Down, PointerKind::Up] {
+                crate::canvas::dispatch(&mut app, &PointerEvent::new(kind, 20.0, 20.0), view);
+            }
+            assert!(app.session.tool_busy() && !app.session.in_interaction(), "other document: {other_document}");
+            placed(&mut app);
+            assert_eq!(names(&app), ["photo.tif"], "other document: {other_document}");
+            assert_eq!(app.session.tool_busy(), !other_document, "other document: {other_document}");
+        }
+    }
 }