From 702b0d78944e79fc3721261be3c495a08ef6730d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=82=BC=ED=98=84?= Date: Mon, 2 Nov 2026 13:49:38 +0900 Subject: [PATCH] M8: a whole ellipse drawn backwards keeps its simple stroke in SVG (#1848) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 * 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 --------- Co-authored-by: Claude Opus 5.5 --- crates/format/src/fig/export/shape.rs | 2 +- crates/format/src/fig/interop_tests.rs | 31 ++++++++++++++++++++++++++ crates/render/src/draw.rs | 2 +- crates/render/src/join_tests.rs | 20 +++++++++++++++++ crates/svg/src/lib.rs | 2 +- crates/svg/src/tests.rs | 21 +++++++++++++++++ 6 files changed, 75 insertions(+), 3 deletions(-) diff --git a/crates/format/src/fig/export/shape.rs b/crates/format/src/fig/export/shape.rs index 03c82cfb1..e4e2cae6b 100644 --- a/crates/format/src/fig/export/shape.rs +++ b/crates/format/src/fig/export/shape.rs @@ -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), diff --git a/crates/format/src/fig/interop_tests.rs b/crates/format/src/fig/interop_tests.rs index b51760c55..fa95d792b 100644 --- a/crates/format/src/fig/interop_tests.rs +++ b/crates/format/src/fig/interop_tests.rs @@ -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 = 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)); +} diff --git a/crates/render/src/draw.rs b/crates/render/src/draw.rs index f48899aea..f0af6b6d0 100644 --- a/crates/render/src/draw.rs +++ b/crates/render/src/draw.rs @@ -217,7 +217,7 @@ pub fn stroke_area(n: &Node, path: &BezPath, closed: bool) -> Arc { 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, diff --git a/crates/render/src/join_tests.rs b/crates/render/src/join_tests.rs index 9fbb4f870..42c931997 100644 --- a/crates/render/src/join_tests.rs +++ b/crates/render/src/join_tests.rs @@ -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); +} diff --git a/crates/svg/src/lib.rs b/crates/svg/src/lib.rs index 58d80ac91..3158d1c0d 100644 --- a/crates/svg/src/lib.rs +++ b/crates/svg/src/lib.rs @@ -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 => { diff --git a/crates/svg/src/tests.rs b/crates/svg/src/tests.rs index c3fb75f8e..ade0e3f30 100644 --- a/crates/svg/src/tests.rs +++ b/crates/svg/src/tests.rs @@ -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:?}"); + } +}