From 84e2d975877681e72d0fdafea57d45983251d482 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 22:37:23 +0000 Subject: [PATCH] address review: close scribbles at the release point; keep vertices when a capped press adds none --- crates/engine/src/tests.rs | 16 ++++++++++++++++ crates/engine/src/tools.rs | 2 ++ crates/engine/src/tools/click.rs | 8 ++++++-- crates/engine/src/tools/pointer.rs | 5 +++-- 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index b7d029f30..f5b5e3a5f 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5039,6 +5039,22 @@ fn a_long_ink_stroke_is_extended_in_place_and_capped() { assert!((2..=tools::MAX_GESTURE_POINTS).contains(&kept), "{kept}"); } +#[test] +fn double_clicking_a_capped_freeform_keeps_its_vertices() { + let mut s = session(); + let ev = |kind, x: f64, y: f64| tools::PointerEvent { kind, x, y, mods: Default::default(), tol: 3.0, pressure: None }; + s.set_tool(tools::ToolKind::Shape { preset: "freeform".into() }); + s.pointer(ev(tools::PointerKind::Down, 100.0, 100.0)).unwrap(); + // Densely clicked vertices, each within the double-click tolerance of the last. + for i in 0..tools::MAX_GESTURE_POINTS + 10 { + s.pointer(ev(tools::PointerKind::Down, 300.0 + i as f64 * 0.001, 100.0)).unwrap(); + } + s.pointer(ev(tools::PointerKind::DoubleClick, 300.0, 100.0)).unwrap(); + let st = s.doc().unwrap(); + let deckcraft_model::Geom::Custom { paths } = &st.shapes().last().unwrap().geom else { panic!("no freeform") }; + assert_eq!(paths[0].d.matches(" L ").count(), tools::MAX_GESTURE_POINTS - 1); +} + fn ink_pressures(s: &Session) -> Vec> { let st = s.doc().unwrap(); let Some(sh) = st.shapes().iter().rev().find(|sh| matches!(sh.kind, deckcraft_model::ShapeKind::Ink { .. })) else { diff --git a/crates/engine/src/tools.rs b/crates/engine/src/tools.rs index 50a0fbb02..2b7f78da9 100644 --- a/crates/engine/src/tools.rs +++ b/crates/engine/src/tools.rs @@ -276,6 +276,8 @@ pub struct ToolState { pub path_preview: Vec, pub path_smooth: bool, pub path_closing: bool, + /// Whether the last freeform press added a vertex (it adds none at the point cap). + pub press_added_vertex: bool, /// The shape in Edit Points: its vertices are handles instead of the box's. pub edit_points: Option, /// The vertex (path, index) whose control handles show in Edit Points. diff --git a/crates/engine/src/tools/click.rs b/crates/engine/src/tools/click.rs index 802eb8c5e..10bc03d98 100644 --- a/crates/engine/src/tools/click.rs +++ b/crates/engine/src/tools/click.rs @@ -4,8 +4,12 @@ use super::{Drag, Hit, Point, Result, Session, ShapeId, ShapeKind, TextSel, Tool impl Session { pub(super) fn double_click(&mut self, p: Point, tol: f64) -> Result { if let Some(Drag::Freeform { mode, mut points, .. }) = self.tool.drag.clone() { - // The double-click's own click added a duplicate vertex. - while points.len() > 2 && points.last().zip(points.get(points.len() - 2)).is_some_and(|(a, b)| (*a - *b).hypot() <= tol) { + // The double-click's own click added a duplicate vertex (none at the point cap, where + // the existing vertices are all kept). + while self.tool.press_added_vertex + && points.len() > 2 + && points.last().zip(points.get(points.len() - 2)).is_some_and(|(a, b)| (*a - *b).hypot() <= tol) + { points.pop(); } self.tool.drag = Some(Drag::Freeform { mode, points, pressed: false }); diff --git a/crates/engine/src/tools/pointer.rs b/crates/engine/src/tools/pointer.rs index 8433a998e..6d88595d7 100644 --- a/crates/engine/src/tools/pointer.rs +++ b/crates/engine/src/tools/pointer.rs @@ -228,7 +228,8 @@ impl Session { self.tool.drag = Some(Drag::Freeform { mode, points, pressed: false }); return self.finish_freeform(true); } - if points.len() < super::MAX_GESTURE_POINTS { + self.tool.press_added_vertex = points.len() < super::MAX_GESTURE_POINTS; + if self.tool.press_added_vertex { points.push(p); } self.tool.path_preview = points.clone(); @@ -551,7 +552,7 @@ impl Session { Some(Drag::Freeform { mode, points, .. }) => { if mode == "scribble" { // A scribble ending back at its start closes into a filled shape. - let closed = points.len() > 3 && points.first().zip(points.last()).is_some_and(|(a, b)| (*a - *b).hypot() <= 6.0); + let closed = points.len() > 3 && points.first().is_some_and(|a| (*a - p).hypot() <= 6.0); self.tool.drag = Some(Drag::Freeform { mode, points, pressed: false }); return self.finish_freeform(closed); }