Apply review fixes

- crates/engine/src/tools/pointer.rs

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);

crates/fonts/src/shape.rs

@@ -37,56 +37,146 @@ pub struct ShapeParams<'a> {
     pub language: Option<&'a str>,
 }
 
+/// Most bytes of text given to the shaper at once. The shaper lets one call grow its buffers to
+/// 256 glyphs per character (a font's substitutions decide how many), so a long item is shaped in
+/// pieces of this size, each with the rest of the paragraph as context.
+const MAX_SHAPE_CHUNK: usize = 1024;
+/// Most glyphs kept per character of a piece: a piece the font expands further is laid out one
+/// glyph per character from the cmap instead.
+const MAX_GLYPHS_PER_CHAR: usize = 32;
+
 /// Shape `para[range]` with `face`, the rest of `para` as context. Glyphs are in visual order
 /// (left to right) and clusters are byte offsets into `para`. `map` substitutes characters (All
 /// Caps) without moving clusters. An empty or out-of-bounds range shapes nothing.
 pub fn shape_range(face: &FontFace, para: &str, range: Range<usize>, params: &ShapeParams, map: impl Fn(char) -> char) -> Vec<ShapedGlyph> {
-    let (Some(pre), Some(text), Some(post)) = (para.get(..range.start), para.get(range.clone()), para.get(range.end..)) else {
+    let (Some(_), Some(text), Some(_)) = (para.get(..range.start), para.get(range.clone()), para.get(range.end..)) else {
         return Vec::new();
     };
-    let mut out = Vec::with_capacity(text.len());
+    let chunks = chunks(text, MAX_SHAPE_CHUNK);
+    let mut pieces: Vec<Vec<ShapedGlyph>> = Vec::with_capacity(chunks.len());
     let shaped = face.hb().map(|hb| {
         let shaper = face.shaper.shaper(&hb).instance(face.instance.as_ref()).build();
-        let mut buf = UnicodeBuffer::new();
-        buf.set_pre_context(pre);
-        buf.set_post_context(post);
-        for (i, c) in text.char_indices() {
-            buf.add(map(c), (range.start + i) as u32);
-        }
-        buf.guess_segment_properties();
-        if let Some(s) = params.script.and_then(|t| Script::from_iso15924_tag(Tag::new(&t))) {
-            buf.set_script(s);
-        }
-        if let Some(l) = params.language.or_else(|| params.script.and_then(crate::script::default_language)).and_then(|l| l.parse::<Language>().ok())
-        {
-            buf.set_language(l);
-        }
-        buf.set_direction(if params.rtl { Direction::RightToLeft } else { Direction::LeftToRight });
-        let gb = shaper.shape(buf, ShapeOptions::new().features(params.features));
-        for (info, pos) in gb.glyph_infos().iter().zip(gb.glyph_positions()) {
-            out.push(ShapedGlyph {
-                gid: info.glyph_id,
-                cluster: info.cluster as usize,
-                x_advance: pos.x_advance,
-                x_offset: pos.x_offset,
-                y_offset: pos.y_offset,
-            });
+        // Pieces of one item share the script the whole item would be guessed to have.
+        let item_script = (chunks.len() > 1 && params.script.is_none()).then(|| {
+            let mut probe = UnicodeBuffer::new();
+            for c in text.chars() {
+                probe.add(map(c), 0);
+            }
+            probe.guess_segment_properties();
+            probe.script()
+        });
+        for c in &chunks {
+            let r = (range.start + c.start)..(range.start + c.end);
+            let (Some(pre), Some(piece), Some(post)) = (para.get(..r.start), para.get(r.clone()), para.get(r.end..)) else { continue };
+            let mut buf = UnicodeBuffer::new();
+            buf.set_pre_context(pre);
+            buf.set_post_context(post);
+            for (i, c) in piece.char_indices() {
+                buf.add(map(c), (r.start + i) as u32);
+            }
+            buf.guess_segment_properties();
+            if let Some(s) = params.script.and_then(|t| Script::from_iso15924_tag(Tag::new(&t))) {
+                buf.set_script(s);
+            } else if let Some(s) = item_script {
+                buf.set_script(s);
+            }
+            if let Some(l) =
+                params.language.or_else(|| params.script.and_then(crate::script::default_language)).and_then(|l| l.parse::<Language>().ok())
+            {
+                buf.set_language(l);
+            }
+            buf.set_direction(if params.rtl { Direction::RightToLeft } else { Direction::LeftToRight });
+            let gb = shaper.shape(buf, ShapeOptions::new().features(params.features));
+            let budget = piece.chars().count().saturating_mul(MAX_GLYPHS_PER_CHAR);
+            if gb.len() > budget {
+                // Font-controlled expansion past anything real text needs: keep it bounded.
+                pieces.push(cmap_glyphs(face, para, r, params.rtl, &map));
+                continue;
+            }
+            pieces.push(
+                gb.glyph_infos()
+                    .iter()
+                    .zip(gb.glyph_positions())
+                    .map(|(info, pos)| ShapedGlyph {
+                        gid: info.glyph_id,
+                        cluster: info.cluster as usize,
+                        x_advance: pos.x_advance,
+                        x_offset: pos.x_offset,
+                        y_offset: pos.y_offset,
+                    })
+                    .collect(),
+            );
         }
     });
-    if shaped.is_none()
-        && let Some(f) = face.skrifa()
-    {
+    if shaped.is_none() {
         // No shaper for this face: one glyph per character from the cmap, in visual order.
-        let cmap = f.charmap();
-        let gm = f.glyph_metrics(Size::unscaled(), face.location());
-        for (i, c) in text.char_indices() {
-            let g = cmap.map(map(c)).unwrap_or_default();
-            let adv = gm.advance_width(g).unwrap_or(face.upem as f32 * 0.5);
-            out.push(ShapedGlyph { gid: g.to_u32(), cluster: range.start + i, x_advance: adv.round() as i32, x_offset: 0, y_offset: 0 });
-        }
-        if params.rtl {
-            out.reverse();
+        return cmap_glyphs(face, para, range, params.rtl, &map);
+    }
+    // Each piece is in visual order; right to left, the later pieces come first.
+    if params.rtl {
+        pieces.reverse();
+    }
+    let mut out = Vec::with_capacity(pieces.iter().map(Vec::len).sum());
+    for p in pieces {
+        out.extend(p);
+    }
+    out
+}
+
+/// One glyph per character of `para[range]` from the cmap, in visual order.
+fn cmap_glyphs(face: &FontFace, para: &str, range: Range<usize>, rtl: bool, map: &impl Fn(char) -> char) -> Vec<ShapedGlyph> {
+    let (Some(text), Some(f)) = (para.get(range.clone()), face.skrifa()) else { return Vec::new() };
+    let cmap = f.charmap();
+    let gm = f.glyph_metrics(Size::unscaled(), face.location());
+    let mut out = Vec::with_capacity(text.len());
+    for (i, c) in text.char_indices() {
+        let g = cmap.map(map(c)).unwrap_or_default();
+        let adv = gm.advance_width(g).unwrap_or(face.upem as f32 * 0.5);
+        out.push(ShapedGlyph { gid: g.to_u32(), cluster: range.start + i, x_advance: adv.round() as i32, x_offset: 0, y_offset: 0 });
+    }
+    if rtl {
+        out.reverse();
+    }
+    out
+}
+
+/// `text` cut into byte ranges of at most `max` bytes (or one character, if longer), at the last
+/// space of a piece where there is one, else before a character that starts a cluster (not a
+/// combining mark or joiner, not after a joiner). Text that fits is one piece.
+fn chunks(text: &str, max: usize) -> Vec<Range<usize>> {
+    use unicode_bidi::BidiClass::{BN, NSM};
+    let mut out = Vec::new();
+    let mut start = 0;
+    while text.len() - start > max {
+        let window = text.get(start..).unwrap_or_default();
+        let mut space = None;
+        let mut boundary = None;
+        let mut any = None;
+        let mut prev: Option<char> = None;
+        for (i, c) in window.char_indices() {
+            if i > max {
+                break;
+            }
+            if i > 0 {
+                any = Some(i);
+                let joined = prev.is_some_and(|p| matches!(unicode_bidi::bidi_class(p), BN));
+                if !joined && !matches!(unicode_bidi::bidi_class(c), NSM | BN) {
+                    boundary = Some(i);
+                    if prev.is_some_and(char::is_whitespace) {
+                        space = Some(i);
+                    }
+                }
+            }
+            prev = Some(c);
         }
+        // A piece always moves forward by at least one character.
+        let first = window.chars().next().map_or(window.len(), char::len_utf8);
+        let cut = space.filter(|&i| i >= max / 2).or(boundary).or(any).unwrap_or(first).max(first);
+        out.push(start..start + cut);
+        start += cut;
+    }
+    if start < text.len() || out.is_empty() {
+        out.push(start..text.len());
     }
     out
 }
@@ -121,4 +211,37 @@ mod tests {
         // Not a char boundary.
         assert!(shape_range(&face, "سنة", 1..2, &ShapeParams::default(), |c| c).is_empty());
     }
+
+    #[test]
+    fn long_items_are_shaped_in_bounded_pieces() {
+        for text in ["word ".repeat(2_000), "e\u{301}".repeat(2_000), "x".repeat(5_000), "\u{1f469}\u{200d}\u{1f4bb} ".repeat(500)] {
+            let pieces = chunks(&text, MAX_SHAPE_CHUNK);
+            assert_eq!(pieces.first().map(|r| r.start), Some(0));
+            assert_eq!(pieces.last().map(|r| r.end), Some(text.len()));
+            for w in pieces.windows(2) {
+                assert_eq!(w[0].end, w[1].start);
+            }
+            for r in &pieces {
+                assert!(!r.is_empty() && r.len() <= MAX_SHAPE_CHUNK);
+                // Never before a combining mark, never after a joiner.
+                let rest = &text[r.end..];
+                assert!(!rest.starts_with('\u{301}'));
+                assert!(!text[..r.end].ends_with('\u{200d}') || rest.is_empty());
+            }
+        }
+        assert_eq!(chunks("short", MAX_SHAPE_CHUNK), vec![0..5]);
+        assert_eq!(chunks("", MAX_SHAPE_CHUNK), vec![0..0]);
+    }
+
+    #[test]
+    fn long_items_keep_every_cluster_in_visual_order() {
+        let face = FontDb::global().face(DEFAULT_FAMILY, "Regular");
+        let text = "ab cd ".repeat(1_000);
+        let g = shape_range(&face, &text, 0..text.len(), &ShapeParams::default(), |c| c);
+        assert_eq!(g.len(), text.len());
+        assert!(g.windows(2).all(|w| w[0].cluster < w[1].cluster));
+        let rtl = shape_range(&face, &text, 0..text.len(), &ShapeParams { rtl: true, ..Default::default() }, |c| c);
+        assert_eq!(rtl.len(), text.len());
+        assert!(rtl.windows(2).all(|w| w[0].cluster > w[1].cluster));
+    }
 }