From a6742180a90b48bfdc84901de7a0eeaab197d666 Mon Sep 17 00:00:00 2001 From: Mateusz Sych <151755914+SychPL@users.noreply.github.com> Date: Fri, 16 Oct 2026 03:27:35 +0200 Subject: [PATCH] Join from a sketch on a body's face joins that body (#1349) (#1526) Thanks @SychPL for the fix. Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/face_join_tests.rs | 41 ++++++++++++++++++++++++ crates/engine/src/cmd/features.rs | 22 ++++++++++--- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/face_join_tests.rs b/crates/engine/src/cmd/face_join_tests.rs index 20dce23b..12d1f95c 100644 --- a/crates/engine/src/cmd/face_join_tests.rs +++ b/crates/engine/src/cmd/face_join_tests.rs @@ -125,3 +125,44 @@ fn face_join_of_a_step_import_keeps_all_32_neighbours_unchanged() { let FeatureKind::Extrude { targets, .. } = &s.doc.feature(result["feature"].as_u64().unwrap()).unwrap().kind else { panic!("extrude") }; assert_eq!(targets, &["Wall"]); } + +/// #1349: a profile sketched on a body's face joins that body, and leaves alone a neighbour +/// whose face it lies on too: a rib on the side of a bracket, drawn down over the motor the +/// bracket sits on, welded the motor in (a join that ran for minutes on the imported motor and +/// failed). Explicit targets still join both. +#[test] +fn join_of_a_profile_sketched_on_a_face_leaves_a_neighbour_it_lies_on() { + for targets in [None, Some(json!(["Bracket", "Motor"]))] { + let mut s = Session::default(); + run(&mut s, "solid.box", json!({"length":40,"width":30,"height":20,"body_name":"Motor"})); + run(&mut s, "solid.box", json!({"corner":[0,0,20],"length":40,"width":30,"height":5,"body_name":"Bracket"})); + let sketch = run(&mut s, "sketch.create", json!({"plane": {"face": [40, 15, 22.5]}}))["sketch"].as_u64().unwrap(); + let plane = s.model.state().sketch(sketch).unwrap().plane; + let at = |y: f64, z: f64| { + let p = plane.to_local(Vec3::new(40.0, y, z)); + json!([p.x, p.y]) + }; + // On the bracket's side and down over the motor's. + run(&mut s, "sketch.rectangle.two_point", json!({"p0": at(5.0, 10.0), "p1": at(25.0, 25.0)})); + run(&mut s, "sketch.finish", json!({})); + let mut params = + json!({"sketch": sketch, "profiles": [{"point": at(15.0, 22.5)}, {"point": at(15.0, 15.0)}], "distance": 1, "operation": "join"}); + if let Some(t) = &targets { + params["targets"] = t.clone(); + } + let result = run(&mut s, "solid.extrude", params); + assert!(s.model.results.iter().all(|r| r.error.is_none()), "{:?}", s.model.results); + let state = s.model.state(); + let volume = |name: &str| solidcraft_kernel::measure(&state.body(name).unwrap().body).unwrap().volume; + if targets.is_some() { + assert_eq!(state.bodies.len(), 1); + assert!((volume("Bracket") - (24_000.0 + 6000.0 + 300.0)).abs() < 1e-6, "{}", volume("Bracket")); + continue; + } + assert_eq!(state.bodies.len(), 2); + assert!((volume("Motor") - 24_000.0).abs() < 1e-6, "the motor is untouched"); + assert!((volume("Bracket") - (6000.0 + 300.0)).abs() < 1e-6, "{}", volume("Bracket")); + let FeatureKind::Extrude { targets, .. } = &s.doc.feature(result["feature"].as_u64().unwrap()).unwrap().kind else { panic!("extrude") }; + assert_eq!(targets, &["Bracket"]); + } +} diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index facd8bee..ce17bea5 100644 --- a/crates/engine/src/cmd/features.rs +++ b/crates/engine/src/cmd/features.rs @@ -15,7 +15,7 @@ pub static COMMANDS: &[CommandSpec] = &[ .at("SOLID", "CREATE") .icon("extrude") .key("E") - .params("distance: expr (or through_all: true); taper?: angle expr; sketch?: id|name (default: active or last sketch); profiles?: all | [index] | [{curves: [ids]}] (the profile whose outer loop is made of exactly these curves, or pieces of them; found again on every recompute, so it follows parameter changes; islands inside are its holes; an error when no profile or several match) | [{point:[x,y]}] (index and point picks are stored by their curves when that finds the same profile); face?: [x,y,z] (extrude a planar body face instead of a sketch profile); face_body?: name (whose face, where faces of several bodies meet at the point); direction?: positive|negative|symmetric (distance is total thickness, split evenly around the start plane); convention?: total_distance|legacy_per_side (default total_distance; legacy preserves older symmetric extents); distance2?; start_offset?: expr (along the sketch normal, also with direction: negative); to?: [x,y,z] (To Object: a face, vertex or point; to_offset?: expr past it); from?: [x,y,z] (From Object: start at that face or point); operation?: new|join|cut|intersect|auto (cut into a body, otherwise join existing solids; new in an empty component); targets?: [body] (Join from a face defaults to that face's body; explicit targets or participants override); name?; body_name?; thin?: true (Thin: a wall along sketch curves instead of the profiles) with curves?: [curve ids] (open chains and closed loops, joined where they meet end to end; default: every drawn curve of the sketch), thickness: expr, side?: center|inside|outside|two_sides (a closed loop's inside is the area it encloses, an open chain's the side it bends toward, a straight line's the left of its first curve as drawn; default center), thickness2?: expr (two_sides: the thickness outside); corners are mitred, crossing walls are one"), + .params("distance: expr (or through_all: true); taper?: angle expr; sketch?: id|name (default: active or last sketch); profiles?: all | [index] | [{curves: [ids]}] (the profile whose outer loop is made of exactly these curves, or pieces of them; found again on every recompute, so it follows parameter changes; islands inside are its holes; an error when no profile or several match) | [{point:[x,y]}] (index and point picks are stored by their curves when that finds the same profile); face?: [x,y,z] (extrude a planar body face instead of a sketch profile); face_body?: name (whose face, where faces of several bodies meet at the point); direction?: positive|negative|symmetric (distance is total thickness, split evenly around the start plane); convention?: total_distance|legacy_per_side (default total_distance; legacy preserves older symmetric extents); distance2?; start_offset?: expr (along the sketch normal, also with direction: negative); to?: [x,y,z] (To Object: a face, vertex or point; to_offset?: expr past it); from?: [x,y,z] (From Object: start at that face or point); operation?: new|join|cut|intersect|auto (cut into a body, otherwise join existing solids; new in an empty component); targets?: [body] (Join from a face, or from a sketch on a body's face, defaults to that face's body; explicit targets or participants override); name?; body_name?; thin?: true (Thin: a wall along sketch curves instead of the profiles) with curves?: [curve ids] (open chains and closed loops, joined where they meet end to end; default: every drawn curve of the sketch), thickness: expr, side?: center|inside|outside|two_sides (a closed loop's inside is the area it encloses, an open chain's the side it bends toward, a straight line's the left of its first curve as drawn; default center), thickness2?: expr (two_sides: the thickness outside); corners are mitred, crossing walls are one"), CommandSpec::new("solid.revolve", "Revolve", revolve) .at("SOLID", "CREATE") .icon("revolve") @@ -552,10 +552,13 @@ fn extrude(s: &mut Session, p: &Value) -> Result { }; let mut targets = string_list(p, "targets"); if operation == Operation::Join && targets.is_empty() && string_list(p, "participants").is_empty() { - // A face's outward extension joins that body. Assembly neighbours can touch only an - // edge of the tool: trying to join them first fails, or absorbs unrelated parts (#226). + // A face's outward extension joins that body, and so does a profile sketched on a body's + // face. Assembly neighbours can touch only an edge of the tool: trying to join them + // first fails, or absorbs unrelated parts (#226). A profile drawn past the face can lie + // on a neighbour's face too: a rib on a bracket's side welded in the motor the bracket + // holds, a join that ran for minutes and failed (#1349). // Store the resolved target so recomputes and reopening keep the same scope. - targets.extend(face_body); + targets.extend(face_body.or_else(|| sketch_face_body(s, sketch))); } let kind = FeatureKind::Extrude { sketch, @@ -572,6 +575,17 @@ fn extrude(s: &mut Session, p: &Value) -> Result { add_feature(s, p, kind) } +/// The body whose face `sketch` is attached to (on the face itself, not offset from it), when +/// it is a body of the active component. +fn sketch_face_body(s: &Session, sketch: u64) -> Option { + let FeatureKind::Sketch { plane: solidcraft_doc::PlaneRef::Face { anchor: Some(anchor), .. }, .. } = &s.doc.feature(sketch)?.kind else { + return None; + }; + let st = s.model.state(); + let body = st.body(&anchor.target.body)?; + (s.doc.model_body_component(body) == s.active_component).then(|| body.name.clone()) +} + /// A thin extrude's wall from its parameters: the sketch's curves it follows (every drawn /// curve when none are given), its thickness and which side of them it lies on. fn thin_param(s: &Session, p: &Value, sketch: u64, cmd: &str) -> Result {