diff --git a/crates/engine/src/cmd/addons/cutting/stencil.rs b/crates/engine/src/cmd/addons/cutting/stencil.rs index a8b4cfe591..dc415ce73d 100644 --- a/crates/engine/src/cmd/addons/cutting/stencil.rs +++ b/crates/engine/src/cmd/addons/cutting/stencil.rs @@ -89,7 +89,12 @@ fn bridges(s: &mut Session, p: &Value) -> Result { }; let refs: Vec<(&PathData, FillRule)> = parts.iter().map(|(p, r)| (p, *r)).collect(); let spec = BridgeSpec { width, count: count as usize, direction }; - let bridged = stencil_bridges(&refs, spec).map_err(|e| EngineError::Other(e.to_string()))?; + let bridged = stencil_bridges(&refs, spec).map_err(|e| match e { + vectorcraft_pathops::PathOpsError::TooComplex => { + EngineError::Other("Stencil Bridges: the selected art is too large or too detailed to bridge".into()) + } + e => EngineError::Other(e.to_string()), + })?; (bridged, style_of(&top), roots) }; let summary = |id: Option, original: Option| { diff --git a/crates/engine/src/cmd/addons/cutting/stencil/tests.rs b/crates/engine/src/cmd/addons/cutting/stencil/tests.rs index ad072ca13d..b21f0fef5c 100644 --- a/crates/engine/src/cmd/addons/cutting/stencil/tests.rs +++ b/crates/engine/src/cmd/addons/cutting/stencil/tests.rs @@ -277,6 +277,34 @@ fn bad_params_are_refused() { assert!(s.execute("addon.cutting.bridges", &json!({"width": 4})).is_ok(), "good params run"); } +#[test] +fn art_too_detailed_to_bridge_is_refused_with_a_message_that_says_so() { + let mut s = session(); + let parent = s.doc().unwrap().target_parent().unwrap(); + let id = s + .edit("dense", |d, sel| { + // More anchors than Stencil Bridges measures. + let mut zigzag = kurbo::BezPath::new(); + zigzag.move_to((100.0, 100.0)); + for i in 1..=vectorcraft_pathops::addons::stencil::MAX_STENCIL_SEGMENTS { + zigzag.line_to((100.0 + i as f64 * 0.001, if i % 2 == 0 { 100.0 } else { 101.0 })); + } + zigzag.close_path(); + let huge = vectorcraft_geom::PathData::from_bezpath(&zigzag); + let n = vectorcraft_doc::Node::path(d.alloc_id(), huge, vectorcraft_doc::Appearance::default()); + let id = n.id; + d.insert(parent, usize::MAX, n)?; + sel.set([id]); + Ok(id) + }) + .unwrap(); + select(&mut s, &[id]); + let steps = undo_steps(&s); + let e = s.execute("addon.cutting.bridges", &json!({"width": 4})).unwrap_err().to_string(); + assert!(e.contains("Stencil Bridges: the selected art is too large or too detailed to bridge"), "{e}"); + assert_eq!(undo_steps(&s), steps); +} + #[test] fn selected_objects_without_outlines_stay_where_they_are() { for keep in [true, false] { diff --git a/crates/engine/src/cmd/addons/cutting/weed/geometry.rs b/crates/engine/src/cmd/addons/cutting/weed/geometry.rs index d9f612114b..cf7df51d80 100644 --- a/crates/engine/src/cmd/addons/cutting/weed/geometry.rs +++ b/crates/engine/src/cmd/addons/cutting/weed/geometry.rs @@ -511,28 +511,43 @@ impl Region { /// The pieces of outline the region shows: every outline of a shape and of a group's members, /// and in a clip group the members' outlines inside the clipping path and the clipping path's - /// outline where the members' area or outlines are. Errs past [`MAX_PAIRS`]. + /// outline where the members' area or outlines are. Errs past [`MAX_PAIRS`], counted over + /// every clip group in the region together. pub(super) fn outline(&self) -> Result, String> { + self.outline_counted(&mut 0) + } + + /// [`Region::outline`], adding the measure's work to `work`. + fn outline_counted(&self, work: &mut usize) -> Result, String> { match self { Region::Shape { polys, .. } => Ok(polys.iter().flat_map(Poly::segments).collect()), Region::Union(parts) => { let mut out = vec![]; for r in parts { - out.extend(r.outline()?); + out.extend(r.outline_counted(work)?); } Ok(out) } Region::Clip { clip, content } => { // Each side's area is bounded by the outlines it shows, so the pieces are cut and // tested against those, never against art a nested clip hides. - let (clip_shown, content_shown) = (clip.outline()?, content.outline()?); - let mut work = 0; - let mut out = within(&content_shown, clip, &clip_shown, &mut work)?; - out.extend(within(&clip_shown, content, &content_shown, &mut work)?); + let (clip_shown, content_shown) = (clip.outline_counted(work)?, content.outline_counted(work)?); + let mut out = within(&content_shown, clip, &clip_shown, work)?; + out.extend(within(&clip_shown, content, &content_shown, work)?); Ok(out) } } } + + /// The points of the region's closed outlines, those a nested clip hides included: what one + /// [`Region::spans`] looks at, at most. + fn closed_points(&self) -> usize { + match self { + Region::Shape { polys, .. } => polys.iter().filter(|p| p.closed).map(|p| p.points.len()).sum(), + Region::Union(parts) => parts.iter().map(Region::closed_points).sum(), + Region::Clip { clip, content } => clip.closed_points().saturating_add(content.closed_points()), + } + } } fn too_complex_clip() -> String { @@ -541,12 +556,13 @@ fn too_complex_clip() -> String { /// The pieces of `segments` inside `region` (or on its outlines), cut where they cross /// `region_segments`, the outlines the region shows. Each segment, and each piece of one tested -/// for being inside, adds the region's segment count to `work`, and the measure errs past -/// [`MAX_PAIRS`]. +/// for being inside, adds the region's closed outline points (hidden ones included) and its +/// shown segment count to `work`, and the measure errs past [`MAX_PAIRS`]. fn within(segments: &[Segment], region: &Region, region_segments: &[Segment], work: &mut usize) -> Result, String> { let mut out = vec![]; + let cost = region.closed_points().saturating_add(region_segments.len()).max(1); let charge = |work: &mut usize| { - *work = work.saturating_add(region_segments.len().max(1)); + *work = work.saturating_add(cost); if *work > MAX_PAIRS { Err(too_complex_clip()) } else { Ok(()) } }; for &(a, b) in segments { diff --git a/crates/engine/src/cmd/addons/cutting/weed/tests.rs b/crates/engine/src/cmd/addons/cutting/weed/tests.rs index 280ae909bb..76f5f656c6 100644 --- a/crates/engine/src/cmd/addons/cutting/weed/tests.rs +++ b/crates/engine/src/cmd/addons/cutting/weed/tests.rs @@ -356,6 +356,55 @@ fn art_a_nested_clip_hides_adds_no_outline() { assert_eq!(weed_box(&g), Some(Rect::new(100.0, 100.0, 150.0, 140.0)), "only the shown rectangle is measured"); } +#[test] +fn art_a_nested_clip_hides_counts_toward_the_clip_limit() { + // A clip group, clipping path A of 20,000 segments, holding a clip group whose small clipping + // path B shows a rectangle and hides a closed zigzag C of 20,000 points across every row of A. + // The inner group shows four outline pieces, but testing each piece of A for being inside it + // looks at all of C. + let mut s = session(); + let parent = s.doc().unwrap().target_parent().unwrap(); + let id = s + .edit("nested clips", |d, sel| { + let mut path = |p: BezPath| Arc::new(Node::path(d.alloc_id(), PathData::from_bezpath(&p), Appearance::default())); + let mut a = BezPath::new(); + a.move_to((0.0, 0.0)); + for i in 1..=20_000 { + a.line_to((f64::from(i) * 0.02, if i % 2 == 0 { 0.0 } else { 0.5 })); + } + a.line_to((400.0, 300.0)); + a.line_to((0.0, 300.0)); + a.close_path(); + let mut c = BezPath::new(); + c.move_to((300.0, 0.0)); + for i in 1..=20_000 { + c.line_to((300.0 + f64::from(i) * 0.002, if i % 2 == 0 { 0.0 } else { 300.0 })); + } + c.close_path(); + let b = path(shapes::rectangle(Rect::new(100.0, 100.0, 150.0, 140.0)).to_bezpath()); + let shown = path(shapes::rectangle(Rect::new(110.0, 110.0, 140.0, 130.0)).to_bezpath()); + let (a, c) = (path(a), path(c)); + let clipped = |mut g: Node| { + if let NodeKind::Group { clip, .. } = &mut g.kind { + *clip = true; + } + g + }; + let inner = Arc::new(clipped(Node::group(d.alloc_id(), vec![b, shown, c]))); + let outer = clipped(Node::group(d.alloc_id(), vec![a, inner])); + let id = outer.id; + d.insert(parent, usize::MAX, outer)?; + sel.set([id]); + Ok(id) + }) + .unwrap(); + select(&mut s, &[id]); + let n = s.doc().unwrap().doc.node_count(); + let e = s.execute("addon.cutting.weed", &json!({"margin": 10, "count": 0})).unwrap_err(); + assert!(e.to_string().contains("too many outline segments"), "{e}"); + assert_eq!(s.doc().unwrap().doc.node_count(), n); +} + #[test] fn art_selected_inside_a_clip_group_is_measured_by_what_shows() { // The rectangle runs far outside the clipping path; selected on its own, it still shows only diff --git a/crates/pathops/src/addons/stencil.rs b/crates/pathops/src/addons/stencil.rs index 437433e40c..a5412fa328 100644 --- a/crates/pathops/src/addons/stencil.rs +++ b/crates/pathops/src/addons/stencil.rs @@ -146,7 +146,7 @@ fn choose(contours: &[Contour], spec: BridgeSpec, work: usize) -> Result<(Vec
0 { return Err(PathOpsError::TooComplex); } picker.spread(spec.count); diff --git a/crates/pathops/src/addons/stencil/tests.rs b/crates/pathops/src/addons/stencil/tests.rs index 48d2f8148e..93ea92cdde 100644 --- a/crates/pathops/src/addons/stencil/tests.rs +++ b/crates/pathops/src/addons/stencil/tests.rs @@ -210,6 +210,37 @@ fn bridges_found_before_the_budget_runs_out_are_kept() { assert!(matches!(choose(&found, spec(4.0, 1, BridgeDirection::Vertical), least - 1), Err(PathOpsError::TooComplex))); } +#[test] +fn joined_art_is_kept_when_lines_in_one_direction_run_out() { + // A ring whose thin left bar has edges of many short zigzag segments: lines across it in the + // horizontal direction look at many more segments than vertical lines across the top bar. + let zigzag = |x: f64, from: f64, to: f64, out: &mut Vec| { + let n = ((to - from).abs() / 0.05) as usize; + out.extend((1..n).map(|i| Point::new(x + if i % 2 == 1 { 0.02 } else { 0.0 }, from + (to - from) * i as f64 / n as f64))); + }; + let mut outer = vec![Point::new(0.0, 0.0), Point::new(200.0, 0.0), Point::new(200.0, 100.0), Point::new(0.0, 100.0)]; + zigzag(0.0, 100.0, 0.0, &mut outer); + let mut hole = vec![Point::new(10.0, 20.0), Point::new(170.0, 20.0), Point::new(170.0, 80.0), Point::new(10.0, 80.0)]; + zigzag(9.98, 80.0, 20.0, &mut hole); + let art = PathData::new(vec![SubPath::polyline(&outer, true), SubPath::polyline(&hole, true)]); + let found = contours(&art, 4.0).unwrap(); + let shortest = spec(4.0, 1, BridgeDirection::Shortest); + let views = (0..180).step_by(SHORTEST_STEP_DEG).count(); + // A share of the work that runs out in the horizontal direction while another direction joins + // the island. + let share = (1..5000) + .find(|&budget| { + let all: Vec = + (0..180).step_by(SHORTEST_STEP_DEG).map(|d| View::new(&found, (d as f64).to_radians(), 4.0, budget).unwrap()).collect(); + let mut picker = Picker::new(&found, &all, 4.0); + picker.connect(); + all.iter().any(View::exhausted) && picker.unjoined() == 0 + }) + .expect("a share of the work that runs out with the island joined"); + let (bridges, unjoined) = choose(&found, shortest, share * views).unwrap(); + assert_eq!((bridges.len(), unjoined), (1, 0)); +} + #[test] fn curves_are_followed_within_a_twentieth_of_the_width() { let circle = PathData::from_bezpath(&kurbo::Circle::new((0.0, 0.0), 100.0).to_path(0.001)); diff --git a/docs/mcp.md b/docs/mcp.md index 46f79de9e0..6195428c7d 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -9270,8 +9270,9 @@ The result is one group named "Weeding" (the box is its member "Weed Box"), adde selected, as one undo step. While a clip group, or a group inside one, is edited in isolation, the group goes on top of the container that holds the outermost clip group around it. With nothing selected, or with a selection that has no outlines, the command fails. It also fails when the art's outlines, followed in straight segments, have more than -200000 points; when groups, symbol instances and live objects are nested more than 128 levels deep; when measuring a -clip group's outlines against each other takes more than 40000000 tests of one segment against another; when, for the +200000 points; when groups, symbol instances and live objects are nested more than 128 levels deep; when measuring the +clip groups' outlines against each other takes more than 40000000 tests of one segment or outline point against another, +counted over every clip group in the selection together and including the outlines a nested clip group hides; when, for the lines one way, more than 10000000 pairs of a line and an outline segment within `gap` of it are found, or closed outline segments are checked for crossings with the lines more than 10000000 times; when the lines would be more than 100000 pieces; and when no line fits outside the art while `box` is false. Result: `{id, box, lines}`: the group's id,