diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index 5f361a8f..9728e54d 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -274,6 +274,14 @@ fn fillet_fitted( } } +/// Add `note` to a feature's warning. +fn append_warning(warning: &mut Option, note: String) { + *warning = Some(match warning.take() { + Some(w) => format!("{w}; {note}"), + None => note, + }); +} + /// A length in mm, without trailing zeros. fn show_mm(v: f64) -> String { let s = format!("{v:.6}"); @@ -285,11 +293,6 @@ impl ModelState { pub fn fitted_radius(&self, feature: u64) -> Option { self.fitted.get(&feature).copied() } - - /// Every reduced fillet radius (by feature id). - pub fn fitted_radii(&self) -> impl Iterator + '_ { - self.fitted.iter().map(|(k, v)| (*k, *v)) - } } /// Model after some prefix of the timeline. @@ -948,10 +951,7 @@ impl Model { }); if r.is_ok() { if let Some(note) = boolean_diagnostics.warning() { - warning = Some(match warning { - Some(existing) => format!("{existing}; {note}"), - None => note, - }); + append_warning(&mut warning, note); } } else { boolean_diagnostics = Default::default(); @@ -2842,7 +2842,11 @@ fn replay( let Some(mb) = st.bodies.get(ti).cloned() else { continue }; let nb = match &src.kind { FeatureKind::Chamfer { distance2, angle, flip, .. } => chamfer_with(vals, &mb.body, &moved, r, distance2, angle, *flip)?, - FeatureKind::Fillet { fit, .. } => fillet_fitted(&mb.body, &moved, r, *fit)?.0, + // A copy is congruent to the original: the radius it was reduced to first. + FeatureKind::Fillet { fit, .. } => match st.fitted.get(&src.id).copied() { + Some(used) => kernel::fillet(&mb.body, &moved, used).or_else(|_| fillet_fitted(&mb.body, &moved, r, *fit).map(|x| x.0))?, + None => fillet_fitted(&mb.body, &moved, r, *fit)?.0, + }, _ => kernel::fillet(&mb.body, &moved, r)?, }; let nb = Traced::step(nb, &[&mb.traced()]); @@ -3595,11 +3599,7 @@ fn eval_feature(doc: &Document, vals: &BTreeMap, f: &Feature, st: let (b, used) = fillet_fitted(&mb.body, &edges, r, *fit)?; if let Some(used) = used { st.fitted.insert(f.id, used); - let note = format!("the radius is reduced to {} mm, the largest that fits these edges", show_mm(used)); - *warning = Some(match warning.take() { - Some(w) => format!("{w}; {note}"), - None => note, - }); + append_warning(warning, format!("the radius is reduced to {} mm, the largest that fits these edges", show_mm(used))); } (b, input) } diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index df8c67da..3adfe75f 100644 --- a/crates/engine/src/cmd/features.rs +++ b/crates/engine/src/cmd/features.rs @@ -790,6 +790,9 @@ fn fillet(s: &mut Session, p: &Value) -> Result { Some("reduce") => solidcraft_doc::FilletFit::Reduce, Some(o) => return Err(bad(cmd, format!("unknown fit `{o}` (exact or reduce)"))), }; + if fit == solidcraft_doc::FilletFit::Reduce && !style.is_constant() { + return Err(bad(cmd, "`fit: reduce` takes a constant radius")); + } let mut out = add_feature(s, p, FeatureKind::Fillet { edges, radius, body: str_(p, "body").map(str::to_string), style, fit })?; // The radius the fillet used, when `fit: reduce` reduced it. if let Some(id) = out.get("feature").and_then(Value::as_u64) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 1a38ba67..f4f78e97 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -157,6 +157,8 @@ pub struct Preview { pub before: Arc, /// The model after the commands. pub after: Arc, + /// What each command replied (as `execute` returns it). + pub replies: Vec, } /// Prepared world geometry depends on design/model identity and placement context, not @@ -693,12 +695,12 @@ impl Session { /// A `timeline.redefine` is shown even while the timeline is rolled back to the feature /// being edited. pub fn preview(&self, commands: &[(String, Value)]) -> Result { - self.scratch().preview_in_place(commands).map(|after| Preview { before: self.model.state(), after: after.model.state() }) + self.scratch().preview_in_place(commands).map(|(after, replies)| Preview { before: self.model.state(), after: after.model.state(), replies }) } /// The same evaluated preview in occurrence coordinates, including uncaptured moves. pub fn preview_world(&self, commands: &[(String, Value)]) -> Result { - self.scratch().preview_in_place(commands).map(|after| Preview { before: self.world_state(), after: after.world_state() }) + self.scratch().preview_in_place(commands).map(|(after, replies)| Preview { before: self.world_state(), after: after.world_state(), replies }) } /// World preview with each session's visibility applied, including new body placements. @@ -709,15 +711,17 @@ impl Session { shown.bodies.retain(|body| session.body_visible(&body.name)); Arc::new(shown) }; - self.scratch().preview_in_place(commands).map(|after| Preview { before: visible(self), after: visible(&after) }) + self.scratch().preview_in_place(commands).map(|(after, replies)| Preview { before: visible(self), after: visible(&after), replies }) } - fn preview_in_place(mut self, commands: &[(String, Value)]) -> Result { + fn preview_in_place(mut self, commands: &[(String, Value)]) -> Result<(Self, Vec)> { if commands.is_empty() { return Err(EngineError::Other("nothing to preview".into())); } + let mut replies = Vec::with_capacity(commands.len()); for (id, params) in commands { let r = self.execute(id, params)?; + replies.push(r.clone()); if id == "timeline.redefine" && let Some(fid) = r.get("feature").and_then(Value::as_u64) && let Some(idx) = self.doc.feature_index(fid) @@ -730,7 +734,7 @@ impl Session { } } } - Ok(self) + Ok((self, replies)) } /// Measure one or two selections (see `inspect.measure` with `items`); errors come back as diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 4442b187..b09c2af9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2815,6 +2815,21 @@ fn fillet_fit_reduce_uses_the_largest_radius_that_fits() { // Saved only when set. let json = serde_json::to_string(&s.doc.features).unwrap(); assert_eq!(json.matches("\"fit\":\"reduce\"").count(), 1, "{json}"); - // An unknown mode is refused. + // An unknown mode is refused, and so is reduce with a radius that is not constant. assert!(s.execute("solid.fillet", &json!({"edges": mids, "radius": 1, "fit": "taper"})).is_err()); + assert!(s.execute("solid.fillet", &json!({"edges": mids, "radius": 1, "fit": "reduce", "type": "chord"})).is_err()); +} + +#[test] +fn a_pattern_of_a_reduced_fillet_rounds_each_copy_alike() { + // #293: a 6 mm wide post on a plate, its four vertical edges at 3.5 reduced to 3, patterned + // three times: every copy is a full round. + let mut s = Session::default(); + run(&mut s, "solid.box", json!({"length": 60, "width": 20, "height": 2})); + run(&mut s, "solid.box", json!({"length": 14, "width": 6, "height": 10, "corner": [2, 7, 2], "operation": "join", "name": "Post"})); + let mids = json!([[2, 7, 7], [16, 7, 7], [2, 13, 7], [16, 13, 7]]); + run(&mut s, "solid.fillet", json!({"edges": mids, "radius": 3.5, "fit": "reduce", "name": "Round"})); + run(&mut s, "solid.pattern.rectangular", json!({"features": ["Post", "Round"], "dir1": [1, 0, 0], "count1": 3, "spacing1": 20})); + let expect = 60.0 * 20.0 * 2.0 + 3.0 * (840.0 - (4.0 - PI) * 9.0 * 10.0); + assert!(rel(volume(&mut s), expect) < 2e-4, "{} vs {expect}", volume(&mut s)); } diff --git a/crates/kernel/src/curveblend.rs b/crates/kernel/src/curveblend.rs index 758852b4..a53f5c97 100644 --- a/crates/kernel/src/curveblend.rs +++ b/crates/kernel/src/curveblend.rs @@ -11,6 +11,7 @@ use std::collections::HashMap; use solidcraft_geom::Vec3; use truck_modeling::{self as mt, builder}; +use crate::blend::too_large; use crate::body::{Solid, from_p3, p3}; use crate::record::Rebuild; use crate::{KernelError, Result}; @@ -691,7 +692,7 @@ fn curve_blend_with_limit(solid: &Solid, ids: &[mt::EdgeID], size: f64, r: f64, let (ta, oa) = a.as_ref().map(at).unwrap_or((0.0, 0.0)); let (tb, ob) = b.as_ref().map(at).unwrap_or((1.0, 0.0)); if oa > size * 1e-5 || ob > size * 1e-5 || ta < -1e-9 || tb > 1.0 + 1e-9 || tb - ta < 1e-6 { - return Err(crate::blend::too_large("the blend is larger than the neighbouring faces")); + return Err(too_large("the blend is larger than the neighbouring faces")); } let a = a.unwrap_or_else(|| seam.front().clone()); let b = b.unwrap_or_else(|| seam.back().clone()); diff --git a/crates/kernel/src/loopblend.rs b/crates/kernel/src/loopblend.rs index 4b9f5b18..7538747e 100644 --- a/crates/kernel/src/loopblend.rs +++ b/crates/kernel/src/loopblend.rs @@ -10,6 +10,7 @@ use std::collections::HashMap; use solidcraft_geom::Vec3; use truck_modeling::{self as mt, builder}; +use crate::blend::too_large; use crate::body::{Solid, from_p3, p3, v3}; use crate::record::Rebuild; use crate::{KernelError, Result}; @@ -356,7 +357,7 @@ fn build( } let p = joint_point(i).ok_or_else(|| unsupported("degenerate loop joint"))?; if !crate::blend::extension_fits(f, p, *q, size * 1e-7) { - return Err(crate::blend::too_large("the blend is larger than the neighbouring faces")); + return Err(too_large("the blend is larger than the neighbouring faces")); } } // The offset arcs must not turn inside out; an arc of exactly the blend's radius shrinks @@ -371,12 +372,12 @@ fn build( *x = true; } } else if toward && a.dist(c) <= r + tol { - return Err(crate::blend::too_large("the blend is larger than a corner radius")); + return Err(too_large("the blend is larger than a corner radius")); } } } if collapsed.iter().all(|x| *x) { - return Err(crate::blend::too_large("the blend is larger than a corner radius")); + return Err(too_large("the blend is larger than a corner radius")); } let mut va: Vec = pa.iter().map(|p| builder::vertex(p3(*p))).collect(); // A collapsed piece starts and ends at one vertex. @@ -546,11 +547,7 @@ fn build( let along = (from_p3(other.point()) - from_p3(v.point())).dot(target - from_p3(v.point())); let reach = (target - from_p3(v.point())).len2(); if along <= reach + tol { - return Err(crate::blend::too_large(if face_side { - "the blend is longer than the next edge" - } else { - "the blend is taller than the walls" - })); + return Err(too_large(if face_side { "the blend is longer than the next edge" } else { "the blend is taller than the walls" })); } let nb = if face_side { va.get(i) } else { vb.get(i) }.ok_or_else(|| KernelError::Failed("joint".into()))?; let slot = seam_ends.entry(e.id()).or_insert_with(|| (e.absolute_clone(), None, None)); @@ -566,7 +563,7 @@ fn build( let a = a.unwrap_or_else(|| e.absolute_front().clone()); let b = b.unwrap_or_else(|| e.absolute_back().clone()); if from_p3(a.point()).dist(from_p3(b.point())) < tol * 10.0 { - return Err(crate::blend::too_large("the blend is longer than the next edge")); + return Err(too_large("the blend is longer than the next edge")); } seam_subst.insert(id, builder::line(&a, &b)); } diff --git a/crates/ui-egui/src/preselect_tests.rs b/crates/ui-egui/src/preselect_tests.rs index 77a82c6e..241ff237 100644 --- a/crates/ui-egui/src/preselect_tests.rs +++ b/crates/ui-egui/src/preselect_tests.rs @@ -543,6 +543,10 @@ fn fillet_radius_fit_reduces_and_shows_the_applied_radius() { let fillet = app.session.doc.features.last().unwrap().id; let e = crate::dialogs::for_feature(&app, fillet, None).unwrap(); assert_eq!(e.extra.get("fit"), Some(&json!("reduce"))); + // Its edit preview shows the radius it uses too. + let cmds = apply_commands(&app, &e).unwrap(); + let job = crate::preview::compute(&app.session, &cmds, crate::preview::Colors::from_tokens(&crate::theme::Tokens::dark())).unwrap(); + assert_eq!(job.applied, Some(3.0)); } /// Apply a dialog's commands to the session; the new feature's id. diff --git a/crates/ui-egui/src/preview.rs b/crates/ui-egui/src/preview.rs index aa3351aa..fd851234 100644 --- a/crates/ui-egui/src/preview.rs +++ b/crates/ui-egui/src/preview.rs @@ -229,7 +229,9 @@ pub fn compute(s: &Session, cmds: &[(String, Value)], colors: Colors) -> Job { let p = real.map_err(|e| e.to_string())?; let mut b = guard(&|| build(&p.before, &p.after, colors))?; b.bounds = state_bounds(&p.after); - b.applied = p.after.fitted_radii().find(|(id, _)| p.before.fitted_radius(*id).is_none()).map(|(_, r)| r); + // The radius a previewed fillet used when it reduced the one asked for (a new + // fillet or an edited one: the feature the replies name). + b.applied = p.replies.iter().filter_map(|r| r.get("feature").and_then(Value::as_u64)).find_map(|id| p.after.fitted_radius(id)); Ok(b) } }