From b33da8a7b0f3dd92d0a2a88ec22ab72f9d028a1d Mon Sep 17 00:00:00 2001
From: Trevor Brown <trevorbrown.dev@gmail.com>
Date: Sat, 7 Nov 2026 11:52:52 -0500
Subject: [PATCH] Ellipse: an unsliced ellipse edits as four anchor points, not
 six (#2080)

A whole ellipse and each ring contour are built as four cubics through the extremes instead of a tolerance-subdivided arc. Sliced ellipses are unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---
 crates/geom/src/lib.rs   | 23 ++++++++++++++++-----
 crates/geom/src/tests.rs | 43 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/crates/geom/src/lib.rs b/crates/geom/src/lib.rs
index a775a4170..f290882d8 100644
--- a/crates/geom/src/lib.rs
+++ b/crates/geom/src/lib.rs
@@ -472,15 +472,28 @@ pub fn ellipse(width: f64, height: f64, start: f64, sweep: f64, inner: f64) -> B
     let mut path = BezPath::new();
     // A whole turn as an arc with these radii: kurbo's `Ellipse` recovers its radii from an
     // affine map, which overflows to infinity for radii near f64's limit (and infinite cubics).
-    let whole = |rx: f64, ry: f64| kurbo::Arc::new(c, (rx, ry), 0.0, TAU, 0.0);
+    // Four cubics through the right, bottom, left and top extremes (clockwise from 3 o'clock),
+    // the same four anchors an ellipse has when edited as a vector; an arc subdivided by
+    // tolerance would give five or six.
+    let whole = |rx: f64, ry: f64| {
+        const K: f64 = 0.552_284_749_830_793_4;
+        let (kx, ky) = (rx * K, ry * K);
+        let mut p = BezPath::new();
+        p.move_to(c + Vec2::new(rx, 0.0));
+        p.curve_to(c + Vec2::new(rx, ky), c + Vec2::new(kx, ry), c + Vec2::new(0.0, ry));
+        p.curve_to(c + Vec2::new(-kx, ry), c + Vec2::new(-rx, ky), c + Vec2::new(-rx, 0.0));
+        p.curve_to(c + Vec2::new(-rx, -ky), c + Vec2::new(-kx, -ry), c + Vec2::new(0.0, -ry));
+        p.curve_to(c + Vec2::new(kx, -ry), c + Vec2::new(rx, -ky), c + Vec2::new(rx, 0.0));
+        p
+    };
     if full && inner <= 0.0 {
-        return whole(rx, ry).to_path(tol);
+        return whole(rx, ry);
     }
     if full {
         // A ring: outer clockwise, inner counter-clockwise.
-        path.extend(whole(rx, ry).path_elements(tol));
-        let inner_path = whole(rx * inner, ry * inner).to_path(tol);
-        path.extend(inner_path.reverse_subpaths());
+        path.extend(whole(rx, ry).elements().iter().copied());
+        let inner_path = whole(rx * inner, ry * inner);
+        path.extend(inner_path.reverse_subpaths().elements().iter().copied());
         return path;
     }
     let outer = kurbo::Arc::new(c, (rx, ry), start, sweep, 0.0);
diff --git a/crates/geom/src/tests.rs b/crates/geom/src/tests.rs
index 459b2b101..909a3c475 100644
--- a/crates/geom/src/tests.rs
+++ b/crates/geom/src/tests.rs
@@ -263,7 +263,7 @@ fn smoothed_polygon_smooths_every_corner() {
 #[test]
 fn ellipse_ring_area() {
     let full = area(&ellipse(100.0, 100.0, 0.0, TAU, 0.0));
-    assert!((full - PI * 2500.0).abs() < 2.0);
+    assert!((full - PI * 2500.0).abs() < 5.0, "{full}");
     let ring = ellipse(100.0, 100.0, 0.0, TAU, 0.5);
     // Winding area of a ring with a reversed inner contour: outer - inner.
     let a = ring.area().abs();
@@ -321,6 +321,47 @@ fn reverse_subpaths_preserves_open_and_closed() {
     assert!(matches!(r.elements().last(), Some(kurbo::PathEl::ClosePath)), "{:?}", r.elements());
 }
 
+fn cubics(path: &BezPath) -> Vec<kurbo::CubicBez> {
+    path.segments()
+        .filter_map(|s| match s {
+            kurbo::PathSeg::Cubic(c) => Some(c),
+            _ => None,
+        })
+        .collect()
+}
+
+#[test]
+fn a_whole_ellipse_is_four_cubics_through_the_extremes() {
+    for (w, h) in [(100.0, 100.0), (200.0, 60.0), (3.0, 900.0)] {
+        let segs = cubics(&ellipse(w, h, 0.0, TAU, 0.0));
+        assert_eq!(segs.len(), 4, "{w}x{h}");
+        let (rx, ry) = (w / 2.0, h / 2.0);
+        let want = [Point::new(rx, h), Point::new(0.0, ry), Point::new(rx, 0.0), Point::new(w, ry)];
+        for (i, s) in segs.iter().enumerate() {
+            assert!((s.p3 - want[i]).hypot() < 1e-9, "{w}x{h}: anchor {i} is {:?}", s.p3);
+            // Handle length is 0.5523 of the radius along the tangent.
+            let k = 0.552_284_749_830_793_4;
+            let (r_along, r_across) = if i % 2 == 0 { (ry, rx) } else { (rx, ry) };
+            let _ = r_across;
+            assert!(((s.p1 - s.p0).hypot() - k * r_along).abs() < 1e-9, "{w}x{h}: handle {i}");
+        }
+    }
+}
+
+#[test]
+fn a_whole_ring_is_four_cubics_per_contour() {
+    let segs = cubics(&ellipse(100.0, 100.0, 0.0, TAU, 0.5));
+    assert_eq!(segs.len(), 8);
+}
+
+#[test]
+fn a_sliced_ellipse_still_builds_its_arc() {
+    let pie = ellipse(100.0, 100.0, 0.0, PI, 0.0);
+    assert!(!cubics(&pie).is_empty());
+    assert!(matches!(pie.elements().last(), Some(kurbo::PathEl::ClosePath)));
+    assert!((area(&pie) - PI * 1250.0).abs() < 2.0);
+}
+
 #[test]
 fn a_huge_ellipse_arc_is_a_few_cubics() {
     let big = ellipse(1e300, 1e300, 0.3, 2.0, 0.5);