Merge pull request #1186 from ramox81/codex/complete-reverse-command

# Conflicts:
#	src/entities/helix.rs

src/app/commands/draw.rs

@@ -596,9 +596,23 @@ impl OpenCADStudio {
 
             "REVERSE" => {
                 use crate::modules::draw::modify::reverse::ReverseCommand;
-                let new_cmd = ReverseCommand::new();
-                self.command_line.push_info(&new_cmd.prompt());
-                self.tabs[i].active_cmd = Some(Box::new(new_cmd));
+                if self.tabs[i].scene.selected.is_empty() {
+                    use crate::modules::draw::select::SelectObjectsCommand;
+                    let selection = SelectObjectsCommand::new("REVERSE");
+                    self.command_line.push_info(&selection.prompt());
+                    self.tabs[i].active_cmd = Some(Box::new(selection));
+                } else {
+                    let replacements = self.tabs[i].scene.selected_entities().into_iter()
+                        .filter(|(handle, _)| !self.tabs[i].scene.is_layer_locked(*handle))
+                        .filter_map(|(handle, entity)| ReverseCommand::reversed(entity)
+                            .map(|reversed| (handle, vec![reversed])))
+                        .collect::<Vec<_>>();
+                    if !replacements.is_empty() {
+                        return Some(self.apply_cmd_result(crate::command::CmdResult::ReplaceMany(
+                            replacements, Vec::new(),
+                        )));
+                    }
+                }
             }
 
             "MEASUREGEOM" => {

src/entities/helix.rs

@@ -97,6 +97,36 @@ fn kernel_curve(helix: &Helix, top_radius: f64) -> Option<HelixCurve> {
     })
 }
 
+/// Reverse generating parameters together with the exact stored spline.
+pub(crate) fn reversed(helix: &Helix) -> Option<Helix> {
+    let original_curve = spline_curve(&helix.spline)?;
+    let parameters = kernel_curve(helix, top_radius(helix))?.reversed()?;
+    let mut result = helix.clone();
+    result.axis_base_point = vector(Vec3::from(parameters.base_center));
+    result.axis_vector = vector(Vec3::from(parameters.axis_direction));
+    result.start_point = vector(Vec3::from(original_curve.point_at(1.0)));
+    result.radius = parameters.top_radius;
+    // Reverse the evaluated curve itself instead of fitting it again.
+    let reversed_curve = original_curve.reversed()?;
+    result.spline.control_points = reversed_curve
+        .control_points()
+        .iter()
+        .map(|point| Vector3::new(point[0], point[1], point[2]))
+        .collect();
+    result.spline.fit_points.reverse();
+    result.spline.weights = if helix.spline.weights.is_empty() {
+        Vec::new()
+    } else {
+        reversed_curve.weights().to_vec()
+    };
+    result.spline.knots = reversed_curve.knots().to_vec();
+    let begin = helix.spline.begin_tangent;
+    let end = helix.spline.end_tangent;
+    result.spline.begin_tangent = Vector3::new(-end.x, -end.y, -end.z);
+    result.spline.end_tangent = Vector3::new(-begin.x, -begin.y, -begin.z);
+    Some(result)
+}
+
 fn projected_direction(direction: Vec3, axis: Vec3) -> Option<Vec3> {
     (direction - axis * direction.dot(axis)).normalize()
 }
@@ -325,6 +355,7 @@ fn apply_geom_prop(helix: &mut Helix, field: &str, value: &str) {
 fn apply_grip(helix: &mut Helix, grip_id: usize, apply: GripApply) {
     let original = helix.clone();
     let top = top_radius(helix);
+    let Some(base_radius) = base_radius(helix) else { return; };
     let Some((base, axis, start_direction)) = axis_frame(helix) else {
         return;
     };
@@ -356,7 +387,6 @@ fn apply_grip(helix: &mut Helix, grip_id: usize, apply: GripApply) {
             if radius <= EPSILON {
                 return;
             }
-            helix.radius = radius;
             helix.start_point = vector(base + radial);
         }
         (2, GripApply::Absolute(position)) => {
@@ -384,7 +414,7 @@ fn apply_grip(helix: &mut Helix, grip_id: usize, apply: GripApply) {
                 *helix = original;
                 return;
             };
-            helix.start_point = vector(base + start_direction * helix.radius);
+            helix.start_point = vector(base + start_direction * base_radius);
         }
         (3, GripApply::Absolute(position)) => {
             let position = kernel(position);
@@ -521,4 +551,54 @@ mod tests {
         apply_geom_prop(&mut helix, "height", "5");
         assert_eq!(helix, original);
     }
+
+    #[test]
+    fn reversing_twice_restores_a_conical_helix_and_radius_roles() {
+        let curve = HelixCurve {
+            base_center: [1.0, 2.0, 3.0],
+            axis_direction: [0.0, 1.0, 1.0],
+            start_direction: [1.0, 0.0, 0.0],
+            base_radius: 1.0,
+            top_radius: 2.5,
+            height: 4.0,
+            turns: 1.25,
+            direction: HelixDirection::Clockwise,
+        };
+        let nurbs = curve.nurbs().expect("valid helix");
+        let mut helix = Helix::new();
+        helix.axis_base_point = Vector3::new(1.0, 2.0, 3.0);
+        let axis = Vec3::from(curve.axis_direction).normalize().unwrap();
+        helix.axis_vector = vector(axis);
+        helix.start_point = Vector3::new(2.0, 2.0, 3.0);
+        helix.radius = curve.top_radius;
+        helix.turns = curve.turns;
+        helix.turn_height = curve.height / curve.turns;
+        helix.handedness = false;
+        helix.spline.degree = nurbs.degree() as i32;
+        helix.spline.control_points = nurbs
+            .control_points()
+            .iter()
+            .map(|point| Vector3::new(point[0], point[1], point[2]))
+            .collect();
+        helix.spline.knots = nurbs.knots().to_vec();
+        helix.spline.weights = nurbs.weights().to_vec();
+
+        let once = reversed(&helix).expect("first reversal");
+        assert!((once.radius - curve.base_radius).abs() < 1.0e-12);
+        assert_eq!(once.turns, helix.turns);
+        assert_eq!(once.turn_height, helix.turn_height);
+        assert_eq!(once.handedness, helix.handedness);
+        let original_curve = spline_curve(&helix.spline).unwrap();
+        let once_curve = spline_curve(&once.spline).unwrap();
+        assert!(Vec3::from(once_curve.point_at(0.0))
+            .distance(Vec3::from(original_curve.point_at(1.0))) < 1.0e-9);
+
+        let twice = reversed(&once).expect("second reversal");
+        assert!((twice.radius - helix.radius).abs() < 1.0e-12);
+        let twice_curve = spline_curve(&twice.spline).unwrap();
+        for parameter in [0.0, 0.2, 0.5, 0.8, 1.0] {
+            assert!(Vec3::from(twice_curve.point_at(parameter))
+                .distance(Vec3::from(original_curve.point_at(parameter))) < 1.0e-8);
+        }
+    }
 }

src/modules/draw/draw/helix.rs

@@ -134,7 +134,7 @@ impl HelixCommand {
         let start = self.center + start_direction * curve.base_radius;
         helix.start_point = Vector3::new(start.x, start.y, start.z);
         helix.axis_vector = Vector3::new(axis.x, axis.y, axis.z);
-        helix.radius = curve.base_radius;
+        helix.radius = curve.top_radius;
         helix.turns = curve.turns;
         helix.turn_height = if curve.turns == 0.0 { 0.0 } else { height / curve.turns };
         helix.handedness = self.counter_clockwise;

src/modules/draw/modify/reverse.rs

@@ -54,7 +54,7 @@ impl ReverseCommand {
     }
 
     /// Build a reversed copy of `entity`, or `None` for an unsupported type.
-    fn reversed(entity: &EntityType) -> Option<EntityType> {
+    pub(crate) fn reversed(entity: &EntityType) -> Option<EntityType> {
         match entity {
             EntityType::Line(line) => {
                 let mut out = line.clone();
@@ -76,6 +76,7 @@ impl ReverseCommand {
                 Some(EntityType::Polyline3D(out))
             }
             EntityType::Spline(sp) => Some(EntityType::Spline(reverse_spline(sp))),
+            EntityType::Helix(helix) => crate::entities::helix::reversed(helix).map(EntityType::Helix),
             _ => None,
         }
     }
@@ -176,9 +177,8 @@ fn reverse_polyline2d(pl: &acadrust::entities::Polyline2D) -> acadrust::entities
     out
 }
 
-/// Reverse a Spline: flip control points and fit points, regenerate the
-/// clamped knot vector. Mirrors `splinedit::apply_spline_op`'s REVERSE branch.
-fn reverse_spline(sp: &Spline) -> Spline {
+/// Reverse stored coordinates in 3D and let the kernel mirror the knot domain.
+pub(super) fn reverse_spline(sp: &Spline) -> Spline {
     let mut out = sp.clone();
     out.control_points.reverse();
     out.fit_points.reverse();
@@ -186,8 +186,11 @@ fn reverse_spline(sp: &Spline) -> Spline {
     if out.weights.len() == out.control_points.len() {
         out.weights.reverse();
     }
-    out.knots =
-        Spline::generate_clamped_knots(out.degree as usize, out.control_points.len());
+    if let Some(curve) = super::spline_ops::spline_to_nurbs(sp) {
+        out.knots = curve.reversed().knots().to_vec();
+    }
+    out.begin_tangent = acadrust::types::Vector3::new(-sp.end_tangent.x, -sp.end_tangent.y, -sp.end_tangent.z);
+    out.end_tangent = acadrust::types::Vector3::new(-sp.begin_tangent.x, -sp.begin_tangent.y, -sp.begin_tangent.z);
     out
 }
 
@@ -197,7 +200,7 @@ impl CadCommand for ReverseCommand {
     }
 
     fn prompt(&self) -> String {
-        t!("REVERSE  Select line, polyline or spline to reverse:").into_owned()
+        t!("REVERSE  Select line, polyline, spline or helix to reverse:").into_owned()
     }
 
     fn needs_entity_pick(&self) -> bool {
@@ -240,3 +243,39 @@ impl CadCommand for ReverseCommand {
 inventory::submit!(crate::command::CommandRegistration {
     names: &["REVERSE"]
 }); // ReverseCommand
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use acadrust::types::Vector3;
+
+    #[test]
+    fn spline_reversal_preserves_knot_spacing_weights_and_tangents() {
+        let mut spline = Spline::new();
+        spline.degree = 2;
+        spline.control_points = vec![
+            Vector3::new(0.0, 0.0, 0.0),
+            Vector3::new(1.0, 2.0, 0.0),
+            Vector3::new(3.0, 1.0, 1.0),
+            Vector3::new(4.0, 0.0, 2.0),
+        ];
+        spline.knots = vec![0.0, 0.0, 0.0, 0.25, 1.0, 1.0, 1.0];
+        spline.weights = vec![1.0, 2.0, 3.0, 4.0];
+        spline.begin_tangent = Vector3::new(1.0, 2.0, 3.0);
+        spline.end_tangent = Vector3::new(4.0, 5.0, 6.0);
+
+        let reversed = reverse_spline(&spline);
+        assert_eq!(reversed.control_points, spline.control_points.iter().rev().cloned().collect::<Vec<_>>());
+        assert_eq!(reversed.weights, vec![4.0, 3.0, 2.0, 1.0]);
+        assert_eq!(reversed.knots, vec![0.0, 0.0, 0.0, 0.75, 1.0, 1.0, 1.0]);
+        assert_eq!(reversed.begin_tangent, Vector3::new(-4.0, -5.0, -6.0));
+        assert_eq!(reversed.end_tangent, Vector3::new(-1.0, -2.0, -3.0));
+
+        let restored = reverse_spline(&reversed);
+        assert_eq!(restored.control_points, spline.control_points);
+        assert_eq!(restored.weights, spline.weights);
+        assert_eq!(restored.knots, spline.knots);
+        assert_eq!(restored.begin_tangent, spline.begin_tangent);
+        assert_eq!(restored.end_tangent, spline.end_tangent);
+    }
+}