1b-agent/OneCAD
1
0 Fork 0
forked from 1b-agent/solidcraft-upstream
Code Pull Requests Activity

fix: faces of an extrude drawn over a face's projected edges keep an exact source (#505)

Browse Source 
A profile traced over a face sketch's projected boundary left the extrude's caps and sides without an exact planar source, so a sketch or offset plane on the new top face was refused. The drawn curve is now the edge's source where it covers a projection. Clearer fixed-plane message; regression tests in the engine, UI harness and design corpus, plus a corpus-wide flat-face attachment check (known gaps under #491-#495).

Fixes #489

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
bherbruckandClaude Opus 5.5 authored and GitHub committed 2026-10-12 23:01:35 -04:00
1 parent 4aaa5457b3
commit 45bc22fc30
11 files changed
+1113 -15
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

apps/solidcraft-cli/tests/corpus/README.md
+6 -2
View File
@@ -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.
apps/solidcraft-cli/tests/corpus/issue-489-stacked-join.expected.json
+157
View File
@@ -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
}
}
apps/solidcraft-cli/tests/corpus/issue-489-stacked-join.solidcraft
+539
View File
@@ -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"
}
apps/solidcraft-cli/tests/design_corpus.rs
+166
View File
@@ -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<Planes>, 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<String>)> {
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<String> = 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<String> = 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 {