From 31b2ddc0cf15d998dc483d1e152f5bfe5f63414e Mon Sep 17 00:00:00 2001 From: Mateusz Sych <151755914+SychPL@users.noreply.github.com> Date: Fri, 16 Oct 2026 19:26:11 +0200 Subject: [PATCH] Split Body: the face a plane split cuts takes an attachment that follows the plane (#565) (#1746) Thanks @SychPL. --- crates/doc/src/eval.rs | 29 ++++++-- crates/engine/src/cmd/lineage_face_tests.rs | 73 +++++++++++++++++++++ crates/kernel/src/lib.rs | 2 +- crates/kernel/src/ops.rs | 13 ++-- docs/action-matrix.md | 8 +-- docs/face-attachments.md | 6 +- tests/action_matrix/baseline.json | 36 +++++----- 7 files changed, 131 insertions(+), 36 deletions(-) diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index a9c4f566..06149643 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -4435,13 +4435,14 @@ fn eval_feature(doc: &Document, vals: &BTreeMap, f: &Feature, st: FeatureKind::Split { body, plane, tool, sketch, line } => { let i = st.bodies.iter().position(|b| &b.name == body).ok_or_else(|| DocError::Unknown(format!("body `{body}`")))?; let Some(mb) = st.bodies.get(i).cloned() else { return Err(DocError::Unknown(format!("body `{body}`"))) }; + let input = mb.traced(); let parts = match tool { // A sketch line: the plane through it along the sketch's normal. _ if sketch.is_some() || line.is_some() => { let (Some(sketch), Some(line)) = (sketch, line) else { return Err(DocError::Invalid("a sketch line tool needs both its sketch and its line".into())); }; - kernel::split_by_plane(&mb.body, &sketch_line_plane(st, sketch.0, line)?)? + split_by_plane_traced(&input, &sketch_line_plane(st, sketch.0, line)?)? } // A face of a body (its surface extended). Some(t) => { @@ -4451,19 +4452,18 @@ fn eval_feature(doc: &Document, vals: &BTreeMap, f: &Feature, st: // The pieces keep the planar roles of the faces they share with the body (#565). kernel::split_body(&mb.body, &kernel::SplitTool::Face { body: &tb.body, face })? .into_iter() - .map(|part| part.with_preserved_planar_sources(&mb.body)) - .collect::, _>>()? + .map(|part| Ok(Traced::step(part.with_preserved_planar_sources(&mb.body)?, &[&input]))) + .collect::>>()? } None => { let pl = doc.resolve_plane_in(vals, plane, 0, Some(st))?; - kernel::split_by_plane(&mb.body, &pl)? + split_by_plane_traced(&input, &pl)? } }; if parts.len() < 2 { return Err(DocError::Invalid("the tool does not split the body".into())); } - let input = mb.traced(); - let mut it = parts.into_iter().map(|part| Traced::step(part, &[&input])); + let mut it = parts.into_iter(); if let (Some(first), Some(slot)) = (it.next(), st.bodies.get_mut(i)) { *slot = ModelBody::traced_new(mb.name.clone(), first, mb.feature); } @@ -5320,6 +5320,23 @@ mod body_name_tests { } } +/// The parts of a body split by a plane, on its positive side first (only the non-empty ones). +/// Each part's cut face is named `section` after the half-space box that cut it, so it takes an +/// attachment that follows the plane (#565); the box's other faces never reach the part. +fn split_by_plane_traced(input: &Traced, plane: &Plane) -> Result> { + let mut parts = Vec::new(); + for positive in [true, false] { + let (part, tool) = kernel::half_space(&input.body, plane, positive)?; + let Some(part) = part else { continue }; + // The box's cap in the plane: its start cap on the positive side, its end cap on the other. + let section = Some("section".to_string()); + let caps = if positive { [section, None] } else { [None, section] }; + let tool = crate::lineage::ProfileRoles { sketch: None, region: None, caps }.tool(tool, None); + parts.push(Traced::step(part, &[input, &tool])); + } + Ok(parts) +} + /// The plane a straight sketch line sweeps along its sketch's normal: through the line, its x /// along the line and its y along the normal (a Split Body tool, #1552). pub fn sketch_line_plane(st: &ModelState, sketch: u64, line: &str) -> Result { diff --git a/crates/engine/src/cmd/lineage_face_tests.rs b/crates/engine/src/cmd/lineage_face_tests.rs index 70224f98..680dfb1f 100644 --- a/crates/engine/src/cmd/lineage_face_tests.rs +++ b/crates/engine/src/cmd/lineage_face_tests.rs @@ -326,3 +326,76 @@ fn every_flat_face_of_an_elbow_up_and_an_elbow_down_takes_a_sketch() { ); every_flat_face_takes_a_sketch(&mut s); } + +/// #565: Split Body by a plane a parameter moves (a raw plane, a construction plane or a sketch +/// line). The face the split cuts on each part is named after the half-space that cut it +/// (`F:section`): a sketch and an offset plane attach to it and follow the plane. +#[test] +fn the_face_a_split_cuts_takes_a_sketch_and_an_offset_plane_that_follow_the_plane() { + for tool in ["plane", "construction", "line"] { + let mut s = Session::default(); + run(&mut s, "parameters.add", json!({"name": "split_x", "expression": "15 mm"})); + run(&mut s, "solid.box", json!({"length": 40, "width": 20, "height": 10, "body_name": "Block"})); + let split = match tool { + "plane" => json!({"body": "Block", "plane": {"origin": ["split_x", 0, 0], "normal": [1, 0, 0]}}), + "construction" => { + run(&mut s, "construct.plane.offset", json!({"base": "YZ", "offset": "split_x", "name": "Cut"})); + json!({"body": "Block", "plane": "Cut"}) + } + _ => { + let id = run(&mut s, "sketch.create", json!({"plane": "XY", "name": "Guide"}))["sketch"].as_u64().unwrap(); + run(&mut s, "sketch.line", json!({"points": [[15, -10], [15, 30]], "ids": ["cut"], "connect": false})); + run(&mut s, "sketch.constraint.horizontal_vertical", json!({"line": "cut", "mode": "vertical"})); + run(&mut s, "sketch.dimension", json!({"entities": ["origin", "cut.start"], "type": "horizontal", "value": "split_x"})); + run(&mut s, "sketch.finish", json!({})); + json!({"body": "Block", "sketch": id, "line": "cut"}) + } + }; + run(&mut s, "solid.split_body", split); + no_errors(&s); + // Each part's cut face (the two lie on one another, so each is picked by body and index). + let cut_face = |s: &Session, body: &str, x: f64| { + let state = s.model.state(); + let b = &state.body(body).unwrap().body; + let index = (0..b.face_count()) + .find(|i| b.planar_face(*i).is_some_and(|p| p.normal().x.abs() > 0.999 && (p.origin.x - x).abs() < 1e-9)) + .unwrap_or_else(|| panic!("{tool}: no cut face on `{body}`")); + json!({"face": [x, 10, 5], "body": body, "index": index}) + }; + let mut normals = Vec::new(); + for (k, body) in ["Block", "Block (1)"].into_iter().enumerate() { + let pick = cut_face(&s, body, 15.); + run(&mut s, "sketch.create", json!({"plane": pick, "name": format!("OnCut{k}")})); + run(&mut s, "sketch.finish", json!({})); + let (plane, certificate) = healthy(&s, &format!("OnCut{k}")); + assert_eq!(certificate, Some(FaceCertificate::Lineage), "{tool} {body}"); + assert!((plane.origin.x - 15.).abs() < 1e-9 && plane.normal().x.abs() > 0.999, "{tool} {body}: {plane:?}"); + normals.push(plane.normal().x); + let pick = cut_face(&s, body, 15.); + run(&mut s, "construct.plane.offset", json!({"base": pick, "offset": 2, "name": format!("Off{k}")})); + } + assert!(normals[0] * normals[1] < 0., "{tool}: the two cut faces face each other's way: {normals:?}"); + no_errors(&s); + + run(&mut s, "parameters.change", json!({"name": "split_x", "expression": "20 mm"})); + no_errors(&s); + let offset = |s: &Session, name: &str| { + let feature = s.doc.find_feature(name).map(|f| f.id).unwrap(); + let state = s.model.state(); + match state.construct.iter().find(|c| c.feature == feature).map(|c| &c.geom) { + Some(solidcraft_doc::construct::ConstructGeom::Plane(p)) => *p, + other => panic!("{tool} {name}: {other:?}"), + } + }; + for (k, n) in normals.iter().enumerate() { + let (plane, _) = healthy(&s, &format!("OnCut{k}")); + assert!((plane.origin.x - 20.).abs() < 1e-9 && (plane.normal().x - n).abs() < 1e-9, "{tool} OnCut{k}: {plane:?}"); + let off = offset(&s, &format!("Off{k}")); + assert!((off.origin.x - (20. + 2. * n)).abs() < 1e-9, "{tool} Off{k}: {off:?}"); + } + let t = reopened(&s); + for k in 0..2 { + assert!((healthy(&t, &format!("OnCut{k}")).0.origin.x - 20.).abs() < 1e-9, "{tool} reopened OnCut{k}"); + } + } +} diff --git a/crates/kernel/src/lib.rs b/crates/kernel/src/lib.rs index a87fa2d6..eb24a714 100644 --- a/crates/kernel/src/lib.rs +++ b/crates/kernel/src/lib.rs @@ -159,7 +159,7 @@ pub use mesh_convert::convert_prismatic_mesh; pub use meshbody::{MAX_TRIANGLES, mesh_body}; pub use native::{decode_native_body, encode_native_body, native_preview_mesh}; pub use offset::with_same_surface; -pub use ops::{BoolOp, boolean, can_join_touching_faces, join_apart, split_by_plane, transform, transform_matrix, union_apart}; +pub use ops::{BoolOp, boolean, can_join_touching_faces, half_space, join_apart, split_by_plane, transform, transform_matrix, union_apart}; pub use polybool::{faceted_boolean, planar_boolean}; pub use polyhedron::{HalfSpace, convex_polyhedron, draft, offset_faces, shell}; pub use prism::{PLANAR_FIT_TOL, extrude_planar_regions, planar_regions_boolean}; diff --git a/crates/kernel/src/ops.rs b/crates/kernel/src/ops.rs index 84ea44e8..f84c768b 100644 --- a/crates/kernel/src/ops.rs +++ b/crates/kernel/src/ops.rs @@ -1428,16 +1428,19 @@ pub fn transform(body: &Body, translate: Vec3, origin: Vec3, axis: Vec3, angle: pub fn split_by_plane(body: &Body, plane: &solidcraft_geom::Plane) -> Result> { let mut out = Vec::new(); for positive in [true, false] { - out.extend(half_space(body, plane, positive)?); + out.extend(half_space(body, plane, positive)?.0); } Ok(out) } -/// The part of a body on one side of a plane (`None` when nothing is there): its intersection -/// with [`half_box`] (whose history it carries, over the body and the box). -pub(crate) fn half_space(body: &Body, plane: &solidcraft_geom::Plane, positive: bool) -> Result> { +/// The part of a body on one side of a plane (`None` when nothing is there), and the tool it is +/// cut with: the part is the body's intersection with [`half_box`] (the tool), whose history it +/// carries, over the body and the box. The box's face in the plane, the part's cut face, is its +/// start cap on the positive side and its end cap on the negative side. +pub fn half_space(body: &Body, plane: &solidcraft_geom::Plane, positive: bool) -> Result<(Option, Body)> { // The box's own construction roles describe no face of the design: the cut face has none. - boolean(body, &half_box(body, plane, positive)?.with_face_sources(None), BoolOp::Intersect) + let tool = half_box(body, plane, positive)?.with_face_sources(None); + Ok((boolean(body, &tool, BoolOp::Intersect)?, tool)) } /// A box covering everything of `body` on one side of a plane, its start cap in the plane. diff --git a/docs/action-matrix.md b/docs/action-matrix.md index 71c38075..b15c3855 100644 --- a/docs/action-matrix.md +++ b/docs/action-matrix.md @@ -257,7 +257,7 @@ resolves; it is recorded as `+edit`. ## Causes -Every failure, grouped by cause (5017 in all). Faces that can't take an attached sketch fail four +Every failure, grouped by cause (5013 in all). Faces that can't take an attached sketch fail four actions each (`face.sketch`, `face.offset_plane`, and `face.press_pull` and `face.extrude`, which make a legacy face sketch there). @@ -286,7 +286,6 @@ make a legacy face sketch there). | 9 | #568 | Construction geometry on model edges, faces and vertices is lost when a parameter moves it | `vertex.plane_three_points+edit` 5, `edge.normal_plane+edit` 2, `face.midplane+edit` 1, `face.shell+edit` 1 | 4 | demo/enclosure_steps `Enclosure/v[F2:top & F4:inner:F2:side:l4 & F4:inner:F3:blend:F2:side:l` | | 9 | | Axis through a face the kernel sees as a cylinder that the command doesn't (sensor enclosure slot ends; not filed yet) | `curved.axis` 9 | 1 | examples/sensor_enclosure `Lid/F15:blend:F14:side:l2\|F14:side:l3` | | 7 | | Other failures in the edit round: the hole cut and blend sizes above, and an extrude from the M0 box's top face whose profile isn't found again (not filed yet) | `face.hole+edit` 3, `face.extrude+edit` 1, `face.press_pull+edit` 1, `edge.chamfer+edit` 1 | 4 | corpus/m0-box-fillet-hole `Body1/F2:top` | -| 4 | #565 | The face a Split Body cuts has no source (the faces it leaves whole keep theirs) | `face.offset_plane` 2, `face.sketch` 2 | 1 | oracle/22-split-body `Cyl (1)/F4:face:3` | | 2 | | A helix edge projected along its axis is a point (expected) | `edge.project` 2 | 1 | oracle/50-sweep-tight-bend `Bend/F3:side:c1#1\|F3:side:c1#2~2` | Where the face history stands (`crates/kernel/src/history.rs`, [face attachments](face-attachments.md)): @@ -297,5 +296,6 @@ chamfers, pattern and mirror copies, sides a Join merged, revolves, lofts, sweep shell rims, the plastic features' faces and the faces around them, and pieces of a split face (by the piece's own name where Offset Face moved it off the others' plane). Sheet metal panels, their rims and the flat ends of bends attach by name. What still refuses a sketch is what makes faces -without recording their history (some cuts, the face a Split Body cuts): press-pull and -extrude-from-face then fall back to a legacy face sketch found again by a point. +without recording their history (some cuts, the face a Split Body cuts with another body's +face): press-pull and extrude-from-face then fall back to a legacy face sketch found again by a +point. diff --git a/docs/face-attachments.md b/docs/face-attachments.md index 50f39112..6c22b96a 100644 --- a/docs/face-attachments.md +++ b/docs/face-attachments.md @@ -24,8 +24,10 @@ has one: the persistent name the kernel's face history gives it, which says what and follows it through every later feature. That covers a Cut's walls and floor (`F5:side:l1`, `F5:top`), a chamfer face (`F7:blend:…`), each copy a pattern or mirror makes (`F4:top@F6.2`: the seed's face and the copy's instance), a side wall a Join merged from two -features (named after the first, answering to the other), and the pieces a later feature split -a face into (all pieces of one name on one oriented plane are that face). A lineage face's +features (named after the first, answering to the other), the pieces a later feature split +a face into (all pieces of one name on one oriented plane are that face), and the face Split Body +cuts with a plane (`F4:section` on each part: a construction plane, a sketch line or a plane given +outright, so a sketch on it follows the plane when a parameter moves it). A lineage face's plane is its current exact plane, measured in a frame that depends on the plane alone: its origin where the definition origin projects onto it, its x axis the first of the definition's X, Y and Z axes not steep to it (`|cos| < 0.6`), projected. The construction certificates diff --git a/tests/action_matrix/baseline.json b/tests/action_matrix/baseline.json index 312a3924..96bda732 100644 --- a/tests/action_matrix/baseline.json +++ b/tests/action_matrix/baseline.json @@ -4805,32 +4805,32 @@ "design": "load" }, "oracle/22-split-body": { - "Cyl (1)/F2:bottom": "!face.extrude face.hole !face.offset_plane !face.press_pull face.shell !face.sketch", - "Cyl (1)/F2:bottom|F2:side:c1": "edge.chamfer edge.fillet edge.normal_plane edge.project", - "Cyl (1)/F2:bottom|F4:face:2": "edge.chamfer edge.fillet edge.normal_plane edge.project", - "Cyl (1)/F2:side:c1": "curved.axis curved.tangent_plane", - "Cyl (1)/F2:side:c1|F4:face:3": "edge.chamfer edge.fillet edge.normal_plane edge.project", - "Cyl (1)/F4:face:2": "curved.axis curved.tangent_plane", - "Cyl (1)/F4:face:2|F4:face:3": "edge.chamfer edge.fillet edge.normal_plane edge.project", - "Cyl (1)/F4:face:3": "!face.extrude face.hole !face.offset_plane !face.press_pull face.shell !face.sketch", + "Cyl (1)/F2:bottom": "face.extrude face.hole face.offset_plane face.press_pull face.shell face.sketch", + "Cyl (1)/F2:bottom|F2:side:c1#1": "edge.chamfer edge.fillet edge.normal_plane edge.project", + "Cyl (1)/F2:bottom|F2:side:c1#2": "edge.chamfer edge.fillet edge.normal_plane edge.project", + "Cyl (1)/F2:side:c1#1": "curved.axis curved.tangent_plane", + "Cyl (1)/F2:side:c1#1|F4:section": "edge.chamfer edge.fillet edge.normal_plane edge.project", + "Cyl (1)/F2:side:c1#2": "curved.axis curved.tangent_plane", + "Cyl (1)/F2:side:c1#2|F4:section": "edge.chamfer edge.fillet edge.normal_plane edge.project", + "Cyl (1)/F4:section": "!face.extrude face.hole face.offset_plane !face.press_pull face.shell face.sketch", "Cyl (1)/body": "body.copy body.mirror body.move body.pattern", - "Cyl (1)/v[F2:bottom & F2:side:c1 & F4:face:2]": "vertex.point", - "Cyl (1)/v[F2:bottom & F2:side:c1 & F4:face:2] (2)": "vertex.point", - "Cyl (1)/v[F2:side:c1 & F4:face:2 & F4:face:3]": "vertex.point", - "Cyl (1)/v[F2:side:c1 & F4:face:2 & F4:face:3] (2)": "vertex.point", + "Cyl (1)/v[F2:bottom & F2:side:c1#1 & F2:side:c1#2]": "vertex.point", + "Cyl (1)/v[F2:bottom & F2:side:c1#1 & F2:side:c1#2] (2)": "vertex.point", + "Cyl (1)/v[F2:side:c1#1 & F2:side:c1#2 & F4:section]": "vertex.point", + "Cyl (1)/v[F2:side:c1#1 & F2:side:c1#2 & F4:section] (2)": "vertex.point", "Cyl/F2:side:c1#1": "curved.axis curved.tangent_plane", "Cyl/F2:side:c1#1|F2:top": "edge.chamfer edge.fillet edge.normal_plane edge.project", - "Cyl/F2:side:c1#1|F4:face:3": "edge.chamfer edge.fillet edge.normal_plane edge.project", + "Cyl/F2:side:c1#1|F4:section": "edge.chamfer edge.fillet edge.normal_plane edge.project", "Cyl/F2:side:c1#2": "curved.axis curved.tangent_plane", "Cyl/F2:side:c1#2|F2:top": "edge.chamfer edge.fillet edge.normal_plane edge.project", - "Cyl/F2:side:c1#2|F4:face:3": "edge.chamfer edge.fillet edge.normal_plane edge.project", - "Cyl/F2:top": "!face.extrude face.hole !face.offset_plane !face.press_pull face.shell !face.sketch", - "Cyl/F4:face:3": "!face.extrude face.hole !face.offset_plane !face.press_pull face.shell !face.sketch", + "Cyl/F2:side:c1#2|F4:section": "edge.chamfer edge.fillet edge.normal_plane edge.project", + "Cyl/F2:top": "face.extrude face.hole face.offset_plane face.press_pull face.shell face.sketch", + "Cyl/F4:section": "!face.extrude face.hole face.offset_plane !face.press_pull face.shell face.sketch", "Cyl/body": "body.copy body.mirror body.move body.pattern", "Cyl/v[F2:side:c1#1 & F2:side:c1#2 & F2:top]": "vertex.point", "Cyl/v[F2:side:c1#1 & F2:side:c1#2 & F2:top] (2)": "vertex.point", - "Cyl/v[F2:side:c1#1 & F2:side:c1#2 & F4:face:3]": "vertex.point", - "Cyl/v[F2:side:c1#1 & F2:side:c1#2 & F4:face:3] (2)": "vertex.point", + "Cyl/v[F2:side:c1#1 & F2:side:c1#2 & F4:section]": "vertex.point", + "Cyl/v[F2:side:c1#1 & F2:side:c1#2 & F4:section] (2)": "vertex.point", "design": "load" }, "oracle/23-bracket-multifeature": {