From df61ee2f848882477c479038681e58f5059aa1ec Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 2 Nov 2026 07:44:39 -0800 Subject: [PATCH 1/2] Canvas: moves after the button-up do not join a stroke A Pencil or brush stroke takes every pointer move of a frame. In the frame that releases the button, the moves that came after the button-up were sent as drags before the release, so the stroke ran out to them and back. The moves now stop at the primary button-up, and the pen pose for the end of the stroke is the one kept for the last move before it. --- crates/ui-egui/src/canvas/drag.rs | 23 +++++++--- crates/ui-egui/src/canvas/input.rs | 7 +-- crates/ui-egui/src/canvas/tests.rs | 71 +++++++++++++++++++++++++++--- 3 files changed, 87 insertions(+), 14 deletions(-) diff --git a/crates/ui-egui/src/canvas/drag.rs b/crates/ui-egui/src/canvas/drag.rs index 1c14379e2..871af252f 100644 --- a/crates/ui-egui/src/canvas/drag.rs +++ b/crates/ui-egui/src/canvas/drag.rs @@ -149,10 +149,17 @@ 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 last move +/// before it is the pointer's position. +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, if it came. + let before_up = release.and_then(|b| { + let up = i.events.iter().position(|e| matches!(e, egui::Event::PointerButton { button, pressed: false, .. } if *button == b))?; + Some(i.events.iter().take(up).filter(|e| matches!(e, egui::Event::PointerMoved(_))).count()) + }); let mut moves = vec![]; if i.events.iter().any(|e| matches!(e, egui::Event::Touch { .. })) { let mut force = None; @@ -173,6 +180,10 @@ pub(super) fn frame_moves(ui: &Ui, stylus: Option<&StylusFeed>) -> (Vec) -> (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..79954a6b0 100644 --- a/crates/ui-egui/src/canvas/tests.rs +++ b/crates/ui-egui/src/canvas/tests.rs @@ -6578,6 +6578,64 @@ 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"); +} + #[test] fn frame_moves_keeps_touch_feed_and_mouse_samples_with_their_pressure() { let ctx = egui::Context::default(); @@ -6586,7 +6644,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 +6655,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 +6664,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 +7259,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}"); } From 49477f8be5e6fd002a8c71422784c5a96a6caea9 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 2 Nov 2026 12:04:05 -0800 Subject: [PATCH 2/2] Canvas: keep the last move before the button-up when it is elsewhere When the release frame held moves after the button-up, the last move before it was dropped as the pointer's position, although the release event carries its own position. That move's sample was lost from the stroke. It is now kept unless it is at the button-up's position, and its tilt and rotation still give the release's pose. --- crates/ui-egui/src/canvas/drag.rs | 32 +++++++++++++++++++++--------- crates/ui-egui/src/canvas/tests.rs | 21 ++++++++++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/crates/ui-egui/src/canvas/drag.rs b/crates/ui-egui/src/canvas/drag.rs index 871af252f..88293880a 100644 --- a/crates/ui-egui/src/canvas/drag.rs +++ b/crates/ui-egui/src/canvas/drag.rs @@ -150,15 +150,20 @@ pub(super) type FrameMove = (Pos2, Option, Option); /// 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). 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 last move -/// before it is the pointer's position. +/// 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, if it came. + // 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| { - let up = i.events.iter().position(|e| matches!(e, egui::Event::PointerButton { button, pressed: false, .. } if *button == b))?; - Some(i.events.iter().take(up).filter(|e| matches!(e, egui::Event::PointerMoved(_))).count()) + 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 { .. })) { @@ -181,10 +186,19 @@ pub(super) fn frame_moves(ui: &Ui, stylus: Option<&StylusFeed>, release: Option< moves = i.events.iter().filter_map(|e| if let egui::Event::PointerMoved(p) = e { Some((*p, None, None)) } else { None }).collect(); } // Cut after pairing: the feed's samples pair with every move of the frame. - if let Some(n) = before_up { - moves.truncate(n); - } - let last_pose = moves.pop().and_then(|(_, _, q)| q); + // The pointer's position is the button-up's: a last move before it that is elsewhere + // stays a move. Its tilt and rotation are still the pose. + let last_pose = match before_up { + Some((n, up_at)) => { + 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 { diff --git a/crates/ui-egui/src/canvas/tests.rs b/crates/ui-egui/src/canvas/tests.rs index 79954a6b0..3e184bef4 100644 --- a/crates/ui-egui/src/canvas/tests.rs +++ b/crates/ui-egui/src/canvas/tests.rs @@ -6636,6 +6636,27 @@ fn frame_moves_stop_at_the_button_up() { 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();