diff --git a/crates/engine/src/shapebuilder_strokes_tests.rs b/crates/engine/src/shapebuilder_strokes_tests.rs index 259dedb14..19aa2c9c5 100644 --- a/crates/engine/src/shapebuilder_strokes_tests.rs +++ b/crates/engine/src/shapebuilder_strokes_tests.rs @@ -137,6 +137,35 @@ fn a_stroked_shape_gives_its_ring_and_keeps_its_look() { assert!(kids.iter().position(|k| *k == o) > kids.iter().position(|k| *k == r), "the outline sits above the shape"); } +#[test] +fn a_centred_stroke_on_a_sharp_star_makes_three_regions() { + let mut s = Session::new(); + // Five sharp tips: a centred stroke's outline folds over itself at each one and meets the + // star's fill, which used to give overlapping regions and a spiked outline along them. + let mut pts = Vec::new(); + for i in 0..10 { + let a = -std::f64::consts::FRAC_PI_2 + i as f64 * std::f64::consts::PI / 5.0; + let r = if i % 2 == 0 { 100.0 } else { 40.0 }; + pts.push([150.0 + r * a.cos(), 150.0 + r * a.sin()]); + } + let v = id(&ex(&mut s, "vector.create", json!({"points": pts, "closed": true}))); + ex(&mut s, "node.set", json!({"id": v, "props": {"fill": "#ff0000", "strokes": ["#0000ff"], "strokeWeight": 8, "strokeAlign": "center"}})); + s.set_selection(vec![v]); + let out = ex(&mut s, "shapeBuilder.regions", json!({})); + assert_eq!(out["regions"].as_array().unwrap().len(), 3, "the inside and the stroke's two halves: {out}"); + let a = crate::shapebuilder::arrangement(&mut s, &[v]).unwrap(); + for (i, r) in a.regions.iter().enumerate() { + for (j, o) in a.regions.iter().enumerate() { + if let (false, Some(p)) = (i == j, r.point) { + assert_eq!(o.path.winding(p), 0, "regions {i} and {j} overlap at {p:?}"); + } + } + } + let mut covers: Vec<(bool, bool)> = a.regions.iter().map(|r| (r.parts.iter().any(|p| p.stroke), r.parts.iter().any(|p| !p.stroke))).collect(); + covers.sort(); + assert_eq!(covers, vec![(false, true), (true, false), (true, true)], "fill only, stroke only, both"); +} + #[test] fn stroke_regions_survive_junk() { for j in [ diff --git a/crates/pathops/src/lib.rs b/crates/pathops/src/lib.rs index fb9cfcfec..40695d66f 100644 --- a/crates/pathops/src/lib.rs +++ b/crates/pathops/src/lib.rs @@ -192,28 +192,43 @@ pub fn faces(paths: &[(BezPath, bool)]) -> Result, PathOpsE if paths.len() > MAX_ARRANGEMENT_INPUTS || paths.iter().any(|(p, _)| !finite(p)) { return Err(PathOpsError::BadInput); } - let closed_paths: Vec = paths.iter().map(|(p, _)| closed(p)).collect(); + // Self-intersecting or self-overlapping inputs (a centred stroke outlined around a sharp + // corner folds over itself and touches the shape's fill) leave the winding on each side of a + // segment ambiguous where they touch, which made the sweep report overlapping faces. Reducing + // every input to the simple contours of the area it fills first keeps the faces disjoint. + let closed_paths: Vec = paths.iter().map(|(p, even_odd)| plain(&closed(p), *even_odd)).collect(); if closed_paths.iter().all(|p| p.elements().is_empty()) { return Ok(Vec::new()); } let bbox = closed_paths.iter().filter(|p| !p.elements().is_empty()).map(Shape::bounding_box).reduce(|a, b| a.union(b)); let m = bbox.map(|b| b.x0.abs().max(b.x1.abs()).max(b.y0.abs()).max(b.y1.abs())).unwrap_or(0.0); let eps = (m * f64::EPSILON * 64.0).max(1e-6); - let even_odd: Vec = paths.iter().map(|(_, e)| *e).collect(); - sweep(|| arrange(&closed_paths, &even_odd, eps)) + sweep(|| arrange(&closed_paths, eps)) } -/// [`faces`] of the closed `paths`, swept with tolerance `eps`. -fn arrange(closed_paths: &[BezPath], even_odd: &[bool], eps: f64) -> Result, PathOpsError> { +/// The area `path` fills under `even_odd` as simple contours (self-intersections and overlapping +/// subpaths resolved), which both fill rules then cover the same. A path the sweep can't handle +/// comes back unchanged, so [`faces`] still tries it as it is. +fn plain(path: &BezPath, even_odd: bool) -> BezPath { + if path.elements().is_empty() { + return BezPath::new(); + } + let rule = if even_odd { linesweeper::FillRule::EvenOdd } else { linesweeper::FillRule::NonZero }; + let swept = sweep(|| linesweeper::binary_op(path, &BezPath::new(), rule, linesweeper::BinaryOp::Union).map_err(|_| PathOpsError::BadInput)); + let Ok(contours) = swept else { return path.clone() }; + let mut out = BezPath::new(); + for c in contours.contours() { + out.extend(c.path.elements().iter().copied()); + } + out +} + +/// [`faces`] of the closed, simple `paths`, swept with tolerance `eps`. +fn arrange(closed_paths: &[BezPath], eps: f64) -> Result, PathOpsError> { let topo = linesweeper::topology::Topology::::from_paths(closed_paths.iter().enumerate().map(|(i, p)| (p, i)), eps) .map_err(|_| PathOpsError::BadInput)?; - let cover_of = |w: &Windings| -> Vec { - w.0.iter() - .enumerate() - .filter(|(i, n)| if even_odd.get(*i).copied().unwrap_or(false) { *n % 2 != 0 } else { **n != 0 }) - .map(|(i, _)| i) - .collect() - }; + // Every path was reduced to simple contours, so either rule covers the same area: non-zero. + let cover_of = |w: &Windings| -> Vec { w.0.iter().enumerate().filter(|(_, n)| **n != 0).map(|(i, _)| i).collect() }; // Every coverage that occurs on either side of an output segment. let mut covers: std::collections::BTreeSet> = std::collections::BTreeSet::new(); for seg in topo.segment_indices() { @@ -465,6 +480,46 @@ mod tests { p.area().abs() } + /// A five-pointed star with sharp tips, and its centred stroke, whose outline folds over + /// itself at every tip (a self-intersecting non-zero input that touches the star's fill). + fn star_and_stroke() -> (BezPath, BezPath) { + let mut star = BezPath::new(); + for i in 0..10 { + let a = -std::f64::consts::FRAC_PI_2 + i as f64 * std::f64::consts::PI / 5.0; + let r = if i % 2 == 0 { 100.0 } else { 40.0 }; + let p = (150.0 + r * a.cos(), 150.0 + r * a.sin()); + if i == 0 { + star.move_to(p); + } else { + star.line_to(p); + } + } + star.close_path(); + (star.clone(), stroke_center(&star, 8.0, &StrokeProps::default())) + } + + #[test] + fn self_intersecting_inputs_make_disjoint_faces() { + let (star, ring) = star_and_stroke(); + // The stroke outline really is self-intersecting: its winding is 2 at the tips. + assert_eq!(ring.winding(Point::new(150.0, 56.0)), 2); + let f = faces(&[(star.clone(), false), (ring.clone(), false)]).unwrap(); + let mut got: Vec> = f.iter().map(|x| x.cover.clone()).collect(); + got.sort(); + assert_eq!(got, vec![vec![0], vec![0, 1], vec![1]], "the star's inside, its stroke's inner half, its outer half"); + // Faces never overlap: no face's interior point is inside another face. + for (i, a) in f.iter().enumerate() { + let Some(p) = figcraft_doc::vector_faces::interior_point(&a.path) else { continue }; + for (j, b) in f.iter().enumerate() { + assert!(i == j || b.path.winding(p) == 0, "faces {i} and {j} overlap at {p:?}"); + } + } + // They cover exactly what the two shapes paint together. + let painted = boolean(&[star, ring], BoolOp::Union).unwrap(); + let all = union_faces(f.iter().map(|x| &x.path)).unwrap(); + assert!((all.area().abs() - painted.area().abs()).abs() < 0.5, "{} vs {}", all.area().abs(), painted.area().abs()); + } + #[test] fn a_panic_in_the_sweep_is_bad_input() { let r: Result<(), PathOpsError> = sweep(|| panic!("sweep bug")); diff --git a/crates/ui-egui/src/builder_poly_tests.rs b/crates/ui-egui/src/builder_poly_tests.rs new file mode 100644 index 000000000..88abfe4f3 --- /dev/null +++ b/crates/ui-egui/src/builder_poly_tests.rs @@ -0,0 +1,39 @@ +//! The shape builder's region outlines: repeats dropped and sub-pixel slivers left out, so the +//! tessellator never sees a zero-length segment at a hairpin (that draws a spike across the canvas). + +use super::clean_poly; +use egui::pos2; + +#[test] +fn a_contour_loses_the_point_it_closes_back_to() { + // A square whose flattened contour ends where it started, as kurbo's flatten emits it. + let sq = vec![pos2(0.0, 0.0), pos2(10.0, 0.0), pos2(10.0, 10.0), pos2(0.0, 10.0), pos2(0.0, 0.0)]; + let out = clean_poly(sq).expect("a square is drawn"); + assert_eq!(out, vec![pos2(0.0, 0.0), pos2(10.0, 0.0), pos2(10.0, 10.0), pos2(0.0, 10.0)]); +} + +#[test] +fn repeated_points_and_stray_values_are_dropped() { + // Triplicated points at the corners are what makes the tessellator divide by a zero normal. + let poly = vec![pos2(0.0, 0.0), pos2(0.0, 0.0), pos2(0.0, 0.0), pos2(20.0, 0.0), pos2(f32::NAN, 5.0), pos2(20.0, 20.0), pos2(0.0, 20.0)]; + let out = clean_poly(poly).expect("a square is drawn"); + assert_eq!(out, vec![pos2(0.0, 0.0), pos2(20.0, 0.0), pos2(20.0, 20.0), pos2(0.0, 20.0)]); +} + +#[test] +fn a_sliver_a_fraction_of_a_pixel_thin_is_left_out() { + // A sliver region that doubles back on itself: 100 px long, 0.002 px thick, closing back. + let sliver = vec![pos2(0.0, 0.0), pos2(100.0, 0.0), pos2(100.0, 0.002), pos2(0.0, 0.0)]; + assert!(clean_poly(sliver).is_none()); + // Two points (a doubled-back line without thickness) and a single point aren't areas either. + assert!(clean_poly(vec![pos2(0.0, 0.0), pos2(10.0, 0.0)]).is_none()); + assert!(clean_poly(vec![pos2(3.0, 4.0)]).is_none()); +} + +#[test] +fn a_thin_but_visible_region_is_kept() { + // 2 px wide and 100 px long: thin, but a real area. + let strip = vec![pos2(0.0, 0.0), pos2(100.0, 0.0), pos2(100.0, 2.0), pos2(0.0, 2.0)]; + let out = clean_poly(strip).expect("a 2 px strip is drawn"); + assert_eq!(out.len(), 4); +} diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index e6dbc3c8c..bafbe82de 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1441,13 +1441,15 @@ fn region_hatch(app: &FigApp, painter: &egui::Painter) { fn builder_overlay(app: &FigApp, painter: &egui::Painter, t: &Tokens, arr: &figcraft_engine::shapebuilder::Arrangement) { let ov = &app.session.tool.overlay; let screen = |r: &figcraft_engine::shapebuilder::Region| -> Vec> { - r.polys.iter().map(|poly| poly.iter().map(|p| app.to_screen(p.x, p.y)).collect()).collect() + r.polys.iter().filter_map(|poly| clean_poly(poly.iter().map(|p| app.to_screen(p.x, p.y)))).collect() }; // Edges read on any fill: a light halo under a thin accent line. for r in &arr.regions { for poly in screen(r) { - painter.add(Shape::closed_line(poly.clone(), stroke(Color32::from_white_alpha(150), 2.5))); - painter.add(Shape::closed_line(poly, stroke(accent(painter), 1.0))); + for seg in segments(&poly) { + painter.add(Shape::line(seg.to_vec(), stroke(Color32::from_white_alpha(150), 2.5))); + painter.add(Shape::line(seg.to_vec(), stroke(accent(painter), 1.0))); + } } } let col = if ov.builder_remove { t.danger } else { accent(painter) }; @@ -1456,6 +1458,33 @@ fn builder_overlay(app: &FigApp, painter: &egui::Painter, t: &Tokens, arr: &figc } } +/// A region contour in screen space, ready to draw: non-finite and repeated points dropped (the +/// last point back at the first, the points a sliver doubles back on) and sub-pixel slivers left +/// out, since they're invisible anyway. +fn clean_poly(points: impl IntoIterator) -> Option> { + let mut out: Vec = Vec::new(); + for q in points { + if !(q.x.is_finite() && q.y.is_finite()) || out.last().is_some_and(|l| l.distance(q) < 0.01) { + continue; + } + out.push(q); + } + if out.len() > 1 && out.first().is_some_and(|f| f.distance(*out.last().unwrap_or(f)) < 0.01) { + out.pop(); + } + let n = out.len(); + let twice_area: f32 = (0..n).map(|k| out[k].x * out[(k + 1) % n].y - out[(k + 1) % n].x * out[k].y).sum(); + (n > 2 && twice_area.abs() > 0.4).then_some(out) +} + +/// A closed polygon as its segments. Drawn segment by segment rather than as one closed path: a +/// joined path asks the tessellator for a mitre at every hairpin a region's sliver doubles back on, +/// and a zero-length segment there divides by a zero normal, which draws a spike across the canvas. +fn segments(poly: &[Pos2]) -> impl Iterator + '_ { + let n = poly.len(); + (0..n).map(move |k| [poly[k], poly[(k + 1) % n]]) +} + /// Polygons hatched and outlined in `col` (an area about to be filled, merged or removed), each /// line on a light halo so it reads on any fill, even one in `col`. fn hatched(painter: &egui::Painter, polys: &[Vec], col: Color32) { @@ -1465,8 +1494,10 @@ fn hatched(painter: &egui::Painter, polys: &[Vec], col: Color32) { painter.line_segment(l, stroke(col, 1.25)); } for poly in polys { - painter.add(Shape::closed_line(poly.clone(), stroke(halo, 4.0))); - painter.add(Shape::closed_line(poly.clone(), stroke(col, 2.0))); + for seg in segments(poly) { + painter.add(Shape::line(seg.to_vec(), stroke(halo, 4.0))); + painter.add(Shape::line(seg.to_vec(), stroke(col, 2.0))); + } } } @@ -1594,6 +1625,10 @@ pub fn fmt(v: f64) -> String { if (r - r.round()).abs() < 1e-9 { format!("{}", r.round() as i64) } else { format!("{r}") } } +#[cfg(test)] +#[path = "builder_poly_tests.rs"] +mod builder_poly_tests; + #[cfg(test)] #[path = "zoom_preview_tests.rs"] mod zoom_preview_tests;