fix(mesh): complete polygon mesh support

src/modules/draw/defaults.rs

@@ -1,5 +1,4 @@
-// Persistent command defaults — last-used values survive across command runs
-// within the same session.  Uses thread_local! so no synchronisation needed.
+// Last-used command values for this session.
 
 use std::cell::Cell;
 
@@ -28,6 +27,10 @@ thread_local! {
     static RECT_FILLET:     Cell<f64> = Cell::new(0.0);
     static DONUT_INNER_DIAMETER: Cell<f64> = Cell::new(0.5);
     static DONUT_OUTER_DIAMETER: Cell<f64> = Cell::new(1.0);
+    static HELIX_BASE_RADIUS: Cell<f64> = Cell::new(1.0);
+    static HELIX_HEIGHT: Cell<f64> = Cell::new(1.0);
+    static HELIX_TURNS: Cell<f64> = Cell::new(3.0);
+    static HELIX_COUNTER_CLOCKWISE: Cell<bool> = Cell::new(true);
 }
 
 macro_rules! accessors {
@@ -70,6 +73,16 @@ accessors!(get_rect_rotation, set_rect_rotation, RECT_ROTATION);
 accessors!(get_rect_chamfer1, set_rect_chamfer1, RECT_CHAMFER1);
 accessors!(get_rect_chamfer2, set_rect_chamfer2, RECT_CHAMFER2);
 accessors!(get_rect_fillet, set_rect_fillet, RECT_FILLET);
+accessors!(get_helix_base_radius, set_helix_base_radius, HELIX_BASE_RADIUS);
+accessors!(get_helix_height, set_helix_height, HELIX_HEIGHT);
+accessors!(get_helix_turns, set_helix_turns, HELIX_TURNS);
+pub fn get_helix_counter_clockwise() -> bool {
+    HELIX_COUNTER_CLOCKWISE.with(|value| value.get())
+}
+
+pub fn set_helix_counter_clockwise(value: bool) {
+    HELIX_COUNTER_CLOCKWISE.with(|current| current.set(value));
+}
 accessors!(
     get_donut_inner_diameter,
     set_donut_inner_diameter,

src/modules/draw/draw/face3d.rs

@@ -0,0 +1,500 @@
+use acadrust::entities::{face3d::InvisibleEdgeFlags, Face3D};
+use acadrust::types::Vector3;
+use acadrust::{EntityType, Handle};
+use cadkernel::geom2d::Tolerance;
+use cadkernel::space::{curve::segments_overlap_collinearly, Vec3 as KernelVec3};
+use glam::DVec3;
+use std::collections::{HashMap, HashSet};
+
+use crate::command::{CadCommand, CmdOption, CmdResult};
+use crate::scene::model::wire_model::WireModel;
+use crate::t;
+
+fn vector(point: DVec3) -> Vector3 {
+    Vector3::new(point.x, point.y, point.z)
+}
+
+fn point(value: Vector3) -> DVec3 {
+    DVec3::new(value.x, value.y, value.z)
+}
+
+fn set_edge_invisible(flags: &mut InvisibleEdgeFlags, edge: usize, invisible: bool) {
+    match edge {
+        0 => flags.set_first_invisible(invisible),
+        1 => flags.set_second_invisible(invisible),
+        2 => flags.set_third_invisible(invisible),
+        3 => flags.set_fourth_invisible(invisible),
+        _ => {}
+    }
+}
+
+fn edge_is_invisible(flags: InvisibleEdgeFlags, edge: usize) -> bool {
+    match edge {
+        0 => flags.is_first_invisible(),
+        1 => flags.is_second_invisible(),
+        2 => flags.is_third_invisible(),
+        3 => flags.is_fourth_invisible(),
+        _ => false,
+    }
+}
+
+fn append_segment(points: &mut Vec<[f64; 3]>, start: [f64; 3], end: [f64; 3]) {
+    if !points.is_empty() {
+        points.push([f64::NAN; 3]);
+    }
+    points.push(start);
+    points.push(end);
+}
+
+pub struct Face3dCommand {
+    points: Vec<DVec3>,
+    invisible: [bool; 4],
+    pending_invisible: bool,
+}
+
+impl Face3dCommand {
+    pub fn new() -> Self {
+        Self {
+            points: Vec::new(),
+            invisible: [false; 4],
+            pending_invisible: false,
+        }
+    }
+
+    fn build(&self, fourth: DVec3) -> Option<EntityType> {
+        if self.points.len() != 3 {
+            return None;
+        }
+        let mut face = Face3D::new(
+            vector(self.points[0]),
+            vector(self.points[1]),
+            vector(self.points[2]),
+            vector(fourth),
+        );
+        for edge in 0..4 {
+            set_edge_invisible(&mut face.invisible_edges, edge, self.invisible[edge]);
+        }
+        Some(EntityType::Face3D(face))
+    }
+
+    fn continue_from_last_edge(&mut self, fourth: DVec3) {
+        let third = self.points[2];
+        let shared_edge_invisible = self.invisible[2];
+        self.points.clear();
+        self.points.push(third);
+        self.points.push(fourth);
+        self.invisible = [shared_edge_invisible, false, false, false];
+        self.pending_invisible = false;
+    }
+
+    fn continue_from_triangle(&mut self) {
+        let second = self.points[1];
+        let third = self.points[2];
+        let shared_edge_invisible = self.invisible[1];
+        self.points.clear();
+        self.points.push(second);
+        self.points.push(third);
+        self.invisible = [shared_edge_invisible, false, false, false];
+        self.pending_invisible = false;
+    }
+}
+
+impl CadCommand for Face3dCommand {
+    fn name(&self) -> &'static str {
+        "3DFACE"
+    }
+
+    fn prompt(&self) -> String {
+        match self.points.len() {
+            0 => t!("3DFACE  Specify first point or [Invisible]:").into_owned(),
+            1 => t!("3DFACE  Specify second point or [Invisible]:").into_owned(),
+            2 => t!("3DFACE  Specify third point or [Invisible] <exit>:").into_owned(),
+            _ => t!("3DFACE  Specify fourth point or [Invisible] <create three-sided face>:")
+                .into_owned(),
+        }
+    }
+
+    fn options(&self) -> Vec<CmdOption> {
+        vec![CmdOption::new(t!("Invisible").as_ref(), "I")]
+    }
+
+    fn on_point(&mut self, picked: DVec3) -> CmdResult {
+        let point_index = self.points.len();
+        if self.pending_invisible && point_index < 4 {
+            self.invisible[point_index] = true;
+        }
+        self.pending_invisible = false;
+        if self.points.len() == 3 {
+            let entity = self.build(picked);
+            self.continue_from_last_edge(picked);
+            return entity.map_or(CmdResult::Cancel, CmdResult::CommitEntity);
+        }
+        self.points.push(picked);
+        CmdResult::NeedPoint
+    }
+
+    fn on_enter(&mut self) -> CmdResult {
+        if self.points.len() != 3 {
+            return CmdResult::Cancel;
+        }
+        if self.pending_invisible {
+            self.invisible[3] = true;
+        }
+        let third = self.points[2];
+        let entity = self.build(third);
+        self.continue_from_triangle();
+        entity.map_or(CmdResult::Cancel, CmdResult::CommitEntity)
+    }
+
+    fn wants_text_input(&self) -> bool {
+        true
+    }
+
+    fn point_step_accepts_keywords(&self) -> bool {
+        true
+    }
+
+    fn on_text_input(&mut self, value: &str) -> Option<CmdResult> {
+        match value.trim().to_ascii_uppercase().as_str() {
+            "I" | "INVISIBLE" => {
+                self.pending_invisible = true;
+                Some(CmdResult::NeedPoint)
+            }
+            _ => None,
+        }
+    }
+
+    fn on_mouse_move(&mut self, cursor: DVec3) -> Option<WireModel> {
+        if self.points.is_empty() {
+            return None;
+        }
+        let mut preview = Vec::new();
+        for edge in 0..self.points.len().saturating_sub(1) {
+            if !self.invisible[edge] {
+                append_segment(
+                    &mut preview,
+                    self.points[edge].to_array(),
+                    self.points[edge + 1].to_array(),
+                );
+            }
+        }
+        let pending_edge = self.points.len() - 1;
+        if !self.invisible[pending_edge] {
+            append_segment(
+                &mut preview,
+                self.points.last()?.to_array(),
+                cursor.to_array(),
+            );
+        }
+        if self.points.len() == 3 && !(self.invisible[3] || self.pending_invisible) {
+            append_segment(&mut preview, cursor.to_array(), self.points[0].to_array());
+        }
+        Some(WireModel::solid_f64(
+            "face3d_preview".to_string(),
+            preview,
+            WireModel::CYAN,
+            false,
+        ))
+    }
+}
+
+#[derive(Clone, Copy, PartialEq, Eq)]
+enum EdgeMode {
+    Toggle,
+    Display,
+    DisplaySelect,
+}
+
+pub struct FaceEdgeCommand {
+    faces: Vec<(Handle, Face3D)>,
+    mode: EdgeMode,
+    show_all_hidden: bool,
+    displayed_faces: HashSet<Handle>,
+    pending_replacements: HashMap<Handle, Face3D>,
+}
+
+impl FaceEdgeCommand {
+    pub fn new(faces: Vec<(Handle, Face3D)>) -> Self {
+        Self {
+            faces,
+            mode: EdgeMode::Toggle,
+            show_all_hidden: false,
+            displayed_faces: HashSet::new(),
+            pending_replacements: HashMap::new(),
+        }
+    }
+
+    fn corners(face: &Face3D) -> [KernelVec3; 4] {
+        [
+            KernelVec3::from(point(face.first_corner).to_array()),
+            KernelVec3::from(point(face.second_corner).to_array()),
+            KernelVec3::from(point(face.third_corner).to_array()),
+            KernelVec3::from(point(face.fourth_corner).to_array()),
+        ]
+    }
+
+    fn nearest_edge(face: &Face3D, picked: KernelVec3) -> Option<usize> {
+        let corners = Self::corners(face);
+        let tolerance = Tolerance::default().linear();
+        let mut best = None;
+        for edge in 0..4 {
+            let start = corners[edge];
+            let end = corners[(edge + 1) % 4];
+            if start.distance(end) <= tolerance {
+                continue;
+            }
+            let distance = picked.distance_to_segment(start, end);
+            if best.is_none_or(|(_, best_distance)| distance < best_distance) {
+                best = Some((edge, distance));
+            }
+        }
+        best.map(|(edge, _)| edge)
+    }
+
+    fn edge_wire(face: &Face3D, edge: usize, name: &str) -> WireModel {
+        let corners = Self::corners(face);
+        WireModel::solid_f64(
+            name.to_string(),
+            vec![corners[edge].to_array(), corners[(edge + 1) % 4].to_array()],
+            WireModel::CYAN,
+            false,
+        )
+    }
+
+    fn collinear_replacements(
+        &mut self,
+        selected_face: usize,
+        selected_edge: usize,
+    ) -> Vec<(Handle, Vec<EntityType>)> {
+        let selected_corners = Self::corners(&self.faces[selected_face].1);
+        let selected_start = selected_corners[selected_edge];
+        let selected_end = selected_corners[(selected_edge + 1) % 4];
+        let make_invisible = !edge_is_invisible(
+            self.faces[selected_face].1.invisible_edges,
+            selected_edge,
+        );
+        let mut replacements = Vec::new();
+        self.pending_replacements.clear();
+        for (handle, face) in &self.faces {
+            let corners = Self::corners(face);
+            let mut updated = face.clone();
+            let mut changed = false;
+            for edge in 0..4 {
+                if segments_overlap_collinearly(
+                    selected_start.to_array(),
+                    selected_end.to_array(),
+                    corners[edge].to_array(),
+                    corners[(edge + 1) % 4].to_array(),
+                    Tolerance::default().linear(),
+                ) && edge_is_invisible(updated.invisible_edges, edge) != make_invisible
+                {
+                    set_edge_invisible(&mut updated.invisible_edges, edge, make_invisible);
+                    changed = true;
+                }
+            }
+            if changed {
+                self.pending_replacements.insert(*handle, updated.clone());
+                replacements.push((*handle, vec![EntityType::Face3D(updated)]));
+            }
+        }
+        replacements
+    }
+
+    fn hidden_edges_wire(&self) -> WireModel {
+        let mut points = Vec::new();
+        for (handle, face) in &self.faces {
+            if !self.show_all_hidden && !self.displayed_faces.contains(handle) {
+                continue;
+            }
+            let corners = Self::corners(face);
+            for edge in 0..4 {
+                if edge_is_invisible(face.invisible_edges, edge)
+                    && corners[edge].distance(corners[(edge + 1) % 4])
+                        > Tolerance::default().linear()
+                {
+                    append_segment(
+                        &mut points,
+                        corners[edge].to_array(),
+                        corners[(edge + 1) % 4].to_array(),
+                    );
+                }
+            }
+        }
+        WireModel::solid_f64(
+            "face3d_hidden_edges".to_string(),
+            points,
+            WireModel::CYAN,
+            false,
+        )
+    }
+}
+
+impl CadCommand for FaceEdgeCommand {
+    fn name(&self) -> &'static str {
+        "EDGE"
+    }
+
+    fn prompt(&self) -> String {
+        match self.mode {
+            EdgeMode::Toggle => {
+                t!("EDGE  Select edge of 3D face to toggle visibility or [Display]:").into_owned()
+            }
+            EdgeMode::Display => {
+                t!("EDGE  Display invisible edges [All/Select] <Select>:").into_owned()
+            }
+            EdgeMode::DisplaySelect => {
+                t!("EDGE  Select 3D faces to display invisible edges <done>:").into_owned()
+            }
+        }
+    }
+
+    fn options(&self) -> Vec<CmdOption> {
+        match self.mode {
+            EdgeMode::Toggle => vec![CmdOption::new(t!("Display").as_ref(), "D")],
+            EdgeMode::Display => vec![
+                CmdOption::new(t!("All").as_ref(), "A"),
+                CmdOption::new(t!("Select").as_ref(), "S"),
+            ],
+            EdgeMode::DisplaySelect => Vec::new(),
+        }
+    }
+
+    fn needs_entity_pick(&self) -> bool {
+        matches!(self.mode, EdgeMode::Toggle | EdgeMode::DisplaySelect)
+    }
+
+    fn entity_pick_includes_fills(&self) -> bool {
+        true
+    }
+
+    fn entity_pick_uses_surface_point(&self) -> bool {
+        true
+    }
+
+    fn entity_pick_highlights_hover(&self) -> bool {
+        true
+    }
+
+    fn on_entity_pick(&mut self, handle: Handle, picked: DVec3) -> CmdResult {
+        let Some(index) = self.faces.iter().position(|(candidate, _)| *candidate == handle) else {
+            return CmdResult::NeedPoint;
+        };
+        if self.mode == EdgeMode::DisplaySelect {
+            self.displayed_faces.insert(handle);
+            return CmdResult::Preview(self.hidden_edges_wire());
+        }
+        let picked = KernelVec3::from(picked.to_array());
+        let Some(edge) = Self::nearest_edge(&self.faces[index].1, picked) else {
+            return CmdResult::NeedPoint;
+        };
+        let replacements = self.collinear_replacements(index, edge);
+        if replacements.is_empty() {
+            CmdResult::NeedPoint
+        } else {
+            CmdResult::ReplaceManyContinue(replacements)
+        }
+    }
+
+    fn on_hover_entity(&mut self, handle: Handle, picked: DVec3) -> Vec<WireModel> {
+        let Some((_, face)) = self.faces.iter().find(|(candidate, _)| *candidate == handle) else {
+            return Vec::new();
+        };
+        if self.mode == EdgeMode::DisplaySelect
+            || self.show_all_hidden
+            || self.displayed_faces.contains(&handle)
+        {
+            let mut points = Vec::new();
+            let corners = Self::corners(face);
+            for edge in 0..4 {
+                if corners[edge].distance(corners[(edge + 1) % 4])
+                    > Tolerance::default().linear()
+                {
+                    append_segment(
+                        &mut points,
+                        corners[edge].to_array(),
+                        corners[(edge + 1) % 4].to_array(),
+                    );
+                }
+            }
+            return vec![WireModel::solid_f64(
+                "face3d_edges".to_string(),
+                points,
+                WireModel::CYAN,
+                false,
+            )];
+        }
+        Self::nearest_edge(face, KernelVec3::from(picked.to_array()))
+            .map(|edge| vec![Self::edge_wire(face, edge, "face3d_edge")])
+            .unwrap_or_default()
+    }
+
+    fn on_text_input(&mut self, value: &str) -> Option<CmdResult> {
+        let keyword = value.trim().to_ascii_uppercase();
+        match self.mode {
+            EdgeMode::Toggle if matches!(keyword.as_str(), "D" | "DISPLAY") => {
+                self.mode = EdgeMode::Display;
+                Some(CmdResult::NeedPoint)
+            }
+            EdgeMode::Display if matches!(keyword.as_str(), "A" | "ALL") => {
+                self.mode = EdgeMode::Toggle;
+                self.show_all_hidden = true;
+                Some(CmdResult::Preview(self.hidden_edges_wire()))
+            }
+            EdgeMode::Display if matches!(keyword.as_str(), "S" | "SELECT") => {
+                self.mode = EdgeMode::DisplaySelect;
+                self.show_all_hidden = false;
+                self.displayed_faces.clear();
+                Some(CmdResult::NeedPoint)
+            }
+            _ => Some(CmdResult::NeedPoint),
+        }
+    }
+
+    fn wants_text_input(&self) -> bool {
+        true
+    }
+
+    fn point_step_accepts_keywords(&self) -> bool {
+        true
+    }
+
+    fn on_point(&mut self, _point: DVec3) -> CmdResult {
+        CmdResult::NeedPoint
+    }
+
+    fn on_enter(&mut self) -> CmdResult {
+        match self.mode {
+            EdgeMode::Display => {
+                self.mode = EdgeMode::DisplaySelect;
+                self.show_all_hidden = false;
+                self.displayed_faces.clear();
+                CmdResult::NeedPoint
+            }
+            EdgeMode::DisplaySelect => {
+                self.mode = EdgeMode::Toggle;
+                CmdResult::Preview(self.hidden_edges_wire())
+            }
+            EdgeMode::Toggle => CmdResult::Cancel,
+        }
+    }
+
+    fn on_entity_replaced(&mut self, old: Handle, new_handles: &[Handle]) {
+        if let Some(new_handle) = new_handles.first().copied() {
+            if let Some((handle, face)) =
+                self.faces.iter_mut().find(|(handle, _)| *handle == old)
+            {
+                if let Some(updated) = self.pending_replacements.remove(&old) {
+                    *face = updated;
+                }
+                *handle = new_handle;
+            }
+            if self.displayed_faces.remove(&old) {
+                self.displayed_faces.insert(new_handle);
+            }
+        }
+    }
+}
+
+inventory::submit!(crate::command::CommandRegistration {
+    names: &["3DFACE", "EDGE"]
+});

src/modules/draw/draw/helix.rs

@@ -1,35 +1,18 @@
-// Parametric helix tool — interactive command.
-//
-// Command: HELIX — build a parametric 3D helix and commit it as a single 3D
-// polyline whose straight segments approximate the curve. The user picks a
-// base centre point, then types the base radius, the top radius (Enter keeps
-// it equal to the base radius for a cylindrical helix), the height, and the
-// number of turns (Enter accepts a default of three turns).
-//
-// The curve is sampled at a fixed angular resolution: with `SEG_PER_TURN`
-// samples per turn the helix has `turns * SEG_PER_TURN` segments. For each
-// sample the radius interpolates linearly from base to top, the elevation
-// interpolates linearly from zero to the height, and the planar position is
-// `(radius*cos(angle), radius*sin(angle))` offset from the centre.
-
-use acadrust::entities::Polyline3D;
+use acadrust::entities::{Helix, HelixConstraint, Spline};
 use acadrust::types::Vector3;
 use acadrust::EntityType;
-use crate::t;
-
-use crate::command::{CadCommand, CmdResult, DynField, WorkingPlane};
-use crate::modules::{IconKind, ModuleEvent, ToolDef};
-use crate::scene::model::wire_model::WireModel;
+use cadkernel::space::{HelixCurve, HelixDirection, Vec3};
 use glam::DVec3;
 
-/// Angular samples per full turn of the helix.
-const SEG_PER_TURN: usize = 36;
-/// Default number of turns when the user accepts the prompt with Enter.
-const DEFAULT_TURNS: f64 = 3.0;
+use crate::command::{CadCommand, CmdOption, CmdResult, DynField, WorkingPlane};
+use crate::modules::draw::defaults;
+use crate::modules::{IconKind, ModuleEvent, ToolDef};
+use crate::scene::model::wire_model::WireModel;
+use crate::t;
 
-// ── Ribbon definition ─────────────────────────────────────────────────────
+const EPSILON: f64 = 1.0e-9;
 
-#[allow(dead_code)] // ribbon definition ready for wiring; command works via the command line
+#[allow(dead_code)]
 pub fn tool() -> ToolDef {
     ToolDef {
         id: "HELIX",
@@ -39,16 +22,18 @@ pub fn tool() -> ToolDef {
     }
 }
 
-// ── Command implementation ────────────────────────────────────────────────
-
-/// Which parameter the command is currently collecting.
 #[derive(Clone, Copy, PartialEq, Eq)]
 enum Step {
     Center,
     BaseRadius,
+    BaseDiameter,
     TopRadius,
-    Height,
+    TopDiameter,
+    Final,
+    AxisEndpoint,
     Turns,
+    TurnHeight,
+    Twist,
 }
 
 pub struct HelixCommand {
@@ -58,58 +43,130 @@ pub struct HelixCommand {
     top_radius: f64,
     height: f64,
     turns: f64,
+    requested_turn_height: Option<f64>,
+    counter_clockwise: bool,
+    constraint: HelixConstraint,
     plane: WorkingPlane,
 }
 
 impl HelixCommand {
     pub fn new() -> Self {
+        let base_radius = defaults::get_helix_base_radius();
         Self {
             step: Step::Center,
             center: DVec3::ZERO,
-            base_radius: 0.0,
-            top_radius: 0.0,
-            height: 0.0,
-            turns: DEFAULT_TURNS,
+            base_radius,
+            top_radius: base_radius,
+            height: defaults::get_helix_height(),
+            turns: defaults::get_helix_turns(),
+            requested_turn_height: None,
+            counter_clockwise: defaults::get_helix_counter_clockwise(),
+            constraint: HelixConstraint::Turns,
             plane: WorkingPlane::default(),
         }
     }
 
-    /// Sample the helix into a list of world points.
-    fn sample_points(&self, base_radius: f64, top_radius: f64, height: f64, turns: f64) -> Vec<DVec3> {
-        let turns = if turns <= 0.0 { DEFAULT_TURNS } else { turns };
-        let segments = ((turns * SEG_PER_TURN as f64).round() as usize).max(1);
-        let mut pts = Vec::with_capacity(segments + 1);
-        for t in 0..=segments {
-            let frac = t as f64 / segments as f64;
-            let angle = turns * std::f64::consts::TAU * frac;
-            let radius = base_radius + (top_radius - base_radius) * frac;
-            let z = height * frac;
-            pts.push(
-                self.center
-                    + self.plane.x * (radius * angle.cos())
-                    + self.plane.y * (radius * angle.sin())
-                    + self.plane.z * z,
-            );
-        }
-        pts
-    }
-
-    /// Build the committed helix entity from the collected parameters.
-    fn build(&self) -> Option<EntityType> {
-        let pts = self.sample_points(self.base_radius, self.top_radius, self.height, self.turns);
-        if pts.len() < 2 {
-            return None;
-        }
-        let verts: Vec<Vector3> = pts.iter().map(|p| Vector3::new(p.x, p.y, p.z)).collect();
-        let mut pl = Polyline3D::from_points(verts);
-        pl.flags.closed = false;
-        Some(EntityType::Polyline3D(pl))
-    }
-
-    /// Parse a positive distance from typed text; returns `None` on failure.
     fn parse_value(text: &str) -> Option<f64> {
         crate::entities::common::parse_typed_length(text)
     }
+
+    fn parse_turns(text: &str) -> Option<f64> {
+        text.trim().replace(',', ".").parse().ok().filter(|value: &f64| value.is_finite())
+    }
+
+    fn axis_start_direction(&self, axis: Vec3) -> Option<Vec3> {
+        let project = |direction: DVec3| {
+            let direction = Vec3::from(direction.to_array());
+            (direction - axis * direction.dot(axis)).normalize()
+        };
+        project(self.plane.x).or_else(|| project(self.plane.y))
+    }
+
+    fn resolved_turns(&self, height: f64) -> f64 {
+        match self.requested_turn_height {
+            Some(turn_height) if turn_height > EPSILON => (height.abs() / turn_height).max(EPSILON),
+            _ => self.turns,
+        }
+    }
+
+    fn kernel_curve(&self, height: f64, axis: DVec3) -> Option<HelixCurve> {
+        let axis = Vec3::from(axis.to_array())
+            .normalize()
+            .or_else(|| Vec3::from(self.plane.z.to_array()).normalize())?;
+        Some(HelixCurve {
+            base_center: self.center.to_array(),
+            axis_direction: axis.to_array(),
+            start_direction: self.axis_start_direction(axis)?.to_array(),
+            base_radius: self.base_radius,
+            top_radius: self.top_radius,
+            height,
+            turns: self.resolved_turns(height),
+            direction: if self.counter_clockwise {
+                HelixDirection::CounterClockwise
+            } else {
+                HelixDirection::Clockwise
+            },
+        })
+    }
+
+    fn build(&self, height: f64, axis: DVec3) -> Option<EntityType> {
+        let curve = self.kernel_curve(height, axis)?;
+        let nurbs = curve.nurbs()?;
+        let axis = DVec3::from_array(curve.axis_direction).normalize_or(self.plane.z);
+        let start_direction = DVec3::from_array(curve.start_direction).normalize_or(self.plane.x);
+        let mut spline = Spline::new();
+        spline.degree = nurbs.degree() as i32;
+        spline.knots = nurbs.knots().to_vec();
+        spline.control_points = nurbs
+            .control_points()
+            .iter()
+            .map(|point| Vector3::new(point[0], point[1], point[2]))
+            .collect();
+        spline.flags.planar = false;
+        spline.flags.rational = nurbs.is_rational();
+        if spline.flags.rational {
+            spline.weights = nurbs.weights().to_vec();
+        }
+
+        let mut helix = Helix::new();
+        helix.spline = spline;
+        helix.axis_base_point = Vector3::new(self.center.x, self.center.y, self.center.z);
+        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.turns = curve.turns;
+        helix.turn_height = height / curve.turns;
+        helix.handedness = self.counter_clockwise;
+        helix.constraint = self.constraint;
+        Some(EntityType::Helix(helix))
+    }
+
+    fn commit(&mut self, height: f64, axis: DVec3) -> CmdResult {
+        let Some(entity) = self.build(height, axis) else {
+            return CmdResult::NeedPoint;
+        };
+        self.height = height;
+        self.turns = self.resolved_turns(height);
+        defaults::set_helix_base_radius(self.base_radius);
+        defaults::set_helix_height(height);
+        defaults::set_helix_turns(self.turns);
+        defaults::set_helix_counter_clockwise(self.counter_clockwise);
+        CmdResult::CommitAndExit(entity)
+    }
+
+    fn preview(&self, height: f64, axis: DVec3) -> Option<WireModel> {
+        let points = self
+            .kernel_curve(height, axis)?
+            .nurbs()?
+            .tessellate_angle(cadkernel::tessellation::DEFAULT_ANGLE);
+        Some(WireModel::solid_f64(
+            "helix_preview".to_string(),
+            points,
+            WireModel::CYAN,
+            false,
+        ))
+    }
 }
 
 impl CadCommand for HelixCommand {
@@ -123,65 +180,158 @@ impl CadCommand for HelixCommand {
 
     fn prompt(&self) -> String {
         match self.step {
-            Step::Center => t!("HELIX  Specify centre point of base:").into_owned(),
-            Step::BaseRadius => t!("HELIX  Specify base radius:").into_owned(),
-            Step::TopRadius => t!("HELIX  Specify top radius <same as base>:").into_owned(),
-            Step::Height => t!("HELIX  Specify helix height:").into_owned(),
-            Step::Turns => t!("HELIX  Enter number of turns <3>:").into_owned(),
+            Step::Center => crate::tf!(
+                "HELIX  Number of turns = {:.4}  Twist = {}\nSpecify center point of base:",
+                self.turns,
+                if self.counter_clockwise { "CCW" } else { "CW" }
+            )
+            .into_owned(),
+            Step::BaseRadius => crate::tf!(
+                "HELIX  Specify base radius or [Diameter] <{:.4}>:",
+                self.base_radius
+            )
+            .into_owned(),
+            Step::BaseDiameter => crate::tf!(
+                "HELIX  Specify base diameter <{:.4}>:",
+                self.base_radius * 2.0
+            )
+            .into_owned(),
+            Step::TopRadius => crate::tf!(
+                "HELIX  Specify top radius or [Diameter] <{:.4}>:",
+                self.base_radius
+            )
+            .into_owned(),
+            Step::TopDiameter => crate::tf!(
+                "HELIX  Specify top diameter <{:.4}>:",
+                self.base_radius * 2.0
+            )
+            .into_owned(),
+            Step::Final => crate::tf!(
+                "HELIX  Specify helix height or [Axis endpoint/Turns/turn Height/tWist] <{:.4}>:",
+                self.height
+            )
+            .into_owned(),
+            Step::AxisEndpoint => t!("HELIX  Specify axis endpoint:").into_owned(),
+            Step::Turns => crate::tf!("HELIX  Enter number of turns <{:.4}>:", self.turns).into_owned(),
+            Step::TurnHeight => crate::tf!(
+                "HELIX  Specify distance between turns <{:.4}>:",
+                self.height.abs() / self.turns.max(EPSILON)
+            )
+            .into_owned(),
+            Step::Twist => crate::tf!(
+                "HELIX  Enter twist direction [Clockwise/Counterclockwise] <{}>:",
+                if self.counter_clockwise { "CCW" } else { "CW" }
+            )
+            .into_owned(),
         }
     }
 
-    fn on_point(&mut self, pt: DVec3) -> CmdResult {
+    fn options(&self) -> Vec<CmdOption> {
+        match self.step {
+            Step::BaseRadius | Step::TopRadius => vec![CmdOption::new(t!("Diameter").as_ref(), "D")],
+            Step::Final => vec![
+                CmdOption::new(t!("Axis endpoint").as_ref(), "A"),
+                CmdOption::new(t!("Turns").as_ref(), "T"),
+                CmdOption::new(t!("Turn Height").as_ref(), "H"),
+                CmdOption::new(t!("Twist").as_ref(), "W"),
+            ],
+            Step::Twist => vec![
+                CmdOption::new(t!("Clockwise").as_ref(), "CW"),
+                CmdOption::new(t!("Counterclockwise").as_ref(), "CCW"),
+            ],
+            _ => Vec::new(),
+        }
+    }
+
+    fn point_step_accepts_keywords(&self) -> bool {
+        matches!(self.step, Step::BaseRadius | Step::TopRadius | Step::Final | Step::Twist)
+    }
+
+    fn on_point(&mut self, point: DVec3) -> CmdResult {
         match self.step {
             Step::Center => {
-                self.center = pt;
+                self.center = point;
                 self.step = Step::BaseRadius;
                 CmdResult::NeedPoint
             }
-            Step::BaseRadius => {
-                // A point click resolves the base radius as the planar distance
-                // from the centre to the picked point.
-                let delta = self.plane.vector_to_local(pt - self.center);
-                let r = delta.x.hypot(delta.y);
-                if r > 0.0 {
-                    self.base_radius = r;
+            Step::BaseRadius | Step::BaseDiameter => {
+                let local = self.plane.vector_to_local(point - self.center);
+                let distance = local.x.hypot(local.y);
+                let radius = if self.step == Step::BaseDiameter {
+                    distance * 0.5
+                } else {
+                    distance
+                };
+                if radius > EPSILON {
+                    self.base_radius = radius;
+                    self.top_radius = radius;
                     self.step = Step::TopRadius;
                 }
                 CmdResult::NeedPoint
             }
-            Step::TopRadius => {
-                let delta = self.plane.vector_to_local(pt - self.center);
-                let r = delta.x.hypot(delta.y);
-                self.top_radius = if r > 0.0 { r } else { self.base_radius };
-                self.step = Step::Height;
+            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 {
+                    distance * 0.5
+                } else {
+                    distance
+                };
+                self.step = Step::Final;
                 CmdResult::NeedPoint
             }
-            Step::Height => {
-                // Use the elevation of the picked point relative to the centre.
-                let h = (pt - self.center).dot(self.plane.z);
-                if h.abs() > 0.0 {
-                    self.height = h;
-                    self.step = Step::Turns;
+            Step::Final => {
+                let height = (point - self.center).dot(self.plane.z);
+                if height.abs() <= EPSILON {
+                    CmdResult::NeedPoint
+                } else {
+                    self.commit(height, self.plane.z)
                 }
-                CmdResult::NeedPoint
             }
-            Step::Turns => CmdResult::NeedPoint,
+            Step::AxisEndpoint => {
+                let vector = point - self.center;
+                let height = vector.length();
+                if height <= EPSILON {
+                    CmdResult::NeedPoint
+                } else {
+                    self.commit(height, vector / height)
+                }
+            }
+            Step::Turns | Step::TurnHeight | Step::Twist => CmdResult::NeedPoint,
         }
     }
 
     fn on_enter(&mut self) -> CmdResult {
         match self.step {
-            Step::TopRadius => {
-                // Enter keeps the top radius equal to the base radius.
+            Step::Center => CmdResult::Cancel,
+            Step::BaseRadius | Step::BaseDiameter => {
                 self.top_radius = self.base_radius;
-                self.step = Step::Height;
+                self.step = Step::TopRadius;
+                CmdResult::NeedPoint
+            }
+            Step::TopRadius | Step::TopDiameter => {
+                self.top_radius = self.base_radius;
+                self.step = Step::Final;
+                CmdResult::NeedPoint
+            }
+            Step::Final => self.commit(self.height, self.plane.z),
+            Step::AxisEndpoint => CmdResult::NeedPoint,
+            Step::Turns => {
+                self.constraint = HelixConstraint::Turns;
+                self.requested_turn_height = None;
+                self.step = Step::Final;
+                CmdResult::NeedPoint
+            }
+            Step::TurnHeight => {
+                self.constraint = HelixConstraint::TurnHeight;
+                self.requested_turn_height = Some(self.height.abs() / self.turns.max(EPSILON));
+                self.step = Step::Final;
+                CmdResult::NeedPoint
+            }
+            Step::Twist => {
+                self.step = Step::Final;
                 CmdResult::NeedPoint
             }
-            Step::Turns => match self.build() {
-                Some(e) => CmdResult::CommitAndExit(e),
-                None => CmdResult::Cancel,
-            },
-            _ => CmdResult::NeedPoint,
         }
     }
 
@@ -190,123 +340,156 @@ impl CadCommand for HelixCommand {
     }
 
     fn wants_text_input(&self) -> bool {
-        // Every step past the centre pick reads a typed numeric value.
         self.step != Step::Center
     }
 
     fn dyn_field(&self) -> DynField {
         match self.step {
-            Step::Center => DynField::Point,
-            Step::BaseRadius | Step::TopRadius => DynField::Distance,
-            Step::Height => DynField::Distance,
-            Step::Turns => DynField::Scalar,
+            Step::Center | Step::AxisEndpoint => DynField::Point,
+            Step::BaseRadius | Step::BaseDiameter | Step::TopRadius | Step::TopDiameter | Step::Final | Step::TurnHeight => DynField::Distance,
+            Step::Turns | Step::Twist => DynField::Scalar,
         }
     }
 
     fn on_text_input(&mut self, text: &str) -> Option<CmdResult> {
-        let t = text.trim();
+        let value = text.trim();
+        let keyword = value.to_ascii_uppercase();
         match self.step {
             Step::Center => None,
+            Step::BaseRadius if matches!(keyword.as_str(), "D" | "DIAMETER") => {
+                self.step = Step::BaseDiameter;
+                Some(CmdResult::NeedPoint)
+            }
+            Step::TopRadius if matches!(keyword.as_str(), "D" | "DIAMETER") => {
+                self.step = Step::TopDiameter;
+                Some(CmdResult::NeedPoint)
+            }
+            Step::Final if matches!(keyword.as_str(), "A" | "AXIS" | "AXIS ENDPOINT") => {
+                self.step = Step::AxisEndpoint;
+                Some(CmdResult::NeedPoint)
+            }
+            Step::Final if matches!(keyword.as_str(), "T" | "TURNS") => {
+                self.step = Step::Turns;
+                Some(CmdResult::NeedPoint)
+            }
+            Step::Final if matches!(keyword.as_str(), "H" | "HEIGHT" | "TURN HEIGHT") => {
+                self.step = Step::TurnHeight;
+                Some(CmdResult::NeedPoint)
+            }
+            Step::Final if matches!(keyword.as_str(), "W" | "TWIST") => {
+                self.step = Step::Twist;
+                Some(CmdResult::NeedPoint)
+            }
             Step::BaseRadius => {
-                if let Some(v) = Self::parse_value(t) {
-                    if v > 0.0 {
-                        self.base_radius = v;
-                        self.step = Step::TopRadius;
-                    }
+                let radius = Self::parse_value(value)?;
+                if radius > EPSILON {
+                    self.base_radius = radius;
+                    self.top_radius = radius;
+                    self.step = Step::TopRadius;
+                }
+                Some(CmdResult::NeedPoint)
+            }
+            Step::BaseDiameter => {
+                let diameter = Self::parse_value(value)?;
+                if diameter > EPSILON {
+                    self.base_radius = diameter * 0.5;
+                    self.top_radius = self.base_radius;
+                    self.step = Step::TopRadius;
                 }
                 Some(CmdResult::NeedPoint)
             }
             Step::TopRadius => {
-                if t.is_empty() {
-                    // Empty entry keeps the top radius equal to the base radius.
-                    self.top_radius = self.base_radius;
-                    self.step = Step::Height;
-                } else if let Some(v) = Self::parse_value(t) {
-                    self.top_radius = if v > 0.0 { v } else { self.base_radius };
-                    self.step = Step::Height;
+                let radius = Self::parse_value(value)?;
+                if radius >= 0.0 {
+                    self.top_radius = radius;
+                    self.step = Step::Final;
                 }
                 Some(CmdResult::NeedPoint)
             }
-            Step::Height => {
-                if let Some(v) = Self::parse_value(t) {
-                    if v != 0.0 {
-                        self.height = v;
-                        self.step = Step::Turns;
-                    }
+            Step::TopDiameter => {
+                let diameter = Self::parse_value(value)?;
+                if diameter >= 0.0 {
+                    self.top_radius = diameter * 0.5;
+                    self.step = Step::Final;
                 }
                 Some(CmdResult::NeedPoint)
             }
+            Step::Final => {
+                let height = Self::parse_value(value)?;
+                (height.abs() > EPSILON).then(|| self.commit(height, self.plane.z))
+            }
+            Step::AxisEndpoint => None,
             Step::Turns => {
-                self.turns = if t.is_empty() {
-                    DEFAULT_TURNS
-                } else {
-                    match Self::parse_value(t) {
-                        Some(v) if v > 0.0 => v,
-                        _ => return Some(CmdResult::NeedPoint),
-                    }
-                };
-                match self.build() {
-                    Some(e) => Some(CmdResult::CommitAndExit(e)),
-                    None => Some(CmdResult::Cancel),
+                let turns = Self::parse_turns(value)?;
+                if turns > EPSILON {
+                    self.turns = turns;
+                    self.requested_turn_height = None;
+                    self.constraint = HelixConstraint::Turns;
+                    self.step = Step::Final;
                 }
+                Some(CmdResult::NeedPoint)
+            }
+            Step::TurnHeight => {
+                let turn_height = Self::parse_value(value)?;
+                if turn_height > EPSILON {
+                    self.requested_turn_height = Some(turn_height);
+                    self.constraint = HelixConstraint::TurnHeight;
+                    self.step = Step::Final;
+                }
+                Some(CmdResult::NeedPoint)
+            }
+            Step::Twist => {
+                match keyword.as_str() {
+                    "C" | "CW" | "CLOCKWISE" => self.counter_clockwise = false,
+                    "CC" | "CCW" | "COUNTERCLOCKWISE" => self.counter_clockwise = true,
+                    _ => return Some(CmdResult::NeedPoint),
+                }
+                self.step = Step::Final;
+                Some(CmdResult::NeedPoint)
             }
         }
     }
 
-    fn on_mouse_move(&mut self, pt: DVec3) -> Option<WireModel> {
-        // Preview the helix once enough parameters are known. Before a base
-        // radius is fixed, track the cursor distance to size a live ring.
+    fn on_mouse_move(&mut self, point: DVec3) -> Option<WireModel> {
         match self.step {
             Step::Center => None,
-            Step::BaseRadius => {
-                let delta = self.plane.vector_to_local(pt - self.center);
-                let r = delta.x.hypot(delta.y);
-                if r <= 0.0 {
-                    return None;
-                }
-                let pts = self.sample_points(r, r, 0.0, 1.0);
-                let raw: Vec<[f64; 3]> = pts.iter().map(|p| [p.x, p.y, p.z]).collect();
-                Some(WireModel::solid_f64(
-                    "helix_preview".to_string(),
-                    raw,
-                    WireModel::CYAN,
-                    false,
-                ))
-            }
-            _ => {
-                // Use the parameters fixed so far; for the height step let the
-                // cursor elevation drive the live height.
-                let height = if self.step == Step::Height {
-                    (pt - self.center).dot(self.plane.z)
+            Step::BaseRadius | Step::BaseDiameter => {
+                let local = self.plane.vector_to_local(point - self.center);
+                let old = (self.base_radius, self.top_radius);
+                let distance = local.x.hypot(local.y);
+                self.base_radius = if self.step == Step::BaseDiameter {
+                    distance * 0.5
                 } else {
-                    self.height
-                };
-                let top = if self.step == Step::TopRadius {
-                    let delta = self.plane.vector_to_local(pt - self.center);
-                    let r = delta.x.hypot(delta.y);
-                    if r > 0.0 {
-                        r
-                    } else {
-                        self.base_radius
-                    }
-                } else {
-                    self.top_radius
-                };
-                let pts = self.sample_points(self.base_radius, top, height, self.turns);
-                if pts.len() < 2 {
-                    return None;
+                    distance
                 }
-                let raw: Vec<[f64; 3]> = pts.iter().map(|p| [p.x, p.y, p.z]).collect();
-                Some(WireModel::solid_f64(
-                    "helix_preview".to_string(),
-                    raw,
-                    WireModel::CYAN,
-                    false,
-                ))
+                .max(EPSILON);
+                self.top_radius = self.base_radius;
+                let preview = self.preview(0.0, self.plane.z);
+                (self.base_radius, self.top_radius) = old;
+                preview
             }
+            Step::TopRadius | Step::TopDiameter => {
+                let local = self.plane.vector_to_local(point - self.center);
+                let old = self.top_radius;
+                let distance = local.x.hypot(local.y);
+                self.top_radius = if self.step == Step::TopDiameter {
+                    distance * 0.5
+                } else {
+                    distance
+                };
+                let preview = self.preview(0.0, self.plane.z);
+                self.top_radius = old;
+                preview
+            }
+            Step::Final => self.preview((point - self.center).dot(self.plane.z), self.plane.z),
+            Step::AxisEndpoint => {
+                let vector = point - self.center;
+                let height = vector.length();
+                (height > EPSILON).then(|| self.preview(height, vector / height)).flatten()
+            }
+            Step::Turns | Step::TurnHeight | Step::Twist => self.preview(self.height, self.plane.z),
         }
     }
 }
 
-// ── Autocomplete registry ─────────────────────────────────
-inventory::submit!(crate::command::CommandRegistration { names: &["HELIX"] });  // HelixCommand
+inventory::submit!(crate::command::CommandRegistration { names: &["HELIX"] });

src/modules/draw/draw/mesh3d.rs

@@ -17,7 +17,7 @@ enum Step {
     Vertices,
 }
 
-/// Creates a legacy M by N polygon mesh from row-major 3D vertex input.
+/// Creates an M by N polygon mesh from row-major 3D input.
 pub struct Mesh3dCommand {
     step: Step,
     m: usize,
@@ -159,6 +159,9 @@ impl CadCommand for Mesh3dCommand {
         if self.step != Step::Vertices {
             return CmdResult::NeedPoint;
         }
+        if !point.is_finite() {
+            return CmdResult::NeedPoint;
+        }
         self.points.push(point);
         if self.points.len() == self.total_vertices() {
             self.build()

src/modules/draw/draw/mod.rs

@@ -5,6 +5,7 @@ pub mod circle;
 pub mod dimcenter;
 pub mod donut;
 pub mod ellipse;
+pub mod face3d;
 pub mod hatch;
 pub mod hatchedit;
 pub mod helix;

src/modules/draw/draw/poly3d.rs

@@ -1,11 +1,4 @@
-// 3D polyline tool — interactive command.
-//
-// Command: 3DPOLY — pick a sequence of points whose Z ordinates may differ and
-// commit them as a single 3D polyline of straight segments. Unlike PLINE (a
-// planar lightweight polyline with bulge arcs), every vertex keeps its own Z,
-// so the result is a true non-planar path. Close (C) adds a closing segment
-// back to the first point; Undo (U) drops the last picked vertex. Enter / Esc
-// finishes the open polyline.
+// 3DPOLY creates open or closed non-planar paths.
 
 use acadrust::entities::Polyline3D;
 use acadrust::types::Vector3;
@@ -67,7 +60,7 @@ impl CadCommand for Poly3dCommand {
     fn prompt(&self) -> String {
         match self.points.len() {
             0 => t!("3DPOLY  Specify start point of polyline:").into_owned(),
-            1 => t!("3DPOLY  Specify next point  [Undo]:").into_owned(),
+            1 | 2 => t!("3DPOLY  Specify next point  [Undo]:").into_owned(),
             _ => t!("3DPOLY  Specify next point  [Close/Undo]:").into_owned(),
         }
     }
@@ -98,10 +91,10 @@ impl CadCommand for Poly3dCommand {
 
     fn on_text_input(&mut self, text: &str) -> Option<CmdResult> {
         match text.trim().to_uppercase().as_str() {
-            "C" | "CLOSE" => match self.build(true) {
-                Some(e) => Some(CmdResult::CommitAndExit(e)),
-                None => Some(CmdResult::NeedPoint),
-            },
+            "C" | "CLOSE" if self.points.len() >= 3 => self
+                .build(true)
+                .map(CmdResult::CommitAndExit),
+            "C" | "CLOSE" => Some(CmdResult::NeedPoint),
             "U" | "UNDO" => {
                 self.points.pop();
                 Some(CmdResult::NeedPoint)

src/modules/draw/modify/pedit.rs

@@ -1,26 +1,15 @@
-// PEDIT command — edit a polyline entity (#263).
-//
-// Flow:
-//   Select polyline — picking a LINE or ARC instead asks "Turn it into one?"
-//   (Yes converts in place and continues editing the result).
-//   Options: Close / Open / Join / Width / Fit / Spline / Decurve / eXit.
-//   Join gathers more segments (lines / arcs / polylines) and merges every
-//   contiguous run through the JOIN machinery. (Vertex editing lives on the
-//   polyline's grips, not in PEDIT.)
-//   Fit smooths the segments into tangent-blended arcs, Spline replaces the
-//   shape with a sampled cubic B-spline of the vertex frame, Decurve
-//   straightens every segment.
+// PEDIT edits polylines and polygon meshes.
 
 use acadrust::entities::LwVertex;
-use cadkernel::geom2d::nurbs::clamped_uniform_knots;
-use cadkernel::geom2d::NurbsCurve;
 use acadrust::types::Vector2;
 use acadrust::{EntityType, Handle};
+use cadkernel::geom2d::nurbs::clamped_uniform_knots;
+use cadkernel::geom2d::NurbsCurve;
 use glam::{DVec2, DVec3};
-use crate::t;
 use rustc_hash::FxHashMap as HashMap;
 
 use crate::command::{CadCommand, CmdResult};
+use crate::t;
 
 const TAU: f64 = std::f64::consts::TAU;
 
@@ -53,6 +42,7 @@ pub struct PeditCommand {
     target: Option<Handle>,
     info: HashMap<u64, PeditTarget>,
     mode: Mode,
+    undo_count: usize,
 }
 
 impl PeditCommand {
@@ -61,6 +51,7 @@ impl PeditCommand {
             target: None,
             info,
             mode: Mode::PickTarget,
+            undo_count: 0,
         }
     }
 
@@ -88,7 +79,6 @@ impl PeditCommand {
         let handle = self.target?;
         self.info.get(&handle.value())?.mesh_size
     }
-
 }
 
 impl CadCommand for PeditCommand {
@@ -140,15 +130,15 @@ impl CadCommand for PeditCommand {
                 CmdOption::new(t!("eXit").as_ref(), "X"),
             ],
             Mode::Options => vec![
-                    CmdOption::new(t!("Close").as_ref(), "C"),
-                    CmdOption::new(t!("Open").as_ref(), "O"),
-                    CmdOption::new(t!("Join").as_ref(), "J"),
-                    CmdOption::new(t!("Width").as_ref(), "W"),
-                    CmdOption::new(t!("Fit").as_ref(), "F"),
-                    CmdOption::new(t!("Spline").as_ref(), "S"),
-                    CmdOption::new(t!("Decurve").as_ref(), "D"),
-                    CmdOption::new(t!("eXit").as_ref(), "X"),
-                ],
+                CmdOption::new(t!("Close").as_ref(), "C"),
+                CmdOption::new(t!("Open").as_ref(), "O"),
+                CmdOption::new(t!("Join").as_ref(), "J"),
+                CmdOption::new(t!("Width").as_ref(), "W"),
+                CmdOption::new(t!("Fit").as_ref(), "F"),
+                CmdOption::new(t!("Spline").as_ref(), "S"),
+                CmdOption::new(t!("Decurve").as_ref(), "D"),
+                CmdOption::new(t!("eXit").as_ref(), "X"),
+            ],
             Mode::MeshVertex(_) => vec![
                 CmdOption::new(t!("Next").as_ref(), "N"),
                 CmdOption::new(t!("Previous").as_ref(), "P"),
@@ -218,19 +208,23 @@ impl CadCommand for PeditCommand {
         // piece as the live target and carry its bookkeeping over.
         if let Some(&nh) = new_handles.first() {
             self.info.remove(&old.value());
-                self.info.insert(
+            self.info.insert(
                 nh.value(),
-                    PeditTarget {
-                        is_poly: true,
-                        convertible: false,
-                        mesh_size: None,
-                    },
+                PeditTarget {
+                    is_poly: true,
+                    convertible: false,
+                    mesh_size: None,
+                },
             );
             self.target = Some(nh);
             self.mode = Mode::Options;
         }
     }
 
+    fn on_pedit_applied(&mut self) {
+        self.undo_count = self.undo_count.saturating_add(1);
+    }
+
     fn wants_text_input(&self) -> bool {
         !matches!(self.mode, Mode::PickTarget | Mode::MeshVertexMove(_))
     }
@@ -332,7 +326,11 @@ impl CadCommand for PeditCommand {
                             handle,
                             op: PeditOp::SetMeshClosedN(false),
                         }),
-                        "U" | "UNDO" => Some(CmdResult::UndoDocument),
+                        "U" | "UNDO" if self.undo_count > 0 => {
+                            self.undo_count -= 1;
+                            Some(CmdResult::UndoDocument)
+                        }
+                        "U" | "UNDO" => Some(CmdResult::NeedPoint),
                         "X" | "EXIT" => Some(CmdResult::Cancel),
                         _ => None,
                     };
@@ -481,6 +479,9 @@ pub fn apply_pedit(entity: &mut EntityType, op: &PeditOp) -> bool {
         },
         PeditOp::SetMeshClosedM(closed) => match entity {
             EntityType::PolygonMesh(mesh) => {
+                if mesh.is_closed_m() == *closed {
+                    return false;
+                }
                 mesh.flags.set(
                     acadrust::entities::polygon_mesh::PolygonMeshFlags::CLOSED_M,
                     *closed,
@@ -491,6 +492,9 @@ pub fn apply_pedit(entity: &mut EntityType, op: &PeditOp) -> bool {
         },
         PeditOp::SetMeshClosedN(closed) => match entity {
             EntityType::PolygonMesh(mesh) => {
+                if mesh.is_closed_n() == *closed {
+                    return false;
+                }
                 mesh.flags.set(
                     acadrust::entities::polygon_mesh::PolygonMeshFlags::CLOSED_N,
                     *closed,
@@ -501,26 +505,38 @@ pub fn apply_pedit(entity: &mut EntityType, op: &PeditOp) -> bool {
         },
         PeditOp::SetMeshSmooth(smooth) => match entity {
             EntityType::PolygonMesh(mesh) => {
+                let mut changed = mesh.smooth_type != *smooth;
                 mesh.smooth_type = *smooth;
                 if mesh.smooth_type
                     != acadrust::entities::polygon_mesh::SurfaceSmoothType::NoSmooth
                 {
-                    if mesh.m_smooth_density < 2 {
+                    if !(2..=256).contains(&mesh.m_smooth_density) {
                         mesh.m_smooth_density = 6;
+                        changed = true;
                     }
-                    if mesh.n_smooth_density < 2 {
+                    if !(2..=256).contains(&mesh.n_smooth_density) {
                         mesh.n_smooth_density = 6;
+                        changed = true;
                     }
                 }
-                true
+                changed
             }
             _ => false,
         },
         PeditOp::MoveMeshVertex { index, point } => match entity {
             EntityType::PolygonMesh(mesh) => {
+                if !point.is_finite() {
+                    return false;
+                }
                 let Some(vertex) = mesh.vertices.get_mut(*index) else {
                     return false;
                 };
+                if vertex.location.x == point.x
+                    && vertex.location.y == point.y
+                    && vertex.location.z == point.z
+                {
+                    return false;
+                }
                 vertex.location = acadrust::types::Vector3::new(point.x, point.y, point.z);
                 true
             }