diff --git a/crates/ui-egui/src/canvas/drag.rs b/crates/ui-egui/src/canvas/drag.rs index 1c14379e2..88293880a 100644 --- a/crates/ui-egui/src/canvas/drag.rs +++ b/crates/ui-egui/src/canvas/drag.rs @@ -149,10 +149,22 @@ pub(super) type FrameMove = (Pos2, Option, Option); /// move (empty when the pen read neither), if any. The last move is the pointer's position; the /// tilt and rotation the feed kept for it come second, if any. 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). -pub(super) fn frame_moves(ui: &Ui, stylus: Option<&StylusFeed>) -> (Vec, Option) { +/// too few for a low Fidelity to follow the hand (#3328). With `release`, the moves stop at that +/// button's button-up in this frame, if it came: the moves after it are hover, and the button-up's +/// position is the pointer's. The last move before it is left out only when it is at that +/// position; its tilt and rotation come second either way. +pub(super) fn frame_moves(ui: &Ui, stylus: Option<&StylusFeed>, release: Option) -> (Vec, Option) { let samples = stylus.map(StylusFeed::take_moves).unwrap_or_default(); ui.input(|i| { + // How many of the frame's moves came before the button-up, and where it came, if it came. + let before_up = release.and_then(|b| { + i.events.iter().enumerate().find_map(|(k, e)| match e { + egui::Event::PointerButton { pos, button, pressed: false, .. } if *button == b => { + Some((i.events.iter().take(k).filter(|e| matches!(e, egui::Event::PointerMoved(_))).count(), *pos)) + } + _ => None, + }) + }); let mut moves = vec![]; if i.events.iter().any(|e| matches!(e, egui::Event::Touch { .. })) { let mut force = None; @@ -173,7 +185,20 @@ pub(super) fn frame_moves(ui: &Ui, stylus: Option<&StylusFeed>) -> (Vec { + moves.truncate(n); + let pose = moves.last().and_then(|&(_, _, q)| q); + if moves.last().is_some_and(|&(p, _, _)| p == up_at) { + moves.pop(); + } + pose + } + None => moves.pop().and_then(|(_, _, q)| q), + }; // A burst of events (a stalled frame) is thinned to a bounded count. const MAX: usize = 64; if moves.len() > MAX { @@ -188,9 +213,11 @@ pub(super) fn frame_moves(ui: &Ui, stylus: Option<&StylusFeed>) -> (Vec (Vec, Option) { - let (moves, last_pose) = frame_moves(ui, app.services.stylus.as_ref()); +/// position come second, for every tool. With `released`, only the moves before the primary +/// button-up count. +pub(super) fn tool_frame_moves(ui: &Ui, app: &VectorcraftApp, released: bool) -> (Vec, Option) { + let release = released.then_some(egui::PointerButton::Primary); + let (moves, last_pose) = frame_moves(ui, app.services.stylus.as_ref(), release); (if app.session.tool_wants_samples() { moves } else { vec![] }, last_pose) } diff --git a/crates/ui-egui/src/canvas/input.rs b/crates/ui-egui/src/canvas/input.rs index 87b35a7c9..3775edeb8 100644 --- a/crates/ui-egui/src/canvas/input.rs +++ b/crates/ui-egui/src/canvas/input.rs @@ -382,7 +382,7 @@ fn drag_held(app: &mut VectorcraftApp, ui: &Ui, f: &Pointing, d: Drag, p: Pos2) // 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, last_pose) = tool_frame_moves(ui, app); + let (moves, last_pose) = tool_frame_moves(ui, app, false); // A frame whose pointer ends where the last one did still moved in between. let backquote = ui.input(|i| i.key_down(egui::Key::Backtick)); let mods_changed = tool_drag_mods_changed(ui, mods(m, space), backquote); @@ -467,8 +467,9 @@ fn drag_released(app: &mut VectorcraftApp, ui: &Ui, f: &Pointing, d: Drag, p: Po // tools commit the last preview, which should be where the pointer let go // (#834). Only once the press has dragged, so a click's jitter stays a click // (the Pen's corner anchor, no nudge with Direct Selection). A tool that builds a - // stroke from the drag's samples gets the frame's other moves before it. - let (moves, last_pose) = tool_frame_moves(ui, app); + // stroke from the drag's samples gets the frame's other moves before it, up to the + // button-up: the moves after it are hover. + let (moves, last_pose) = tool_frame_moves(ui, app, true); let pen = move_pose(app, &xf, last_pose); if let Some(last) = last_drag { let backquote = ui.input(|i| i.key_down(egui::Key::Backtick)); diff --git a/crates/ui-egui/src/canvas/tests.rs b/crates/ui-egui/src/canvas/tests.rs index 499ff8a7e..3e184bef4 100644 --- a/crates/ui-egui/src/canvas/tests.rs +++ b/crates/ui-egui/src/canvas/tests.rs @@ -6578,6 +6578,85 @@ fn a_drag_ends_where_the_button_went_up_in_its_frame() { assert!(app.session.active().unwrap().interaction.is_none(), "committed"); } +/// The moves that come after the button-up in the release's frame are hover: a stroke built from +/// the drag's samples does not take them. +#[test] +fn moves_after_the_button_up_do_not_join_the_stroke() { + use egui::{Event, Modifiers}; + let mut app = VectorcraftApp::new(Session::new(), Default::default()); + app.session.execute("file.new", &json!({"width": 400, "height": 300})).unwrap(); + app.ui.view.smart_guides = false; + app.ui.view.snap_to_point = false; + let ctx = egui::Context::default(); + frame(&mut app, &ctx, vec![]); + let xf = Xf::new(app.canvas_rect.unwrap(), app.view().unwrap()); + let button = |pos, pressed| Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Modifiers::NONE }; + let center = app.canvas_rect.unwrap().center(); + app.select_tool("pencil"); + let a = center - vec2(100.0, 0.0); + let end = a + vec2(120.0, 0.0); + frame(&mut app, &ctx, vec![Event::PointerMoved(a)]); + frame(&mut app, &ctx, vec![button(a, true)]); + for x in [20.0, 40.0, 60.0] { + frame(&mut app, &ctx, vec![Event::PointerMoved(a + vec2(x, 0.0))]); + } + let hover = [end + vec2(30.0, 100.0), end + vec2(60.0, 120.0), end + vec2(90.0, 140.0)]; + let mut events = vec![Event::PointerMoved(a + vec2(90.0, 0.0)), Event::PointerMoved(end), button(end, false)]; + events.extend(hover.map(Event::PointerMoved)); + frame(&mut app, &ctx, events); + let st = app.session.active().unwrap(); + let r = st.doc.node(st.selection.objects[0]).unwrap().geometric_bounds().unwrap(); + let (left, right) = (xf.to_doc(a), xf.to_doc(end)); + assert!((r.x0 - left.x).abs() < 1.0 && (r.x1 - right.x).abs() < 1.0 && r.height() < 1.0, "the stroke stops at the release: {r:?}"); +} + +/// [`frame_moves`] with a release keeps the moves before the button-up, and the pose the feed kept +/// for the last of them. +#[test] +fn frame_moves_stop_at_the_button_up() { + let ctx = egui::Context::default(); + let (a, b, c, d) = (pos2(10.0, 20.0), pos2(20.0, 30.0), pos2(30.0, 40.0), pos2(40.0, 50.0)); + let up = egui::Event::PointerButton { pos: b, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }; + let events = vec![egui::Event::PointerMoved(a), egui::Event::PointerMoved(b), up, egui::Event::PointerMoved(c), egui::Event::PointerMoved(d)]; + let feed = StylusFeed::default(); + let at_b = PenPose { tilt: Some([20.0, 0.0]), rotation: None }; + let after = PenPose { tilt: Some([50.0, 0.0]), rotation: None }; + for (p, pose) in [(0.4, PenPose::default()), (0.6, at_b), (0.7, after), (0.8, after)] { + feed.set_move(crate::stylus::PenSample { pressure: p, pose, ..Default::default() }); + } + let (mut moves, mut last) = (vec![], None); + let release = Some(egui::PointerButton::Primary); + let mut out = + ctx.run_ui(egui::RawInput { events: events.clone(), ..Default::default() }, |ui| (moves, last) = frame_moves(ui, Some(&feed), release)); + out.textures_delta.clear(); + assert_eq!(moves, [(a, Some(0.4), Some(PenPose::default()))], "the moves after the button-up are left out"); + assert_eq!(last, Some(at_b), "the pose is the last move's before the button-up"); + let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| moves = frame_moves(ui, None, None).0); + out.textures_delta.clear(); + assert_eq!(moves, [(a, None, None), (b, None, None), (c, None, None)], "without a release every move counts"); +} + +/// A button-up away from the last move before it keeps that move: the release takes only its own +/// position, and the pose is still the last move's. +#[test] +fn frame_moves_keep_the_last_move_when_the_button_went_up_elsewhere() { + let ctx = egui::Context::default(); + let (a, b, up_at, c) = (pos2(10.0, 20.0), pos2(20.0, 30.0), pos2(25.0, 35.0), pos2(30.0, 40.0)); + let up = egui::Event::PointerButton { pos: up_at, button: egui::PointerButton::Primary, pressed: false, modifiers: Default::default() }; + let events = vec![egui::Event::PointerMoved(a), egui::Event::PointerMoved(b), up, egui::Event::PointerMoved(c)]; + let feed = StylusFeed::default(); + let at_b = PenPose { tilt: Some([20.0, 0.0]), rotation: None }; + for (p, pose) in [(0.4, PenPose::default()), (0.6, at_b), (0.7, PenPose::default())] { + feed.set_move(crate::stylus::PenSample { pressure: p, pose, ..Default::default() }); + } + let (mut moves, mut last) = (vec![], None); + let release = Some(egui::PointerButton::Primary); + let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| (moves, last) = frame_moves(ui, Some(&feed), release)); + out.textures_delta.clear(); + assert_eq!(moves, [(a, Some(0.4), Some(PenPose::default())), (b, Some(0.6), Some(at_b))], "the move before the button-up is kept"); + assert_eq!(last, Some(at_b), "the pose is the last move's before the button-up"); +} + #[test] fn frame_moves_keeps_touch_feed_and_mouse_samples_with_their_pressure() { let ctx = egui::Context::default(); @@ -6586,7 +6665,7 @@ fn frame_moves_keeps_touch_feed_and_mouse_samples_with_their_pressure() { let c = pos2(30.0, 40.0); let events = vec![egui::Event::PointerMoved(a), egui::Event::PointerMoved(b), egui::Event::PointerMoved(c)]; let mut mouse = vec![]; - let mut out = ctx.run_ui(egui::RawInput { events: events.clone(), ..Default::default() }, |ui| mouse = frame_moves(ui, None).0); + let mut out = ctx.run_ui(egui::RawInput { events: events.clone(), ..Default::default() }, |ui| mouse = frame_moves(ui, None, None).0); out.textures_delta.clear(); // A mouse keeps its moves too, without a pressure or a pose. assert_eq!(mouse, [(a, None, None), (b, None, None)], "the last move follows the normal pointer path"); @@ -6597,7 +6676,8 @@ fn frame_moves_keeps_touch_feed_and_mouse_samples_with_their_pressure() { feed.set_move(crate::stylus::PenSample { pressure: p, pose, ..Default::default() }); } let (mut samples, mut last) = (vec![], None); - let mut out = ctx.run_ui(egui::RawInput { events: events.clone(), ..Default::default() }, |ui| (samples, last) = frame_moves(ui, Some(&feed))); + let mut out = + ctx.run_ui(egui::RawInput { events: events.clone(), ..Default::default() }, |ui| (samples, last) = frame_moves(ui, Some(&feed), None)); out.textures_delta.clear(); assert_eq!( samples, @@ -6605,17 +6685,17 @@ fn frame_moves_keeps_touch_feed_and_mouse_samples_with_their_pressure() { "paired from the end, each with its own pose; a lift's 0 is no pressure" ); assert_eq!(last, Some(leaning), "the last move's pose comes with the moves"); - let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| (samples, last) = frame_moves(ui, Some(&feed))); + let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| (samples, last) = frame_moves(ui, Some(&feed), None)); out.textures_delta.clear(); assert_eq!(samples, [(a, None, None), (b, None, None)], "the moves were taken: a mouse's moves again"); assert_eq!(last, None); 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).0); + let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| samples = frame_moves(ui, None, None).0); out.textures_delta.clear(); assert_eq!(samples, [(a, Some(0.2), None), (b, Some(0.8), None)], "the last sample follows the normal pointer path"); let events = (0..200).flat_map(|n| pen(egui::TouchPhase::Move, pos2(n as f32, 0.0), 0.5)).collect(); - let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| samples = frame_moves(ui, None).0); + let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| samples = frame_moves(ui, None, None).0); out.textures_delta.clear(); assert!(samples.len() <= 64, "a stalled frame stays bounded"); } @@ -7200,7 +7280,7 @@ fn only_the_freehand_tools_get_every_move_of_a_frame() { { 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).0); + let mut out = ctx.run_ui(egui::RawInput { events: events.clone(), ..Default::default() }, |ui| moves = tool_frame_moves(ui, &app, false).0); out.textures_delta.clear(); assert_eq!(moves.len(), if every { 19 } else { 0 }, "{tool}"); }