diff --git a/src/modules/draw/modify/fillet.rs b/src/modules/draw/modify/fillet.rs index 96e014f3e..1d299c76e 100644 --- a/src/modules/draw/modify/fillet.rs +++ b/src/modules/draw/modify/fillet.rs @@ -794,6 +794,16 @@ impl FilletEntity { } } +/// The refusal a no-trim FILLET prints for anything but two independent Lines. +const NO_TRIM_LINES_ONLY: &str = "No-trim fillet supports two independent lines only."; + +/// No-trim FILLET is defined for two independent Lines only: the addition path +/// keeps both sources untouched, which the arc / polyline branches cannot +/// express without falling back to trimming. +fn no_trim_pair_supported(first: &FilletEntity, second: &FilletEntity) -> bool { + matches!(first, FilletEntity::Line(_)) && matches!(second, FilletEntity::Line(_)) +} + /// Compute FILLET between two entities (Line, Arc, or LwPolyline segment). /// Returns (trimmed_e1, trimmed_e2, optional_fillet_arc). /// For same-poly corner fillet the two returned entities are identical (the rebuilt poly). @@ -1479,6 +1489,9 @@ enum FilletStep { pub struct FilletCommand { radius: f64, + /// Whether the picked source entities are shortened to the tangent points. + /// No-trim is defined for two independent Lines only. + trim: bool, step: FilletStep, all_entities: Vec, entity_index: ModifyEntityIndex, @@ -1499,6 +1512,7 @@ impl FilletCommand { let entity_index = ModifyEntityIndex::build(&all_entities); Self { radius: radius as f64, + trim: defaults::get_fillet_trim(), step: FilletStep::First, all_entities, entity_index, @@ -1560,6 +1574,17 @@ impl FilletCommand { CmdResult::ReplaceManyContinue(replacements) } + /// Keep both source lines and add only the tangent arc. The host commits + /// the addition as one undo step and notifies this command of its new handle. + fn continue_after_addition(&mut self, addition: EntityType) -> CmdResult { + self.all_entities.push(addition.clone()); + self.entity_index = ModifyEntityIndex::build(&self.all_entities); + self.step = FilletStep::First; + self.resume_second = None; + self.made += 1; + CmdResult::CommitEntities(vec![addition]) + } + /// Undo option: take back the last fillet of this command. The host /// undoes the document step and then hands the restored entities back /// through `on_document_undone`. @@ -1608,7 +1633,7 @@ impl CadCommand for FilletCommand { FilletStep::RadiusSecondPoint { .. } => { crate::t!("FILLET Specify second point for radius:").into_owned() } - FilletStep::TrimMode => if defaults::get_fillet_trim() { + FilletStep::TrimMode => if self.trim { crate::t!("FILLET Enter Trim mode option [Trim/No trim] :") } else { crate::t!("FILLET Enter Trim mode option [Trim/No trim] :") @@ -1633,11 +1658,22 @@ impl CadCommand for FilletCommand { } opts.push(CmdOption::new("Polyline", "P")); opts.push(CmdOption::new("Radius", "R")); - opts.push(CmdOption::new("Trim", "T")); + opts.push(if self.trim { + CmdOption::new("No trim", "N") + } else { + CmdOption::new("Trim", "T") + }); opts } FilletStep::Second { .. } => { - vec![CmdOption::new("Radius", "R"), CmdOption::new("Trim", "T")] + vec![ + CmdOption::new("Radius", "R"), + if self.trim { + CmdOption::new("No trim", "N") + } else { + CmdOption::new("Trim", "T") + }, + ] } FilletStep::TrimMode => { vec![CmdOption::new("Trim", "T"), CmdOption::new("No trim", "N")] @@ -1686,8 +1722,14 @@ impl CadCommand for FilletCommand { FilletStep::TrimMode => { match text.trim().to_uppercase().as_str() { "" => {} - "T" | "TRIM" => defaults::set_fillet_trim(true), - "N" | "NO TRIM" | "NOTRIM" => defaults::set_fillet_trim(false), + "T" | "TRIM" => { + self.trim = true; + defaults::set_fillet_trim(true); + } + "N" | "NO TRIM" | "NOTRIM" => { + self.trim = false; + defaults::set_fillet_trim(false); + } _ => return Some(CmdResult::NeedPoint), } self.resume_after_radius(); @@ -1696,6 +1738,14 @@ impl CadCommand for FilletCommand { FilletStep::First | FilletStep::Second { .. } => { let t = text.trim(); let upper = t.to_uppercase(); + if matches!(upper.as_str(), "N" | "NOTRIM" | "NO TRIM" | "TRIM=FALSE") { + self.trim = false; + return Some(CmdResult::NeedPoint); + } + if matches!(upper.as_str(), "T" | "TRIM" | "TRIM=TRUE") { + self.trim = true; + return Some(CmdResult::NeedPoint); + } if matches!(self.step, FilletStep::First) && upper == "P" { self.step = FilletStep::Polyline; return Some(CmdResult::NeedPoint); @@ -1799,6 +1849,11 @@ impl CadCommand for FilletCommand { other => FilletEntity::from_entity(other), }); if let Some(e) = e1 { + // Do not let no-trim silently fall through to the normal + // trimming path for a first object it cannot preserve. + if !self.trim && !matches!(e, FilletEntity::Line(_)) { + return CmdResult::ReportError(NO_TRIM_LINES_ONLY.to_string()); + } self.step = FilletStep::Second { h1: handle, e1: e, @@ -1832,18 +1887,20 @@ impl CadCommand for FilletCommand { if let Some(e2) = e2 { match compute_fillet_entities(&e1, click1, &e2, click, self.radius) { - // No trim: the objects stay as they are; only the arc - // is added (#1558). - Some((_, _, maybe_arc)) if !defaults::get_fillet_trim() => { - let Some(arc) = maybe_arc else { - return CmdResult::NeedPoint; - }; - self.step = FilletStep::First; - self.resume_second = None; - self.made += 1; - CmdResult::CommitEntity(arc) - } Some((new_e1, new_e2, maybe_arc)) => { + // In no-trim mode only two independent Lines are + // supported. Reuse the solver for the same arc + // branch, then discard its shortened-line results. + if !self.trim { + if !no_trim_pair_supported(&e1, &e2) { + self.step = FilletStep::First; + self.resume_second = None; + return CmdResult::ReportError(NO_TRIM_LINES_ONLY.to_string()); + } + return maybe_arc + .map(|arc| self.continue_after_addition(arc)) + .unwrap_or(CmdResult::NeedPoint); + } let mut first_replacements = vec![new_e1]; if let Some(arc) = maybe_arc { first_replacements.push(arc); @@ -1858,6 +1915,10 @@ impl CadCommand for FilletCommand { } None => CmdResult::NeedPoint, } + } else if !self.trim { + self.step = FilletStep::First; + self.resume_second = None; + CmdResult::ReportError(NO_TRIM_LINES_ONLY.to_string()) } else { CmdResult::NeedPoint } @@ -1930,11 +1991,14 @@ impl CadCommand for FilletCommand { // For non-LwPoly, skip same-entity hover. let _ = h1; if let Some(e2) = e2 { + if !self.trim && !no_trim_pair_supported(&e1, &e2) { + return vec![]; + } if let Some((new_e1, new_e2, maybe_arc)) = compute_fillet_entities(&e1, click1, &e2, click, self.radius) { let mut out = Vec::new(); - if defaults::get_fillet_trim() { + if self.trim { out.push(WireModel::solid( "fillet_e1".into(), entity_pts(&new_e1), @@ -1988,6 +2052,27 @@ impl CadCommand for FilletCommand { _ => CmdResult::NeedPoint, } } + fn preserve_commit_layer(&self) -> bool { + // Derived no-trim geometry stays on the picked line's layer rather + // than being moved to the current creation layer. + true + } + + fn on_entities_committed(&mut self, entities: &[EntityType]) { + let mut handles = entities.iter().map(|entity| entity.common().handle); + for entity in self + .all_entities + .iter_mut() + .filter(|entity| entity.common().handle.is_null()) + { + let Some(handle) = handles.next() else { + break; + }; + entity.common_mut().handle = handle; + } + self.entity_index = ModifyEntityIndex::build(&self.all_entities); + } + fn on_entity_replaced(&mut self, _old: Handle, new_handles: &[Handle]) { let mut handles = new_handles.iter().copied(); for entity in self @@ -2622,19 +2707,16 @@ mod tests { 1.0, vec![line(0.0, 0.0, 10.0, 0.0, 1), line(0.0, 0.0, 0.0, 10.0, 2)], ); - assert_eq!(keywords(&command), ["P", "R", "T"]); + assert_eq!(keywords(&command), ["P", "R", "N"]); assert!(command.on_text_input("U").is_none(), "nothing to undo yet"); command.on_entity_pick(Handle::new(1), DVec3::new(5.0, 0.0, 0.0)); assert!(matches!( command.on_entity_pick(Handle::new(2), DVec3::new(0.0, 5.0, 0.0)), CmdResult::ReplaceManyContinue(_) )); - assert_eq!(keywords(&command), ["U", "P", "R", "T"]); - assert!(matches!( - command.on_text_input("U"), - Some(CmdResult::UndoDocument) - )); - assert_eq!(keywords(&command), ["P", "R", "T"]); + assert_eq!(keywords(&command), ["U", "P", "R", "N"]); + assert!(matches!(command.on_text_input("U"), Some(CmdResult::UndoDocument))); + assert_eq!(keywords(&command), ["P", "R", "N"]); // The host hands the restored document back; the cache follows it. let mut doc = codec::CadDocument::new(); let _ = doc.add_entity(line(0.0, 0.0, 20.0, 0.0, 7)); @@ -2643,6 +2725,89 @@ mod tests { assert_eq!(command.all_entities[0].common().handle, Handle::new(7)); } + #[test] + fn no_trim_fillet_adds_only_an_arc_and_keeps_source_handles() { + let first = Handle::new(1); + let second = Handle::new(2); + let original = vec![ + line(0.0, 0.0, 10.0, 0.0, 1), + line(0.0, 0.0, 0.0, 10.0, 2), + ]; + let mut command = FilletCommand::new(1.0, original); + assert!(matches!(command.on_text_input("N"), Some(CmdResult::NeedPoint))); + assert!(!command.trim); + assert!(matches!( + command.on_entity_pick(first, DVec3::new(5.0, 0.0, 0.0)), + CmdResult::NeedPoint + )); + let addition = match command.on_entity_pick(second, DVec3::new(0.0, 5.0, 0.0)) { + CmdResult::CommitEntities(mut entities) => { + assert_eq!(entities.len(), 1); + entities.pop().unwrap() + } + _ => panic!("no-trim FILLET should add one arc"), + }; + assert!(matches!(addition, EntityType::Arc(_))); + assert!(command + .all_entities + .iter() + .any(|entity| entity.common().handle == first)); + assert!(command + .all_entities + .iter() + .any(|entity| entity.common().handle == second)); + assert!(command + .all_entities + .iter() + .any(|entity| matches!(entity, EntityType::Arc(arc) if arc.common.handle.is_null()))); + + let mut committed = addition; + committed.common_mut().handle = Handle::new(10); + command.on_entities_committed(&[committed]); + assert!(command + .all_entities + .iter() + .any(|entity| entity.common().handle == Handle::new(10))); + let source_lines: Vec<_> = command + .all_entities + .iter() + .filter_map(|entity| match entity { + EntityType::Line(line) => Some((line.start, line.end)), + _ => None, + }) + .collect(); + assert_eq!(source_lines[0], (Vector3::new(0.0, 0.0, 0.0), Vector3::new(10.0, 0.0, 0.0))); + assert_eq!(source_lines[1], (Vector3::new(0.0, 0.0, 0.0), Vector3::new(0.0, 10.0, 0.0))); + } + + #[test] + fn no_trim_fillet_refuses_non_line_first_picks() { + let mut arc = ArcEnt::new(); + arc.center = Vector3::new(10.0, 10.0, 0.0); + arc.radius = 5.0; + arc.start_angle = std::f64::consts::PI; + arc.end_angle = std::f64::consts::PI * 1.5; + arc.common.handle = Handle::new(2); + let mut command = FilletCommand::new( + 1.0, + vec![ + line(0.0, 10.0, 20.0, 10.0, 1), + EntityType::Arc(arc), + ], + ); + assert!(matches!(command.on_text_input("N"), Some(CmdResult::NeedPoint))); + assert!(matches!( + command.on_entity_pick(Handle::new(2), DVec3::new(10.0, 5.0, 0.0)), + CmdResult::ReportError(_) + )); + assert!(matches!(command.step, FilletStep::First)); + assert!(matches!( + command.on_entity_pick(Handle::new(1), DVec3::new(5.0, 10.0, 0.0)), + CmdResult::NeedPoint + )); + assert!(matches!(command.step, FilletStep::Second { .. })); + } + #[test] fn chamfer_multiple_keeps_going_and_single_ends() { let lines = || vec![line(0.0, 0.0, 10.0, 0.0, 1), line(0.0, 0.0, 0.0, 10.0, 2)];