From b4c1cdbceab8b6ec5aa9913c90a80d803e58a13d Mon Sep 17 00:00:00 2001 From: Vikram Bhaskaran Date: Mon, 12 Oct 2026 23:49:45 +1100 Subject: [PATCH 1/3] Fill the hole a stroke leaves when it is wider than the closed shape. A closed path stroked wider than it is came back as a ring, because the inner offset crossed the middle. When that outline no longer covers the path, the outer contour is kept so the stroke paints solid. --- crates/effects/src/stroke.rs | 65 +++++++++++++++++++++++++++++- crates/effects/src/stroke/tests.rs | 15 +++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/crates/effects/src/stroke.rs b/crates/effects/src/stroke.rs index 5dc045dd..1254f14d 100644 --- a/crates/effects/src/stroke.rs +++ b/crates/effects/src/stroke.rs @@ -19,7 +19,7 @@ mod width; use std::borrow::Cow; -use kurbo::{BezPath, PathEl, PathSeg}; +use kurbo::{BezPath, ParamCurve, PathEl, PathSeg, Shape}; use vectorcraft_doc::{LineCap, LineJoin, StrokeAlign, StrokeLayer}; pub(crate) use vectorcraft_geom::bez::{kink, segments, subpaths, tangent, unit}; @@ -116,10 +116,71 @@ pub fn line_outline(line: &BezPath, st: &StrokeLayer, width: f64, tol: f64) -> B out.extend(dot_outline(&d.dots, width, st.cap, tol).iter()); out } - (None, None) => kurbo::stroke(line.iter(), &style(st, width), &kurbo::StrokeOpts::default(), tol), + (None, None) => { + let outline = kurbo::stroke(line.iter(), &style(st, width), &kurbo::StrokeOpts::default(), tol); + // A stroke wider than a closed shape comes back as a ring: the inner offset crossed + // the middle and left a hole (#1396). The outer contour is the stroke. + fill_crossed_stroke(line, outline) + } } } +/// When `outline` does not cover `line`, drop contours that sit inside its largest one. +fn fill_crossed_stroke(line: &BezPath, outline: BezPath) -> BezPath { + if !is_closed(line) || outline_covers(line, &outline) { + return outline; + } + let parts = closed_parts(&outline); + if parts.len() < 2 { + return outline; + } + let area = |p: &BezPath| { + let b = p.bounding_box(); + b.width() * b.height() + }; + let Some(outer) = parts.iter().max_by(|a, b| area(a).total_cmp(&area(b))) else { + return outline; + }; + let ob = outer.bounding_box(); + let outer_area = area(outer); + let mut out = BezPath::new(); + for p in &parts { + let b = p.bounding_box(); + let inside = b.x0 >= ob.x0 - 1e-6 && b.y0 >= ob.y0 - 1e-6 && b.x1 <= ob.x1 + 1e-6 && b.y1 <= ob.y1 + 1e-6; + if (area(p) - outer_area).abs() <= 1e-6 || !inside { + out.extend(p.elements().iter().copied()); + } + } + out +} + +fn closed_parts(bp: &BezPath) -> Vec { + let mut out = Vec::new(); + let mut cur = BezPath::new(); + for el in bp.elements() { + if matches!(el, PathEl::MoveTo(_)) && !cur.elements().is_empty() { + out.push(std::mem::take(&mut cur)); + } + cur.push(*el); + } + if !cur.elements().is_empty() { + out.push(cur); + } + out +} + +/// A point on each segment of `line` lies in `outline` (the stroke still covers its path). +fn outline_covers(line: &BezPath, outline: &BezPath) -> bool { + let mut saw = false; + for seg in line.segments() { + saw = true; + if !outline.contains(seg.eval(0.5)) { + return false; + } + } + saw +} + /// Append `seg` to `out` (which must already have a current point at its start). pub(crate) fn push_seg(out: &mut BezPath, seg: &PathSeg) { match seg { diff --git a/crates/effects/src/stroke/tests.rs b/crates/effects/src/stroke/tests.rs index acc8a757..e50616b5 100644 --- a/crates/effects/src/stroke/tests.rs +++ b/crates/effects/src/stroke/tests.rs @@ -37,6 +37,21 @@ fn near(a: f64, b: f64, tol: f64) -> bool { (a - b).abs() <= tol } +/// #1396: a stroke wider than the shape still covers the interior and reaches half the weight past it. +/// A thinner stroke keeps the hole in the middle. +#[test] +fn a_stroke_wider_than_the_shape_covers_the_interior() { + let circle = kurbo::Circle::new(Point::new(0.0, 0.0), 5.0).to_path(0.1); + let st = stroke(40.0, |_| {}); + let outline = line_outline(&circle, &st, 40.0, 0.25); + let b = outline.bounding_box(); + assert!(b.x0 < -24.0 && b.x1 > 24.0, "half the weight past the radius: {b:?}"); + assert!(kurbo::Shape::contains(&outline, Point::ZERO), "the middle of a fat stroke is painted"); + let thin = line_outline(&circle, &stroke(4.0, |_| {}), 4.0, 0.25); + assert!(!kurbo::Shape::contains(&thin, Point::ZERO), "a narrow stroke stays a ring"); + assert!(kurbo::Shape::contains(&thin, Point::new(5.0, 0.0))); +} + // ---------------------------------------------------------------- dashes and dots #[test] From aed5d380e9892a9e453dc53e939a35c5710a0ba9 Mon Sep 17 00:00:00 2001 From: Vikram Bhaskaran Date: Mon, 12 Oct 2026 23:50:38 +1100 Subject: [PATCH 2/3] Cover a small rectangle in the fat-stroke test. --- crates/effects/src/stroke/tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/effects/src/stroke/tests.rs b/crates/effects/src/stroke/tests.rs index e50616b5..3023b06d 100644 --- a/crates/effects/src/stroke/tests.rs +++ b/crates/effects/src/stroke/tests.rs @@ -50,6 +50,8 @@ fn a_stroke_wider_than_the_shape_covers_the_interior() { let thin = line_outline(&circle, &stroke(4.0, |_| {}), 4.0, 0.25); assert!(!kurbo::Shape::contains(&thin, Point::ZERO), "a narrow stroke stays a ring"); assert!(kurbo::Shape::contains(&thin, Point::new(5.0, 0.0))); + let square = line_outline(&Rect::new(0.0, 0.0, 8.0, 8.0).to_path(0.1), &st, 40.0, 0.25); + assert!(kurbo::Shape::contains(&square, Point::new(4.0, 4.0)), "a fat stroke on a small rectangle covers it"); } // ---------------------------------------------------------------- dashes and dots From 1168c319b037eac11c20a9171b9e6bed435ade15 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Mon, 12 Oct 2026 20:18:29 +0700 Subject: [PATCH 3/3] Close the hole whenever the inner offset of a closed stroke has crossed over The check added for #1396 dropped the inner contour only when the path itself fell in the hole, which takes a stroke more than twice as wide as the shape. Between one and two times the size (a 30 pt ellipse with a 50 pt stroke), and on rectangles with round or bevel joins, the hole stayed. An offset contour is now left out when no point of it lies half the weight from its subpath: it is then nowhere the edge of the stroke. Subpaths are judged one by one, and only when the stroke is at least as wide as the subpath's bounds, so ordinary strokes take the same single call as before. --- crates/effects/src/stroke.rs | 95 +++++++++++++----------------- crates/effects/src/stroke/tests.rs | 82 ++++++++++++++++++++++++++ crates/render/src/tests.rs | 44 ++++++++++++++ 3 files changed, 167 insertions(+), 54 deletions(-) diff --git a/crates/effects/src/stroke.rs b/crates/effects/src/stroke.rs index 1254f14d..6980c002 100644 --- a/crates/effects/src/stroke.rs +++ b/crates/effects/src/stroke.rs @@ -19,7 +19,7 @@ mod width; use std::borrow::Cow; -use kurbo::{BezPath, ParamCurve, PathEl, PathSeg, Shape}; +use kurbo::{BezPath, ParamCurve, ParamCurveNearest, PathEl, PathSeg, Shape}; use vectorcraft_doc::{LineCap, LineJoin, StrokeAlign, StrokeLayer}; pub(crate) use vectorcraft_geom::bez::{kink, segments, subpaths, tangent, unit}; @@ -116,69 +116,56 @@ pub fn line_outline(line: &BezPath, st: &StrokeLayer, width: f64, tol: f64) -> B out.extend(dot_outline(&d.dots, width, st.cap, tol).iter()); out } - (None, None) => { - let outline = kurbo::stroke(line.iter(), &style(st, width), &kurbo::StrokeOpts::default(), tol); - // A stroke wider than a closed shape comes back as a ring: the inner offset crossed - // the middle and left a hole (#1396). The outer contour is the stroke. - fill_crossed_stroke(line, outline) - } + (None, None) => plain_outline(line, &style(st, width), tol), } } -/// When `outline` does not cover `line`, drop contours that sit inside its largest one. -fn fill_crossed_stroke(line: &BezPath, outline: BezPath) -> BezPath { - if !is_closed(line) || outline_covers(line, &outline) { - return outline; - } - let parts = closed_parts(&outline); - if parts.len() < 2 { - return outline; - } - let area = |p: &BezPath| { - let b = p.bounding_box(); - b.width() * b.height() +/// `line` stroked with `style`, to fill with the non-zero rule. +/// +/// kurbo offsets a closed subpath to both sides, as two contours. Once half the weight is more +/// than the shape can hold (its inradius), the inner one has crossed the middle and comes out +/// the far side: wound against the outer contour it would cut a hole where the pen in fact +/// overlaps itself (#1396). Such a contour is left out, so the stroke paints solid. +// shortcut: closed subpaths of plain strokes only. An open path that turns back on itself inside +// a wider stroke (and dashes on such a shape) can still cancel; fix it in the offsetting if reported. +fn plain_outline(line: &BezPath, style: &kurbo::Stroke, tol: f64) -> BezPath { + let stroke = |els: &[PathEl]| kurbo::stroke(els.iter().copied(), style, &kurbo::StrokeOpts::default(), tol); + let els = line.elements(); + // Only a stroke at least as wide as a closed subpath's bounds can cross over: everything + // else takes the one call it always did. + let may_cross = |sub: &[PathEl]| { + let b = BezPath::from_vec(sub.to_vec()).bounding_box(); + style.width >= b.width().min(b.height()) }; - let Some(outer) = parts.iter().max_by(|a, b| area(a).total_cmp(&area(b))) else { - return outline; - }; - let ob = outer.bounding_box(); - let outer_area = area(outer); + let subs = subpaths(line); + if !subs.iter().any(|(r, closed)| *closed && els.get(r.clone()).is_some_and(may_cross)) { + return stroke(els); + } + let reach = style.width / 2.0 - 2.0 * tol; let mut out = BezPath::new(); - for p in &parts { - let b = p.bounding_box(); - let inside = b.x0 >= ob.x0 - 1e-6 && b.y0 >= ob.y0 - 1e-6 && b.x1 <= ob.x1 + 1e-6 && b.y1 <= ob.y1 + 1e-6; - if (area(p) - outer_area).abs() <= 1e-6 || !inside { - out.extend(p.elements().iter().copied()); + for (r, closed) in subs { + let Some(sub) = els.get(r) else { continue }; + let outline = stroke(sub); + if !(closed && may_cross(sub)) { + out.extend(outline.iter()); + continue; + } + let sub = BezPath::from_vec(sub.to_vec()); + for (c, _) in subpaths(&outline) { + let Some(contour) = outline.elements().get(c) else { continue }; + if reaches_edge(contour, &sub, reach) { + out.extend(contour.iter().copied()); + } } } out } -fn closed_parts(bp: &BezPath) -> Vec { - let mut out = Vec::new(); - let mut cur = BezPath::new(); - for el in bp.elements() { - if matches!(el, PathEl::MoveTo(_)) && !cur.elements().is_empty() { - out.push(std::mem::take(&mut cur)); - } - cur.push(*el); - } - if !cur.elements().is_empty() { - out.push(cur); - } - out -} - -/// A point on each segment of `line` lies in `outline` (the stroke still covers its path). -fn outline_covers(line: &BezPath, outline: &BezPath) -> bool { - let mut saw = false; - for seg in line.segments() { - saw = true; - if !outline.contains(seg.eval(0.5)) { - return false; - } - } - saw +/// Whether some point of `contour` lies at least `reach` from `path`: an offset contour that +/// never does is nowhere the edge of the stroke. +fn reaches_edge(contour: &[PathEl], path: &BezPath, reach: f64) -> bool { + let far = |p: kurbo::Point| path.segments().all(|s| s.nearest(p, 1e-6).distance_sq >= reach * reach); + kurbo::segments(contour.iter().copied()).any(|s| far(s.eval(0.0)) || far(s.eval(0.5))) } /// Append `seg` to `out` (which must already have a current point at its start). diff --git a/crates/effects/src/stroke/tests.rs b/crates/effects/src/stroke/tests.rs index 3023b06d..a7d8b403 100644 --- a/crates/effects/src/stroke/tests.rs +++ b/crates/effects/src/stroke/tests.rs @@ -321,3 +321,85 @@ fn degenerate_paths_and_patterns_neither_panic_nor_hang() { } assert!(dash(&line(0.0, 1e-9), &dashed(&[1e-12, 1e-12], 0.0)).is_none(), "too fine to advance: solid"); } + +/// Grid points within `reach` of the inside of `path` (not past an open end) that `outline` +/// leaves unpainted. +fn unpainted_near(path: &BezPath, outline: &BezPath, reach: f64) -> Vec { + let b = path.bounding_box().inflate(reach, reach); + let ends: Vec = if is_closed(path) { vec![] } else { path.segments().flat_map(|s| [s.start(), s.end()]).collect() }; + let ends = [ends.first().copied(), ends.last().copied()]; + let mut missed = Vec::new(); + for i in 0..=60 { + for j in 0..=60 { + let p = Point::new(b.x0 + b.width() * f64::from(i) / 60.0, b.y0 + b.height() * f64::from(j) / 60.0); + let nearest = path + .segments() + .map(|s| (s, kurbo::ParamCurveNearest::nearest(&s, p, 1e-6))) + .min_by(|a, b| a.1.distance_sq.total_cmp(&b.1.distance_sq)) + .map(|(s, n)| (s.eval(n.t), n.distance_sq.sqrt())); + let Some((at, dist)) = nearest else { continue }; + let past_end = ends.iter().flatten().any(|e| e.distance(at) < 1e-6); + if dist <= reach && !past_end && outline.winding(p) == 0 { + missed.push(p); + } + } + } + missed +} + +/// #1396: whatever the weight is next to the size of the shape, every point within half the +/// weight of the path is painted (the stroke is the path swept by a pen that wide). +#[test] +fn a_fat_stroke_paints_everything_within_half_its_weight() { + let mut shapes: Vec<(String, BezPath)> = Vec::new(); + for r in [3.0, 6.0, 10.0, 15.0, 20.0, 24.0, 40.0] { + let circle = kurbo::Circle::new(Point::ZERO, r).to_path(0.01); + let mut ellipse = kurbo::Ellipse::new(Point::ZERO, (r, r / 2.0), 0.3).to_path(0.01); + ellipse.close_path(); + let arc = BezPath::from_vec(circle.elements().iter().take(3).copied().collect()); + let mut l = BezPath::new(); + l.move_to((0.0, 0.0)); + for p in [(r * 2.0, 0.0), (r * 2.0, r), (r, r), (r, r * 2.0), (0.0, r * 2.0)] { + l.line_to(p); + } + l.close_path(); + let mut two = circle.clone(); + two.extend(kurbo::Circle::new(Point::new(200.0, 0.0), 60.0).to_path(0.01).iter()); + shapes.extend([ + (format!("circle r={r}"), circle), + (format!("ellipse {r}"), ellipse), + (format!("rect {r}"), Rect::new(0.0, 0.0, r, r * 1.5).to_path(0.1)), + (format!("open arc r={r}"), arc), + (format!("L {r}"), l), + (format!("circle r={r} beside a large one"), two), + ]); + } + let mut failures = Vec::new(); + for (name, path) in &shapes { + for join in [LineJoin::Miter, LineJoin::Round, LineJoin::Bevel] { + let outline = line_outline(path, &stroke(50.0, |s| s.join = join), 50.0, 0.05); + // A bevel stops short of the pen's reach at a corner. + let reach = if join == LineJoin::Bevel { 16.0 } else { 24.5 }; + let missed = unpainted_near(path, &outline, reach); + if !missed.is_empty() { + failures.push(format!("{name} {join:?}: {} unpainted, e.g. {:?} ({} contours)", missed.len(), missed.first(), moves(&outline))); + } + } + } + assert!(failures.is_empty(), "{}", failures.join("\n")); +} + +/// A stroke the shape has room for keeps its hole, also beside a subpath that has none. +#[test] +fn a_stroke_narrower_than_the_shape_keeps_its_hole() { + let mut path = kurbo::Circle::new(Point::ZERO, 30.0).to_path(0.01); + path.extend(kurbo::Circle::new(Point::new(200.0, 0.0), 10.0).to_path(0.01).iter()); + for join in [LineJoin::Miter, LineJoin::Round, LineJoin::Bevel] { + let outline = line_outline(&path, &stroke(50.0, |s| s.join = join), 50.0, 0.05); + assert_eq!(outline.winding(Point::ZERO), 0, "{join:?}: the hole of the wide ring"); + assert_eq!(outline.winding(Point::new(4.0, 0.0)), 0, "{join:?}: up to its inner edge"); + assert_ne!(outline.winding(Point::new(6.0, 0.0)), 0, "{join:?}"); + assert_ne!(outline.winding(Point::new(200.0, 0.0)), 0, "{join:?}: the small circle is solid"); + assert_eq!(moves(&outline), 3, "{join:?}: two contours for the ring, one for the blob"); + } +} diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 70533ae8..f60a9a11 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -50,6 +50,50 @@ fn stroke_draws_outline_only() { assert_eq!(r.pixel(50, 50), [255, 255, 255, 255]); } +/// #1396: a stroke wider than its ellipse is a solid blob, pixel for pixel the disc a renderer +/// with a native stroke (the PDF and SVG exports) draws, at every alignment and zoom. +#[test] +fn a_stroke_wider_than_its_ellipse_paints_solid() { + let doc = |r: f64, fill: Paint, stroke: Paint, w: f64, align: StrokeAlign| { + let mut d = Document::new(100.0, 100.0); + let id = d.alloc_id(); + let mut n = Node::path(id, shapes::ellipse(Rect::new(50.0 - r, 50.0 - r, 50.0 + r, 50.0 + r)), Appearance::basic(fill, stroke, w)); + if let Some(st) = n.appearance.stroke_mut() { + st.align = align; + } + let l = d.layers[0].id; + d.insert(Some(l), 0, n).unwrap(); + d + }; + let black = || Paint::solid(Color::BLACK); + let render = |d: &Document, zoom: f64| { + let view = Affine::translate((50.0, 50.0)) * Affine::scale(zoom) * Affine::translate((-50.0, -50.0)); + Renderer::new().render(d, 100, 100, view, &RenderOptions { background: Some([255, 255, 255, 255]), ..Default::default() }) + }; + // (ellipse radius, weight, alignment, radius of the disc it paints) + for (r, w, align, disc) in [ + (10.0, 60.0, StrokeAlign::Center, 40.0), + (6.0, 50.0, StrokeAlign::Center, 31.0), + (15.0, 50.0, StrokeAlign::Center, 40.0), + (10.0, 30.0, StrokeAlign::Outside, 40.0), + (10.0, 30.0, StrokeAlign::Inside, 10.0), + ] { + for zoom in [0.5, 1.0, 2.0] { + let got = render(&doc(r, Paint::None, black(), w, align), zoom); + if align == StrokeAlign::Outside { + // The shape itself stays unpainted, and the stroke around it has no gap. + assert_eq!(got.pixel(50, 50), [255, 255, 255, 255], "{r} {w} {align:?} at {zoom}"); + assert_eq!(got.pixel(50 + (12.0 * zoom) as u32, 50), [0, 0, 0, 255], "{r} {w} {align:?} at {zoom}"); + continue; + } + let want = render(&doc(disc, black(), Paint::None, 0.0, StrokeAlign::Center), zoom); + let off = + (0..100).flat_map(|y| (0..100).map(move |x| (x, y))).filter(|&(x, y)| got.pixel(x, y)[0].abs_diff(want.pixel(x, y)[0]) > 40).count(); + assert!(off <= 8, "{r} {w} {align:?} at {zoom}: {off} pixels differ from the disc"); + } + } +} + #[test] fn inside_stroke_stays_inside() { let mut d = Document::new(100.0, 100.0);