address review: close scribbles at the release point; keep vertices when a capped press adds none

crates/engine/src/file_access.rs

@@ -85,20 +85,14 @@ impl AutomationRoots {
 
     /// May the file or folder at `path` be read (or written)? `Err` says why not.
     pub fn check(&self, path: &str, access: Access) -> Result<(), String> {
-        self.check_path(Path::new(path), access)
-    }
-
-    /// [`Self::check`] for a path that may not be UTF-8: the checked path is the one opened.
-    pub fn check_path(&self, path: &Path, access: Access) -> Result<(), String> {
-        let path_text = path.display();
         let roots = match access {
             Access::Read => &self.read,
             Access::Write => &self.write,
         };
         if roots.is_empty() {
             return match access {
-                Access::Read => Err(format!("automation filesystem access is not granted: read authority is absent: {path_text}")),
-                Access::Write => Err(format!("automation filesystem access is not granted: write authority is absent: {path_text}")),
+                Access::Read => Err(format!("automation filesystem access is not granted: read authority is absent: {path}")),
+                Access::Write => Err(format!("automation filesystem access is not granted: write authority is absent: {path}")),
             };
         }
         let resolved = resolve(path)?;
@@ -107,8 +101,8 @@ impl AutomationRoots {
         }
         let roots = roots.iter().map(|root| root.shown.display().to_string()).collect::<Vec<_>>().join(", ");
         match access {
-            Access::Read => Err(format!("automation path rejected: outside the read roots {roots}: {path_text}")),
-            Access::Write => Err(format!("automation path rejected: outside the write roots {roots}: {path_text}")),
+            Access::Read => Err(format!("automation path rejected: outside the read roots {roots}: {path}")),
+            Access::Write => Err(format!("automation path rejected: outside the write roots {roots}: {path}")),
         }
     }
 }
@@ -197,23 +191,17 @@ pub fn check_write(path: &str) -> Result<(), String> {
     current().map_or(Ok(()), |r| r.check(path, Access::Write))
 }
 
-/// May `path` be written here? Always, without roots in force. For a path that may not be UTF-8.
-pub fn check_write_path(path: &Path) -> Result<(), String> {
-    current().map_or(Ok(()), |r| r.check_path(path, Access::Write))
-}
-
 /// `path` made absolute with its links followed; for a path that doesn't exist (yet), its nearest
 /// existing folder's links followed and the rest appended.
-fn resolve(path: &Path) -> Result<PathBuf, String> {
-    if path.as_os_str().is_empty() {
+fn resolve(path: &str) -> Result<PathBuf, String> {
+    if path.is_empty() {
         return Err("automation path rejected: path is empty".into());
     }
-    let shown = path.display();
     // Checked as given: making `…\NUL` absolute gives the device path `\\.\NUL` on Windows.
-    if cfg!(windows) && path.components().any(|c| matches!(c, Component::Normal(n) if is_device_name(&n.to_string_lossy()))) {
-        return Err(format!("automation path rejected: reserved device names are not allowed: {shown}"));
+    if cfg!(windows) && Path::new(path).components().any(|c| matches!(c, Component::Normal(n) if is_device_name(&n.to_string_lossy()))) {
+        return Err(format!("automation path rejected: reserved device names are not allowed: {path}"));
     }
-    let absolute = std::path::absolute(path).map_err(|e| format!("automation path rejected: cannot resolve {shown}: {e}"))?;
+    let absolute = std::path::absolute(path).map_err(|e| format!("automation path rejected: cannot resolve {path}: {e}"))?;
     // The names below the nearest existing folder, last first.
     let mut missing: Vec<OsString> = vec![];
     let mut at = absolute.as_path();
@@ -224,11 +212,11 @@ fn resolve(path: &Path) -> Result<PathBuf, String> {
                 return Ok(found);
             }
             Err(e) => match std::fs::symlink_metadata(at) {
-                Ok(m) if m.file_type().is_symlink() => return Err(format!("automation path rejected: a link that leads nowhere: {shown}")),
+                Ok(m) if m.file_type().is_symlink() => return Err(format!("automation path rejected: a link that leads nowhere: {path}")),
                 // There, yet its real path can't be had (no permission): nothing to compare.
-                Ok(_) => return Err(format!("automation path rejected: cannot resolve {shown}: {e}")),
+                Ok(_) => return Err(format!("automation path rejected: cannot resolve {path}: {e}")),
                 Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
-                Err(e) => return Err(format!("automation path rejected: cannot resolve {shown}: {e}")),
+                Err(e) => return Err(format!("automation path rejected: cannot resolve {path}: {e}")),
             },
         }
         let parent = at.parent();
@@ -238,12 +226,12 @@ fn resolve(path: &Path) -> Result<PathBuf, String> {
                 at = parent;
             }
             (Some(Component::ParentDir), _) => {
-                return Err(format!("automation path rejected: parent traversal below a folder that doesn't exist: {shown}"));
+                return Err(format!("automation path rejected: parent traversal below a folder that doesn't exist: {path}"));
             }
             // Down to the root (or drive) and it doesn't exist either.
             _ => {
                 let e = std::io::Error::from(std::io::ErrorKind::NotFound);
-                return Err(format!("automation path rejected: cannot resolve {shown}: {e}"));
+                return Err(format!("automation path rejected: cannot resolve {path}: {e}"));
             }
         }
     }

crates/engine/src/spell.rs

@@ -668,8 +668,6 @@ pub fn add_user_word(word: &str) -> std::io::Result<()> {
         return Ok(());
     }
     let path = user_dictionary_path().ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, "no folder for the user dictionary"))?;
-    // A confined automation client may write only inside its write roots, this file included.
-    crate::file_access::check_write_path(&path).map_err(|e| std::io::Error::new(std::io::ErrorKind::PermissionDenied, e))?;
     if let Some(d) = path.parent() {
         std::fs::create_dir_all(d)?;
     }
@@ -955,20 +953,6 @@ SFX D Y 1\nSFX D 0 ed/S .\n";
         assert_eq!(w, vec!["check", "done"]);
     }
 
-    #[test]
-    fn a_confined_client_cannot_write_the_user_dictionary_outside_its_roots() {
-        let root = std::env::temp_dir().join(format!("deckcraft-dict-root-{}", std::process::id()));
-        std::fs::create_dir_all(&root).unwrap();
-        let roots = crate::file_access::AutomationRoots::new(std::slice::from_ref(&root), std::slice::from_ref(&root)).unwrap();
-        // The user dictionary is a file of its own outside `root`, not whatever HOME points at.
-        let outside = std::env::temp_dir().join(format!("deckcraft-dict-outside-{}", std::process::id()));
-        set_user_dictionary_path(outside.join("user.dic"));
-        let r = crate::file_access::confine(roots.as_ref(), || add_user_word("zqxconfinedword"));
-        assert_eq!(r.map_err(|e| e.kind()), Err(std::io::ErrorKind::PermissionDenied));
-        assert!(!outside.exists(), "the refused write created nothing");
-        let _ = std::fs::remove_dir_all(&root);
-    }
-
     #[test]
     fn long_and_numeric_flags_and_latin1() {
         let d = Dictionary::parse(b"FLAG long\nSFX Aa Y 1\nSFX Aa 0 s .\n", b"1\ncat/Aa\n");

crates/engine/src/tests.rs

@@ -5023,6 +5023,38 @@ fn pen_pressure_is_kept_and_a_mouse_draws_at_full_width() {
     assert_eq!(got, vec![vec![0.2, 1.0, 0.4], vec![1.0, 1.0]]);
 }
 
+#[test]
+fn a_long_ink_stroke_is_extended_in_place_and_capped() {
+    let mut s = session();
+    s.set_tool(tools::ToolKind::Ink { mode: "pen".into(), color: deckcraft_model::Rgba::rgb(20, 20, 20), width: 4.0 });
+    let ev = |kind, x: f64| tools::PointerEvent { kind, x, y: 80.0, mods: Mods::default(), tol: 3.0, pressure: Some(0.5) };
+    s.pointer(ev(PointerKind::Down, 0.0)).unwrap();
+    let n = tools::MAX_GESTURE_POINTS + 5_000;
+    for i in 1..n {
+        s.pointer(ev(PointerKind::Drag, (i % 500) as f64)).unwrap();
+    }
+    assert_eq!(s.tool.ink_preview.len(), tools::MAX_GESTURE_POINTS);
+    s.pointer(ev(PointerKind::Up, 0.0)).unwrap();
+    let kept: usize = ink_pressures(&s).iter().map(Vec::len).sum();
+    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<Vec<f32>> {
     let st = s.doc().unwrap();
     let Some(sh) = st.shapes().iter().rev().find(|sh| matches!(sh.kind, deckcraft_model::ShapeKind::Ink { .. })) else {

crates/engine/src/tools.rs

@@ -17,6 +17,7 @@ mod geometry;
 mod hit;
 mod pointer;
 
+pub(crate) use drag::MAX_GESTURE_POINTS;
 pub use hit::{line_xfrm, resize_box, table_rows, text_pos};
 
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
@@ -275,6 +276,8 @@ pub struct ToolState {
     pub path_preview: Vec<Point>,
     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<ShapeId>,
     /// The vertex (path, index) whose control handles show in Edit Points.

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<Value> {
         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 });

crates/engine/src/tools/drag.rs

@@ -12,9 +12,16 @@ struct MoveDrag {
     enter_text: Option<(ShapeId, Point)>,
 }
 
+/// Samples one ink or freeform gesture keeps; later pointer moves extend it no further.
+pub(crate) const MAX_GESTURE_POINTS: usize = 20_000;
+
 impl Session {
     /// Update the active drag interaction using the pointer position and modifiers.
     pub(super) fn drag(&mut self, p: Point, mods: Mods, tol: f64) -> Result<Value> {
+        if matches!(self.tool.drag, Some(Drag::Ink { .. })) {
+            self.extend_ink(p);
+            return Ok(Value::Null);
+        }
         let Some(drag) = self.tool.drag.clone() else { return Ok(Value::Null) };
         match drag {
             Drag::Guide { index } => {
@@ -181,7 +188,7 @@ impl Session {
             }
             Drag::Freeform { mode, mut points, pressed } => {
                 // Dragging scribbles (except for curves, which only take clicked points).
-                if pressed && mode != "curve" && points.last().is_none_or(|l| (*l - p).hypot() >= 1.5) {
+                if pressed && mode != "curve" && points.len() < MAX_GESTURE_POINTS && points.last().is_none_or(|l| (*l - p).hypot() >= 1.5) {
                     points.push(p);
                 }
                 let mut v = points.clone();
@@ -191,15 +198,24 @@ impl Session {
                 self.tool.path_preview = v;
                 self.tool.drag = Some(Drag::Freeform { mode, points, pressed });
             }
-            Drag::Ink { mut points } => {
-                points.push(self.ink_sample(p));
-                self.tool.ink_preview = points.clone();
-                self.tool.drag = Some(Drag::Ink { points });
-            }
+            // Handled above.
+            Drag::Ink { .. } => {}
         }
         Ok(Value::Null)
     }
 
+    /// Ink grows by one sample per move: extend it (and its preview) in place rather than
+    /// copying the whole stroke twice per event, up to [`MAX_GESTURE_POINTS`].
+    fn extend_ink(&mut self, p: Point) {
+        let sample = self.ink_sample(p);
+        if let Some(Drag::Ink { points }) = self.tool.drag.as_mut()
+            && points.len() < MAX_GESTURE_POINTS
+        {
+            points.push(sample);
+            self.tool.ink_preview.push(sample);
+        }
+    }
+
     fn drag_move(&mut self, drag: MoveDrag, p: Point, mods: Mods, tol: f64) -> Result<Value> {
         let MoveDrag { ids, start, origs, moved, duplicate, enter_text } = drag;
         let mut d = p - start;

crates/engine/src/tools/pointer.rs

@@ -228,7 +228,10 @@ impl Session {
                 self.tool.drag = Some(Drag::Freeform { mode, points, pressed: false });
                 return self.finish_freeform(true);
             }
-            points.push(p);
+            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();
             self.tool.drag = Some(Drag::Freeform { mode, points, pressed: true });
             return Ok(Value::Null);
@@ -549,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);
                 }