From 3b0f626a4f4106e865fed85ee29673338d6965c9 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 21 Oct 2026 22:18:25 -0700 Subject: [PATCH] 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 | 8 +-- crates/engine/src/cmd/shaper.rs | 20 +++---- crates/engine/src/tests_build.rs | 84 +++++++++++++++++++++++++++- crates/engine/src/tests_shaper.rs | 28 ++++++++++ crates/tools/src/builder.rs | 88 +++++++++++++++++++++++++++--- 5 files changed, 200 insertions(+), 28 deletions(-) diff --git a/crates/engine/src/cmd/buildcmds.rs b/crates/engine/src/cmd/buildcmds.rs index ccebb2ba7..a4f620e9b 100644 --- a/crates/engine/src/cmd/buildcmds.rs +++ b/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 { 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 { 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())); diff --git a/crates/engine/src/cmd/shaper.rs b/crates/engine/src/cmd/shaper.rs index 503229f94..a65ab3e47 100644 --- a/crates/engine/src/cmd/shaper.rs +++ b/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 { 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 = - 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 = 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 { } 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); diff --git a/crates/engine/src/tests_build.rs b/crates/engine/src/tests_build.rs index e7ab082f5..da2215a90 100644 --- a/crates/engine/src/tests_build.rs +++ b/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 }; diff --git a/crates/engine/src/tests_shaper.rs b/crates/engine/src/tests_shaper.rs index 0de25e814..16c7c8b96 100644 --- a/crates/engine/src/tests_shaper.rs +++ b/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() { diff --git a/crates/tools/src/builder.rs b/crates/tools/src/builder.rs index 385ed7bd9..2142769ec 100644 --- a/crates/tools/src/builder.rs +++ b/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) -> Touched { + let mut touched = Touched::default(); + let mut last: Option = 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) -> Vec { + 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 = 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 = 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; 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, to: Point) { - let pts = match from { - Some(a) => sample_polyline(&[a, to], cx.tol(2.0)), - None => vec![to], - }; + let pts: Vec = 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.