Merge pull request #4841 from dotCooCoo/fix/dxf-dxf-ellipse

DXF export: ellipses and unevenly scaled circles are written as ELLIPSE

crates/cad/src/scene.rs

@@ -10,7 +10,7 @@ use kurbo::PathEl;
 use vectorcraft_color::Paint;
 use vectorcraft_doc::{AppearanceItem, Dash, Document, ImageObject, Node, NodeKind, StrokeLayer, SymbolBoxes, TextKind, TextObject, Unit};
 use vectorcraft_effects::stroke;
-use vectorcraft_geom::{Affine, BezPath, FillRule, PathData, Point, Rect};
+use vectorcraft_geom::{Affine, BezPath, FillRule, PathData, Point, Rect, Vec2};
 
 use crate::aci::nearest_aci;
 use crate::writer::{Codes, Drawing, EntityStyle, Handle, ImageDef, Layer, LineType, fixed};
@@ -184,6 +184,46 @@ fn circular_subpath(sub: &Sub) -> Option<CircularSub> {
     Some(CircularSub { centre, radius, samples })
 }
 
+/// A closed subpath that is the four-cubic circle under an affine map: an ellipse, or a circle
+/// scaled out of proportion. `u` and `v` are the conjugate semi-diameters through its first two
+/// anchors.
+struct EllipticalSub {
+    centre: Point,
+    u: Vec2,
+    v: Vec2,
+}
+
+/// Fit a closed subpath of four cubics to one ellipse. Its anchors give the center and two
+/// conjugate semi-diameters; the subpath mapped by their inverse must pass [`circular_subpath`] as
+/// the unit circle, so the same tolerance and turn checks apply.
+fn elliptical_subpath(sub: &Sub) -> Option<EllipticalSub> {
+    if !sub.closed || sub.segs.len() != 4 {
+        return None;
+    }
+    let (a0, a1, a2) = (sub.start, sub.segs.first()?.end(), sub.segs.get(1)?.end());
+    let centre = a0.midpoint(a2);
+    let (u, v) = (a0 - centre, a1 - centre);
+    let det = u.cross(v);
+    let scale2 = u.hypot2().max(v.hypot2());
+    if !det.is_finite() || !scale2.is_finite() || det.abs() <= scale2 * 1e-9 {
+        return None;
+    }
+    let unit = |p: Point| {
+        let d = p - centre;
+        Point::new(d.cross(v) / det, u.cross(d) / det)
+    };
+    let mut segs = Vec::with_capacity(4);
+    for seg in &sub.segs {
+        let Seg::Cubic(a, b, to) = *seg else { return None };
+        segs.push(Seg::Cubic(unit(a), unit(b), unit(to)));
+    }
+    let circle = circular_subpath(&Sub { start: unit(sub.start), segs, closed: true })?;
+    if circle.centre.to_vec2().hypot() > 5e-4 || (circle.radius - 1.0).abs() > 5e-4 {
+        return None;
+    }
+    Some(EllipticalSub { centre, u, v })
+}
+
 fn angle_deg(p: Point, centre: Point) -> f64 {
     (p.y - centre.y).atan2(p.x - centre.x).to_degrees().rem_euclid(360.0)
 }
@@ -764,6 +804,8 @@ impl<'a> Scene<'a> {
                 // Native circles/arcs are preferred by CAD/CAM software. In R12, however, a
                 // stroked curve must stay on the flattened POLYLINE path so groups 40/41 retain
                 // its width. Non-circular curves continue through the existing fallback.
+            } else if v.handles() && self.elliptical(&sub, style) {
+                // R13 and later write an ellipse as a native ELLIPSE.
             } else if v.handles() {
                 self.spline(&sub, style);
             } else {
@@ -901,6 +943,53 @@ impl<'a> Scene<'a> {
         true
     }
 
+    /// Write a recognized elliptical subpath as a native DXF ELLIPSE (R13 and later): its center,
+    /// the end of its major axis relative to the center, and the minor to major ratio. Returns
+    /// false when the curve is not an ellipse, leaving the caller to write it as a spline.
+    fn elliptical(&mut self, sub: &Sub, style: &EntityStyle) -> bool {
+        let Some(last) = sub.segs.last().map(|s| s.end()) else { return false };
+        if !sub.closed || last.distance(sub.start) > 1e-9 {
+            return false;
+        }
+        let Some(ellipse) = elliptical_subpath(sub) else { return false };
+        let centre = self.to_dxf * ellipse.centre;
+        let u = self.to_dxf * (ellipse.centre + ellipse.u) - centre;
+        let v = self.to_dxf * (ellipse.centre + ellipse.v) - centre;
+        // The semi-axes are the square roots of the eigenvalues of [[a, b], [b, d]] = u u^T + v v^T.
+        let (a, b, d) = (u.x * u.x + v.x * v.x, u.x * u.y + v.x * v.y, u.y * u.y + v.y * v.y);
+        let mean = (a + d) / 2.0;
+        let spread = ((a - d) / 2.0).hypot(b);
+        let (major, minor) = ((mean + spread).sqrt(), (mean - spread).max(0.0).sqrt());
+        let angle = 0.5 * (2.0 * b).atan2(a - d);
+        let axis = Vec2::new(major * angle.cos(), major * angle.sin());
+        let ratio = (minor / major).min(1.0);
+        // A thinner ellipse stays a spline: DXF readers such as ezdxf need a ratio of 1e-6 or more.
+        if !centre.x.is_finite() || !centre.y.is_finite() || !axis.x.is_finite() || !axis.y.is_finite() || !ratio.is_finite() || ratio < 1e-6 {
+            return false;
+        }
+
+        // The drawing extents take the ellipse's bounding box: half-widths hypot(u.x, v.x) and
+        // hypot(u.y, v.y) about the center.
+        let (c, eu, ev) = (ellipse.centre, ellipse.u, ellipse.v);
+        let (hx, hy) = (eu.x.hypot(ev.x), eu.y.hypot(ev.y));
+        for p in [Point::new(c.x - hx, c.y), Point::new(c.x + hx, c.y), Point::new(c.x, c.y - hy), Point::new(c.x, c.y + hy)] {
+            self.pt(p);
+        }
+
+        let layer = self.layer.clone();
+        let h = self.handle();
+        self.e.entity("ELLIPSE", h, fixed::MODEL_RECORD, &layer, style, "AcDbEllipse");
+        self.e.xyz(10, centre.x, centre.y);
+        self.e.xyz(11, axis.x, axis.y);
+        self.e.normal();
+        // The ratio and the end parameter are written in full: rounded to six decimals, a thin
+        // ellipse's minor axis would change size, and a whole turn would read back as an open curve.
+        self.e.raw(40, &ratio.to_string());
+        self.e.num(41, 0.0);
+        self.e.raw(42, &std::f64::consts::TAU.to_string());
+        true
+    }
+
     /// A curved subpath as one cubic B-spline whose clamped knots repeat at every joint, so it
     /// passes through every anchor (straight segments become straight cubics).
     fn spline(&mut self, sub: &Sub, style: &EntityStyle) {

crates/cad/src/tests.rs

@@ -4,7 +4,7 @@ use std::sync::Arc;
 
 use vectorcraft_color::{Color, Paint};
 use vectorcraft_doc::{Appearance, CharStyle, Dash, Document, ImageBlob, ImageObject, Node, NodeKind, TextObject, Unit};
-use vectorcraft_geom::{Affine, PathData, Point, Rect, shapes};
+use vectorcraft_geom::{Affine, ParamCurve, PathData, Point, Rect, Shape, shapes};
 
 use crate::*;
 
@@ -208,10 +208,129 @@ fn circles_and_circular_arcs_are_native_dxf_entities() {
         assert_eq!((all(&e[1], 10), all(&e[1], 20), all(&e[1], 40)), (vec![120.0], vec![50.0], vec![30.0]), "{version:?}");
         assert_eq!((all(&e[1], 50), all(&e[1], 51)), (vec![0.0], vec![90.0]), "{version:?}");
     }
+}
 
-    // An actual ellipse is not close enough to a circle and remains a spline.
-    let ellipse = doc_with(|d| vec![path(d, shapes::ellipse(Rect::new(20.0, 20.0, 100.0, 60.0)), Paint::None, Paint::solid(Color::BLACK), 1.0)]);
-    assert_eq!(kinds(&export(&ellipse, &opts(&ellipse)).unwrap()), ["SPLINE"]);
+/// The path `p` with `xf` applied.
+fn transformed(p: &PathData, xf: Affine) -> PathData {
+    PathData::from_bezpath(&(xf * p.to_bezpath()))
+}
+
+fn close_to(a: &[f64], b: &[f64], tol: f64) -> bool {
+    a.len() == b.len() && a.iter().zip(b).all(|(x, y)| (x - y).abs() <= tol)
+}
+
+#[test]
+fn ellipses_and_unevenly_scaled_circles_are_native_ellipses() {
+    let circle = shapes::ellipse(Rect::new(20.0, 20.0, 80.0, 80.0));
+    let d = doc_with(|d| {
+        let stroke = || Paint::solid(Color::BLACK);
+        vec![
+            // 80 × 40 pt about (60, 40): DXF center (60, 60), major axis (40, 0), ratio 0.5.
+            path(d, shapes::ellipse(Rect::new(20.0, 20.0, 100.0, 60.0)), Paint::None, stroke(), 1.0),
+            // A 60 pt circle scaled 150% across about (50, 50): 90 × 60 pt, ratio 2/3.
+            path(
+                d,
+                transformed(&circle, Affine::translate((50.0, 50.0)) * Affine::scale_non_uniform(1.5, 1.0) * Affine::translate((-50.0, -50.0))),
+                Paint::None,
+                stroke(),
+                1.0,
+            ),
+            // The first ellipse turned 30° about its center, filled: the outline is the ellipse too.
+            path(
+                d,
+                transformed(&shapes::ellipse(Rect::new(20.0, 20.0, 100.0, 60.0)), Affine::rotate_about(30f64.to_radians(), Point::new(60.0, 40.0))),
+                Paint::solid(Color::BLACK),
+                Paint::None,
+                0.0,
+            ),
+        ]
+    });
+    for version in DxfVersion::ALL.into_iter().filter(|version| *version >= DxfVersion::R13) {
+        let out = export(&d, &DxfOptions { version, ..opts(&d) }).unwrap();
+        let e = entities(&out);
+        assert_eq!(e.iter().map(|x| kind(x)).collect::<Vec<_>>(), ["ELLIPSE", "ELLIPSE", "ELLIPSE", "HATCH"], "{version:?}");
+        let ellipse = |x: &Pairs| (all(x, 10), all(x, 20), all(x, 11), all(x, 21), all(x, 40), all(x, 41), all(x, 42));
+        let (cx, cy, mx, my, ratio, start, end) = ellipse(&e[0]);
+        assert_eq!((cx, cy, mx, my, ratio), (vec![60.0], vec![60.0], vec![40.0], vec![0.0], vec![0.5]), "{version:?}");
+        assert!(close_to(&start, &[0.0], 1e-9) && close_to(&end, &[std::f64::consts::TAU], 1e-12), "{version:?}: a whole turn");
+        let (cx, cy, mx, my, ratio, ..) = ellipse(&e[1]);
+        assert!(close_to(&[cx, cy, mx, my, ratio].concat(), &[50.0, 50.0, 45.0, 0.0, 2.0 / 3.0], 1e-6), "{version:?}: {:?}", e[1]);
+        // Turned 30° counterclockwise on screen, so clockwise with DXF's y up.
+        let (cx, cy, mx, my, ratio, ..) = ellipse(&e[2]);
+        let (cos, sin) = (30f64.to_radians().cos(), 30f64.to_radians().sin());
+        assert!(close_to(&[cx, cy, mx, my, ratio].concat(), &[60.0, 60.0, 40.0 * cos, -40.0 * sin, 0.5], 1e-6), "{version:?}: {:?}", e[2]);
+        assert!(e[0].contains(&(100, "AcDbEllipse".into())), "{version:?}");
+    }
+    // R12 has no ELLIPSE: the curves are flattened polylines.
+    let r12 = kinds(&export(&d, &DxfOptions { version: DxfVersion::R12, ..opts(&d) }).unwrap());
+    assert!(!r12.iter().any(|k| k == "ELLIPSE") && r12.iter().any(|k| k == "POLYLINE"), "{r12:?}");
+    // A thin ellipse keeps its ratio in full (six decimals would write 0.000002).
+    let thin = doc_with(|d| vec![path(d, shapes::ellipse(Rect::new(0.0, 50.0, 200.0, 50.0003)), Paint::None, Paint::solid(Color::BLACK), 1.0)]);
+    let e = entities(&export(&thin, &opts(&thin)).unwrap());
+    assert_eq!(kind(&e[0]), "ELLIPSE");
+    assert!(close_to(&all(&e[0], 40), &[1.5e-6], 1e-10), "{:?}", e[0]);
+}
+
+#[test]
+fn exported_ellipses_read_back_as_the_same_ellipse() {
+    let xf = Affine::rotate_about(30f64.to_radians(), Point::new(60.0, 40.0));
+    let d = doc_with(|d| {
+        vec![path(d, transformed(&shapes::ellipse(Rect::new(20.0, 20.0, 100.0, 60.0)), xf), Paint::None, Paint::solid(Color::BLACK), 1.0)]
+    });
+    let out = export(&d, &opts(&d)).unwrap();
+    assert_eq!(kinds(&out), ["ELLIPSE"]);
+    let o = ImportOptions { unit: Some(Unit::Points), scale: Some(1.0), center: false, ..ImportOptions::default() };
+    let back = import(&out.bytes, &o).unwrap().document;
+    let mut paths = vec![];
+    back.walk(|n| {
+        if let NodeKind::Path { path, .. } = &n.kind {
+            paths.push(path.to_bezpath());
+        }
+    });
+    assert_eq!(paths.len(), 1);
+    assert!(paths[0].elements().last().is_some_and(|el| matches!(el, kurbo::PathEl::ClosePath)), "the ellipse reads back closed");
+    // Every point of the curve read back lies on the 80 × 40 pt ellipse turned 30°, once both are centered.
+    let bp = &paths[0];
+    let found = bp.bounding_box().center();
+    let inverse = xf.inverse();
+    let original = Point::new(60.0, 40.0);
+    for seg in bp.segments() {
+        for i in 0..=16 {
+            let p = seg.eval(f64::from(i) / 16.0) - found.to_vec2() + original.to_vec2();
+            let q = inverse * p;
+            let r = ((q.x - 60.0) / 40.0).hypot((q.y - 40.0) / 20.0);
+            assert!((r - 1.0).abs() < 1e-3, "{p:?} is off the ellipse ({r})");
+        }
+    }
+}
+
+#[test]
+fn closed_four_cubic_shapes_that_are_not_writable_ellipses_stay_splines() {
+    // A rounded square: four cubics through the ellipse's anchors with longer handles.
+    let (c, r, k) = (Point::new(50.0, 50.0), 30.0, 30.0 * 0.8);
+    let mut squircle = vectorcraft_geom::BezPath::new();
+    squircle.move_to(Point::new(c.x + r, c.y));
+    squircle.curve_to(Point::new(c.x + r, c.y - k), Point::new(c.x + k, c.y - r), Point::new(c.x, c.y - r));
+    squircle.curve_to(Point::new(c.x - k, c.y - r), Point::new(c.x - r, c.y - k), Point::new(c.x - r, c.y));
+    squircle.curve_to(Point::new(c.x - r, c.y + k), Point::new(c.x - k, c.y + r), Point::new(c.x, c.y + r));
+    squircle.curve_to(Point::new(c.x + k, c.y + r), Point::new(c.x + r, c.y + k), Point::new(c.x + r, c.y));
+    squircle.close_path();
+    // An egg: one half wider than the other.
+    let egg = shapes::ellipse(Rect::new(110.0, 20.0, 190.0, 60.0)).to_bezpath();
+    let mut els: Vec<kurbo::PathEl> = egg.elements().to_vec();
+    if let Some(kurbo::PathEl::CurveTo(a, b, _)) = els.get_mut(1) {
+        a.x -= 10.0;
+        b.x -= 10.0;
+    }
+    let d = doc_with(|d| {
+        vec![
+            path(d, PathData::from_bezpath(&squircle), Paint::None, Paint::solid(Color::BLACK), 1.0),
+            path(d, PathData::from_bezpath(&kurbo::BezPath::from_vec(els)), Paint::None, Paint::solid(Color::BLACK), 1.0),
+            // An ellipse thinner than the smallest ratio DXF readers take (5e-7 against 1e-6).
+            path(d, shapes::ellipse(Rect::new(0.0, 90.0, 200.0, 90.0001)), Paint::None, Paint::solid(Color::BLACK), 1.0),
+        ]
+    });
+    assert_eq!(kinds(&export(&d, &opts(&d)).unwrap()), ["SPLINE", "SPLINE", "SPLINE"]);
 }
 
 #[test]

docs/gaps.md

@@ -254,7 +254,7 @@ from G14 and the budget run of G15: **~170–270 h** one agent, ~50–80 h wall
 
 - **Done:** Numeric fields keep a text drag's start when the pointer gains focus and movement exceeds the click threshold (#1326, in part). A completed focus click selects the whole value; keyboard focus, double-click and numeric stepping remain regression-tested. Short movements still classified as clicks can select all; that case remains open.
 
-- **Done:** DXF reads control-point SPLINE closure (#1299 follow-up after #1320); open curves keep their open state. DXF export now recognizes circular cubic paths conservatively and writes native CIRCLE/ARC where stroke semantics permit; R12 weighted strokes remain POLYLINE, and a non-circular ellipse keeps the existing fallback. CNC preset and VCarve QA remain open. Escape leaves Artboard editing for Selection; during a drag it first cancels the gesture. Headless egui key events cover idle Escape, active-drag cancellation and a subsequent Escape in `tests/distortkeys.rs`.
+- **Done:** DXF reads control-point SPLINE closure (#1299 follow-up after #1320); open curves keep their open state. DXF export now recognizes circular cubic paths conservatively and writes native CIRCLE/ARC where stroke semantics permit; R12 weighted strokes remain POLYLINE. A closed four-cubic ellipse, or a circle scaled out of proportion, is written as a native ELLIPSE in R13 and later; R12 keeps the existing fallback. CNC preset and VCarve QA remain open. Escape leaves Artboard editing for Selection; during a drag it first cancels the gesture. Headless egui key events cover idle Escape, active-drag cancellation and a subsequent Escape in `tests/distortkeys.rs`.
 
 - **Done:** Simplify and Add Anchor Points clear stale component selections on rewritten paths; Simplify previews and reports original/new point counts (#1272).
 - **Done:** Object › Path › Simplify opens its slider at the Auto-Simplify level for the selection and previews that result, as the [manual-simplify page](https://helpx.adobe.com/illustrator/desktop/draw-shapes-and-paths/modify-paths/manually-simplify-paths.html) describes; simplifying only a Direct Selection part of a path is done too (G1).