Merge pull request #4227 from ncic/pencil-mouse-samples

Canvas: a mouse's moves between frames reach the tool too

crates/engine/src/tooling.rs

@@ -415,6 +415,12 @@ impl Session {
         self.tool.wants_ticks()
     }
 
+    /// Does the active tool build a stroke from every position the pointer passed
+    /// ([`vectorcraft_tools::Tool::wants_samples`])?
+    pub fn tool_wants_samples(&self) -> bool {
+        self.tool.wants_samples()
+    }
+
     /// Is the active tool waiting for work it runs on another thread ([`Tool::waiting`])?
     pub fn tool_waiting(&self) -> bool {
         self.tool.waiting()

crates/tools/src/draw2/gesture.rs

@@ -533,6 +533,11 @@ impl Tool for GestureTool {
             _ => vec![],
         }
     }
+    /// The freehand tools build their stroke from every position the pointer passed; the Join tool
+    /// only needs where its scribble crosses the ends.
+    fn wants_samples(&self) -> bool {
+        matches!(self.id, "pencil" | "paintbrush" | "blobBrush" | "eraser" | "knife" | "smooth" | "pathEraser")
+    }
     /// `[` and `]` are the Blob Brush's and the Eraser's: their size (#1661).
     fn claims_key(&self, _cx: &ToolContext, key: ToolKey) -> bool {
         matches!(key, ToolKey::BracketLeft | ToolKey::BracketRight) && matches!(self.id, "blobBrush" | "eraser")
@@ -874,6 +879,16 @@ mod tests {
     /// Holding Alt shows the Paintbrush's closing cursor before the press, and the Pencil's and the
     /// Paintbrush's while drawing (before a press, Alt gives the Pencil its straight-line cursor); a release
     /// with Alt still held closes the path, and Alt let go before the release leaves it open.
+    /// The freehand tools take every position the pointer passed (`wants_samples`); the Join tool
+    /// doesn't need them.
+    #[test]
+    fn the_freehand_tools_want_every_sample() {
+        for id in ["pencil", "paintbrush", "blobBrush", "eraser", "knife", "smooth", "pathEraser"] {
+            assert!(GestureTool::new(id).wants_samples(), "{id}");
+        }
+        assert!(!GestureTool::new("join").wants_samples());
+    }
+
     #[test]
     fn alt_shows_the_closing_cursor_and_a_release_with_alt_closes() {
         let (d, _) = doc_with_rect();

crates/tools/src/lib.rs

@@ -923,6 +923,14 @@ pub trait Tool: Send {
     fn wants_ticks(&self) -> bool {
         false
     }
+    /// Does the tool build a stroke from the drag's samples (the Pencil and the other freehand
+    /// tools)? The host then hands it every position the pointer passed between two frames, not
+    /// only the last, so a fast stroke keeps its curve. Every other tool gets one drag a frame, so
+    /// one whose drag preview is costly (moving a large selection, Free Distort, the Width tool)
+    /// doesn't redo it for each sample.
+    fn wants_samples(&self) -> bool {
+        false
+    }
     /// Is the tool waiting for work it runs on another thread (the planar map of a large selection
     /// for the Shape Builder and the Live Paint Bucket)? It highlights no region under the pointer
     /// until the work is done. While this holds, the desktop app calls [`Tool::poll`] each frame,

crates/tools/src/symbolism.rs

@@ -261,6 +261,9 @@ impl Tool for BrushPaintTool {
             self.inner.set_option(key, v);
         }
     }
+    fn wants_samples(&self) -> bool {
+        self.inner.wants_samples()
+    }
     fn busy(&self) -> bool {
         self.inner.busy()
     }

crates/ui-egui/src/canvas.rs

@@ -196,9 +196,9 @@ fn pen_pressure(ui: &Ui, app: &VectorcraftApp, pressed: bool) -> f32 {
 
 /// The positions this frame's pointer moves passed through before its last one, each with the
 /// pen force a touch event gave just before it, or the pressure the host's feed kept for that move
-/// ([`StylusFeed::take_moves`], macOS), if any. The last move is the pointer's position. Only for
-/// pens and fingers (frames with touch events or with the feed's moves): a mouse's moves are left
-/// as they were.
+/// ([`StylusFeed::take_moves`], macOS), if any. The last move is the pointer's position. A mouse's
+/// moves count too, with no pressure: a fast mouse stroke otherwise keeps one position a frame, a
+/// sample every few pixels, too few for a low Fidelity to follow the hand (#3328).
 fn frame_moves(ui: &Ui, stylus: Option<&StylusFeed>) -> Vec<(Pos2, Option<f32>)> {
     let pressures = stylus.map(StylusFeed::take_moves).unwrap_or_default();
     ui.input(|i| {
@@ -219,6 +219,8 @@ fn frame_moves(ui: &Ui, stylus: Option<&StylusFeed>) -> Vec<(Pos2, Option<f32>)>
             let forces = pressures.iter().rev().map(|f| (*f > 0.0).then_some(*f)).chain(std::iter::repeat(None));
             moves = points.into_iter().rev().zip(forces).collect();
             moves.reverse();
+        } else {
+            moves = i.events.iter().filter_map(|e| if let egui::Event::PointerMoved(p) = e { Some((*p, None)) } else { None }).collect();
         }
         moves.pop();
         // A burst of events (a stalled frame) is thinned to a bounded count.
@@ -231,6 +233,15 @@ fn frame_moves(ui: &Ui, stylus: Option<&StylusFeed>) -> Vec<(Pos2, Option<f32>)>
     })
 }
 
+/// The moves of this frame ([`frame_moves`]) the active tool gets before the pointer's position:
+/// all of them for a tool that builds a stroke from them ([`vectorcraft_engine::Session::tool_wants_samples`]), none
+/// for the others, which get one drag a frame. The moves are read either way, so the host's feed
+/// pairs the next frame's with its own.
+fn tool_frame_moves(ui: &Ui, app: &VectorcraftApp) -> Vec<(Pos2, Option<f32>)> {
+    let moves = frame_moves(ui, app.services.stylus.as_ref());
+    if app.session.tool_wants_samples() { moves } else { vec![] }
+}
+
 pub fn mods(m: egui::Modifiers, space: bool) -> Mods {
     Mods { shift: m.shift, alt: m.alt, cmd: m.command, ctrl: m.ctrl, space }
 }
@@ -1185,13 +1196,16 @@ fn handle_input(app: &mut VectorcraftApp, ui: &Ui, resp: &egui::Response, rect:
                     ui.data_mut(|dd| dd.insert_temp(drag_id(), Drag::Art));
                 }
                 Drag::Tool => {
-                    if tool_drag_moved(ui, &xf, p, &pointer) {
-                        // In a frame with touch events (a pen on Windows or the web, a finger) or a
-                        // macOS pen's moves, every position the pen passed, not just the last: a fast
-                        // stroke keeps its curve (a pen reports far more often than frames come).
-                        // Mouse events, a pen's on X11 among them, give the last position only, with
-                        // the host's latest pressure.
-                        for (at, force) in frame_moves(ui, app.services.stylus.as_ref()) {
+                    // Every position the pen or mouse passed this frame, not just the last, for a
+                    // tool that builds a stroke from them: a fast stroke keeps its curve (a pen or a
+                    // mouse reports far more often than frames come). Touch events (a pen on Windows
+                    // or the web, a finger) and a macOS pen's moves carry their own pressure; a
+                    // mouse's, a pen's on X11 among them, take the host's latest. Other tools get
+                    // the last position only, so a costly drag preview runs once a frame.
+                    let moves = tool_frame_moves(ui, app);
+                    // A frame whose pointer ends where the last one did still moved in between.
+                    if tool_drag_moved(ui, &xf, p, &pointer) || !moves.is_empty() {
+                        for (at, force) in moves {
                             let pressure = force.unwrap_or_else(|| pen_pressure(ui, app, false));
                             dispatch(app, &PointerEvent { kind: PointerKind::Drag, pos: xf.to_doc(at), mods: mods(m, space), pressure }, view);
                         }
@@ -9127,7 +9141,7 @@ mod tests {
     }
 
     #[test]
-    fn frame_moves_keeps_touch_samples_and_pressure_but_leaves_mouse_unchanged() {
+    fn frame_moves_keeps_touch_feed_and_mouse_samples_with_their_pressure() {
         let ctx = egui::Context::default();
         let a = pos2(10.0, 20.0);
         let b = pos2(20.0, 30.0);
@@ -9136,7 +9150,8 @@ mod tests {
         let mut mouse = vec![];
         let mut out = ctx.run_ui(egui::RawInput { events: events.clone(), ..Default::default() }, |ui| mouse = frame_moves(ui, None));
         out.textures_delta.clear();
-        assert!(mouse.is_empty());
+        // A mouse keeps its moves too, without a pressure.
+        assert_eq!(mouse, [(a, None), (b, None)], "the last move follows the normal pointer path");
         // A macOS pen: the feed kept a pressure for each move, the last for the last.
         let feed = StylusFeed::default();
         for p in [0.9, 0.3, 0.0, 0.5] {
@@ -9148,7 +9163,7 @@ mod tests {
         assert_eq!(samples, [(a, Some(0.3)), (b, None)], "paired from the end; a lift's 0 is no pressure");
         let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| samples = frame_moves(ui, Some(&feed)));
         out.textures_delta.clear();
-        assert!(samples.is_empty(), "the moves were taken: a mouse's moves again");
+        assert_eq!(samples, [(a, None), (b, None)], "the moves were taken: a mouse's moves again");
         let events = [pen(egui::TouchPhase::Move, a, 0.2), pen(egui::TouchPhase::Move, b, 0.8), pen(egui::TouchPhase::Move, c, 0.6)].concat();
         let mut samples = vec![];
         let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| samples = frame_moves(ui, None));
@@ -9546,6 +9561,84 @@ mod tests {
         assert!(!visible(&app), "the grid stays hidden");
     }
 
+    /// #3328: every position a mouse reports within one frame reaches the Pencil, not only the
+    /// frame's last one, so a fast stroke keeps the hand's movement.
+    #[test]
+    fn a_mouse_stroke_keeps_every_move_of_a_frame() {
+        let mut app = VectorcraftApp::new(Session::new(), Default::default());
+        app.session.execute("file.new", &json!({"width": 800, "height": 600})).unwrap();
+        let ctx = egui::Context::default();
+        frame(&mut app, &ctx, vec![]);
+        app.select_tool("pencil");
+        let xf = Xf::new(app.canvas_rect.unwrap(), app.view().unwrap());
+        let at = |x: f64, y: f64| xf.to_screen(Point::new(x, y));
+        let button =
+            |p, pressed| egui::Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE };
+        let start = at(100.0, 100.0);
+        frame(&mut app, &ctx, vec![egui::Event::PointerMoved(start), button(start, true)]);
+        // One frame of a fast stroke: a zigzag of 20 moves, 6 pt apart.
+        let zigzag: Vec<_> = (1..=20).map(|i| at(100.0 + 6.0 * f64::from(i), if i % 2 == 0 { 100.0 } else { 112.0 })).collect();
+        frame(&mut app, &ctx, zigzag.iter().map(|p| egui::Event::PointerMoved(*p)).collect());
+        let end = *zigzag.last().unwrap();
+        frame(&mut app, &ctx, vec![button(end, false)]);
+        let (cmd, p) = app.session.journal.last().unwrap().clone();
+        assert_eq!(cmd, "path.freehand");
+        let pts = p["points"].as_array().unwrap();
+        let expected: Vec<_> = std::iter::once(start)
+            .chain(zigzag.iter().copied())
+            .map(|p| {
+                let p = xf.to_doc(p);
+                json!([p.x, p.y])
+            })
+            .collect();
+        assert_eq!(pts, expected.as_slice(), "the stroke keeps every move of the frame, in order");
+    }
+
+    /// A frame whose moves end where the last frame's did still reaches the Pencil: the stroke went
+    /// out and came back in between.
+    #[test]
+    fn a_mouse_stroke_keeps_a_frame_that_ends_where_it_started() {
+        let mut app = VectorcraftApp::new(Session::new(), Default::default());
+        app.session.execute("file.new", &json!({"width": 800, "height": 600})).unwrap();
+        let ctx = egui::Context::default();
+        frame(&mut app, &ctx, vec![]);
+        app.select_tool("pencil");
+        let xf = Xf::new(app.canvas_rect.unwrap(), app.view().unwrap());
+        let at = |x: f64, y: f64| xf.to_screen(Point::new(x, y));
+        let button =
+            |p, pressed| egui::Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE };
+        let (start, a) = (at(100.0, 100.0), at(160.0, 100.0));
+        frame(&mut app, &ctx, vec![egui::Event::PointerMoved(start), button(start, true)]);
+        frame(&mut app, &ctx, vec![egui::Event::PointerMoved(a)]);
+        // Out to a bump and back to `a`, all in one frame.
+        let bump = [at(170.0, 80.0), at(180.0, 70.0), at(170.0, 80.0), a];
+        frame(&mut app, &ctx, bump.iter().map(|p| egui::Event::PointerMoved(*p)).collect());
+        frame(&mut app, &ctx, vec![button(a, false)]);
+        let (_, p) = app.session.journal.last().unwrap().clone();
+        let n = p["points"].as_array().unwrap().len();
+        assert!(n >= 5, "the bump's moves are kept: {n} points");
+    }
+
+    /// Only a tool that builds a stroke from the drag's samples gets every move of a frame (#4227):
+    /// the Selection tool and the other tools get one drag a frame, at the pointer's position, so a
+    /// costly drag preview runs once.
+    #[test]
+    fn only_the_freehand_tools_get_every_move_of_a_frame() {
+        let mut app = VectorcraftApp::new(Session::new(), Default::default());
+        app.session.execute("file.new", &json!({"width": 800, "height": 600})).unwrap();
+        let ctx = egui::Context::default();
+        let events: Vec<_> = (0..20).map(|i| egui::Event::PointerMoved(pos2(100.0 + i as f32 * 5.0, 100.0))).collect();
+        for (tool, every) in
+            [("pencil", true), ("paintbrush", true), ("blobBrush", true), ("selection", false), ("directSelection", false), ("width", false)]
+        {
+            app.select_tool(tool);
+            let mut moves = vec![];
+            let mut out = ctx.run_ui(egui::RawInput { events: events.clone(), ..Default::default() }, |ui| moves = tool_frame_moves(ui, &app));
+            out.textures_delta.clear();
+            assert_eq!(moves.len(), if every { 19 } else { 0 }, "{tool}");
+        }
+    }
+
     /// #491: a pen tap whose press and lift come in one frame selects what it tapped, and the
     /// lift (the pointer gone with it) doesn't drag the selection to the canvas's centre.
     #[test]