diff --git a/src/entities/helix.rs b/src/entities/helix.rs index aa753c6e3..fafc69e24 100644 --- a/src/entities/helix.rs +++ b/src/entities/helix.rs @@ -53,9 +53,16 @@ fn spline_curve(spline: &Spline) -> Option { fn axis_frame(helix: &Helix) -> Option<(Vec3, Vec3, Vec3)> { let base = point(helix.axis_base_point); let axis = point(helix.axis_vector).normalize()?; - let radial = point(helix.start_point) - base; - let start_direction = (radial - axis * radial.dot(axis)).normalize()?; - Some((base, axis, start_direction)) + let tangent = spline_curve(&helix.spline) + .map(|curve| Vec3::from(curve.tangent_at(0.0))) + .and_then(Vec3::normalize) + .or_else(|| perpendicular(axis))?; + let (axis, radial, _) = HelixCurve::frame_from_points(base.to_array(),point(helix.axis_vector).to_array(),point(helix.start_point).to_array(),tangent.to_array())?; + Some((base,Vec3::from(axis),Vec3::from(radial))) +} + +fn base_radius(helix: &Helix) -> Option { + radius_from_axis(helix, point(helix.start_point)) } fn radius_from_axis(helix: &Helix, value: Vec3) -> Option { @@ -65,6 +72,7 @@ fn radius_from_axis(helix: &Helix, value: Vec3) -> Option { } fn top_radius(helix: &Helix) -> f64 { + if helix.turns == 0.0 { return helix.radius; } spline_curve(&helix.spline) .map(|curve| Vec3::from(curve.point_at(1.0))) .and_then(|endpoint| radius_from_axis(helix, endpoint)) @@ -77,7 +85,7 @@ fn kernel_curve(helix: &Helix, top_radius: f64) -> Option { base_center: base.to_array(), axis_direction: axis.to_array(), start_direction: start_direction.to_array(), - base_radius: helix.radius, + base_radius: base_radius(helix)?, top_radius, height: helix.turns * helix.turn_height, turns: helix.turns, @@ -105,12 +113,18 @@ fn perpendicular(axis: Vec3) -> Option { } fn rebuild(helix: &mut Helix, top_radius: f64) -> bool { - let Some(curve) = kernel_curve(helix, top_radius) else { + rebuild_with_radial(helix, top_radius, None) +} + +fn rebuild_with_radial(helix: &mut Helix, top_radius: f64, radial: Option) -> bool { + let Some(mut curve) = kernel_curve(helix, top_radius) else { return false; }; + if let Some(radial) = radial { curve.start_direction = radial.to_array(); } let Some(nurbs) = curve.nurbs() else { return false; }; + helix.radius = top_radius; helix.spline.degree = nurbs.degree() as i32; helix.spline.knots = nurbs.knots().to_vec(); helix.spline.control_points = nurbs @@ -127,7 +141,7 @@ fn rebuild(helix: &mut Helix, top_radius: f64) -> bool { helix.spline.flags.closed = false; helix.spline.flags.periodic = false; helix.spline.flags.rational = nurbs.is_rational(); - helix.spline.flags.planar = false; + helix.spline.flags.planar = curve.height == 0.0; helix.spline.flags.linear = false; true } @@ -194,7 +208,7 @@ fn properties(helix: &Helix) -> Vec { edit(t!("Height").as_ref(), "height", height), edit(t!("Turns").as_ref(), "turns", helix.turns), edit(t!("Turn height").as_ref(), "turn_height", helix.turn_height), - edit(t!("Base radius").as_ref(), "base_radius", helix.radius), + edit(t!("Base radius").as_ref(), "base_radius", base_radius(helix).unwrap_or(0.0)), edit(t!("Top radius").as_ref(), "top_radius", top_radius), choice(t!("Twist").as_ref(), "twist", &twist, &twist_options), ro(t!("Turn slope").as_ref(), "turn_slope", format_angle(turn_slope)), @@ -249,10 +263,15 @@ fn apply_geom_prop(helix: &mut Helix, field: &str, value: &str) { helix.axis_base_point = vector(base); helix.start_point = vector(point(helix.start_point) + delta); } - "height" if number.abs() > EPSILON => { + "height" => { + if helix.turns == 0.0 && number != 0.0 { + return; + } + if number < 0.0 { helix.axis_vector = vector(-point(helix.axis_vector)); } + let number = number.abs(); if helix.constraint == HelixConstraint::TurnHeight && helix.turn_height.abs() > EPSILON { let turns = number / helix.turn_height; - if turns <= EPSILON { + if turns < 0.0 { return; } helix.turns = turns; @@ -260,16 +279,18 @@ fn apply_geom_prop(helix: &mut Helix, field: &str, value: &str) { helix.turn_height = number / helix.turns.max(EPSILON); } } - "turns" if number > EPSILON => { - if helix.constraint == HelixConstraint::TurnHeight { + "turns" if number >= 0.0 => { + if number == 0.0 { helix.turns = 0.0; } + else if helix.constraint == HelixConstraint::TurnHeight { helix.turns = number; } else { helix.turns = number; helix.turn_height = old_height / number; } } - "turn_height" if number.abs() > EPSILON => { - if helix.constraint == HelixConstraint::Turns { + "turn_height" if number >= 0.0 => { + if number == 0.0 { helix.turn_height = 0.0; } + else if helix.constraint == HelixConstraint::Turns { helix.turn_height = number; } else { let turns = old_height / number; @@ -280,12 +301,13 @@ fn apply_geom_prop(helix: &mut Helix, field: &str, value: &str) { helix.turns = turns; } } - "base_radius" if number > EPSILON => { + "base_radius" if number >= 0.0 => { let Some((base, _, start_direction)) = axis_frame(helix) else { return; }; - helix.radius = number; helix.start_point = vector(base + start_direction * number); + if !rebuild_with_radial(helix, top, Some(start_direction)) { *helix = original; } + return; } "top_radius" if number >= 0.0 => { if !rebuild(helix, number) { @@ -455,3 +477,48 @@ impl Transformable for Helix { apply_transform(self, transform); } } + +#[cfg(test)] +mod tests { + use super::*; + + fn zero_base_point_helix() -> Helix { + let curve = HelixCurve { + base_center: [1.0, 2.0, 3.0], + axis_direction: [0.0, 0.0, 1.0], + start_direction: [1.0, 0.0, 0.0], + base_radius: 0.0, + top_radius: 2.0, + height: 0.0, + turns: 0.0, + direction: HelixDirection::CounterClockwise, + }; + let nurbs = curve.nurbs().unwrap(); + let mut helix = Helix::new(); + helix.axis_base_point = Vector3::new(1.0, 2.0, 3.0); + helix.axis_vector = Vector3::UNIT_Z; + helix.start_point = helix.axis_base_point; + helix.radius = 2.0; + helix.turns = 0.0; + helix.turn_height = 0.0; + helix.spline.degree = nurbs.degree() as i32; + helix.spline.knots = nurbs.knots().to_vec(); + helix.spline.control_points = nurbs + .control_points() + .iter() + .map(|point| Vector3::new(point[0], point[1], point[2])) + .collect(); + helix + } + + #[test] + fn zero_base_point_locus_retains_an_editable_frame() { + let mut helix = zero_base_point_helix(); + assert_eq!(base_radius(&helix), Some(0.0)); + assert_eq!(top_radius(&helix), 2.0); + + let original = helix.clone(); + apply_geom_prop(&mut helix, "height", "5"); + assert_eq!(helix, original); + } +} diff --git a/src/modules/draw/draw/helix.rs b/src/modules/draw/draw/helix.rs index fbeb2ca63..9c055310d 100644 --- a/src/modules/draw/draw/helix.rs +++ b/src/modules/draw/draw/helix.rs @@ -122,7 +122,7 @@ impl HelixCommand { .iter() .map(|point| Vector3::new(point[0], point[1], point[2])) .collect(); - spline.flags.planar = false; + spline.flags.planar = height == 0.0; spline.flags.rational = nurbs.is_rational(); if spline.flags.rational { spline.weights = nurbs.weights().to_vec(); @@ -136,7 +136,7 @@ impl HelixCommand { helix.axis_vector = Vector3::new(axis.x, axis.y, axis.z); helix.radius = curve.base_radius; helix.turns = curve.turns; - helix.turn_height = height / curve.turns; + helix.turn_height = if curve.turns == 0.0 { 0.0 } else { height / curve.turns }; helix.handedness = self.counter_clockwise; helix.constraint = self.constraint; Some(EntityType::Helix(helix)) @@ -262,7 +262,7 @@ impl CadCommand for HelixCommand { } else { distance }; - if radius > EPSILON { + if radius >= 0.0 { self.base_radius = radius; self.top_radius = radius; self.step = Step::TopRadius; @@ -272,21 +272,20 @@ impl CadCommand for HelixCommand { Step::TopRadius | Step::TopDiameter => { let local = self.plane.vector_to_local(point - self.center); let distance = local.x.hypot(local.y); - self.top_radius = if self.step == Step::TopDiameter { + let top_radius = if self.step == Step::TopDiameter { distance * 0.5 } else { distance }; - self.step = Step::Final; + if self.base_radius > 0.0 || top_radius > 0.0 { + self.top_radius = top_radius; + self.step = Step::Final; + } CmdResult::NeedPoint } Step::Final => { let height = (point - self.center).dot(self.plane.z); - if height.abs() <= EPSILON { - CmdResult::NeedPoint - } else { - self.commit(height, self.plane.z) - } + self.commit(height, self.plane.z) } Step::AxisEndpoint => { let vector = point - self.center; @@ -310,6 +309,9 @@ impl CadCommand for HelixCommand { CmdResult::NeedPoint } Step::TopRadius | Step::TopDiameter => { + if self.base_radius == 0.0 { + return CmdResult::NeedPoint; + } self.top_radius = self.base_radius; self.step = Step::Final; CmdResult::NeedPoint @@ -382,7 +384,7 @@ impl CadCommand for HelixCommand { } Step::BaseRadius => { let radius = Self::parse_value(value)?; - if radius > EPSILON { + if radius >= 0.0 { self.base_radius = radius; self.top_radius = radius; self.step = Step::TopRadius; @@ -391,7 +393,7 @@ impl CadCommand for HelixCommand { } Step::BaseDiameter => { let diameter = Self::parse_value(value)?; - if diameter > EPSILON { + if diameter >= 0.0 { self.base_radius = diameter * 0.5; self.top_radius = self.base_radius; self.step = Step::TopRadius; @@ -400,7 +402,7 @@ impl CadCommand for HelixCommand { } Step::TopRadius => { let radius = Self::parse_value(value)?; - if radius >= 0.0 { + if radius >= 0.0 && (self.base_radius > 0.0 || radius > 0.0) { self.top_radius = radius; self.step = Step::Final; } @@ -408,7 +410,7 @@ impl CadCommand for HelixCommand { } Step::TopDiameter => { let diameter = Self::parse_value(value)?; - if diameter >= 0.0 { + if diameter >= 0.0 && (self.base_radius > 0.0 || diameter > 0.0) { self.top_radius = diameter * 0.5; self.step = Step::Final; } @@ -416,11 +418,15 @@ impl CadCommand for HelixCommand { } Step::Final => { let height = Self::parse_value(value)?; - (height.abs() > EPSILON).then(|| self.commit(height, self.plane.z)) + Some(self.commit(height, self.plane.z)) } Step::AxisEndpoint => None, Step::Turns => { let turns = Self::parse_turns(value)?; + if turns > 500.0 { + self.step = Step::Final; + return Some(CmdResult::NeedPoint); + } if turns > EPSILON { self.turns = turns; self.requested_turn_height = None; @@ -493,3 +499,26 @@ impl CadCommand for HelixCommand { } inventory::submit!(crate::command::CommandRegistration { names: &["HELIX"] }); + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn zero_turn_build_keeps_finite_native_parameters() { + let mut command = HelixCommand::new(); + command.center = DVec3::new(1.0, 2.0, 3.0); + command.base_radius = 2.0; + command.top_radius = 4.0; + command.turns = 0.0; + let EntityType::Helix(helix) = command.build(0.0, DVec3::Z).unwrap() else { + panic!("expected helix"); + }; + + assert_eq!(helix.turn_height, 0.0); + assert!(helix.turn_height.is_finite()); + assert!(helix.spline.flags.planar); + assert_eq!(helix.spline.control_points.len(), 2); + assert_eq!(helix.spline.control_points[0], helix.spline.control_points[1]); + } +}