M8: a whole ellipse drawn backwards keeps its simple stroke in SVG (#1848)

* M8: a whole ellipse drawn backwards keeps its simple stroke in SVG

An ellipse with a sweep of -2π goes all the way round, as the renderer
and the .fig export already read it. The SVG export's simple stroke
checked `sweep >= 2π` only, so such an ellipse's inside or outside
stroke was written as a filled even-odd ring instead of a stroke.

All three places now ask `figcraft_geom::full_sweep`, as the geometry
and the bounds do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* M8: a backwards whole ellipse's stroke ring in the renderer and .fig export, tested

Review follow-up (#1848): the renderer's stroke ring and the .fig
export's stroke geometry read a sweep of -2π as whole too, now through
`full_sweep`. A test for each checks a backwards ellipse strokes the
same ring as a forward one, and fails if either goes back to checking
positive sweeps only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* M8: the .fig backwards-ellipse test compares the stroke ring itself

Review follow-up (#1848): the test checked only that one stroke path was
written. It now compares the path's commands and winding rule for the
forward and backwards ellipse, and that the ring is even-odd.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
김삼현
2026-11-01 23:49:38 -05:00
committed by GitHub
co-authored by Claude Opus 5.5
parent fa8faf216d
commit 702b0d7894
6 changed files with 75 additions and 3 deletions
+1 -1
View File
@@ -282,7 +282,7 @@ impl<'d> Exporter<'d> {
if !(plain && weight.is_finite() && weight > 0.0 && w.is_finite() && h.is_finite()) {
return;
}
let whole = matches!(n.kind, NodeKind::Ellipse { sweep, inner, .. } if inner == 0.0 && sweep.abs() >= std::f64::consts::TAU - 1e-9);
let whole = matches!(n.kind, NodeKind::Ellipse { sweep, inner, .. } if inner == 0.0 && figcraft_doc::geom::full_sweep(sweep));
if boxy || whole {
let (out, inset) = match sp.align {
StrokeAlign::Inside => (0.0, weight),
+31
View File
@@ -449,3 +449,34 @@ fn local_samples_agree_with_the_written_schema() {
}
}
}
/// An ellipse drawn the other way round (a sweep of −2π) is whole: its inside stroke is written
/// as the same ring a forward one's is, path and winding rule alike.
#[test]
fn a_whole_ellipse_drawn_backwards_carries_its_stroke_ring() {
let ring = |sweep: f64| {
let mut doc = sample();
let ids: Vec<NodeId> = doc.ids().collect();
for id in ids {
if let Some(n) = doc.node_mut(id)
&& let NodeKind::Ellipse { sweep: s, .. } = &mut n.kind
{
*s = sweep;
}
}
let out = std::cell::RefCell::new(None);
with_changes(&doc, |of, msg| {
let blobs = msg.get("blobs").unwrap().list();
let geometry = of("ELLIPSE").get("strokeGeometry").map(|g| {
let p = g.list().first().unwrap().clone();
let bytes = blobs.get(num(&p, "commandsBlob") as usize).and_then(|b| b.get("bytes")).and_then(Kv::bytes).unwrap();
(name(&p, "windingRule").to_string(), bytes.to_vec())
});
*out.borrow_mut() = geometry;
});
out.into_inner()
};
let forwards = ring(std::f64::consts::TAU).expect("a forward ellipse writes its stroke ring");
assert_eq!(forwards.0, "ODD");
assert_eq!(ring(-std::f64::consts::TAU), Some(forwards));
}
+1 -1
View File
@@ -217,7 +217,7 @@ pub fn stroke_area(n: &Node, path: &BezPath, closed: bool) -> Arc<BezPath> {
p.apply_affine(Affine::translate((x0, y0)));
Some(p)
}
NodeKind::Ellipse { sweep, inner, .. } if sweep.abs() >= std::f64::consts::TAU - 1e-9 && inner == 0.0 => {
NodeKind::Ellipse { sweep, inner, .. } if figcraft_geom::full_sweep(sweep) && inner == 0.0 => {
Some(figcraft_geom::kurbo::Ellipse::from_rect(Rect::new(x0, y0, x0 + ww, y0 + hh)).to_path(0.05))
}
_ => None,
+20
View File
@@ -44,3 +44,23 @@ fn outside_bevel_join_cuts_outer_corner() {
fn outside_miter_join_keeps_square_corner() {
assert_ne!(covered(StrokeJoin::Miter), 0);
}
/// An ellipse drawn the other way round (a sweep of −2π) is whole: its inside stroke is the same
/// ring of two outlines as a forward one's.
#[test]
fn a_whole_ellipse_drawn_backwards_strokes_as_a_ring() {
let ring = |sweep: f64| {
let mut d = Document::new("ring");
let p = d.first_page().unwrap();
let id = d.create(p, NodeKind::Ellipse { start: 0.0, sweep, inner: 0.0 }, "E", 0.0, 0.0, 60.0, 40.0).unwrap();
let n = d.get_mut(id).unwrap();
n.strokes = vec![Paint::solid(Color::BLACK)];
(n.stroke.weight, n.stroke.align) = (6.0, StrokeAlign::Inside);
let outline: BezPath = figcraft_geom::kurbo::Ellipse::from_rect(figcraft_geom::kurbo::Rect::new(0.0, 0.0, 60.0, 40.0)).to_path(0.1);
(*crate::draw::stroke_area(d.get(id).unwrap(), &outline, true)).clone()
};
let forwards = ring(std::f64::consts::TAU);
let moves = forwards.elements().iter().filter(|e| matches!(e, figcraft_geom::kurbo::PathEl::MoveTo(_))).count();
assert_eq!(moves, 2, "two outlines: {forwards:?}");
assert_eq!(ring(-std::f64::consts::TAU), forwards);
}
+1 -1
View File
@@ -802,7 +802,7 @@ impl W<'_> {
return None;
}
match n.kind {
NodeKind::Ellipse { sweep, inner, .. } if inner == 0.0 && sweep >= std::f64::consts::TAU - 1e-9 => {
NodeKind::Ellipse { sweep, inner, .. } if inner == 0.0 && figcraft_geom::full_sweep(sweep) => {
figcraft_geom::kurbo::Shape::to_path(&figcraft_geom::kurbo::Ellipse::from_rect(r), 0.1)
}
NodeKind::Rectangle | NodeKind::Frame | NodeKind::Component { .. } | NodeKind::Instance { .. } if n.corners.smoothing == 0.0 => {
+21
View File
@@ -429,3 +429,24 @@ fn an_exported_dashed_line_opens_as_the_same_line() {
d.subtree(frame).iter().filter_map(|i| d.node(*i)).filter(|n| !n.stroke.dashes.is_empty()).map(|n| n.stroke.dashes.clone()).collect();
assert!(matches!(dashes.as_slice(), [d] if d.len() == 2 && d.iter().all(|v| (v - 10.7).abs() < 1e-4)), "{dashes:?}");
}
/// An ellipse drawn the other way round (a sweep of −2π) is as whole as one drawn forwards: its
/// stroke inside or outside exports as a stroke, the same as the forward one's.
#[test]
fn a_whole_ellipse_drawn_backwards_keeps_its_simple_stroke() {
let export = |sweep: f64, align: figcraft_doc::StrokeAlign| {
let mut d = Document::new("backwards");
let p = d.first_page().unwrap();
let n = d.create(p, NodeKind::Ellipse { start: 0.0, sweep, inner: 0.0 }, "Ellipse", 0.0, 0.0, 100.0, 60.0).unwrap();
let node = d.get_mut(n).unwrap();
node.fills.clear();
node.strokes = vec![Paint::solid(Color::from_hex("000000").unwrap())];
(node.stroke.weight, node.stroke.align) = (8.0, align);
export_svg(&d, &[n], &SvgOptions { simplify_stroke: true, ..SvgOptions::default() }).unwrap()
};
for align in [figcraft_doc::StrokeAlign::Inside, figcraft_doc::StrokeAlign::Outside] {
let (forwards, backwards) = (export(std::f64::consts::TAU, align), export(-std::f64::consts::TAU, align));
assert!(forwards.contains("stroke-width=\"8\""), "{align:?}: {forwards}");
assert_eq!(backwards, forwards, "{align:?}");
}
}