From 954d3f3116674a2c98036c224f3eb3d62ba80dc5 Mon Sep 17 00:00:00 2001 From: ncic <1848265+ncic@users.noreply.github.com> Date: Mon, 26 Oct 2026 06:28:02 +0100 Subject: [PATCH 1/2] Canvas: a mouse's moves between frames reach the tool too (#3328) The canvas hands a dragging tool every position the pointer passed within a frame for pens and fingers (touch events) and for a macOS pen (the stylus feed's moves, #852), but a mouse got the frame's last position only: one sample per frame. A fast mouse stroke kept a position every 10 to 20 px, too few for any Fidelity to follow the hand, and fewer still while the app is busy and frames come slower. frame_moves now keeps a mouse's moves as well, without a pressure (the tool takes the host's latest, a mouse's being full; a pen's on X11 among them), with the same bound of 64 moves for a stalled frame, as touch and pen moves already reach every tool. Tests: a_mouse_stroke_keeps_every_move_of_a_frame (20 moves in one frame reach the Pencil; it got 2 points before this change), and frame_moves_keeps_touch_feed_and_mouse_samples_with_their_pressure replaces the test that a mouse's moves were left out. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/canvas.rs | 51 ++++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index a951ef290..904886bf0 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/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)> { 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)> 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. @@ -1186,11 +1188,11 @@ fn handle_input(app: &mut VectorcraftApp, ui: &Ui, resp: &egui::Response, rect: } 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. + // Every position the pen or mouse passed this frame, not just the last: 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. for (at, force) in frame_moves(ui, app.services.stylus.as_ref()) { 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 +9129,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 +9138,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 +9151,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 +9549,32 @@ 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(); + assert!(pts.len() >= 20, "the stroke keeps the frame's moves: {} points", pts.len()); + } + /// #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] From 21a9e854e11dc9823428ecff59fd23f5862777db Mon Sep 17 00:00:00 2001 From: ncic <1848265+ncic@users.noreply.github.com> Date: Mon, 26 Oct 2026 07:27:29 +0100 Subject: [PATCH 2/2] Tools: only the freehand tools get every move of a frame (#4227 review) Every pointer move of a frame went to whatever tool was active, so with a fast mouse (or a pen) a tool whose drag preview is costly (the Selection tool moving a large selection, Free Distort, the Width tool) redid it for each, up to 64 times a frame. Tool::wants_samples (default false) says a tool builds a stroke from the drag's samples: the Pencil, Paintbrush, Blob Brush, Eraser, Knife, Smooth and Path Eraser answer true (the Paintbrush through its wrapper), the Join tool doesn't. The canvas still reads the frame's moves every frame, so the macOS feed's pressures stay paired, but replays them only to such a tool; every other tool gets one drag a frame at the latest position. A frame whose moves end where the last frame's did still reaches the tool. Tests: only_the_freehand_tools_get_every_move_of_a_frame (Pencil, Paintbrush and Blob Brush get the 19 moves before the last, Selection, Direct Selection and Width none), a_mouse_stroke_keeps_a_frame_that_ends_where_it_started, the_freehand_tools_want_every_sample, and a_mouse_stroke_keeps_every_move_of_a_frame now checks every position in order, not only the count. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/tooling.rs | 6 +++ crates/tools/src/draw2/gesture.rs | 15 ++++++ crates/tools/src/lib.rs | 8 ++++ crates/tools/src/symbolism.rs | 3 ++ crates/ui-egui/src/canvas.rs | 80 +++++++++++++++++++++++++++---- 5 files changed, 104 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/tooling.rs b/crates/engine/src/tooling.rs index deb42c2c2..a2e074377 100644 --- a/crates/engine/src/tooling.rs +++ b/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() diff --git a/crates/tools/src/draw2/gesture.rs b/crates/tools/src/draw2/gesture.rs index e506799e7..9a7043112 100644 --- a/crates/tools/src/draw2/gesture.rs +++ b/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(); diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index 0774fdf66..cafb6def1 100644 --- a/crates/tools/src/lib.rs +++ b/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, diff --git a/crates/tools/src/symbolism.rs b/crates/tools/src/symbolism.rs index b555ac9ea..bb04f7ab3 100644 --- a/crates/tools/src/symbolism.rs +++ b/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() } diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 904886bf0..e1601c0d8 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -233,6 +233,15 @@ fn frame_moves(ui: &Ui, stylus: Option<&StylusFeed>) -> Vec<(Pos2, Option)> }) } +/// 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)> { + 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 } } @@ -1187,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) { - // Every position the pen or mouse passed this frame, not just the last: 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. - 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); } @@ -9572,7 +9584,59 @@ mod tests { let (cmd, p) = app.session.journal.last().unwrap().clone(); assert_eq!(cmd, "path.freehand"); let pts = p["points"].as_array().unwrap(); - assert!(pts.len() >= 20, "the stroke keeps the frame's moves: {} points", pts.len()); + 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