Merge pull request #5441 from storytold/fix/sec-linear-scans-r7

Linear-time scans: Add Anchor Points, swatch library adds, export placements, width profiles, symbolism previews

crates/doc/src/appearance/profile.rs

@@ -34,11 +34,11 @@ impl WidthProfile {
         if t <= p[0].0 {
             return (p[0].1, p[0].2);
         }
-        for w in p.windows(2) {
-            if t <= w[1].0 {
-                let u = (t - w[0].0) / (w[1].0 - w[0].0).max(1e-9);
-                return (w[0].1 + (w[1].1 - w[0].1) * u, w[0].2 + (w[1].2 - w[0].2) * u);
-            }
+        // (Positions go forward; found in log time, so a long profile read from a file is cheap to ask.)
+        let i = p.partition_point(|q| q.0 < t);
+        if let (Some(a), Some(b)) = (i.checked_sub(1).and_then(|j| p.get(j)), p.get(i)) {
+            let u = (t - a.0) / (b.0 - a.0).max(1e-9);
+            return (a.1 + (b.1 - a.1) * u, a.2 + (b.2 - a.2) * u);
         }
         p.last().map_or((1.0, 1.0), |l| (l.1, l.2))
     }
@@ -90,10 +90,12 @@ impl WidthProfile {
     /// discontinuous point (two points at `t`, within `1e-7`).
     pub fn around(&self, t: f64) -> ((f64, f64), (f64, f64)) {
         let p = &self.points;
-        if let Some(i) = p.iter().position(|q| (q.0 - t).abs() <= 1e-7) {
-            let j = i + p[i..].iter().take_while(|q| (q.0 - p[i].0).abs() <= 1e-9).count() - 1;
+        // The same distance test below, so a point exactly 1e-7 before `t` is never skipped by rounding.
+        let i = p.partition_point(|q| q.0 < t && (q.0 - t).abs() > 1e-7);
+        if let Some(first) = p.get(i).filter(|q| (q.0 - t).abs() <= 1e-7) {
+            let j = i + p[i..].partition_point(|q| q.0 - first.0 <= 1e-9) - 1;
             if j > i {
-                return ((p[i].1, p[i].2), (p[j].1, p[j].2));
+                return ((first.1, first.2), (p[j].1, p[j].2));
             }
         }
         let v = self.at(t);
@@ -334,3 +336,26 @@ pub struct SavedProfile {
     pub name: String,
     pub profile: WidthProfile,
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn width_profile_lookups_by_search() {
+        let p = WidthProfile { points: vec![(0.0, 0.0, 0.0), (0.5, 1.0, 1.0), (0.5, 0.5, 0.25), (1.0, 0.0, 0.0)] };
+        assert_eq!(p.at(-1.0), (0.0, 0.0));
+        assert_eq!(p.at(0.25), (0.5, 0.5));
+        assert_eq!(p.at(0.75), (0.25, 0.125));
+        assert_eq!(p.at(2.0), (0.0, 0.0));
+        // A step: the widths just before and just after it.
+        assert_eq!(p.around(0.5), ((1.0, 1.0), (0.5, 0.25)));
+        assert_eq!(p.around(0.25), ((0.5, 0.5), (0.5, 0.5)));
+        // Many points at one position are one step, found without a pass over them all.
+        let many = WidthProfile { points: (0..1000).map(|i| (0.5, i as f64, i as f64)).collect() };
+        assert_eq!(many.around(0.5), ((0.0, 0.0), (999.0, 999.0)));
+        // 0.4999999 == 0.5 - 1e-7, yet its distance from 0.5 rounds above 1e-7: it is not part of the step.
+        let near = WidthProfile { points: vec![(0.4999999, 2.0, 2.0), (0.5, 1.0, 1.0), (0.5, 0.5, 0.25)] };
+        assert_eq!(near.around(0.5), ((1.0, 1.0), (0.5, 0.25)));
+    }
+}

crates/effects/src/stroke/width.rs

@@ -114,13 +114,16 @@ pub(super) fn split_at(pts: &[Point], corner: &[bool], closed: bool, marks: &[f6
     let seg_count = if closed { n } else { n.saturating_sub(1) };
     let mut out = (Vec::with_capacity(n + marks.len()), Vec::with_capacity(n + marks.len()));
     let mut acc = 0.0;
+    // Sorted once, so each segment finds its marks by search, not by a pass over all of them.
+    let mut marks = marks.to_vec();
+    marks.sort_by(f64::total_cmp);
     for i in 0..seg_count {
         let (a, b) = (pts[i], pts[(i + 1) % n]);
         let len = a.distance(b);
         out.0.push(a);
         out.1.push(corner[i]);
-        let mut inner: Vec<f64> = marks.iter().copied().filter(|d| *d > acc + 1e-9 && *d < acc + len - 1e-9).collect();
-        inner.sort_by(f64::total_cmp);
+        let from = marks.partition_point(|d| *d <= acc + 1e-9);
+        let mut inner: Vec<f64> = marks.iter().skip(from).copied().take_while(|d| *d < acc + len - 1e-9).collect();
         inner.dedup_by(|a, b| (*a - *b).abs() <= 1e-9);
         for d in inner {
             out.0.push(a.lerp(b, (d - acc) / len));

crates/engine/src/cmd/place/document.rs

@@ -338,8 +338,9 @@ pub(crate) fn full_documents(d: &Document) -> (Cow<'_, Document>, Vec<String>) {
         Cow::Owned(doc) => doc,
     };
     let mut need: Vec<PlacedDocument> = vec![];
+    let mut seen = std::collections::HashSet::new();
     doc.visit_placed(|_, p| {
-        if doc.images.get(&p.key).is_some_and(ImageBlob::is_proxy) && !need.iter().any(|n| n.key == p.key) {
+        if doc.images.get(&p.key).is_some_and(ImageBlob::is_proxy) && seen.insert(p.key.clone()) {
             need.push(p.clone());
         }
     });

crates/engine/src/cmd/swatchlib.rs

@@ -13,6 +13,7 @@ use serde_json::{Value, json};
 use vectorcraft_color::libraries::{BuiltinLibrary, GRADIENT_LIBRARIES, SWATCH_LIBRARIES, builtin_library};
 use vectorcraft_color::palette_io::{self, PaletteFormat};
 use vectorcraft_color::recolor::Palette;
+use vectorcraft_color::swatch::REGISTRATION;
 use vectorcraft_color::{Paint, Swatch, SwatchGroup, SwatchLibrary, default_swatches};
 use vectorcraft_doc::pattern::pattern_paint;
 
@@ -587,8 +588,9 @@ fn copy_to_user(s: &mut Session, p: &Value) -> Result<Value> {
 /// ungrouped): a group's name brings the group, a swatch's name the swatch alone; no names, all.
 fn picked<'a>(lib: &'a SwatchLibrary, names: &[String], cmd: &str) -> Result<Vec<(Option<&'a str>, &'a Swatch)>> {
     let mut out: Vec<(Option<&str>, &Swatch)> = vec![];
+    let mut seen: HashSet<&str> = HashSet::new();
     let mut push = |g: Option<&'a str>, w: &'a Swatch| {
-        if !out.iter().any(|(_, x)| x.name == w.name) {
+        if seen.insert(w.name.as_str()) {
             out.push((g, w));
         }
     };
@@ -606,6 +608,23 @@ fn picked<'a>(lib: &'a SwatchLibrary, names: &[String], cmd: &str) -> Result<Vec
     Ok(out)
 }
 
+/// Every name a swatch or colour group of `d` uses (they share one namespace, with the registration swatch).
+fn taken_names(d: &vectorcraft_doc::Document) -> HashSet<String> {
+    let names = d.swatches_iter().map(|w| &w.name).chain(d.swatch_groups.iter().map(|g| &g.name));
+    names.cloned().chain([REGISTRATION.to_string()]).collect()
+}
+
+/// [`vectorcraft_doc::Document::free_swatch_name`] against `taken`, which takes the name returned.
+fn free_in(taken: &mut HashSet<String>, base: &str) -> String {
+    let name = if taken.contains(base) {
+        (2..).map(|i| format!("{base} {i}")).find(|n| !taken.contains(n)).unwrap_or_else(|| base.to_string())
+    } else {
+        base.to_string()
+    };
+    taken.insert(name.clone());
+    name
+}
+
 fn add(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "swatch.library.add";
     let (info, lib) = library_param(s, p, C)?;
@@ -621,11 +640,21 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
     // same pattern) are reused, not added again.
     let patterns = patternlib::library_doc(&info.id);
     let d = &s.doc()?.doc;
+    // The document's swatches by name (the first of a name wins, as in `Document::swatch`), looked
+    // up once so that a large palette takes linear time.
+    let mut by_name: std::collections::HashMap<&str, &Swatch> = std::collections::HashMap::new();
+    for w in d.swatches_iter() {
+        by_name.entry(w.name.as_str()).or_insert(w);
+    }
     let there = |w: &Swatch| match (&w.paint, &patterns) {
         (Paint::Pattern { pattern, .. }, Some(src)) => {
             patternlib::existing_pattern(d, src, pattern).and_then(|target| patternlib::pattern_swatch(d, &target))
         }
-        _ => d.swatch(&w.name).is_some_and(|x| x.paint == w.paint).then(|| w.name.clone()),
+        _ => match by_name.get(w.name.as_str()) {
+            Some(x) => x.paint == w.paint,
+            None => w.name == REGISTRATION && d.swatch(&w.name).is_some_and(|x| x.paint == w.paint),
+        }
+        .then(|| w.name.clone()),
     };
     // Library name → the name of the document's swatch.
     let mut existing: Vec<(String, String)> = vec![];
@@ -647,6 +676,7 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
             // already had, or one made here.
             let before = d.swatch_groups.len();
             let mut placed: Vec<(&str, usize)> = vec![];
+            let mut taken = taken_names(d);
             for (group, w) in fresh {
                 if let (Paint::Pattern { pattern, .. }, Some(src)) = (&w.paint, &patterns) {
                     // The pattern comes with its swatch; a pattern the document has without one
@@ -661,9 +691,10 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
                         }
                     };
                     out.push((w.name.clone(), name));
+                    taken = taken_names(d);
                     continue;
                 }
-                let name = d.free_swatch_name(&w.name);
+                let name = free_in(&mut taken, &w.name);
                 let sw = Swatch { name: name.clone(), ..(*w).clone() };
                 match group {
                     Some(g) => {
@@ -671,7 +702,7 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
                         let i = match found.or_else(|| d.swatch_groups.iter().take(before).position(|x| x.name == *g)) {
                             Some(i) => i,
                             None => {
-                                let name = d.free_swatch_name(g);
+                                let name = free_in(&mut taken, g);
                                 d.swatch_groups.push(SwatchGroup { name, swatches: vec![] });
                                 d.swatch_groups.len() - 1
                             }

crates/pathops/src/edit.rs

@@ -3,7 +3,7 @@
 
 use std::collections::{BTreeMap, BTreeSet};
 
-use kurbo::{CubicBez, PathSeg, Point, Rect, Vec2};
+use kurbo::{CubicBez, ParamCurve, PathSeg, Point, Rect, Vec2};
 use vectorcraft_geom::path::measured_arclen;
 use vectorcraft_geom::{Anchor, AnchorKind, PathData, SubPath};
 
@@ -628,9 +628,32 @@ fn golden_min(f: impl Fn(f64) -> (CubicBez, f64, usize), mut a: f64, mut b: f64)
 pub fn add_anchor_points(path: &PathData) -> PathData {
     let mut out = path.clone();
     for sp in &mut out.subpaths {
-        for seg in (0..sp.segment_count()).rev() {
-            sp.insert_anchor(seg, 0.5);
+        let n = sp.segment_count();
+        if n == 0 {
+            continue;
         }
+        // Built in one pass (inserting into the middle of the vector shifts the rest each time):
+        // anchor k lands at 2k and the new anchor of segment k at 2k + 1.
+        let mut v = Vec::with_capacity(sp.anchors.len() * 2);
+        for (k, a) in sp.anchors.iter().enumerate() {
+            v.push(*a);
+            if k < n {
+                v.push(*a);
+            }
+        }
+        for seg in 0..n {
+            let c = sp.segment(seg);
+            let (l, r) = (c.subsegment(0.0..0.5), c.subsegment(0.5..1.0));
+            if sp.segment_is_line(seg) {
+                v[2 * seg + 1] = Anchor::corner(l.p3);
+            } else {
+                let at = (2 * seg + 2) % v.len();
+                v[2 * seg].h_out = l.p1;
+                v[at].h_in = r.p2;
+                v[2 * seg + 1] = Anchor { p: l.p3, h_in: l.p2, h_out: r.p1, kind: AnchorKind::Smooth };
+            }
+        }
+        sp.anchors = v;
     }
     out
 }

crates/pathops/tests/it/ops.rs

@@ -515,6 +515,21 @@ fn add_anchor_points_doubles() {
     assert!(r.anchors().any(|(_, _, a)| a.p == Point::new(5.0, 0.0)));
 }
 
+/// The one-pass build gives what inserting a midpoint into each segment (last first) gives, open or closed.
+#[test]
+fn add_anchor_points_matches_inserting() {
+    let open = PathData::single(SubPath::new(circle(0.0, 0.0, 10.0).subpaths[0].anchors.clone(), false));
+    for src in [circle(0.0, 0.0, 10.0), rect(0.0, 0.0, 10.0, 10.0), open] {
+        let mut want = src.clone();
+        for sp in &mut want.subpaths {
+            for seg in (0..sp.segment_count()).rev() {
+                sp.insert_anchor(seg, 0.5);
+            }
+        }
+        assert_eq!(add_anchor_points(&src), want);
+    }
+}
+
 #[test]
 fn average_axes() {
     let p = PathData::single(SubPath::polyline(&[Point::new(0.0, 0.0), Point::new(10.0, 4.0), Point::new(5.0, 8.0)], false));

crates/tools/src/lib.rs

@@ -323,7 +323,7 @@ pub(crate) const MAX_STROKE_SAMPLES: usize = 100_000;
 /// `n / PREVIEW_ALL_UNTIL` samples. Each preview carries every sample so far, so previewing at
 /// every sample cost the square of the stroke's length (a gesture of many drags sent at once
 /// spent seconds); this way it is about `PREVIEW_ALL_UNTIL` per sample.
-const PREVIEW_ALL_UNTIL: usize = 1000;
+pub(crate) const PREVIEW_ALL_UNTIL: usize = 1000;
 
 /// Whether a drag that made the stroke `n` samples long previews it ([`PREVIEW_ALL_UNTIL`]).
 pub(crate) fn previews_at(n: usize) -> bool {

crates/tools/src/symbolism.rs

@@ -11,7 +11,7 @@ use serde_json::{Value, json};
 use vectorcraft_geom::{Point, Shape};
 
 use crate::draw2::{FEEDBACK, GestureTool, points_json};
-use crate::{Action, Cursor, Mods, Overlay, PointerEvent, PointerKind, Tool, ToolContext, ToolKey};
+use crate::{Action, Cursor, MAX_STROKE_SAMPLES, Mods, Overlay, PointerEvent, PointerKind, Tool, ToolContext, ToolKey, previews_at};
 
 /// Create a symbolism tool or the brush-aware Paintbrush (None = not one of ours).
 pub fn create(id: &str) -> Option<Box<dyn Tool>> {
@@ -26,6 +26,8 @@ pub fn create(id: &str) -> Option<Box<dyn Tool>> {
 pub struct SymbolismTool {
     id: &'static str,
     points: Vec<Point>,
+    /// How many points the last preview had ([`previews_at`] thins previews of a long drag).
+    previewed: usize,
     active: bool,
     hover: Option<Point>,
     alt: bool,
@@ -53,7 +55,19 @@ impl SymbolismTool {
             "symbolStyler" => "symbolStyler",
             _ => "symbolSprayer",
         };
-        Self { id, points: vec![], active: false, hover: None, alt: false, diameter: 80.0, intensity: 5.0, density: 5.0, symbol: None, style: None }
+        Self {
+            id,
+            points: vec![],
+            previewed: 0,
+            active: false,
+            hover: None,
+            alt: false,
+            diameter: 80.0,
+            intensity: 5.0,
+            density: 5.0,
+            symbol: None,
+            style: None,
+        }
     }
 
     fn label(&self) -> &'static str {
@@ -103,6 +117,19 @@ impl SymbolismTool {
     }
 }
 
+impl SymbolismTool {
+    /// Ends the drag: what is committed is the last preview, so the whole drag is shown first.
+    fn finish(&mut self, cx: &ToolContext) -> Vec<Action> {
+        self.active = false;
+        let stale = (self.previewed != self.points.len()).then(|| {
+            let (c, v) = self.command(cx);
+            Action::Preview(c, v)
+        });
+        self.points.clear();
+        stale.into_iter().chain([Action::Commit]).collect()
+    }
+}
+
 impl Tool for SymbolismTool {
     fn id(&self) -> &'static str {
         self.id
@@ -116,6 +143,7 @@ impl Tool for SymbolismTool {
         match ev.kind {
             PointerKind::Down => {
                 self.points = vec![p];
+                self.previewed = 1;
                 self.active = true;
                 self.alt = ev.mods.alt;
                 let (c, v) = self.command(cx);
@@ -124,18 +152,19 @@ impl Tool for SymbolismTool {
             PointerKind::Drag if self.active => {
                 // Sprayer: thin the samples to a fraction of the brush; others: every few pixels.
                 let step = if self.id == "symbolSprayer" { (self.diameter * 0.15).max(cx.tol(3.0)) } else { cx.tol(3.0) };
-                if self.points.last().is_some_and(|l| l.distance(p) < step) {
+                // Each preview carries every point so far: a long drag stops growing and previews less often.
+                if self.points.len() >= MAX_STROKE_SAMPLES || self.points.last().is_some_and(|l| l.distance(p) < step) {
                     return vec![];
                 }
                 self.points.push(p);
+                if !previews_at(self.points.len()) {
+                    return vec![];
+                }
+                self.previewed = self.points.len();
                 let (c, v) = self.command(cx);
                 vec![Action::Preview(c, v)]
             }
-            PointerKind::Up if self.active => {
-                self.active = false;
-                self.points.clear();
-                vec![Action::Commit]
-            }
+            PointerKind::Up if self.active => self.finish(cx),
             _ => vec![],
         }
     }
@@ -157,11 +186,9 @@ impl Tool for SymbolismTool {
         }
         vec![]
     }
-    fn deactivate(&mut self, _cx: &ToolContext) -> Vec<Action> {
+    fn deactivate(&mut self, cx: &ToolContext) -> Vec<Action> {
         if self.active {
-            self.active = false;
-            self.points.clear();
-            return vec![Action::Commit];
+            return self.finish(cx);
         }
         vec![]
     }
@@ -294,6 +321,39 @@ mod tests {
         assert!(!t.busy());
     }
 
+    #[test]
+    fn a_long_drag_previews_less_often_and_commits_all_of_it() {
+        let (d, _) = doc_with_rect();
+        let (s, p) = (Selection::default(), paint());
+        let cx = cx(&d, &s, &p);
+        let mut t = SymbolismTool::new("symbolShifter");
+        t.pointer(&cx, &PointerEvent::new(PointerKind::Down, 0.0, 0.0));
+        let n = 7 * crate::PREVIEW_ALL_UNTIL + 1; // not a point that previews
+        let previews = (1..n).filter(|&i| !t.pointer(&cx, &PointerEvent::new(PointerKind::Drag, 50.0 * i as f64, 0.0)).is_empty()).count();
+        assert!(previews < n * 3 / 4, "{previews} previews of {n} points");
+        let up = t.pointer(&cx, &PointerEvent::new(PointerKind::Up, 0.0, 0.0));
+        let last = up.iter().rev().nth(1);
+        assert!(matches!(last, Some(Action::Preview(_, v)) if v["points"].as_array().unwrap().len() == n), "{up:?}");
+        assert_eq!(up.last(), Some(&Action::Commit));
+    }
+
+    #[test]
+    fn switching_tools_mid_drag_previews_the_whole_drag_before_committing() {
+        let (d, _) = doc_with_rect();
+        let (s, p) = (Selection::default(), paint());
+        let cx = cx(&d, &s, &p);
+        let mut t = SymbolismTool::new("symbolShifter");
+        t.pointer(&cx, &PointerEvent::new(PointerKind::Down, 0.0, 0.0));
+        let n = 7 * crate::PREVIEW_ALL_UNTIL + 1; // not a point that previews
+        for i in 1..n {
+            t.pointer(&cx, &PointerEvent::new(PointerKind::Drag, 50.0 * i as f64, 0.0));
+        }
+        let a = t.deactivate(&cx);
+        assert!(matches!(a.first(), Some(Action::Preview(_, v)) if v["points"].as_array().unwrap().len() == n), "{a:?}");
+        assert_eq!(a.last(), Some(&Action::Commit));
+        assert!(!t.busy());
+    }
+
     #[test]
     fn adjust_tools_map_to_symbol_adjust() {
         let (d, _) = doc_with_rect();