SPLINE Close is periodic, options report; POLYGON typed radius has a flat bottom; CIRCLE rejects bad input (fixes #238) (#240)

SPLINE Close now builds a smooth closed (periodic) curve through the fit points
instead of repeating the first point. The geometry gets Spline::from_fit_points_closed:
chord-length parameters including the closing chord, a periodic knot vector, and the
first three control points repeated at the end. The fit data no longer duplicates the
first point. DXF writes group 70 bit 2 (periodic) for it, and a file whose closed spline
needs refitting is refitted closed. SPLINEDIT Close/refit/move and fit-point grips keep
a closed spline periodic.

SPLINE Method asks Fit/CV, and CV continues as SPLINE by control vertices. Knots, Object,
start/end Tangency and toLerance report "not available yet" and stay at the prompt.
Garbage text reports "Point or option keyword required.".

POLYGON with a typed radius puts the bottom edge horizontal for Inscribed and for
Circumscribed. The JSON form uses the same orientation when no angle is given.

CIRCLE reports "Point or option keyword required." for an invalid token at the centre
prompt, and "Invalid point." at the 3P/2P point prompts, then asks again.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Valentin Klein
2026-10-10 15:34:24 -04:00
committed by GitHub
co-authored by Claude Opus 5.5
parent afe8b93255
commit c55feb79da
11 changed files with 278 additions and 33 deletions
+62 -17
View File
@@ -40,7 +40,7 @@ pub fn specs() -> Vec<CommandSpec> {
CommandSpec::new("polygon", "Polygon", run_polygon)
.menu(&["Draw", "Polygon"])
.alias(&["pol"])
.params("{sides, center, radius, inscribed?: bool, angle?} | {sides, edge: [[x,y],[x,y]]}")
.params("{sides, center, radius, inscribed?: bool, angle?: deg to a vertex (inscribed) or edge midpoint (default: bottom edge horizontal)} | {sides, edge: [[x,y],[x,y]]}")
.interactive(|_| Ok(Box::new(PolygonM::default()))),
CommandSpec::new("ellipse", "Ellipse", run_ellipse)
.menu(&["Draw", "Ellipse", "Center"])
@@ -189,14 +189,13 @@ fn run_polygon(s: &mut Session, p: &Value) -> Result<Value> {
} else {
let c = point_req("polygon", p, "center")?;
let r = f64_req("polygon", p, "radius")?;
let ang = f64_or(p, "angle", 90.0).to_radians();
let inscribed = bool_or(p, "inscribed", true);
polygon_vertices(
c,
n,
Vec2::polar(c, r, if inscribed { ang } else { ang - std::f64::consts::PI / n as f64 + std::f64::consts::PI / n as f64 }),
inscribed,
)
// Without an angle, the bottom edge is horizontal, as with a typed radius.
let rp = match p.get("angle").and_then(Value::as_f64).filter(|a| a.is_finite()) {
Some(a) => Vec2::polar(c, r, a.to_radians()),
None => polygon_typed_radius_point(c, n, r, inscribed),
};
polygon_vertices(c, n, rp, inscribed)
};
added(s.add_entity(lwpoly(vs, true))?)
}
@@ -737,6 +736,9 @@ impl Interactive for CircleM {
}
}
Input::Enter => Ok(Step::Cancel),
// Garbage at a point prompt: report it and ask again.
Input::Text(_) if self.mode == 0 && self.pts.is_empty() => Err(crate::EngineError::Other("Point or option keyword required.".into())),
Input::Text(_) => Err(crate::EngineError::Other("Invalid point.".into())),
_ => Ok(Step::Continue),
}
}
@@ -833,7 +835,7 @@ impl Interactive for PolygonM {
}
(Some(c), Some(ins), Input::Text(t)) => {
let r = number(&t).ok_or_else(|| crate::EngineError::Other("Requires numeric distance or point.".into()))?;
let rp = if ins { c + Vec2::Y * r } else { c - Vec2::Y * r };
let rp = polygon_typed_radius_point(c, n, r, ins);
s.add_entity(lwpoly(polygon_vertices(c, n, rp, ins), true))?;
return Ok(Step::Done);
}
@@ -1128,13 +1130,26 @@ impl Interactive for XlineM {
#[derive(Default)]
struct SplineM {
pts: Vec<Vec2>,
asking_method: bool,
/// `Method` > `CV` hands over to the control-vertex machine (`spline.cv`).
delegate: Option<Box<dyn Interactive>>,
}
impl Interactive for SplineM {
fn name(&self) -> &'static str {
"SPLINE"
}
fn prompt(&self, _s: &Session) -> Prompt {
fn begin(&mut self, s: &mut Session) -> Result<Step> {
s.echo("Current settings: Method=Fit Knots=Chord");
Ok(Step::Continue)
}
fn prompt(&self, s: &Session) -> Prompt {
if let Some(d) = &self.delegate {
return d.prompt(s);
}
if self.asking_method {
return Prompt::new("Enter spline creation method", super::curves::KW).kw(&["Fit", "CV"]).default("Fit");
}
match self.pts.len() {
0 => Prompt::new("Specify first point", Accept::POINT).kw(&["Method", "Knots", "Object"]),
1 => Prompt::new("Enter next point", Accept::POINT).kw(&["start Tangency", "toLerance"]).base_opt(self.pts.last().copied()),
@@ -1145,6 +1160,27 @@ impl Interactive for SplineM {
}
}
fn input(&mut self, s: &mut Session, i: Input) -> Result<Step> {
if let Some(d) = &mut self.delegate {
return d.input(s, i);
}
if self.asking_method {
match i {
Input::Keyword(k) if k == "CV" => {
let Some(factory) = find_command("spline.cv").and_then(|c| c.interactive) else {
return Err(crate::EngineError::Other("CV: not available yet".into()));
};
let mut m = factory(s)?;
self.asking_method = false;
let step = m.begin(s)?;
self.delegate = Some(m);
return Ok(step);
}
Input::Keyword(_) | Input::Enter => self.asking_method = false,
Input::Text(_) => return Err(crate::EngineError::Other("Invalid option keyword.".into())),
_ => {}
}
return Ok(Step::Continue);
}
match i {
Input::Point(p) => {
self.pts.push(p);
@@ -1155,24 +1191,33 @@ impl Interactive for SplineM {
Ok(Step::Continue)
}
Input::Keyword(k) if k == "Close" => {
if let Some(f) = self.pts.first().copied() {
self.pts.push(f);
}
let mut sp = Spline::from_fit_points(&self.pts);
sp.closed = true;
s.add_entity(EntityKind::Spline(sp))?;
// A smooth closed (periodic) curve, not a repeat of the first point.
s.add_entity(EntityKind::Spline(Spline::from_fit_points_closed(&self.pts)))?;
Ok(Step::Done)
}
Input::Keyword(k) if k == "Method" => {
self.asking_method = true;
Ok(Step::Continue)
}
Input::Keyword(k) => {
// Knots, Object, start/end Tangency, toLerance: stay at this prompt.
s.echo(format!("{k}: not available yet"));
Ok(Step::Continue)
}
Input::Enter => {
if self.pts.len() >= 2 {
s.add_entity(EntityKind::Spline(Spline::from_fit_points(&self.pts)))?;
}
Ok(Step::Done)
}
Input::Text(_) => Err(crate::EngineError::Other("Point or option keyword required.".into())),
_ => Ok(Step::Continue),
}
}
fn preview(&self, _s: &Session, c: Vec2) -> Vec<EntityKind> {
fn preview(&self, s: &Session, c: Vec2) -> Vec<EntityKind> {
if let Some(d) = &self.delegate {
return d.preview(s, c);
}
if self.pts.is_empty() {
return Vec::new();
}
+9
View File
@@ -38,6 +38,15 @@ pub fn polygon_vertices(center: Vec2, sides: usize, radius_pt: Vec2, inscribed:
(0..n).map(|i| PolyVertex::new(Vec2::polar(center, r, a0 + step * i as f64))).collect()
}
/// The radius point for a polygon whose radius is typed: the bottom edge is horizontal in both
/// modes (inscribed: a vertex half a side's angle past straight down; circumscribed: the bottom
/// edge's midpoint straight down).
pub fn polygon_typed_radius_point(center: Vec2, sides: usize, radius: f64, inscribed: bool) -> Vec2 {
let down = -std::f64::consts::FRAC_PI_2;
let a = if inscribed { down + std::f64::consts::PI / sides.clamp(3, 1024) as f64 } else { down };
Vec2::polar(center, radius, a)
}
/// Polygon from an edge (first two vertices).
pub fn polygon_from_edge(p1: Vec2, p2: Vec2, sides: usize) -> Vec<PolyVertex> {
let n = sides.clamp(3, 1024);
+3 -9
View File
@@ -649,9 +649,7 @@ fn spline_close(sp: &Spline) -> Spline {
return sp.clone();
}
let mut out = if sp.fit.len() >= 2 {
let mut f = sp.fit.clone();
f.extend(sp.fit.first().copied());
Spline::from_fit_points(&f)
Spline::from_fit_points_closed(&sp.fit)
} else {
let mut c = sp.control.clone();
c.extend(sp.control.first().copied());
@@ -704,9 +702,7 @@ fn run_splinedit(s: &mut Session, p: &Value) -> Result<Value> {
if fit.len() < 2 || fit.len() > MAX_GEN {
return Err(other("The spline has no fit data; give `fit` points."));
}
let mut n = Spline::from_fit_points(&fit);
n.closed = sp.closed;
EntityKind::Spline(n)
EntityKind::Spline(Spline::from_fit(&fit, sp.closed))
}
"polyline" => EntityKind::LwPolyline(spline_to_poly(sp, p.get("precision").and_then(Value::as_u64).unwrap_or(10) as usize)),
"move" => {
@@ -716,9 +712,7 @@ fn run_splinedit(s: &mut Session, p: &Value) -> Result<Value> {
if !n.fit.is_empty() {
let f = n.fit.get_mut(i).ok_or_else(|| bad("splinedit", "index out of range"))?;
*f = to;
let closed = n.closed;
n = Spline::from_fit_points(&n.fit);
n.closed = closed;
n = Spline::from_fit(&n.fit, n.closed);
} else {
let c = n.control.get_mut(i).ok_or_else(|| bad("splinedit", "index out of range"))?;
*c = to;
+1 -1
View File
@@ -151,7 +151,7 @@ fn splinedit_json_and_command_line() {
let mut s = Session::new();
let sp = h(&s.execute("spline", &json!({"fit": [[0, 0], [2, 2], [4, 0], [6, 2]]})).unwrap());
s.execute("splinedit", &json!({"handle": sp, "option": "close"})).unwrap();
assert!(matches!(kind(&s, &sp), EntityKind::Spline(x) if x.closed && x.fit.len() == 5));
assert!(matches!(kind(&s, &sp), EntityKind::Spline(x) if x.closed && x.is_periodic() && x.fit.len() == 4));
s.execute("splinedit", &json!({"handle": sp, "option": "open"})).unwrap();
assert!(matches!(kind(&s, &sp), EntityKind::Spline(x) if !x.closed && x.fit.len() == 4));
s.execute("splinedit", &json!({"handle": sp, "option": "reverse"})).unwrap();
+1 -4
View File
@@ -181,10 +181,7 @@ pub fn stretch_grip(kind: &EntityKind, index: usize, to: Vec2) -> Option<EntityK
} else {
let mut fit = sp.fit.clone();
*fit.get_mut(index)? = to;
let closed = sp.closed;
let mut n = Spline::from_fit_points(&fit);
n.closed = closed;
*sp = n;
*sp = Spline::from_fit(&fit, sp.closed);
}
}
EntityKind::Ray(r) | EntityKind::XLine(r) => {
+79
View File
@@ -0,0 +1,79 @@
//! Command-line behaviour of the SPLINE, POLYGON and CIRCLE prompt machines.
use cadcraft_doc::EntityKind;
use cadcraft_engine::Session;
use cadcraft_geom::Vec2;
fn last(s: &Session) -> EntityKind {
s.doc().unwrap().model.iter().last().map(|e| e.kind.clone()).unwrap()
}
fn logged(s: &Session, text: &str) -> bool {
s.log.iter().any(|l| l.contains(text))
}
#[test]
fn spline_close_makes_a_smooth_periodic_curve() {
let mut s = Session::new();
s.cmdline("spline 0,0 4,0 4,3 0,3 c").unwrap();
assert!(s.running.is_none());
let EntityKind::Spline(sp) = last(&s) else { panic!("no spline") };
assert!(sp.closed && sp.is_periodic());
assert_eq!(sp.fit.len(), 4, "the first point is not repeated");
let (lo, hi) = sp.domain();
assert!(sp.eval(lo).near(Vec2::ZERO, 1e-9) && sp.eval(hi).near(Vec2::ZERO, 1e-9));
let h = 1e-6;
let out = (sp.eval(lo + h) - sp.eval(lo)).normalized();
let back = (sp.eval(hi) - sp.eval(hi - h)).normalized();
assert!(out.near(back, 1e-4), "no corner at the seam");
}
#[test]
fn spline_options_report_and_stay_at_the_prompt() {
let mut s = Session::new();
s.cmdline("spline k o 0,0 t 5,5 l 10,0 xyz").unwrap();
for k in ["Knots: not available yet", "Object: not available yet", "start Tangency: not available yet", "toLerance: not available yet"] {
assert!(logged(&s, k), "{k}");
}
assert!(logged(&s, "Point or option keyword required."));
s.cmdline("").unwrap();
let EntityKind::Spline(sp) = last(&s) else { panic!("no spline") };
assert_eq!(sp.fit, [Vec2::ZERO, Vec2::new(5.0, 5.0), Vec2::new(10.0, 0.0)], "no point was swallowed");
// Method > CV switches to drawing by control vertices.
s.cmdline("spline m cv 0,0 1,1 2,0 3,1").unwrap();
s.cmdline("").unwrap();
let EntityKind::Spline(sp) = last(&s) else { panic!("no spline") };
assert!(sp.fit.is_empty() && sp.control.len() == 4);
}
/// The two lowest vertices of a closed polyline share their y (the bottom edge is horizontal).
fn bottom_y(k: &EntityKind) -> Option<f64> {
let EntityKind::LwPolyline(p) = k else { return None };
let mut ys: Vec<f64> = p.vertices.iter().map(|v| v.p.y).collect();
ys.sort_by(f64::total_cmp);
((ys[0] - ys[1]).abs() < 1e-9).then_some(ys[0])
}
#[test]
fn polygon_typed_radius_has_a_horizontal_bottom_edge() {
let mut s = Session::new();
let pentagon = -2.0 * (std::f64::consts::PI / 5.0).cos();
for (input, low) in
[("polygon 6 0,0 i 2", -(3f64.sqrt())), ("polygon 6 0,0 c 2", -2.0), ("polygon 4 0,0 i 2", -(2f64.sqrt())), ("polygon 5 0,0 i 2", pentagon)]
{
s.cmdline(input).unwrap();
let y = bottom_y(&last(&s));
assert!(y.is_some_and(|y| (y - low).abs() < 1e-9), "{input}: bottom edge at {y:?}, want {low}");
}
}
#[test]
fn circle_center_prompt_rejects_garbage() {
let mut s = Session::new();
s.cmdline("circle d").unwrap();
assert!(logged(&s, "Point or option keyword required."));
assert!(s.running.is_some(), "still at the centre prompt");
s.cmdline("0,0 5").unwrap();
assert!(matches!(last(&s), EntityKind::Circle(c) if (c.radius - 5.0).abs() < 1e-9));
}
+73
View File
@@ -87,6 +87,79 @@ impl Spline {
Spline { degree, knots, control, weights: Vec::new(), fit: fit.to_vec(), closed: false }
}
/// Smooth closed (periodic) cubic interpolation through fit points. Chord-length parameters
/// include the closing chord; the knot vector is the periodic (unclamped) extension and the
/// first `degree` control points repeat at the end, so the curve has continuous tangent and
/// curvature at the seam. A repeated closing fit point is dropped.
pub fn from_fit_points_closed(fit: &[Vec2]) -> Spline {
let mut pts = fit.to_vec();
if pts.len() > 3 && pts.first().zip(pts.last()).is_some_and(|(a, b)| a.near(*b, 1e-12)) {
pts.pop();
}
let n = pts.len();
let fallback = |pts: &[Vec2]| {
let mut f = pts.to_vec();
f.extend(pts.first().copied());
let mut s = Spline::from_fit_points(&f);
s.fit = pts.to_vec();
s.closed = true;
s
};
let p = 3;
if n < p {
return fallback(&pts);
}
// Parameters t_0 = 0 .. t_n = 1 (t_n is the return to the first point).
let mut t = vec![0.0; n + 1];
let mut total = 0.0;
for i in 1..=n {
total += pts.get(i - 1).zip(pts.get(i % n)).map(|(a, b)| a.dist(*b)).unwrap_or(0.0).max(1e-12);
if let Some(x) = t.get_mut(i) {
*x = total;
}
}
for x in t.iter_mut() {
*x /= total;
}
// Periodic knots: knots[j] = t_(j-p), extended by whole periods on both sides.
let tk = |i: isize| {
let (q, r) = (i.div_euclid(n as isize), i.rem_euclid(n as isize) as usize);
t.get(r).copied().unwrap_or(0.0) + q as f64
};
let m = n + p;
let knots: Vec<f64> = (0..m + p + 1).map(|j| tk(j as isize - p as isize)).collect();
// Interpolation at t_i; control index k and k + n are the same unknown.
let mut a = vec![vec![0.0; n]; n];
for (i, row) in a.iter_mut().enumerate() {
let u = t.get(i).copied().unwrap_or(0.0);
let span = find_span(m - 1, p, u, &knots);
for (k, v) in basis(span, u, p, &knots).iter().enumerate() {
if let Some(cell) = row.get_mut((span + k - p) % n) {
*cell += *v;
}
}
}
let xs: Vec<f64> = pts.iter().map(|q| q.x).collect();
let ys: Vec<f64> = pts.iter().map(|q| q.y).collect();
let (Some(cx), Some(cy)) = (solve(a.clone(), xs), solve(a, ys)) else { return fallback(&pts) };
let mut control: Vec<Vec2> = cx.into_iter().zip(cy).map(|(x, y)| Vec2::new(x, y)).collect();
if control.iter().any(|c| !c.x.is_finite() || !c.y.is_finite()) {
return fallback(&pts);
}
control.extend_from_within(..p);
Spline { degree: p, knots, control, weights: Vec::new(), fit: pts, closed: true }
}
/// Interpolation through fit points, open or closed (see `from_fit_points_closed`).
pub fn from_fit(fit: &[Vec2], closed: bool) -> Spline {
if closed { Spline::from_fit_points_closed(fit) } else { Spline::from_fit_points(fit) }
}
/// Closed with an unclamped (periodic) knot vector, as DXF flags it (group 70 bit 2).
pub fn is_periodic(&self) -> bool {
self.closed && self.knots.first() != self.knots.get(self.degree)
}
pub fn is_valid(&self) -> bool {
let n = self.control.len();
n > self.degree && self.degree >= 1 && self.knots.len() == n + self.degree + 1 && self.knots.windows(2).all(|w| w.first() <= w.get(1))
+23
View File
@@ -123,6 +123,29 @@ fn spline_interpolates_fit_points() {
assert!(s.eval(1.0).near(fit[4], 1e-9));
}
#[test]
fn closed_spline_is_smooth_at_the_seam() {
let fit = [Vec2::new(0.0, 0.0), Vec2::new(4.0, 0.0), Vec2::new(4.0, 3.0), Vec2::new(0.0, 3.0)];
let s = Spline::from_fit_points_closed(&fit);
assert!(s.is_valid() && s.closed && s.is_periodic());
assert_eq!(s.fit.len(), 4, "the first fit point is not repeated");
let (lo, hi) = s.domain();
assert!(s.eval(lo).near(fit[0], 1e-9) && s.eval(hi).near(fit[0], 1e-9));
let pts = s.tessellate(1e-4);
for f in fit {
assert!(pts.iter().any(|p| p.dist(f) < 0.05), "fit point {f:?} missed");
}
// Same tangent leaving and arriving at the seam (an open fit through a repeated point has a corner there).
let h = 1e-6;
let out = (s.eval(lo + h) - s.eval(lo)).normalized();
let back = (s.eval(hi) - s.eval(hi - h)).normalized();
assert!(out.near(back, 1e-4), "kink at the seam: {out:?} vs {back:?}");
// A repeated closing point is dropped.
let mut dup = fit.to_vec();
dup.push(fit[0]);
assert_eq!(Spline::from_fit_points_closed(&dup).fit.len(), 4);
}
#[test]
fn ellipse_points() {
let e = Ellipse::full(Vec2::ZERO, Vec2::new(4.0, 0.0), 0.5);
+1 -1
View File
@@ -253,7 +253,7 @@ fn entity(kind: &str, tags: &[Tag]) -> Option<(Common, EntityKind)> {
let fit = t.pts(11);
let mut sp = Spline { degree, knots, control, weights, fit: fit.clone(), closed: t.i(70).unwrap_or(0) & 1 != 0 };
if !sp.is_valid() && fit.len() >= 2 {
sp = Spline::from_fit_points(&fit);
sp = Spline::from_fit(&fit, sp.closed);
} else if !sp.is_valid() && sp.control.len() >= 2 {
sp = Spline::from_control(sp.control.clone(), degree);
}
+1 -1
View File
@@ -467,7 +467,7 @@ fn entity(w: &mut W, d: &Drawing, e: &Entity, owner: &str, paper: bool, cx: &Ctx
EntityKind::Spline(s) => {
w.s(0, "SPLINE");
common(w, e, owner, paper, "AcDbSpline");
let flags = 8 | i64::from(s.closed) | if s.weights.is_empty() { 0 } else { 4 };
let flags = 8 | i64::from(s.closed) | if s.is_periodic() { 2 } else { 0 } | if s.weights.is_empty() { 0 } else { 4 };
w.i(70, flags);
w.i(71, s.degree as i64);
w.i(72, s.knots.len() as i64);
+25
View File
@@ -0,0 +1,25 @@
//! A closed fit-point spline is periodic: DXF flags it closed + periodic (group 70 bits 1 and 2)
//! and it reads back as the same smooth closed curve.
use cadcraft_doc::{Common, Drawing, EntityKind, Space};
use cadcraft_geom::{Spline, Vec2};
#[test]
fn closed_fit_spline_roundtrips_as_periodic() {
let fit = [Vec2::new(0.0, 0.0), Vec2::new(4.0, 0.0), Vec2::new(4.0, 3.0), Vec2::new(0.0, 3.0)];
let sp = Spline::from_fit_points_closed(&fit);
let mut d = Drawing::new_metric();
d.add(&Space::Model, Common::default(), EntityKind::Spline(sp.clone())).unwrap();
let bytes = cadcraft_io::write(&d, "a.dxf").unwrap();
let text = String::from_utf8(bytes.clone()).unwrap();
let spline = &text[text.find("SPLINE").unwrap()..];
assert!(spline.contains("70\r\n11\r\n"), "planar + closed + periodic");
let back = cadcraft_io::read(&bytes, "a.dxf").unwrap();
let Some(EntityKind::Spline(r)) = back.model.iter().map(|e| e.kind.clone()).next() else { panic!("no spline") };
assert!(r.closed && r.is_periodic());
assert_eq!(r.fit.len(), 4);
for i in 0..=20 {
let t = i as f64 / 20.0;
assert!(r.eval(t).near(sp.eval(t), 1e-9));
}
}