fix: fillet fit review follow-ups (#404)
* fix: fillet fit review follow-ups From the review of #388 (merged before these landed): - The Fillet dialog's Applied Radius showed nothing when editing a fillet that was already reduced: the preview found the radius by diffing the states before and after. The engine's preview now keeps the command replies, and the dialog reads the radius of the feature they name. - Pattern copies of a reduced fillet try the radius the original was reduced to first, instead of each searching again (up to 16 fillets). - fit: reduce with a chord or variable radius is refused instead of being silently ignored. - One helper appends a note to a feature's warning; too_large is imported like unsupported in the loop and curve blends. Refs #293 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01TWc1smHYwTSVrVc8SNvS44 * Use the solidcraft_doc crate name after the rename Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01HWLPLy7ug7URSK5WybjRNa --------- Co-authored-by: Claude Opus 5.5 <[email protected]> Co-authored-by: bherbruck <[email protected]>
This commit is contained in:
8 files changed
+58
-32
No files matched your search
@@ -790,6 +790,9 @@ fn fillet(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
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)
|
||||
|
||||
@@ -157,6 +157,8 @@ pub struct Preview {
|
||||
pub before: Arc<ModelState>,
|
||||
/// The model after the commands.
|
||||
pub after: Arc<ModelState>,
|
||||
/// What each command replied (as `execute` returns it).
|
||||
pub replies: Vec<Value>,
|
||||
}
|
||||
|
||||
/// 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<Preview> {
|
||||
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<Preview> {
|
||||
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<Self> {
|
||||
fn preview_in_place(mut self, commands: &[(String, Value)]) -> Result<(Self, Vec<Value>)> {
|
||||
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
|
||||
|
||||
@@ -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));
|
||||
}
|
||||
Reference in new issue
Block a user