Shape Builder: a drag picks every region it crosses, however narrow

A drag tested points spaced by a step that grew with the selection (at
least 0.25 pt), and the tool's highlight tested points 2 screen pixels
apart. A region narrower than the step, such as a thin overlap, fell
between samples: it kept its piece of each source, the merge left strips
behind, or the drag failed with "no region under the pointer" while the
highlight showed the region. Zoomed out, the highlight could miss a
region the merge then took.

The drag now finds where each segment crosses a region outline and tests
the middle of every stretch between crossings, so every region it
crosses is picked. The tool's highlight and shapeBuilder.merge use the
same picking, and Shaper's scribble picks the faces it crosses the same
way. In erase mode the line pieces and edges are still found within the
tolerance, and a segment that crosses one also tests the crossing point.

Shaper's scribble found the strokes to erase at points at least 0.5 pt
apart, so with a tolerance below 1 pt it could pass over a stroke
without erasing it. It now also tests where the scribble crosses each
stroke and points tolerance / 2 apart.

crates/engine/src/cmd/buildcmds.rs

@@ -19,8 +19,7 @@ use vectorcraft_doc::{Appearance, Document, GapOptions, GapSize, LiveShape, Node
 use vectorcraft_geom::{FillRule, PathData, Point, Rect, SubPath};
 use vectorcraft_pathops as po;
 use vectorcraft_tools::builder::{
-    self as b, BuilderMap, EDGE_NAME, FACE_NAME, LIVE_PAINT_NAME, SOURCES_NAME, Touched, edge_near, face_at, is_live_paint, sample_polyline,
-    sorted_roots,
+    self as b, BuilderMap, EDGE_NAME, FACE_NAME, LIVE_PAINT_NAME, SOURCES_NAME, Touched, edge_near, face_at, is_live_paint, sorted_roots,
 };
 use vectorcraft_trace as tr;
 
@@ -296,7 +295,6 @@ fn sb_merge(s: &mut Session, p: &Value) -> Result<Value> {
         if map.shapes.is_empty() {
             return Err(EngineError::Other("Shape Builder: select paths or compound paths".into()));
         }
-        let step = (d.bounds_of(&roots, false).map(|r| r.width().max(r.height())).unwrap_or(100.0) / 2000.0).max(0.25);
         if let Some(tol) = split_tol
             && let Some(&q) = pts.first()
             && let Some((i, run)) = map.stroke_at(&outlines, q, tol)
@@ -329,9 +327,7 @@ fn sb_merge(s: &mut Session, p: &Value) -> Result<Value> {
             return Ok((ids, None, touched, Some(split)));
         }
         let mut touched = Touched { regions: marquee.map(|(a, b)| map.regions_in_marquee(a, b)).unwrap_or_default(), ..Default::default() };
-        for q in sample_polyline(&pts, step) {
-            touched.add(map.hit(q, piece_tol));
-        }
+        touched.extend(&map.hits_along(&pts, piece_tol));
         touched.edges = map.erased_edges(&touched);
         if touched.is_empty() {
             return Err(EngineError::Other("Shape Builder: no region under the pointer".into()));

crates/engine/src/cmd/shaper.rs

@@ -9,7 +9,7 @@ use vectorcraft_doc::shaper::{EdgeEdit, FILL_PREFIX, FaceEdit, FaceSelector, SOU
 use vectorcraft_doc::{Appearance, AppearanceItem, Document, Node, NodeId, NodeKind};
 use vectorcraft_geom::{FillRule, PathData, Point, Rect, Shape as _};
 use vectorcraft_pathops as po;
-use vectorcraft_tools::builder::{self as b, BuilderMap, Hit};
+use vectorcraft_tools::builder::{self as b, BuilderMap};
 
 const MAX_SOURCES: usize = 64;
 const MAX_POINTS: usize = 4096;
@@ -617,8 +617,9 @@ fn scribble(s: &mut Session, p: &Value) -> Result<Value> {
     let edits = face_edits(&map, &src, &spec);
     let filled = |i: usize| edits.get(&i).is_none_or(|e| !e.erase) && map.regions.get(i).and_then(|r| fill_source(&src, &r.0)).is_some();
     let components = visible_components(&map, &src, &edits);
+    // Every face the scribble crosses, however narrow.
     let faces: std::collections::BTreeSet<usize> =
-        points.iter().filter_map(|p| map.region_at(*p)).filter(|i| filled(*i)).filter_map(|i| components.get(i).copied()).collect();
+        map.hits_along(&pts, None).regions.into_iter().filter(|i| filled(*i)).filter_map(|i| components.get(i).copied()).collect();
     let touched: Vec<usize> = components.iter().enumerate().filter(|(_, c)| faces.contains(c)).map(|(i, _)| i).collect();
     let first = points.first().copied().unwrap_or(Point::ZERO);
     let last = points.last().copied().unwrap_or(first);
@@ -642,19 +643,12 @@ fn scribble(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let mut strokes = 0;
     if touched.is_empty() {
-        let mut piece_ids = vec![];
+        // Every stroke the scribble crosses or passes within `tolerance` of, however small that is.
+        let mut hits = map.hits_along(&pts, Some(tolerance));
         for point in &points {
-            let index = match map.hit(*point, Some(tolerance)) {
-                Some(Hit::Line(i)) => Some(map.edges.len() + i),
-                Some(Hit::Edge(i)) => Some(i),
-                _ => None,
-            };
-            if let Some(i) = index
-                && !piece_ids.contains(&i)
-            {
-                piece_ids.push(i);
-            }
+            hits.add(map.hit(*point, Some(tolerance)));
         }
+        let piece_ids = hits.edges.iter().copied().chain(hits.lines.iter().map(|i| map.edges.len() + i));
         for i in piece_ids {
             if let Some(e) = pieces(&map).get(i).and_then(|p| edge_selector(&map, &src, p)) {
                 spec.edges.push(e);

crates/engine/src/tests_build.rs

@@ -49,8 +49,13 @@ fn area(s: &Session, id: NodeId) -> f64 {
 
 /// Rect A (0..100) and rect B (50..150), both 100 tall, selected. A red, B blue.
 fn two(s: &mut Session) -> (NodeId, NodeId) {
+    two_at(s, 50.0)
+}
+
+/// [`two`] with B at `x`..`x`+100.
+fn two_at(s: &mut Session, x: f64) -> (NodeId, NodeId) {
     let a = rect(s, 0.0, 0.0, 100.0, 100.0);
-    let b = rect(s, 50.0, 0.0, 100.0, 100.0);
+    let b = rect(s, x, 0.0, 100.0, 100.0);
     set_fill(s, a, Color::rgb(1.0, 0.0, 0.0));
     set_fill(s, b, Color::rgb(0.0, 0.0, 1.0));
     s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap();
@@ -81,6 +86,35 @@ fn shape_builder_drag_merges_touched_regions() {
     assert_eq!(layer_children(&s), 2);
 }
 
+#[test]
+fn shape_builder_drag_merges_a_region_thinner_than_the_sampling_step() {
+    let mut s = session();
+    two_at(&mut s, 99.9);
+    assert_eq!(s.execute("shapeBuilder.regions", &json!({})).unwrap()["regions"].as_array().unwrap().len(), 3);
+    let r = s.execute("shapeBuilder.merge", &json!({"points": [[50, 50], [150, 50]]})).unwrap();
+    assert_eq!(r["regions"], 3, "the 0.1 pt overlap too");
+    let out = ids(&r);
+    assert_eq!(out.len(), 1, "{out:?}");
+    assert_eq!(outline(&node(&s, out[0])).0.subpaths.len(), 1);
+    assert!((area(&s, out[0]) - 19990.0).abs() < 1.0, "{}", area(&s, out[0]));
+}
+
+#[test]
+fn shape_builder_drag_merges_a_narrow_overlap_when_far_art_is_selected() {
+    let mut s = session();
+    let (a, b) = two_at(&mut s, 99.0);
+    // A far square makes the selection 4,010 pt wide.
+    let far = rect(&mut s, 4000.0, 4000.0, 10.0, 10.0);
+    s.execute("select.set", &json!({"ids": [a.0, b.0, far.0]})).unwrap();
+    let r = s.execute("shapeBuilder.merge", &json!({"points": [[50.3, 50], [150.3, 50]]})).unwrap();
+    assert_eq!(r["regions"], 3);
+    let out = ids(&r);
+    assert_eq!(out.len(), 2, "the merged shape and the far square: {out:?}");
+    let merged = NodeId(r["merged"].as_u64().unwrap());
+    assert_eq!(outline(&node(&s, merged)).0.subpaths.len(), 1);
+    assert!((area(&s, merged) - 19900.0).abs() < 1.0, "{}", area(&s, merged));
+}
+
 #[test]
 fn shape_builder_click_splits_out_one_region() {
     let mut s = session();
@@ -331,6 +365,54 @@ fn shape_builder_tool_drives_the_command() {
     assert_eq!(layer_children(&s), 1);
 }
 
+#[test]
+fn shape_builder_tool_merges_the_narrow_shape_it_highlights_in_a_wide_selection() {
+    let mut s = session();
+    let strip = rect(&mut s, 199.5, 100.0, 1.0, 100.0);
+    // A far square makes the selection 9,800.5 pt wide.
+    let far = rect(&mut s, 9900.0, 100.0, 100.0, 100.0);
+    s.execute("select.set", &json!({"ids": [strip.0, far.0]})).unwrap();
+    let v = ViewInfo::default();
+    s.select_tool("shapeBuilder", v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Down, 190.0, 150.0), v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Drag, 210.0, 150.0), v).unwrap();
+    assert_eq!(s.overlays(v).len(), 3, "the strip's hatch and outline, and the drag path");
+    s.pointer(&PointerEvent::new(PointerKind::Up, 210.0, 150.0), v).unwrap();
+    assert_eq!(s.journal.last().unwrap().0, "shapeBuilder.merge");
+    assert_eq!(layer_children(&s), 2);
+}
+
+#[test]
+fn shape_builder_tool_highlights_and_merges_a_narrow_overlap() {
+    let mut s = session();
+    two_at(&mut s, 99.95);
+    let v = ViewInfo::default();
+    s.select_tool("shapeBuilder", v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Down, 50.3, 50.0), v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Drag, 150.3, 50.0), v).unwrap();
+    assert_eq!(s.overlays(v).len(), 7, "the three regions' hatches and outlines, and the drag path");
+    s.pointer(&PointerEvent::new(PointerKind::Up, 150.3, 50.0), v).unwrap();
+    assert_eq!(layer_children(&s), 1, "one shape, no leftover strips");
+    let d = &s.doc().unwrap().doc;
+    let merged = d.children(d.default_layer()).unwrap()[0].id;
+    assert_eq!(outline(&node(&s, merged)).0.subpaths.len(), 1);
+}
+
+#[test]
+fn shape_builder_tool_zoomed_out_picks_a_narrow_shape() {
+    let mut s = session();
+    let strip = rect(&mut s, 199.5, 100.0, 1.0, 100.0);
+    let near = rect(&mut s, 300.0, 100.0, 3.0, 100.0);
+    s.execute("select.set", &json!({"ids": [strip.0, near.0]})).unwrap();
+    let v = ViewInfo { zoom: 0.05, ..Default::default() };
+    s.select_tool("shapeBuilder", v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Down, 190.0, 150.0), v).unwrap();
+    s.pointer(&PointerEvent::new(PointerKind::Drag, 260.0, 150.0), v).unwrap();
+    assert_eq!(s.overlays(v).len(), 3, "the strip's hatch and outline, and the drag path");
+    s.pointer(&PointerEvent::new(PointerKind::Up, 260.0, 150.0), v).unwrap();
+    assert_eq!(s.journal.last().unwrap().0, "shapeBuilder.merge");
+}
+
 /// The first and last stop colours of a gradient paint.
 fn gradient_ends(p: &Paint) -> Option<([f32; 3], [f32; 3])> {
     let Paint::Gradient(g) = p else { return None };

crates/engine/src/tests_shaper.rs

@@ -108,6 +108,18 @@ fn shaper_interior_punch_removes_the_whole_visible_front_face() {
     }
 }
 
+#[test]
+fn shaper_scribble_merges_a_face_narrower_than_its_sampling_step() {
+    let mut s = session();
+    rectangle(&mut s, 0.0, "#0000ff");
+    s.execute("paint.setFill", &json!({"color": "#ff0000", "ids": []})).unwrap();
+    s.execute("shape.rectangle", &json!({"x": 49.8, "y": 10, "width": 0.4, "height": 80})).unwrap();
+    // Every crossing of the 0.4 pt strip falls between two of the 2 pt samples.
+    let g = scribble(&mut s, &[[25.0, 20.0], [75.0, 35.0], [30.0, 50.0], [75.0, 65.0]]);
+    assert_eq!(fill(&s, g, 50.0, 50.0).as_deref(), Some("#0000ff"), "the strip merges into the face around it");
+    assert_eq!(fill(&s, g, 25.0, 50.0).as_deref(), Some("#0000ff"));
+}
+
 #[test]
 fn shaper_exiting_scribble_erases_only_touched_fill_and_boundary_strokes() {
     let mut s = session();
@@ -141,6 +153,22 @@ fn shaper_erases_only_protruding_stroke_pieces_keeps_fill_and_original_line() {
     assert_eq!(node(&s, line), &original_line);
 }
 
+#[test]
+fn shaper_scribble_with_a_small_tolerance_erases_a_stroke_it_crosses_between_samples() {
+    for (tolerance, from, to) in [(0.1, 140.25, 160.25), (0.01, 50.0123, 250.0123)] {
+        let mut s = session();
+        s.execute("paint.setFill", &json!({"color":"#ffff00"})).unwrap();
+        s.execute("shape.ellipse", &json!({"x":100,"y":100,"width":100,"height":100})).unwrap();
+        s.execute("shape.line", &json!({"x1":50,"y1":150,"x2":250,"y2":150})).unwrap();
+        // No sample along the stroke lands within `tolerance` of the line at y = 150.
+        let v = s.execute("shaper.scribble", &json!({"points": [[60.0, from], [60.0, to]], "tolerance": tolerance})).unwrap();
+        assert_eq!(v["strokes"], json!(1), "tolerance {tolerance}: {v}");
+        let g = NodeId(v["id"].as_u64().unwrap());
+        assert!(!stroke(&s, g, 75.0, 150.0));
+        assert!(stroke(&s, g, 150.0, 150.0));
+    }
+}
+
 /// Export Selection crops a Shaper group to the art it shows, not to its transparent source art.
 #[test]
 fn shaper_export_selection_crops_to_the_shown_art() {

crates/tools/src/builder.rs

@@ -392,6 +392,74 @@ impl BuilderMap {
         Some((i, run))
     }
 
+    /// What a gesture along the polyline `pts` touches, in order: [`Self::hit`] at each vertex and
+    /// at the points [`Self::probes`] picks between them, so it touches every region it crosses,
+    /// however narrow.
+    pub fn hits_along(&self, pts: &[Point], piece_tol: Option<f64>) -> Touched {
+        let mut touched = Touched::default();
+        let mut last: Option<Point> = None;
+        for &p in pts {
+            if let Some(a) = last {
+                for q in self.probes(a, p, piece_tol) {
+                    touched.add(self.hit(q, piece_tol));
+                }
+            }
+            touched.add(self.hit(p, piece_tol));
+            last = Some(p);
+        }
+        touched
+    }
+
+    /// The points strictly between `a` and `b`, in order, that [`Self::hits_along`] tests: the
+    /// middle of each stretch between two places where the segment crosses a region's outline, and
+    /// with `piece_tol` (erasing) those crossings, the places where it crosses a line piece or an
+    /// edge, and points `piece_tol / 2` apart (at most 4,000 per segment), so the line pieces and
+    /// edges the segment crosses or passes close to are touched too.
+    fn probes(&self, a: Point, b: Point, piece_tol: Option<f64>) -> Vec<Point> {
+        let len = a.distance(b);
+        if !(len.is_finite() && len > 0.0) {
+            return vec![];
+        }
+        let line = kurbo::Line::new(a, b);
+        let lb = Rect::from_points(a, b);
+        let mut cross: Vec<f64> = vec![];
+        for (_, bp, rb) in &self.regions {
+            if rb.x0 > lb.x1 || rb.x1 < lb.x0 || rb.y0 > lb.y1 || rb.y1 < lb.y0 {
+                continue;
+            }
+            for seg in bp.segments() {
+                cross.extend(seg.intersect_line(line).iter().map(|x| x.line_t));
+            }
+        }
+        cross.retain(|t| t.is_finite() && *t > 0.0 && *t < 1.0);
+        cross.sort_by(f64::total_cmp);
+        // Crossings closer than this along the segment are one place.
+        let gap = 1e-9 / len;
+        let mut ts: Vec<f64> = vec![];
+        let mut from = 0.0;
+        for &t in cross.iter().chain([1.0].iter()) {
+            if t - from > gap {
+                ts.push((from + t) / 2.0);
+                from = t;
+            }
+        }
+        if let Some(tol) = piece_tol {
+            ts.extend(cross.iter().copied());
+            for (piece, pb) in self.lines.iter().chain(&self.edges) {
+                if pb.x0 > lb.x1 || pb.x1 < lb.x0 || pb.y0 > lb.y1 || pb.y1 < lb.y0 {
+                    continue;
+                }
+                for seg in piece.path.to_bezpath().segments() {
+                    ts.extend(seg.intersect_line(line).iter().map(|x| x.line_t).filter(|t| t.is_finite() && *t > 0.0 && *t < 1.0));
+                }
+            }
+            let n = ((len / (tol / 2.0).max(1e-3)).ceil() as usize).min(4000);
+            ts.extend((1..n).map(|k| k as f64 / n as f64));
+            ts.sort_by(f64::total_cmp);
+        }
+        ts.into_iter().map(|t| a.lerp(b, t)).collect()
+    }
+
     /// The regions just either side of the middle of `piece` (None: outside every region).
     pub fn sides(&self, piece: &po::Shape) -> [Option<usize>; 2] {
         let Some(sp) = piece.path.subpaths.first() else { return [None; 2] };
@@ -446,6 +514,14 @@ impl Touched {
         }
     }
 
+    /// Adds what `other` touched, in its order.
+    pub fn extend(&mut self, other: &Touched) {
+        let regions = other.regions.iter().map(|&i| Hit::Region(i));
+        for hit in regions.chain(other.lines.iter().map(|&i| Hit::Line(i))).chain(other.edges.iter().map(|&i| Hit::Edge(i))) {
+            self.add(Some(hit));
+        }
+    }
+
     pub fn is_empty(&self) -> bool {
         self.regions.is_empty() && self.lines.is_empty() && self.edges.is_empty()
     }
@@ -741,16 +817,12 @@ impl ShapeBuilderTool {
         alt.then(|| cx.tol(LINE_TOL_PX))
     }
 
+    /// Adds what the drag from `from` to `to` touches, picked as `shapeBuilder.merge` picks it.
     fn touch(&mut self, cx: &ToolContext, from: Option<Point>, to: Point) {
-        let pts = match from {
-            Some(a) => sample_polyline(&[a, to], cx.tol(2.0)),
-            None => vec![to],
-        };
+        let pts: Vec<Point> = from.into_iter().chain([to]).collect();
         let tol = Self::line_tol(cx, self.alt);
-        let map = self.cache.get(cx, self.alt, self.opts.open_filled_as_closed);
-        for p in pts {
-            self.touched.add(map.hit(p, tol));
-        }
+        let touched = self.cache.get(cx, self.alt, self.opts.open_filled_as_closed).hits_along(&pts, tol);
+        self.touched.extend(&touched);
     }
 
     /// Selection Straight Line: the drag touches what the line from its first point to `p` crosses.