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 a951ef290..e1601c0d8 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. @@ -231,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 } } @@ -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]