diff --git a/apps/solidcraft-cli/tests/corpus/README.md b/apps/solidcraft-cli/tests/corpus/README.md index 8fa26209..a4b4a5b7 100644 --- a/apps/solidcraft-cli/tests/corpus/README.md +++ b/apps/solidcraft-cli/tests/corpus/README.md @@ -17,6 +17,10 @@ hostile variants of it never panic. | `modern-plastic` | bd98035 | shell, lip, boss, rest, snap fit, plastic rule | | `naming-split-pieces` | 9a979cf | format 13 names: a slot splits the top; fillet, chamfer and offset face name its pieces by number (`F2:top#2`) | | `split-body-pieces` | v0.3.2 (07e263d) | a slot along X cuts a block into two bodies; a fillet, a chamfer and an appearance refer to the pieces by body name (`Body1`, `Body1 (1)`) | +| `issue-489-stacked-join` | v0.6.0 (809ecf7), desktop | a 70 × 84 extrude, then a 70 × 62 strip joined on its top face, sketched partly over the face's projected edges (issue #489; expected values from main at 2880525) | -The scripts that made them are in `scripts/`. Never regenerate a file with a newer build: the -point is that it was written by the old one. Add new files for new eras instead. +The scripts that made them are in `scripts/` (`issue-489-stacked-join` was drawn in the desktop +app and has none). Every flat face of every final body must also take an attached sketch and an +offset plane; the faces that can't yet are listed with their issues in `design_corpus.rs`. + +Never regenerate a file with a newer build: the point is that it was written by the old one. Add new files for new eras instead. diff --git a/apps/solidcraft-cli/tests/corpus/issue-489-stacked-join.expected.json b/apps/solidcraft-cli/tests/corpus/issue-489-stacked-join.expected.json new file mode 100644 index 00000000..a0bc391a --- /dev/null +++ b/apps/solidcraft-cli/tests/corpus/issue-489-stacked-join.expected.json @@ -0,0 +1,157 @@ +{ + "bodies": [ + { + "area_mm2": 25751.999999999945, + "bbox": { + "max": [ + 35.0, + 42.0, + 48.00000000000001 + ], + "min": [ + -35.0, + -42.000000000000014, + 0.0 + ] + }, + "center_of_mass": [ + 5.524909928104648e-14, + -1.0945382096961234e-12, + 22.366336633663238 + ], + "density_g_cm3": 1.29, + "edges": 24, + "face_types": { + "plane": 10 + }, + "faces": 10, + "kernel": { + "edges": 24, + "faces": 10, + "shells": 1, + "vertices": 16 + }, + "mass_g": 328.33079999999734, + "material": "Default", + "name": "Body1", + "vertices": 16, + "volume_mm3": 254519.9999999979 + } + ], + "body_count": 1, + "params": [ + { + "comment": "", + "error": null, + "expression": "84", + "model": true, + "name": "d1", + "unit": "mm", + "value": 84.0 + }, + { + "comment": "", + "error": null, + "expression": "70", + "model": true, + "name": "d2", + "unit": "mm", + "value": 70.0 + }, + { + "comment": "", + "error": null, + "expression": "62", + "model": true, + "name": "d4", + "unit": "mm", + "value": 62.0 + } + ], + "sketches": [ + { + "curves": 6, + "dof": 0, + "id": 1, + "name": "Sketch1", + "profiles": 1, + "status": "solved", + "visible": false + }, + { + "curves": 10, + "dof": 1, + "id": 3, + "name": "Sketch2", + "profiles": 3, + "status": "solved", + "visible": false + } + ], + "timeline": [ + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 1, + "ms": 0.131943, + "name": "Sketch1", + "rolled_back": false, + "suppressed": false, + "type": "Sketch", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 2, + "ms": 1.0812709999999999, + "name": "Extrude1", + "rolled_back": false, + "suppressed": false, + "type": "ExtrudeFeature", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 3, + "ms": 0.450002, + "name": "Sketch2", + "rolled_back": false, + "suppressed": false, + "type": "Sketch", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 4, + "ms": 5.191514000000001, + "name": "Extrude2", + "rolled_back": false, + "suppressed": false, + "type": "ExtrudeFeature", + "warning": null + } + ], + "total": { + "area_mm2": 25751.999999999945, + "edges": 24, + "faces": 10, + "mass_g": 328.33079999999734, + "vertices": 16, + "volume_mm3": 254519.9999999979 + } +} diff --git a/apps/solidcraft-cli/tests/corpus/issue-489-stacked-join.solidcraft b/apps/solidcraft-cli/tests/corpus/issue-489-stacked-join.solidcraft new file mode 100644 index 00000000..9e029246 --- /dev/null +++ b/apps/solidcraft-cli/tests/corpus/issue-489-stacked-join.solidcraft @@ -0,0 +1,539 @@ +{ + "features": [ + { + "id": 1, + "name": "Sketch1", + "plane": { + "name": "XY", + "type": "origin" + }, + "sketch": { + "constraints": [ + { + "id": "k1", + "l": 0, + "type": "horizontal" + }, + { + "id": "k2", + "l": 2, + "type": "horizontal" + }, + { + "id": "k3", + "l": 1, + "type": "vertical" + }, + { + "id": "k4", + "l": 3, + "type": "vertical" + }, + { + "id": "k5", + "l": 4, + "p": 5, + "type": "midpoint" + }, + { + "id": "k6", + "p": 0, + "q": 5, + "type": "coincident" + }, + { + "id": "k7", + "l": 1, + "param": "d1", + "type": "length", + "value": 84.0 + }, + { + "id": "k8", + "l": 0, + "param": "d2", + "type": "length", + "value": 70.0 + } + ], + "counters": { + "k": 8, + "l": 6, + "p": 5 + }, + "curves": [ + { + "a": 1, + "b": 2, + "construction": false, + "id": "l1", + "type": "line" + }, + { + "a": 2, + "b": 3, + "construction": false, + "id": "l2", + "type": "line" + }, + { + "a": 3, + "b": 4, + "construction": false, + "id": "l3", + "type": "line" + }, + { + "a": 4, + "b": 1, + "construction": false, + "id": "l4", + "type": "line" + }, + { + "a": 1, + "b": 3, + "construction": true, + "id": "l5", + "type": "line" + }, + { + "a": 2, + "b": 4, + "construction": true, + "id": "l6", + "type": "line" + } + ], + "points": [ + { + "fixed": true, + "id": "origin", + "pos": [ + 0.0, + 0.0 + ] + }, + { + "fixed": false, + "id": "p1", + "pos": [ + -35.0, + -42.0 + ] + }, + { + "fixed": false, + "id": "p2", + "pos": [ + 35.0, + -42.0 + ] + }, + { + "fixed": false, + "id": "p3", + "pos": [ + 35.0, + 42.0 + ] + }, + { + "fixed": false, + "id": "p4", + "pos": [ + -35.0, + 42.0 + ] + }, + { + "fixed": false, + "id": "p5", + "pos": [ + 0.0, + 0.0 + ] + } + ] + }, + "suppressed": false, + "type": "sketch" + }, + { + "extent": { + "direction": "positive", + "distance": "30" + }, + "id": 2, + "name": "Extrude1", + "operation": "new_body", + "param_names": [ + "d3" + ], + "profiles": { + "loops": [ + [ + "l1", + "l2", + "l3", + "l4" + ] + ], + "type": "curves" + }, + "sketch": 1, + "suppressed": false, + "type": "extrude" + }, + { + "id": 3, + "name": "Sketch2", + "plane": { + "anchor": { + "author_path": [], + "component": 0, + "coordinates": [ + 0.0, + 0.0 + ], + "path": [], + "target": { + "body": "Body1", + "certificate": { + "high": true, + "holes": [], + "kind": "extrude_cap", + "outer": [ + "l1", + "l2", + "l3", + "l4" + ], + "sketch": 1 + }, + "name": "F2:cap:high:ed6121d9bed14d3fd103f347231990d103349f33b7874ad292d3c4d3539266c3", + "origin_feature": 2, + "point": [ + 0.0, + 0.0, + 30.0 + ] + }, + "x_axis": [ + 1.0, + 0.0 + ] + }, + "at": [ + 2.012802880253332, + 7.4635742926698185, + 30.000000000000114 + ], + "name": "F2:top", + "plane": { + "origin": [ + 0.0, + 0.0, + 30.0 + ], + "x_dir": [ + 1.0, + 0.0, + 0.0 + ], + "y_dir": [ + 0.0, + 1.0, + 0.0 + ] + }, + "type": "face" + }, + "sketch": { + "constraints": [ + { + "id": "k1", + "l": 4, + "type": "horizontal" + }, + { + "id": "k2", + "l": 6, + "type": "horizontal" + }, + { + "id": "k3", + "l": 5, + "type": "vertical" + }, + { + "id": "k4", + "l": 7, + "type": "vertical" + }, + { + "id": "k5", + "l": 8, + "p": 9, + "type": "midpoint" + }, + { + "id": "k6", + "p": 0, + "q": 9, + "type": "coincident" + }, + { + "id": "k7", + "p": 7, + "param": "d4", + "q": 6, + "text": [ + -0.2, + 9.200000000000003 + ], + "type": "distance", + "value": 62.0 + } + ], + "counters": { + "j": 1, + "k": 7, + "l": 10, + "p": 9 + }, + "curves": [ + { + "a": 1, + "b": 2, + "construction": false, + "id": "l1", + "link": "j1", + "type": "line" + }, + { + "a": 3, + "b": 1, + "construction": false, + "id": "l2", + "link": "j1", + "type": "line" + }, + { + "a": 2, + "b": 4, + "construction": false, + "id": "l3", + "link": "j1", + "type": "line" + }, + { + "a": 4, + "b": 3, + "construction": false, + "id": "l4", + "link": "j1", + "type": "line" + }, + { + "a": 5, + "b": 6, + "construction": false, + "id": "l5", + "type": "line" + }, + { + "a": 6, + "b": 7, + "construction": false, + "id": "l6", + "type": "line" + }, + { + "a": 7, + "b": 8, + "construction": false, + "id": "l7", + "type": "line" + }, + { + "a": 8, + "b": 5, + "construction": false, + "id": "l8", + "type": "line" + }, + { + "a": 5, + "b": 7, + "construction": true, + "id": "l9", + "type": "line" + }, + { + "a": 6, + "b": 8, + "construction": true, + "id": "l10", + "type": "line" + } + ], + "links": [ + { + "auto_face": true, + "face_name": "F2:cap:high:ed6121d9bed14d3fd103f347231990d103349f33b7874ad292d3c4d3539266c3", + "id": "j1", + "kind": "project", + "source": { + "at": [ + 11.666666666666666, + -13.999999999999995, + 30.0 + ], + "body": "Body1", + "type": "face" + } + } + ], + "points": [ + { + "fixed": true, + "id": "origin", + "pos": [ + 0.0, + 0.0 + ] + }, + { + "fixed": true, + "id": "p1", + "link": "j1", + "pos": [ + -35.0, + -42.0 + ] + }, + { + "fixed": true, + "id": "p2", + "link": "j1", + "pos": [ + -35.0, + 42.0 + ] + }, + { + "fixed": true, + "id": "p3", + "link": "j1", + "pos": [ + 35.0, + -42.0 + ] + }, + { + "fixed": true, + "id": "p4", + "link": "j1", + "pos": [ + 35.0, + 42.0 + ] + }, + { + "fixed": false, + "id": "p5", + "pos": [ + -35.0, + -31.000000000002185 + ] + }, + { + "fixed": false, + "id": "p6", + "pos": [ + 35.0, + -31.00000000000349 + ] + }, + { + "fixed": false, + "id": "p7", + "pos": [ + 35.0, + 30.99999999999603 + ] + }, + { + "fixed": false, + "id": "p8", + "pos": [ + -35.0, + 30.999999999995318 + ] + }, + { + "fixed": false, + "id": "p9", + "pos": [ + 0.0, + 0.0 + ] + } + ] + }, + "suppressed": false, + "type": "sketch" + }, + { + "extent": { + "direction": "positive", + "distance": "18" + }, + "id": 4, + "name": "Extrude2", + "operation": "join", + "param_names": [ + "d5" + ], + "profiles": { + "loops": [ + [ + "l1", + "l5", + "l4", + "l7" + ] + ], + "type": "curves" + }, + "sketch": 3, + "suppressed": false, + "type": "extrude" + } + ], + "format": "solidcraft/14", + "name": "Untitled", + "next_id": 6, + "params": [ + { + "expr": "84", + "model": true, + "name": "d1", + "unit": "mm" + }, + { + "expr": "70", + "model": true, + "name": "d2", + "unit": "mm" + }, + { + "expr": "62", + "model": true, + "name": "d4", + "unit": "mm" + } + ], + "units": "mm" +} diff --git a/apps/solidcraft-cli/tests/design_corpus.rs b/apps/solidcraft-cli/tests/design_corpus.rs index 7d3a8921..125331fb 100644 --- a/apps/solidcraft-cli/tests/design_corpus.rs +++ b/apps/solidcraft-cli/tests/design_corpus.rs @@ -131,6 +131,172 @@ fn body_names_saved_before_exact_history_keep_their_pieces() { assert!(s.doc.appearances.bodies.contains_key("Body1 (1)")); } +/// Flat faces that can't take an attached sketch yet, by cause: (design, body, the faces' +/// planes as outward normal and distance from the origin, or every flat face of the body, +/// issue). Face indices are not used: they can change between builds. Each issue has an +/// ignored test below that checks its faces; a listed plane whose face attaches fails the main +/// check until it is removed from here. +type Planes = &'static [([f64; 3], f64)]; +const NO_ATTACHMENT_YET: [(&str, &str, Option, u32); 9] = [ + ("issue-489-stacked-join", "Body1", Some(&[([1., 0., 0.], 35.), ([-1., 0., 0.], 35.)]), 491), + ("issue-489-stacked-join", "Body1", Some(&[([0., 0., 1.], 30.)]), 492), + ("m0-revolve-primitives", "Body2", Some(&[([-FRAC_1_SQRT_2, 0., FRAC_1_SQRT_2], -21. * FRAC_1_SQRT_2)]), 493), + ("split-body-pieces", "Body1", Some(&[([0., FRAC_1_SQRT_2, FRAC_1_SQRT_2], 38.5 * FRAC_1_SQRT_2), ([0., -1., 0.], -18.)]), 493), + ("split-body-pieces", "Body1 (1)", Some(&[([0., 1., 0.], 12.)]), 493), + ("m0-revolve-primitives", "Body1", None, 494), + ("naming-split-pieces", "Body1", None, 494), + ("modern-plastic", "Body1", None, 494), + ("modern-sheet-metal", "Body1", None, 495), +]; +use std::f64::consts::FRAC_1_SQRT_2; + +/// The issue a face is listed under, and whether it is listed by its plane (not as every face). +fn known_without_attachment(design: &str, body: &str, normal: [f64; 3], distance: f64) -> Option<(u32, bool)> { + let same = |(n, d): &([f64; 3], f64)| (0..3).all(|i| (n[i] - normal[i]).abs() < 1e-6) && (d - distance).abs() < 1e-4; + NO_ATTACHMENT_YET.iter().find_map(|(d, b, planes, issue)| { + if *d != design || *b != body { + return None; + } + match planes { + None => Some((*issue, false)), + Some(planes) => planes.iter().any(same).then_some((*issue, true)), + } + }) +} + +fn undo_to(s: &mut Session, features: usize) { + for _ in 0..4 { + if s.doc.features.len() <= features { + return; + } + s.execute("edit.undo", &json!({})).unwrap(); + } + assert_eq!(s.doc.features.len(), features, "undo did not remove the added feature"); +} + +/// Try a sketch and an offset plane on every flat face of every final body of every corpus +/// design, through the same face pick the desktop sends (body, face index, occurrence path, a +/// point on it), without `fixed`. Undo leaves the design as it was for the next face. Returns +/// (design, body, face, issue it is known under, what went wrong or None). +fn flat_face_attachments() -> Vec<(String, String, usize, Option<(u32, bool)>, Option)> { + use solidcraft_engine::doc::{FaceAttachmentStatus, apply_point}; + let mut out = Vec::new(); + for p in corpus() { + let name = p.file_stem().unwrap_or_default().to_string_lossy().to_string(); + let mut s = open(&p); + let state = s.model.state(); + let placed = s.placed_document(); + let mut picks = Vec::new(); + for (world, definition, component, path, frame) in placed.body_frames_with_source(&state) { + let Some(body) = state.bodies.iter().find(|b| b.definition_name() == definition && placed.model_body_component(b) == component) else { + continue; + }; + let mesh = body.body.tessellate(0.01).unwrap(); + for index in 0..body.body.face_count() { + if body.body.planar_face(index).is_none() { + continue; + } + // The middle of the face's largest triangle: well inside the face. + let point = mesh + .triangles + .iter() + .zip(&mesh.tri_face) + .filter(|(_, f)| **f as usize == index) + .filter_map(|(t, _)| mesh.tri(t)) + .max_by(|[a, b, c], [d, e, f]| (*b - *a).cross(*c - *a).len().total_cmp(&(*e - *d).cross(*f - *d).len())) + .map(|[a, b, c]| (a + b + c) * (1.0 / 3.0)); + let Some(point) = point else { continue }; + let point = apply_point(&frame, point); + let normal = + body.body.planar_face(index).map(|plane| solidcraft_engine::doc::apply_vector(&frame, plane.normal())).unwrap_or_default(); + let key = ([normal.x, normal.y, normal.z], normal.dot(point)); + picks.push((world.clone(), index, key, json!({"face": point, "body": world, "index": index, "path": path}))); + } + } + let before = s.doc.features.len(); + for (body, index, (normal, distance), plane) in picks { + let mut problem = None; + match s.execute("sketch.create", &json!({"plane": plane})) { + Ok(created) => { + let id = created["sketch"].as_u64().unwrap_or(0); + match s.placed_document().sketch_face_attachment(id, &s.model) { + FaceAttachmentStatus::Healthy { .. } | FaceAttachmentStatus::Sheet { .. } => {} + other => problem = Some(format!("sketch attachment {other:?}")), + } + s.execute("sketch.finish", &json!({})).unwrap(); + undo_to(&mut s, before); + } + Err(e) => problem = Some(format!("sketch.create: {e}")), + } + match s.execute("construct.plane.offset", &json!({"base": plane, "offset": 5})) { + Ok(_) => undo_to(&mut s, before), + Err(e) => problem = Some(problem.map_or(String::new(), |p| p + "; ") + &format!("construct.plane.offset: {e}")), + } + let known = known_without_attachment(&name, &body, normal, distance); + out.push((name.clone(), body, index, known, problem)); + } + assert!(s.model.results.iter().all(|r| r.error.is_none()), "{name}: left with errors after undo"); + } + out +} + +/// Every flat face of every corpus design takes a durable attachment (issue #489: the top face +/// of an extrude sketched over a face's projected boundary had "no exact planar source"), +/// except the faces listed in [`NO_ATTACHMENT_YET`] under their open issues. +#[test] +fn every_flat_face_of_every_corpus_design_takes_an_attached_sketch() { + let all = flat_face_attachments(); + assert!(all.len() > 50, "only {} faces checked", all.len()); + let problems: Vec = all + .iter() + .filter_map(|(design, body, face, known, problem)| match (known, problem) { + (None, Some(problem)) => Some(format!("{design}: {body} face {face}: {problem}")), + (Some((issue, true)), None) => Some(format!("{design}: {body} face {face} attaches now: remove it from NO_ATTACHMENT_YET (#{issue})")), + _ => None, + }) + .collect(); + assert!(problems.is_empty(), "{} of {} faces:\n{}", problems.len(), all.len(), problems.join("\n")); +} + +fn known_faces_attach(issue: u32) { + let problems: Vec = flat_face_attachments() + .into_iter() + .filter(|(.., known, problem)| known.is_some_and(|(known, _)| known == issue) && problem.is_some()) + .map(|(design, body, face, _, problem)| format!("{design}: {body} face {face}: {}", problem.unwrap_or_default())) + .collect(); + assert!(problems.is_empty(), "{}", problems.join("\n")); +} + +#[test] +#[ignore = "fails until #491 is fixed"] +fn a_side_face_join_merged_from_two_extrudes_takes_an_attached_sketch() { + known_faces_attach(491); +} + +#[test] +#[ignore = "fails until #492 is fixed"] +fn a_face_split_into_pieces_by_a_later_feature_takes_an_attached_sketch() { + known_faces_attach(492); +} + +#[test] +#[ignore = "fails until #493 is fixed"] +fn cut_walls_and_chamfer_faces_take_an_attached_sketch() { + known_faces_attach(493); +} + +#[test] +#[ignore = "fails until #494 is fixed"] +fn faces_after_revolve_offset_face_and_boss_take_an_attached_sketch() { + known_faces_attach(494); +} + +#[test] +#[ignore = "fails until #495 is fixed"] +fn sheet_metal_thickness_faces_take_an_attached_sketch() { + known_faces_attach(495); +} + /// A small deterministic generator (xorshift). struct Rng(u64); impl Rng { diff --git a/crates/doc/src/config_certificates.rs b/crates/doc/src/config_certificates.rs index 21b27a33..1176e251 100644 --- a/crates/doc/src/config_certificates.rs +++ b/crates/doc/src/config_certificates.rs @@ -348,7 +348,9 @@ pub(crate) fn flat_source_matches(certificate: &FaceCertificate, source: &solidc /// Match a generated segment to one exact analytic sketch curve. Coincident /// candidates are ambiguous and never resolved by distance or candidate order. pub(crate) struct ExactSketchCurves<'a> { - segments: Vec<(&'a str, Vec)>, + /// Each analytic curve's id, whether it is projected (linked) reference geometry, and its + /// segments. + segments: Vec<(&'a str, bool, Vec)>, } impl<'a> ExactSketchCurves<'a> { @@ -379,22 +381,33 @@ impl<'a> ExactSketchCurves<'a> { } else { sketch.sketch.segs(index) }; - segments.push((curve.id.as_str(), originals)); + segments.push((curve.id.as_str(), curve.link.is_some(), originals)); } Self { segments } } + /// The one sketch curve a generated segment lies on. A curve drawn over projected + /// reference geometry (a profile traced over a face's live boundary) is that segment's + /// source: the projection only marks where it was drawn. Two drawn curves, or two + /// projections, over the same segment stay ambiguous. pub(crate) fn find(&self, generated: &Seg2) -> Option { - let mut found = None; - for (id, originals) in &self.segments { + let mut drawn = None; + let mut projected = None; + for (id, linked, originals) in &self.segments { if originals.iter().any(|original| exact_segment_contains(original, generated)) { - if found.is_some() { - return None; + let slot = if *linked { &mut projected } else { &mut drawn }; + if slot.is_some() { + *slot = Some(None); + } else { + *slot = Some(Some(*id)); } - found = Some(*id); } } - found.map(str::to_owned) + match (drawn, projected) { + (Some(id), _) => id.map(str::to_owned), + (None, Some(id)) => id.map(str::to_owned), + (None, None) => None, + } } } @@ -607,6 +620,29 @@ mod tests { assert_eq!(ExactSketchCurves::new(&moved).find(&partial), None); } + #[test] + fn a_curve_drawn_over_a_projected_boundary_is_the_source() { + use solidcraft_geom::Vec2; + // A face sketch's live boundary, with a profile edge traced over it (issue #489). + let mut sketch = solidcraft_sketch::Sketch::new(); + let projected = sketch.add_line(Vec2::ZERO, Vec2::new(10., 0.), None, None, Some("edge")).unwrap(); + sketch.curves[projected].link = Some("boundary".into()); + let sketch = solved(sketch); + let edge = Seg2::Line { a: Vec2::new(10., 0.), b: Vec2::ZERO }; + assert_eq!(ExactSketchCurves::new(&sketch).find(&edge).as_deref(), Some("edge"), "a lone projection is its own source"); + let mut traced = sketch.clone(); + traced.sketch.add_line(Vec2::ZERO, Vec2::new(10., 0.), None, None, Some("drawn")).unwrap(); + assert_eq!(ExactSketchCurves::new(&traced).find(&edge).as_deref(), Some("drawn")); + // Two drawn curves over it, or two projections, stay ambiguous. + let mut twice = traced.clone(); + twice.sketch.add_line(Vec2::ZERO, Vec2::new(10., 0.), None, None, Some("again")).unwrap(); + assert_eq!(ExactSketchCurves::new(&twice).find(&edge), None); + let mut projections = sketch.clone(); + let other = projections.sketch.add_line(Vec2::ZERO, Vec2::new(10., 0.), None, None, Some("other")).unwrap(); + projections.sketch.curves[other].link = Some("edge link".into()); + assert_eq!(ExactSketchCurves::new(&projections).find(&edge), None); + } + #[test] #[ignore = "manual optimized certification microbenchmark; no timing assertion"] fn benchmark_prepared_extrude_sources() { diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index 8ebae7f9..1214db0f 100644 --- a/crates/engine/src/cmd/features.rs +++ b/crates/engine/src/cmd/features.rs @@ -1532,3 +1532,7 @@ mod offset_edit_guard_tests; #[cfg(test)] #[path = "offset_plane_face_tests.rs"] mod offset_plane_face_tests; + +#[cfg(test)] +#[path = "stacked_face_tests.rs"] +mod stacked_face_tests; diff --git a/crates/engine/src/cmd/sketch_face.rs b/crates/engine/src/cmd/sketch_face.rs index 42395abf..1d55685c 100644 --- a/crates/engine/src/cmd/sketch_face.rs +++ b/crates/engine/src/cmd/sketch_face.rs @@ -183,6 +183,19 @@ fn capture_impl(s: &Session, value: &Value, cmd: &str, internal: bool) -> Result } None } + Err(solidcraft_doc::DocError::Invalid(message)) + if matches!( + message.as_str(), + "this face has no exact planar source; choose a fixed plane instead" + | "this face has no supported construction certificate; choose a fixed plane instead" + ) => + { + return Err(bad( + cmd, + "this face can't keep an attachment: it was merged from more than one feature's faces or reshaped by an edit. \ + Pick another face, or use a fixed plane here (fixed:true), which won't follow later changes to the face", + )); + } Err(error) => return Err(bad(cmd, format!("{error}; choose a supported exact face or explicitly use fixed:true"))), }; Ok(PlaneRef::Face { plane, at, name, anchor }) diff --git a/crates/engine/src/cmd/stacked_face_tests.rs b/crates/engine/src/cmd/stacked_face_tests.rs new file mode 100644 index 00000000..c9b708dd --- /dev/null +++ b/crates/engine/src/cmd/stacked_face_tests.rs @@ -0,0 +1,133 @@ +//! Issue #489: a sketch or an offset plane on the top face of an extrude that was itself +//! sketched on a face. Sketching a profile over the whole face draws curves on top of the face's +//! projected boundary; the extrude's faces must still carry an exact, certified planar source. +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +use crate::Session; +use serde_json::{Value, json}; +use solidcraft_doc::FaceAttachmentStatus; + +fn run(s: &mut Session, command: &str, params: Value) -> Value { + s.execute(command, ¶ms).unwrap_or_else(|e| panic!("{command} {params}: {e}")) +} + +fn profile(s: &mut Session, circle: bool, full: bool) { + if circle { + run(s, "sketch.circle.center", json!({"center": [20, 20], "diameter": if full { 40 } else { 16 }})); + } else if full { + run(s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [40, 40]})); + } else { + run(s, "sketch.rectangle.two_point", json!({"p0": [10, 10], "p1": [30, 30]})); + } +} + +/// A 40 mm base (square or round) 10 mm tall, then a second extrude 10 mm up from a sketch on +/// its top face (or on a plane at that height), joined or as a new body. +fn two_extrudes(circle: bool, full: bool, on_face: bool, operation: &str) -> Session { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "Base"})); + profile(&mut s, circle, true); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"distance": 10, "name": "First"})); + let plane = if on_face { json!({"face": [20, 20, 10]}) } else { json!("XY") }; + let mut params = json!({"plane": plane, "name": "Upper"}); + if !on_face { + params["offset"] = json!(10); + } + run(&mut s, "sketch.create", params); + profile(&mut s, circle, full); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"distance": 10, "operation": operation, "name": "Second"})); + s +} + +fn healthy(s: &Session, sketch: &str) -> solidcraft_geom::Plane { + let id = s.doc.find_feature(sketch).map(|f| f.id).unwrap(); + match s.placed_document().sketch_face_attachment(id, &s.model) { + FaceAttachmentStatus::Healthy { plane, .. } => plane, + other => panic!("{sketch}: {other:?}"), + } +} + +fn no_errors(s: &Session) { + assert!(s.model.results.iter().all(|r| r.error.is_none()), "{:?}", s.model.results); +} + +#[test] +fn sketch_and_offset_plane_on_the_top_face_of_a_face_sketched_extrude() { + for operation in ["join", "new"] { + for circle in [false, true] { + for full in [true, false] { + for on_face in [true, false] { + let case = format!("{operation} circle={circle} full={full} on_face={on_face}"); + let mut s = two_extrudes(circle, full, on_face, operation); + let top = [20., 20., 20.]; + s.execute("sketch.create", &json!({"plane": {"face": top}, "name": "Top"})).unwrap_or_else(|e| panic!("{case}: {e}")); + run(&mut s, "sketch.finish", json!({})); + let plane = healthy(&s, "Top"); + assert!((plane.origin.z - 20.).abs() < 1e-9 && (plane.normal().z - 1.).abs() < 1e-9, "{case}: {plane:?}"); + s.execute("construct.plane.offset", &json!({"base": {"face": top}, "offset": 5, "name": "Above"})) + .unwrap_or_else(|e| panic!("{case}: {e}")); + // The second extrude's flat side is an exact source too, unless a full-face + // Join merged it with the first extrude's side: that face belongs to neither, + // so it offers the explicit fixed plane with a readable reason. + if !circle { + let side = if full { [40., 20., 15.] } else { [30., 20., 15.] }; + let params = json!({"plane": {"face": side}, "name": "Side"}); + if full && operation == "join" { + let error = s.execute("sketch.create", ¶ms).unwrap_err().to_string(); + assert!(error.contains("merged") && error.contains("fixed:true"), "{case}: {error}"); + run(&mut s, "sketch.create", json!({"plane": {"face": side, "fixed": true}, "name": "Side"})); + } else { + s.execute("sketch.create", ¶ms).unwrap_or_else(|e| panic!("{case} side: {e}")); + assert!((healthy(&s, "Side").normal().x - 1.).abs() < 1e-9, "{case}"); + } + run(&mut s, "sketch.finish", json!({})); + } + s.refresh(); + no_errors(&s); + } + } + } + } +} + +/// Three extrudes, each sketched over the whole top face of the one before; editing the first +/// one's height moves every later sketch with its face. +#[test] +fn three_stacked_extrudes_follow_an_edit_of_the_first() { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "S1"})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [40, 40]})); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"distance": 10, "name": "E1"})); + for (k, top) in [(2, 10.), (3, 20.)] { + run(&mut s, "sketch.create", json!({"plane": {"face": [20, 20, top]}, "name": format!("S{k}")})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [40, 40]})); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"distance": 10, "operation": "join", "name": format!("E{k}")})); + } + run(&mut s, "sketch.create", json!({"plane": {"face": [20, 20, 30]}, "name": "S4"})); + run(&mut s, "sketch.circle.center", json!({"center": [20, 20], "diameter": 10})); + run(&mut s, "sketch.finish", json!({})); + s.refresh(); + no_errors(&s); + assert_eq!(s.model.state().bodies.len(), 1); + for (name, z) in [("S2", 10.), ("S3", 20.), ("S4", 30.)] { + assert!((healthy(&s, name).origin.z - z).abs() < 1e-9, "{name}"); + } + + run(&mut s, "timeline.edit", json!({"feature": "E1", "set": {"extent": {"distance": "25"}}})); + s.refresh(); + no_errors(&s); + for (name, z) in [("S2", 25.), ("S3", 35.), ("S4", 45.)] { + assert!((healthy(&s, name).origin.z - z).abs() < 1e-9, "{name} after edit"); + } + let volume: f64 = s.model.state().bodies.iter().map(|b| solidcraft_kernel::measure(&b.body).unwrap().volume).sum(); + assert!((volume - 40. * 40. * 45.).abs() < 1e-6, "{volume}"); + + // Saved and reopened, every attachment still resolves. + let mut reopened = Session::new(solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap()); + reopened.refresh(); + no_errors(&reopened); + assert!((healthy(&reopened, "S4").origin.z - 45.).abs() < 1e-9); +} diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 255715ce..e2468998 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -2321,7 +2321,7 @@ pub fn start_sketch(app: &mut SolveApp, sel: &Sel) -> Result<(), String> { _ => return Err("Select an origin plane, construction plane or planar body face".into()), }; let before = app.cam; - app.run("sketch.create", json!({ "plane": plane }))?; + app.run("sketch.create", json!({ "plane": plane })).map_err(|error| crate::friendly_error("sketch.create", &error))?; app.pre_sketch_cam = Some(before); look_at_sketch(app); // On a face: look at the picked point, not the plane's origin (a corner of the face), diff --git a/crates/ui-egui/src/sketch_attachment_ui_tests.rs b/crates/ui-egui/src/sketch_attachment_ui_tests.rs index 8e39cfaa..e63862fc 100644 --- a/crates/ui-egui/src/sketch_attachment_ui_tests.rs +++ b/crates/ui-egui/src/sketch_attachment_ui_tests.rs @@ -330,3 +330,43 @@ fn nested_anchor_saved_fixed_conversion_preserves_rotated_second_author_plane() h.app.run("edit.undo", json!({})).unwrap(); assert_eq!(h.app.session.doc.to_json(), before); } + +/// Issue #489: extrude, sketch a rectangle over the whole top face (its corners snap to the +/// face's projected boundary), extrude it up as a Join, then pick the new top face for a sketch. +#[test] +fn picked_top_face_of_a_stacked_join_extrude_takes_a_sketch() { + let mut h = Harness::new(); + h.app.ui.show_origin = false; + h.app.run("sketch.create", json!({"plane":"XY","name":"Base"})).unwrap(); + h.app.run("sketch.rectangle.two_point", json!({"p0":[0,0],"p1":[40,40]})).unwrap(); + h.app.run("sketch.finish", json!({})).unwrap(); + h.app.run("solid.extrude", json!({"distance":10,"name":"First"})).unwrap(); + h.call("ui.view", json!({"view":"top"})); + h.call("ui.view", json!({"view":"fit"})); + h.settle(); + let pick_face = |h: &mut Harness, z: f64| { + h.step(&json!({"start":"sketch.create"})).unwrap(); + h.step(&json!({"click":{"world":[20,20,z]}})).unwrap(); + h.settle(); + let error = h.app.dialog.as_ref().and_then(|d| d.error.clone()); + let sketch = h.app.session.active_sketch.unwrap_or_else(|| panic!("top face at z={z} must take a sketch: {error:?}")); + let FeatureKind::Sketch { plane: PlaneRef::Face { anchor: Some(_), .. }, .. } = &h.app.session.doc.feature(sketch).unwrap().kind else { + panic!("the top face at z={z} must give a certified attachment"); + }; + sketch + }; + pick_face(&mut h, 10.); + h.step(&json!({"start":"sketch.rectangle.two_point"})).unwrap(); + h.step(&json!({"click":{"world":[0,0,10]}})).unwrap(); + h.step(&json!({"click":{"world":[40,40,10]}})).unwrap(); + h.step(&json!({"key":"Escape"})).unwrap(); + h.app.finish_sketch(); + h.app.run("solid.extrude", json!({"distance":10,"operation":"join","name":"Second"})).unwrap(); + assert_eq!(h.app.session.model.state().bodies.len(), 1); + h.settle(); + let top = pick_face(&mut h, 20.); + let FaceAttachmentStatus::Healthy { plane, .. } = h.app.session.placed_document().sketch_face_attachment(top, &h.app.session.model) else { + panic!("attachment must be healthy"); + }; + assert!((plane.origin.z - 20.).abs() < 1e-9); +} diff --git a/docs/face-attachments.md b/docs/face-attachments.md index d8747995..074a6882 100644 --- a/docs/face-attachments.md +++ b/docs/face-attachments.md @@ -6,12 +6,18 @@ and orientation are stored in the source face's canonical plane. Editing the sou or moving its placement therefore updates the sketch plane while preserving its local geometry and dimensions. -Supported planar sources are exact Box faces, untapered New Body Extrude caps and straight -sketch sides, and planar faces of immutable STEP imports. Extrude caps also identify their +Supported planar sources are exact Box faces, untapered New Body and Join Extrude caps and +straight sketch sides, and planar faces of immutable STEP imports. Extrude caps also identify their outer boundary and holes by source curve IDs. Reordering profiles cannot substitute another -profile's cap. Curved surfaces cannot host a planar sketch through this workflow. +profile's cap. A profile drawn over a face sketch's projected boundary (a rectangle +traced over the whole face, or a strip whose sides lie on the face's edges) is identified by +the drawn curves: where a drawn curve and a projected one cover the same edge, the drawn curve +is the source. Two drawn curves, or two projections, over one edge remain ambiguous. Curved surfaces cannot host a planar sketch through this workflow. Edited surfaces without retained construction provenance, ambiguous sources, and unsupported -generators require an explicit fixed plane instead. A nearby or parallel face is never used +generators require an explicit fixed plane instead. A face that a Join merged from two features' coplanar +faces belongs to neither, so it also needs a fixed plane. The error says why, and the Create +Sketch dialog then offers **Use fixed plane**, which places the sketch on that face's current +plane without following later changes. A nearby or parallel face is never used to repair a new attachment automatically. The flat faces of a sheet metal body are the exception: they have no construction