From d9ad816eb2294835a66765e8b6749a82b067c479 Mon Sep 17 00:00:00 2001 From: aapolipponen Date: Sat, 31 Oct 2026 06:46:37 +0200 Subject: [PATCH 01/13] =?UTF-8?q?difftest:=20intersect.curve=5Fclass=5Fp7?= =?UTF-8?q?=20=E2=80=94=20endpoint=20conflicts=20and=20old=20faces=20on=20?= =?UTF-8?q?real=20faces=20(follow-up=20to=20#1824)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/units/intersect_curve_class_p7.rs | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/crates/difftest/src/units/intersect_curve_class_p7.rs b/crates/difftest/src/units/intersect_curve_class_p7.rs index 5752743bab..995c4c3f9b 100644 --- a/crates/difftest/src/units/intersect_curve_class_p7.rs +++ b/crates/difftest/src/units/intersect_curve_class_p7.rs @@ -915,6 +915,62 @@ fn edge_cases() -> Vec<(String, Value)> { OpIn::Fillet { t: 0, j: 3, todo: 1 }, ]; out.push(("edge_empty".into(), json!(Script { objects: objs, face: None, cc: [empty(), empty()], ops }))); + out.extend(real_endpoint_cases()); + out +} + +/// Deliberately conflicting endpoint records exercise upstream's gap-based arbitration. +/// Both ends name the bottom edge; the forward tangent points inside and the backward +/// tangent outside. Equal gaps leave the middle unknown, unequal gaps select one end. +fn real_endpoint_cases() -> Vec<(String, Value)> { + let face = FaceIn { + tol: 1e-5, + o: [0.0; 3], + x: [1.0, 0.0, 0.0], + y: [0.0, 1.0, 0.0], + domain: [0.0, 0.0, 1.0, 1.0], + surf: None, + loops: Some(hfp::LoopsIn::Edges { loops: vec![vec![ + [0.0, 0.0, 0.0], [1.0, 0.0, 0.0], + [1.0, 1.0, 0.0], [0.0, 1.0, 0.0], + ]] }), + }; + // Objects: curve 0, old-face stand-in 1; real face 2, loop 3, edge 4, vertex 5. + let objects = vec![ObjIn::Curve(CurveIn::Line(LineIn { + p: [0.5, 0.0, 0.0], v: [0.0, 1.0, 0.0], + domain: [0.0, 1.0], scale: 1.0, inside_out: false, + })), ObjIn::Face(None)]; + let mut out = Vec::new(); + for (name, start, middle, end, old) in [ + ("max_edge", unknown(), unknown(), pc(103, Some(4), 0.5, 0.0), None), + ("max_vertex", unknown(), unknown(), pc(104, Some(5), 1.0, 0.0), None), + ("conflict_equal", pc(103, Some(4), 0.5, 0.0), unknown(), pc(103, Some(4), 0.5, 0.0), None), + ("conflict_prefer_min", pc(103, Some(4), 0.5, 0.0), unknown(), pc(103, Some(4), 0.5, 1e-6), None), + ("conflict_prefer_max", pc(103, Some(4), 0.5, 1e-6), unknown(), pc(103, Some(4), 0.5, 0.0), None), + ("old_face_update", pc(103, Some(4), 0.5, 0.0), pc(102, Some(1), 0.5, 0.0), unknown(), Some(1)), + ("old_face_mismatch", pc(103, Some(4), 0.5, 0.0), pc(102, Some(2), 0.5, 0.0), unknown(), Some(1)), + ] { + let cc = CcIn { curve: Some(0), s1: None, uv1: None, ignore: false, + tol: 1e-5, fillet: None, + ivls: vec![IvlIn { a: 0.0, b: 1.0, pcs: [start, middle, end] }] }; + let mut objects = objects.clone(); + if name.starts_with("max_") { + let (p, v) = if name == "max_vertex" { + ([0.5, 0.5, 0.0], [-0.5, -0.5, 0.0]) + } else { + ([0.5, 0.5, 0.0], [0.0, -0.5, 0.0]) + }; + objects[0] = ObjIn::Curve(CurveIn::Line(LineIn { + p, v, domain: [0.0, 1.0], + scale: if name == "max_vertex" { 0.5_f64.sqrt() } else { 0.5 }, + inside_out: false, + })); + } + let script = Script { objects, face: Some(face.clone()), cc: [cc.clone(), cc], + ops: vec![OpIn::ClassifyFace { t: 0, j: 0, do_point: false, + face: Some(2), old, uv_ok: false, real: true }] }; + out.push((format!("real_endpoints_{name}"), json!(script))); + } out } @@ -1093,6 +1149,34 @@ mod tests { assert_eq!(&out[..3], &[2.0, 1001.0, 3.0]); } + /// Endpoint arbitration and old-face updates compare full state against C++. + #[test] + fn real_endpoint_branches_match_cpp() { + for (name, inputs) in real_endpoint_cases() { + let script: Script = serde_json::from_value(inputs).unwrap(); + let native = ffi::run(ffi_face(&script).as_ref(), &cpp_input(&script)).unwrap(); + let actual = rust_run(&script).ok().unwrap(); + assert_eq!(actual, native, "{name}"); + assert_eq!(actual[1], 0.0, "{name}: operation must succeed"); + // op, status, interval count, interval bounds, start PC (17 fields), mid PC. + let mid = 22; + let expected = match name.as_str() { + "real_endpoints_conflict_equal" | "real_endpoints_conflict_prefer_max" => 100.0, + _ => 102.0, + }; + assert_eq!(actual[mid], expected, "{name}: middle classification"); + if expected == 102.0 { + assert_eq!(actual[mid + 1], 2.0, "{name}: middle face"); + } + if name == "real_endpoints_old_face_mismatch" { + assert_eq!(actual[mid + 17], 100.0, "mismatched old face must return before propagating to the end"); + } else if name == "real_endpoints_old_face_update" { + assert_eq!(actual[mid + 17], 102.0, "updated middle must propagate to the end"); + assert_eq!(actual[mid + 18], 2.0); + } + } + } + /// Real-face twins of every variant exist and are comparable (not deferred). #[test] fn real_twins_are_comparable() { From 4cac522573f26fda1b0f1fc7df24e2fd6971923c Mon Sep 17 00:00:00 2001 From: chris Date: Fri, 30 Oct 2026 22:38:49 -0700 Subject: [PATCH 02/13] claim: diagnose bottle sweep validity after UB-170 Co-Authored-By: Codex From 3b616f927801e04399047d3933c0891781cb78a4 Mon Sep 17 00:00:00 2001 From: aapolipponen Date: Sat, 31 Oct 2026 07:39:24 +0200 Subject: [PATCH 03/13] =?UTF-8?q?claim:=20fix:=20geom.extent3d=20=E2=80=94?= =?UTF-8?q?=20opt-in:=20segment=20containment=20needs=20both=20endpoints?= =?UTF-8?q?=20(WR-3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From f56a1f80a4b13d4645a2fb8df6e299fe7201aadf Mon Sep 17 00:00:00 2001 From: Brett Lawson Date: Sat, 31 Oct 2026 07:08:19 +0100 Subject: [PATCH 04/13] claim: WR-645 face tight box divides cells at the surface's knots (fork 326de73) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MNXLXj2sJf943y2dDF36Vy From 9d6e2af0210ebe4de3b04bb9ef687805803528da Mon Sep 17 00:00:00 2001 From: bherbruck Date: Sat, 31 Oct 2026 02:08:49 -0400 Subject: [PATCH 05/13] difftest: revive dead fixture cases (dead-case B) Rejected cases across 8 units go from 27 to 8; every unit still passes. Fixtures only. Co-Authored-By: Claude Opus 5.5 (1M context) --- fixtures/boolean.merge_p4/manifold.json | 7 +- fixtures/curve.line/constructor_refusals.json | 60 +++++ fixtures/intersect.curve_class_p9/usage.json | 14 +- .../synchronize.json | 7 +- .../review_box_construction.json | 10 +- .../constructor_refusals.json | 49 +++++ .../level2_assert_valid.json | 16 +- .../topology.assert_valid/level2_tests.json | 8 +- .../constructor_refusals.json | 207 ++++++++++++++++++ 9 files changed, 349 insertions(+), 29 deletions(-) create mode 100644 fixtures/curve.line/constructor_refusals.json create mode 100644 fixtures/primitive.api_primitives/constructor_refusals.json create mode 100644 fixtures/topology.brep_p8/constructor_refusals.json diff --git a/fixtures/boolean.merge_p4/manifold.json b/fixtures/boolean.merge_p4/manifold.json index ed8ba8750e..aa63886650 100644 --- a/fixtures/boolean.merge_p4/manifold.json +++ b/fixtures/boolean.merge_p4/manifold.json @@ -7657,14 +7657,14 @@ } }, { - "name": "plane-domain-constructor-rejection", + "name": "plane-domain-1e6-largest-buildable", "inputs": { "primary": [ 3, 0, 0, 0, - 2000000, + 1000000, 1, 0 ], @@ -7678,7 +7678,8 @@ 0 ], "operation": 0 - } + }, + "note": "The largest plane-face U domain the pinned constructor builds here (1e6; 2e6 is refused, now topology.brep_p8 constructor_refusals.json)." } ] } diff --git a/fixtures/curve.line/constructor_refusals.json b/fixtures/curve.line/constructor_refusals.json new file mode 100644 index 0000000000..66c4332d1e --- /dev/null +++ b/fixtures/curve.line/constructor_refusals.json @@ -0,0 +1,60 @@ +{ + "cases": [ + { + "name": "segment_coincident_ends_refused", + "note": "Constructor status (both sides refuse; compared as a status): SmLine::CreateLineSegment with coincident ends over [0, 1]. It was the setup of offset.offset_executive_p1 coincident_analytic_line_constructor_rejection (now shortest_buildable_analytic_line), which now runs just inside the limit.", + "inputs": { + "make": { + "kind": "segment", + "a": [ + 0, + 0, + 0 + ], + "b": [ + 0, + 0, + 0 + ], + "interval": [ + 0, + 1 + ] + }, + "ops": [ + { + "op": "ends" + } + ] + } + }, + { + "name": "segment_1e_14_refused", + "note": "Constructor status (both sides refuse; compared as a status): SmLine::CreateLineSegment of a 1e-14 line over [0, 1] (1e-12 builds). It was the setup of offset.offset_executive_p1 shortest_buildable_analytic_line, which now runs just inside the limit.", + "inputs": { + "make": { + "kind": "segment", + "a": [ + 0, + 0, + 0 + ], + "b": [ + 1e-14, + 0, + 0 + ], + "interval": [ + 0, + 1 + ] + }, + "ops": [ + { + "op": "ends" + } + ] + } + } + ] +} diff --git a/fixtures/intersect.curve_class_p9/usage.json b/fixtures/intersect.curve_class_p9/usage.json index 29bdd435c6..5bf13b46f2 100644 --- a/fixtures/intersect.curve_class_p9/usage.json +++ b/fixtures/intersect.curve_class_p9/usage.json @@ -660,23 +660,25 @@ } }, { - "name": "collapsed_rectangle_setup_rejected", + "name": "collapsed_surface_face_topology", "upstream_ref": "source/SMLib/src/SmCurveClass.cpp:16225; review #151", "inputs": { "op": 1, "kind": 2, - "rectangle": true - } + "rectangle": false + }, + "note": "The collapsed kind-2 surface built through MakeFaceTopology (rectangle off): the nearest setup the pinned C++ builds (the rectangle form is refused, now topology.brep_p8 constructor_refusals.json)." }, { - "name": "tiny_zero_zone_rectangle_setup_rejected", + "name": "smallest_buildable_zero_zone_rectangle", "upstream_ref": "source/SMLib/src/SmCurveClass.cpp:16225; review #151", "inputs": { "op": 1, - "scale": 1e-06, + "scale": 0.0001, "zone": 0, "rectangle": true - } + }, + "note": "The smallest rectangle scale the pinned fixture builds with zone 0 (1e-4; 1e-5 and 1e-6 are refused at every zone, now topology.brep_p8 constructor_refusals.json)." }, { "name": "vertex_at_source_start", diff --git a/fixtures/offset.offset_executive_p1/synchronize.json b/fixtures/offset.offset_executive_p1/synchronize.json index f1ab76e786..48ffda9698 100644 --- a/fixtures/offset.offset_executive_p1/synchronize.json +++ b/fixtures/offset.offset_executive_p1/synchronize.json @@ -2370,7 +2370,7 @@ } }, { - "name": "coincident_analytic_line_constructor_rejection", + "name": "shortest_buildable_analytic_line", "inputs": { "op": "synchronize", "first": [ @@ -2383,7 +2383,7 @@ 0 ], "end": [ - 0, + 1e-12, 0, 0 ], @@ -2401,7 +2401,8 @@ "second": [], "null_original": false, "null_other": false - } + }, + "note": "The shortest analytic line the pinned CreateLineSegment builds here (1e-12; a zero or 1e-14 line is refused with 1007, now curve.line constructor_refusals.json)." }, { "name": "near_degenerate_analytic_line_positive_length_survives", diff --git a/fixtures/offset.offset_executive_p2/review_box_construction.json b/fixtures/offset.offset_executive_p2/review_box_construction.json index cd4846838a..4c7c2b14de 100644 --- a/fixtures/offset.offset_executive_p2/review_box_construction.json +++ b/fixtures/offset.offset_executive_p2/review_box_construction.json @@ -1,14 +1,14 @@ { - "coverage": "PR #201 must-fix2: CreateBox ignores face-creation statuses. Inspect actual native face/edge/vertex counts before selector resolution; incomplete source or foreign boxes are constructor Rejected(SM_ERR=1001), not built API errors or traversal exceptions. Both reported scales use tolerance 1e-9 and public distance 0.5. Valid 6/12/8 box controls retain Built(SM_ERR_INVALID_INPUT=1007) for null/foreign selections. No size cutoff is implied.", + "coverage": "PR #201 must-fix2: CreateBox ignores face-creation statuses. Inspect actual native face/edge/vertex counts before selector resolution; incomplete source or foreign boxes are constructor Rejected(SM_ERR=1001), not built API errors or traversal exceptions. Both reported scales use tolerance 1e-9 and public distance 0.5. Valid 6/12/8 box controls retain Built(SM_ERR_INVALID_INPUT=1007) for null/foreign selections. No size cutoff is implied. The *_selected cases use the smallest boxes the pinned constructor builds (3e-5 at the origin, 3e-3 at 1e6); the refusals of the 1e-6 and 1e-3-at-1e6 boxes are primitive.api_primitives constructor_refusals.json status cases.", "cases": [ {"name":"review_box_tiny_source_null", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[1e-6,1e-6,1e-6]},"faces":[{"origin":"null"}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, - {"name":"review_box_tiny_source_selected", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[1e-6,1e-6,1e-6]},"faces":[{"origin":"source","index":0}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, + {"name":"review_box_smallest_buildable_source_selected", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[3e-5,3e-5,3e-5]},"faces":[{"origin":"source","index":0}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, {"name":"review_box_translated_source_null", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[1e6,1e6,1e6],"size":[1e-3,1e-3,1e-3]},"faces":[{"origin":"null"}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, - {"name":"review_box_translated_source_selected", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[1e6,1e6,1e6],"size":[1e-3,1e-3,1e-3]},"faces":[{"origin":"source","index":0}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, + {"name":"review_box_smallest_buildable_translated_source_selected", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[1e6,1e6,1e6],"size":[3e-3,3e-3,3e-3]},"faces":[{"origin":"source","index":0}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, {"name":"review_box_tiny_foreign_null", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review foreign constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[2,3,4]},"foreign":{"kind":"box","origin":[0,0,0],"size":[1e-6,1e-6,1e-6]},"faces":[{"origin":"null"}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, - {"name":"review_box_tiny_foreign_selected", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review foreign constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[2,3,4]},"foreign":{"kind":"box","origin":[0,0,0],"size":[1e-6,1e-6,1e-6]},"faces":[{"origin":"foreign","index":0}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, + {"name":"review_box_smallest_buildable_foreign_selected", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review foreign constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[2,3,4]},"foreign":{"kind":"box","origin":[0,0,0],"size":[3e-5,3e-5,3e-5]},"faces":[{"origin":"foreign","index":0}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, {"name":"review_box_translated_foreign_null", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review foreign constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[2,3,4]},"foreign":{"kind":"box","origin":[1e6,1e6,1e6],"size":[1e-3,1e-3,1e-3]},"faces":[{"origin":"null"}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, - {"name":"review_box_translated_foreign_selected", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review foreign constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[2,3,4]},"foreign":{"kind":"box","origin":[1e6,1e6,1e6],"size":[1e-3,1e-3,1e-3]},"faces":[{"origin":"foreign","index":0}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, + {"name":"review_box_smallest_buildable_translated_foreign_selected", "upstream_ref":"source/SMLib/src/SmPrimitiveCreation.cpp:294-344;source/SM_API/src/SmApiPrimitives.cpp:306-324 (review foreign constructor regression)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[2,3,4]},"foreign":{"kind":"box","origin":[1e6,1e6,1e6],"size":[3e-3,3e-3,3e-3]},"faces":[{"origin":"foreign","index":0}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, {"name":"review_box_valid_source_null_control", "upstream_ref":"source/SM_API/src/SmApiPrimitives.cpp:306-324 (review valid constructor control)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[2,3,4]},"faces":[{"origin":"null"}],"tolerance":1e-9,"config":{"offset_distance":0.5}}}, {"name":"review_box_valid_foreign_selected_control", "upstream_ref":"source/SM_API/src/SmApiPrimitives.cpp:306-324 (review valid foreign constructor control)", "inputs":{"operation":"api_shell","source":{"kind":"box","origin":[0,0,0],"size":[2,3,4]},"foreign":{"kind":"box","origin":[0,0,0],"size":[2,3,4]},"faces":[{"origin":"foreign","index":0}],"tolerance":1e-9,"config":{"offset_distance":0.5}}} ] diff --git a/fixtures/primitive.api_primitives/constructor_refusals.json b/fixtures/primitive.api_primitives/constructor_refusals.json new file mode 100644 index 0000000000..b9b9325199 --- /dev/null +++ b/fixtures/primitive.api_primitives/constructor_refusals.json @@ -0,0 +1,49 @@ +{ + "cases": [ + { + "name": "box_1e6_size_1e_6_refused", + "note": "Constructor status (both sides refuse; compared as a status): SmApiCreateBox of a 1e-6 cube at the origin. It was the setup of offset.offset_executive_p2 review_box_*_selected, which now runs just inside the limit.", + "inputs": { + "op": "box", + "args": [ + 0, + 0, + 0, + 1e-06, + 1e-06, + 1e-06 + ] + } + }, + { + "name": "box_size_1e_5_refused", + "note": "Constructor status (both sides refuse; compared as a status): SmApiCreateBox of a 1e-5 cube at the origin (3e-5 builds). It was the setup of offset.offset_executive_p2 review_box_smallest_buildable_*, which now runs just inside the limit.", + "inputs": { + "op": "box", + "args": [ + 0, + 0, + 0, + 1e-05, + 1e-05, + 1e-05 + ] + } + }, + { + "name": "box_at_1e6_size_1e_3_refused", + "note": "Constructor status (both sides refuse; compared as a status): SmApiCreateBox of a 1e-3 cube at (1e6, 1e6, 1e6) (3e-3 builds). It was the setup of offset.offset_executive_p2 review_box_*translated*_selected, which now runs just inside the limit.", + "inputs": { + "op": "box", + "args": [ + 1000000.0, + 1000000.0, + 1000000.0, + 0.001, + 0.001, + 0.001 + ] + } + } + ] +} diff --git a/fixtures/topology.assert_valid/level2_assert_valid.json b/fixtures/topology.assert_valid/level2_assert_valid.json index a1f421be41..0033427398 100644 --- a/fixtures/topology.assert_valid/level2_assert_valid.json +++ b/fixtures/topology.assert_valid/level2_assert_valid.json @@ -322,7 +322,7 @@ } }, { - "name": "l2_thin_face_level2", + "name": "l2_thinnest_buildable_face_level2", "upstream_ref": "source/SMLib/src/SmBrep.cpp:AssertValid", "inputs": { "brep": { @@ -344,7 +344,7 @@ "dom": [ 0, 0, - 1e-13, + 1e-12, 1 ] }, @@ -352,7 +352,7 @@ } }, { - "name": "l2_thin_face_capi2", + "name": "l2_thinnest_buildable_face_capi2", "upstream_ref": "source/SMLib/src/SmBrep.cpp:AssertValid", "inputs": { "brep": { @@ -374,7 +374,7 @@ "dom": [ 0, 0, - 1e-13, + 1e-12, 1 ] }, @@ -383,7 +383,7 @@ } }, { - "name": "l2_thin_face2_level2", + "name": "l2_thinnest_buildable_face2_level2", "upstream_ref": "source/SMLib/src/SmBrep.cpp:AssertValid", "inputs": { "brep": { @@ -405,7 +405,7 @@ "dom": [ -1, -1, - -0.9999999999995, + -0.99999999999, 1 ] }, @@ -413,7 +413,7 @@ } }, { - "name": "l2_thin_face2_capi2", + "name": "l2_thinnest_buildable_face2_capi2", "upstream_ref": "source/SMLib/src/SmBrep.cpp:AssertValid", "inputs": { "brep": { @@ -435,7 +435,7 @@ "dom": [ -1, -1, - -0.9999999999995, + -0.99999999999, 1 ] }, diff --git a/fixtures/topology.assert_valid/level2_tests.json b/fixtures/topology.assert_valid/level2_tests.json index 6c20567be8..ed6fc22e25 100644 --- a/fixtures/topology.assert_valid/level2_tests.json +++ b/fixtures/topology.assert_valid/level2_tests.json @@ -157,7 +157,7 @@ } }, { - "name": "l2_thin_face_tests", + "name": "l2_thinnest_buildable_face_tests", "upstream_ref": "source/SMLib/src/SmFace.cpp:sm_AssertTestFace32", "inputs": { "brep": { @@ -179,7 +179,7 @@ "dom": [ 0, 0, - 1e-13, + 1e-12, 1 ] }, @@ -187,7 +187,7 @@ } }, { - "name": "l2_thin_face2_tests", + "name": "l2_thinnest_buildable_face2_tests", "upstream_ref": "source/SMLib/src/SmFace.cpp:sm_AssertTestFace32", "inputs": { "brep": { @@ -209,7 +209,7 @@ "dom": [ -1, -1, - -0.9999999999995, + -0.99999999999, 1 ] }, diff --git a/fixtures/topology.brep_p8/constructor_refusals.json b/fixtures/topology.brep_p8/constructor_refusals.json new file mode 100644 index 0000000000..d5972aecbe --- /dev/null +++ b/fixtures/topology.brep_p8/constructor_refusals.json @@ -0,0 +1,207 @@ +{ + "cases": [ + { + "name": "plane_face_u_domain_2e6", + "note": "Constructor status (both sides refuse; compared as a status): CreateFaceFromSurface on a plane over [0, 2e6] x [0, 1] (1e6 builds). It was the setup of boolean.merge_p4 plane-domain-constructor-rejection (now plane-domain-1e6-largest-buildable), which now runs just inside the limit.", + "inputs": { + "op": "from_surface", + "surface": { + "plane": { + "o": [ + 0, + 0, + 0 + ], + "n": [ + 0, + 0, + 1 + ] + } + }, + "dom": [ + 0, + 0, + 2000000.0, + 1 + ] + } + }, + { + "name": "plane_face_1e_13_wide", + "note": "Constructor status (both sides refuse; compared as a status): CreateFaceFromSurface on a plane over [0, 1e-13] x [0, 1] (1e-12 builds). It was the setup of topology.assert_valid l2_thin_face_* (now l2_thinnest_buildable_face_*), which now runs just inside the limit.", + "inputs": { + "op": "from_surface", + "surface": { + "plane": { + "o": [ + 0, + 0, + 0 + ], + "n": [ + 0, + 0, + 1 + ] + } + }, + "dom": [ + 0, + 0, + 1e-13, + 1 + ] + } + }, + { + "name": "tilted_plane_face_5e_13_wide", + "note": "Constructor status (both sides refuse; compared as a status): CreateFaceFromSurface on the plane through (1, 2, 3) with normal (0, 1, 1) over [-1, -1 + 5e-13] x [-1, 1] (1e-11 builds). It was the setup of topology.assert_valid l2_thin_face2_* (now l2_thinnest_buildable_face2_*), which now runs just inside the limit.", + "inputs": { + "op": "from_surface", + "surface": { + "plane": { + "o": [ + 1, + 2, + 3 + ], + "n": [ + 0, + 1, + 1 + ] + } + }, + "dom": [ + -1, + -1, + -0.9999999999995, + 1 + ] + } + }, + { + "name": "collapsed_bilinear_patch_face", + "note": "Constructor status (both sides refuse; compared as a status): CreateFaceFromSurface on the bilinear patch with a collapsed side (curve_class_p9's kind 2). It was the setup of intersect.curve_class_p9 collapsed_rectangle_setup_rejected (now collapsed_surface_face_topology), which now runs just inside the limit.", + "inputs": { + "op": "from_surface", + "surface": { + "bspline": { + "net": [ + [ + [ + 0, + 0, + 0, + -1.7976931348623157e+308 + ], + [ + 0, + 0.0, + 0, + -1.7976931348623157e+308 + ] + ], + [ + [ + 1.0, + 0, + 0, + -1.7976931348623157e+308 + ], + [ + 1.0, + 1.0, + 0, + -1.7976931348623157e+308 + ] + ] + ], + "p": 1, + "q": 1, + "u": [ + 0, + 0, + 1, + 1 + ], + "v": [ + 0, + 0, + 1, + 1 + ] + } + }, + "dom": [ + 0, + 0, + 1, + 1 + ] + } + }, + { + "name": "bilinear_patch_1e_6_face", + "note": "Constructor status (both sides refuse; compared as a status): CreateFaceFromSurface on a 1e-6 bilinear patch (curve_class_p9's rectangle at scale 1e-6; 1e-4 builds there). It was the setup of intersect.curve_class_p9 tiny_zero_zone_rectangle_setup_rejected (now smallest_buildable_zero_zone_rectangle), which now runs just inside the limit.", + "inputs": { + "op": "from_surface", + "surface": { + "bspline": { + "net": [ + [ + [ + 0, + 0, + 0, + -1.7976931348623157e+308 + ], + [ + 0, + 1e-06, + 0, + -1.7976931348623157e+308 + ] + ], + [ + [ + 1e-06, + 0, + 0, + -1.7976931348623157e+308 + ], + [ + 1e-06, + 1e-06, + 0, + -1.7976931348623157e+308 + ] + ] + ], + "p": 1, + "q": 1, + "u": [ + 0, + 0, + 1, + 1 + ], + "v": [ + 0, + 0, + 1, + 1 + ] + } + }, + "dom": [ + 0, + 0, + 1, + 1 + ] + } + } + ] +} From 8e340e0f3f3feef72fe6967a33f453a405dff6df Mon Sep 17 00:00:00 2001 From: aapolipponen Date: Sat, 31 Oct 2026 08:09:54 +0200 Subject: [PATCH 06/13] =?UTF-8?q?difftest:=20intersect.curve=5Fclass=5Fp7?= =?UTF-8?q?=20=E2=80=94=20real=5Fendpoint=5Fcases=20avoids=20direct=20inde?= =?UTF-8?q?xing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../difftest/src/units/intersect_curve_class_p7.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/difftest/src/units/intersect_curve_class_p7.rs b/crates/difftest/src/units/intersect_curve_class_p7.rs index 995c4c3f9b..0900976d72 100644 --- a/crates/difftest/src/units/intersect_curve_class_p7.rs +++ b/crates/difftest/src/units/intersect_curve_class_p7.rs @@ -960,11 +960,13 @@ fn real_endpoint_cases() -> Vec<(String, Value)> { } else { ([0.5, 0.5, 0.0], [0.0, -0.5, 0.0]) }; - objects[0] = ObjIn::Curve(CurveIn::Line(LineIn { - p, v, domain: [0.0, 1.0], - scale: if name == "max_vertex" { 0.5_f64.sqrt() } else { 0.5 }, - inside_out: false, - })); + if let Some(slot) = objects.first_mut() { + *slot = ObjIn::Curve(CurveIn::Line(LineIn { + p, v, domain: [0.0, 1.0], + scale: if name == "max_vertex" { 0.5_f64.sqrt() } else { 0.5 }, + inside_out: false, + })); + } } let script = Script { objects, face: Some(face.clone()), cc: [cc.clone(), cc], ops: vec![OpIn::ClassifyFace { t: 0, j: 0, do_point: false, From c80421a97c78fb510cd6d132e438fbf71858f05a Mon Sep 17 00:00:00 2001 From: Brett Lawson Date: Sat, 31 Oct 2026 07:13:23 +0100 Subject: [PATCH 07/13] fix: a face's tight box divides its cells at the surface's knots (opt-in, WR-645) Ports tangency-labs/smlib@326de73 (sm_CellDivision), the second half of the fork's trimmed-face tight box search (WR-588, #1799): face_directed_max divides an undecided cell each way at the surface's knot strictly inside it (by more than CopySubPatch's 1e-8 snap) nearest its middle, at its middle where it holds none, so a cell over a knot off the domain's dyadic points comes to lie within one span and cell_monotone can read it. Under the same opt-in Kernel::fixes.face_tight_box_searches_trim; flag-off code is unchanged. Regression test: a 270-degree rational cylinder (knots at 1/3, 2/3) trimmed to z <= 0.3 boxes to z 0.30859375 without the division, 0.3 with it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MNXLXj2sJf943y2dDF36Vy --- crates/solidcore/src/kernel.rs | 2 +- crates/solidcore/src/query/face/p3.rs | 58 ++++++-- .../issue_1799_tight_box_divides_at_knots.rs | 135 ++++++++++++++++++ ...ls-over-off-dyadic-knots-stay-undecided.md | 10 ++ 4 files changed, 196 insertions(+), 9 deletions(-) create mode 100644 crates/solidcore/tests/issue_1799_tight_box_divides_at_knots.rs create mode 100644 plan/upstream-issues/WR-645--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md diff --git a/crates/solidcore/src/kernel.rs b/crates/solidcore/src/kernel.rs index c0a051d81a..92f19cc5a1 100644 --- a/crates/solidcore/src/kernel.rs +++ b/crates/solidcore/src/kernel.rs @@ -516,7 +516,7 @@ pub struct UpstreamFixes { pub fillet_trace_rejects_short_step: bool, /// `SmCurve::CalculateTightBoundingBox` adds only the B-spline knots strictly inside the queried interval (the interval's ends are already in the box). Upstream adds every tangent-discontinuous knot and always the last knot whatever the interval, so the tight box of an edge on part of a NURBS curve (an imported sphere cap's seam, 30 deg of a quarter-circle NURBS) reaches points the edge never does (WR-474, #1054; the fork's \"a curve's tight box holds the knots inside its interval only\"). pub curve_tight_box_interval_knots: bool, - /// `SmFace::CalculateTightBoundingBox` searches a trimmed face's own extreme inside its trim (subdividing the UV domain from the vertex and edge extremes, bounded at depth 16 / 4096 cells), not only the surface's global extreme, so a face whose surface peaks outside the trim is not boxed too small (WR-588, #1799; tangency-labs/smlib@9d44c1b). + /// `SmFace::CalculateTightBoundingBox` searches a trimmed face's own extreme inside its trim (subdividing the UV domain from the vertex and edge extremes, bounded at depth 16 / 4096 cells, an undecided cell divided at the surface's knot inside it nearest its middle, else at its middle), not only the surface's global extreme, so a face whose surface peaks outside the trim is not boxed too small (WR-588, #1799; tangency-labs/smlib@9d44c1b), nor one whose rational surface has knots off the domain's dyadic points too large (WR-645; tangency-labs/smlib@326de73). pub face_tight_box_searches_trim: bool, /// `SmTopologyIntersector::IntersectTopology` skips a face pair whose face boxes lie farther apart than the intersection tolerance (a box that cannot be computed, or holds nothing, rules nothing out), and `SmBrep::MakeEdgeInFaceWithVU`, on a plane, leaves in place without classifying an inner loop whose test point lies outside the box of the new face's outer loop (grown by the edges' tolerances, at the retry tolerance). Upstream pairs every face of one sheet with every face of the other through whole-surface checks and classifies every inner loop against each imprinted face, so a 2D boolean of a sheet with n holes takes time in n^2 (WR-527, #1526/#1527; the fork's tangency-labs/smlib@60b1c6a). pub boolean_skips_faces_apart: bool, diff --git a/crates/solidcore/src/query/face/p3.rs b/crates/solidcore/src/query/face/p3.rs index 8604edea90..22d476cbd9 100644 --- a/crates/solidcore/src/query/face/p3.rs +++ b/crates/solidcore/src/query/face/p3.rs @@ -11,7 +11,7 @@ //! | Rust | Upstream | //! |-----------------------------------------------|---------------------------------------------| //! | [`calculate_tight_bounding_box`], [`calculate_tight_bounding_box_into`] | `SmFace::CalculateTightBoundingBox` | -//! | `face_directed_max` (opt-in, WR-588), `cell_directed_max`, `cell_monotone` | `sm_FaceDirectedMax`, `sm_CellDirectedMax`, `sm_CellMonotone` (the fork tangency-labs/smlib@9d44c1b) | +//! | `face_directed_max` (opt-in, WR-588), `cell_directed_max`, `cell_monotone`, `cell_division` (WR-645) | `sm_FaceDirectedMax`, `sm_CellDirectedMax`, `sm_CellMonotone` (the fork tangency-labs/smlib@9d44c1b), `sm_CellDivision` (tangency-labs/smlib@326de73) | //! | [`face_classify_uv_line`] | `sm_FaceClassifyUVLine` (file static) | //! | [`face_classify_uv_line_mut`] | `sm_FaceClassifyUVLine` (in place) | //! | [`find_and_classify_loop_bisector`], [`LoopBisector`] | `sms_FindAndClassifyLoopBisector` | @@ -587,10 +587,37 @@ fn eval_pos(eval: &mut SurfaceEval<'_>) -> Result { /// value, so the box is never smaller than the face. const FACE_EXTREME_MAX_CELLS: usize = 4096; -/// How deep [`face_directed_max`] halves a cell (`sm_lFaceExtremeMaxDepth`), each way. A +/// How deep [`face_directed_max`] divides a cell (`sm_lFaceExtremeMaxDepth`), each way. A /// cell left undecided at this depth counts with its own greatest value. const FACE_EXTREME_MAX_DEPTH: u32 = 16; +/// How near a cell's side a knot may lie and still divide it +/// (`sm_dFaceExtremeKnotSnap`): `CopySubPatch` snaps a requested side onto a knot within +/// this distance, so a part nearer than that is no cell of its own. +const FACE_EXTREME_KNOT_SNAP: f64 = 1.0e-08; + +/// Where a cell of `[lo, hi]` is divided along one way (`sm_CellDivision`, WR-645, +/// tangency-labs/smlib@326de73): at the knot strictly inside it (by more than +/// [`FACE_EXTREME_KNOT_SNAP`]) nearest its middle, the first such in `knots` on a tie, +/// else at its middle. Divided at knots, a cell comes to lie within one knot span that +/// way, as [`cell_monotone`] asks of a rational patch; halved at middles alone, a cell +/// over a knot off the dyadic points of the domain (a screw surface's at every eighth of +/// a turn) never does. +fn cell_division(knots: &[f64], lo: f64, hi: f64) -> f64 { + let mid = 0.5 * (lo + hi); + let (mut at, mut off) = (mid, crate::math::BIG_DOUBLE); + for &knot in knots { + if knot > lo + FACE_EXTREME_KNOT_SNAP + && knot < hi - FACE_EXTREME_KNOT_SNAP + && (knot - mid).abs() < off + { + at = knot; + off = (knot - mid).abs(); + } + } + at +} + /// Whether `x` is above zero, false for NaN (the negation of `x > 0`, without the lint). fn is_positive(x: f64) -> bool { x > 0.0 @@ -849,7 +876,8 @@ fn cell_monotone(k: &Kernel, surface: &Surface, cell: &Extent2d, dir: Vec3) -> R /// on the face's boundary or on the cell's own sides, which `best` and the neighbouring /// cells hold; /// -/// otherwise the cell is halved each way. A cell left at [`FACE_EXTREME_MAX_DEPTH`] or past +/// otherwise the cell is divided each way ([`cell_division`]: at a knot inside it, else at +/// its middle; WR-645). A cell left at [`FACE_EXTREME_MAX_DEPTH`] or past /// [`FACE_EXTREME_MAX_CELLS`] counts with its greatest value. The cells are an explicit /// stack (no recursion), taken last first, as the fork's. fn face_directed_max( @@ -862,6 +890,15 @@ fn face_directed_max( dir: Vec3, mut best: f64, ) -> Result { + // (`IsKindOf(SmBSplineSurface_TYPE)`: `GetKnots` through `GetOrCreateGwNurbPointer`) + let (knots_u, knots_v) = if has_b_spline_box(surface) { + ( + surface.knots_with_nurb(k, SurfParamType::U, None)?.0, + surface.knots_with_nurb(k, SurfParamType::V, None)?.0, + ) + } else { + (Vec::new(), Vec::new()) + }; let mut cells: Vec<(Extent2d, u32)> = vec![(*uv_domain, 0)]; let mut divided = 0usize; while let Some((cell, depth)) = cells.pop() { @@ -898,13 +935,18 @@ fn face_directed_max( best = top.top; continue; } - let mid = cell.mid(); let (u0, v0, u1, v1) = (cell.min().x, cell.min().y, cell.max().x, cell.max().y); + // WR-645 (tangency-labs/smlib@326de73): divided at a knot inside the cell, else at + // its middle (`sm_CellDivision`). + let at = Point2::new( + cell_division(&knots_u, u0, u1), + cell_division(&knots_v, v0, v1), + ); let child_depth = depth + 1; - cells.push((Extent2d::from_bounds(u0, v0, mid.x, mid.y), child_depth)); - cells.push((Extent2d::from_bounds(mid.x, v0, u1, mid.y), child_depth)); - cells.push((Extent2d::from_bounds(u0, mid.y, mid.x, v1), child_depth)); - cells.push((Extent2d::from_bounds(mid.x, mid.y, u1, v1), child_depth)); + cells.push((Extent2d::from_bounds(u0, v0, at.x, at.y), child_depth)); + cells.push((Extent2d::from_bounds(at.x, v0, u1, at.y), child_depth)); + cells.push((Extent2d::from_bounds(u0, at.y, at.x, v1), child_depth)); + cells.push((Extent2d::from_bounds(at.x, at.y, u1, v1), child_depth)); } Ok(best) } diff --git a/crates/solidcore/tests/issue_1799_tight_box_divides_at_knots.rs b/crates/solidcore/tests/issue_1799_tight_box_divides_at_knots.rs new file mode 100644 index 0000000000..eaa479e546 --- /dev/null +++ b/crates/solidcore/tests/issue_1799_tight_box_divides_at_knots.rs @@ -0,0 +1,135 @@ +//! WR-645 (tangency-labs/smlib@326de73), the second half of #1799's trimmed-face tight box +//! (`Kernel::fixes.face_tight_box_searches_trim`, WR-588): the search divides an undecided +//! cell at the surface's knot inside it, not only at its middle. +//! +//! A rational patch is read monotone only within one knot span. Halved at middles alone, a +//! cell over a knot off the dyadic points of the face's domain never comes to lie within one +//! span, so along the trim it stays undecided down to the depth limit and counts with its +//! surface's top: the box overshoots. SolidCraft's case: an M10x1.5 thread cut from a shaft +//! of radius 5 and length 12 boxed to z 12.46875; its screw walls have a knot every eighth +//! of a turn. +//! +//! Here: the 270-degree cylinder of radius 1 about z (three rational quadratic quarter +//! spans, knots at u = 1/3 and 2/3), z = v over [0, 1], trimmed to z <= 0.3. Its box is +//! (-1, -1, 0) to (1, 1, 0.3). +#![allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::panic, + clippy::indexing_slicing +)] // test helpers + +use std::f64::consts::FRAC_1_SQRT_2; + +use solidcore::curve::curve::Curve; +use solidcore::curve::line::Line; +use solidcore::geom::extent1d::Extent1d; +use solidcore::geom::extent3d::Extent3d; +use solidcore::math::Point3; +use solidcore::nurbs::b_spline_curve::BSplineCurve; +use solidcore::nurbs::b_spline_surface::BSplineSurface; +use solidcore::nurbs::crv_basic::Curve as NurbCurve; +use solidcore::nurbs::math::CPoint; +use solidcore::nurbs::srf_basic::Surface as NurbSurface; +use solidcore::primitive::api_trim_ops::trim_surface_with_3d_curves; +use solidcore::query::face::p3::calculate_tight_bounding_box; +use solidcore::surface::surface::Surface; +use solidcore::topology::brep::p1::faces; +use solidcore::Kernel; + +/// The arc's knots: three quarter spans, double interior knots at 1/3 and 2/3. +fn arc_knots() -> Vec { + let (a, b) = (1.0 / 3.0, 2.0 / 3.0); + vec![0.0, 0.0, 0.0, a, a, b, b, 1.0, 1.0, 1.0] +} + +/// The arc's control points `(x, y)` and weights, from angle 0 to 270 degrees. +fn arc_net() -> Vec<(f64, f64, f64)> { + let s = FRAC_1_SQRT_2; + vec![ + (1.0, 0.0, 1.0), + (1.0, 1.0, s), + (0.0, 1.0, 1.0), + (-1.0, 1.0, s), + (-1.0, 0.0, 1.0), + (-1.0, -1.0, s), + (0.0, -1.0, 1.0), + ] +} + +/// The arc at height `z`, reversed when `backwards` (the knots are symmetric). +fn arc(z: f64, backwards: bool) -> Curve { + let mut net = arc_net(); + if backwards { + net.reverse(); + } + let pw = net + .into_iter() + .map(|(x, y, w)| CPoint::new(x * w, y * w, z * w, w)) + .collect(); + let nurb = NurbCurve::new(pw, 2, arc_knots()).unwrap(); + Curve::from(BSplineCurve::from_nurb(nurb, 3, false)) +} + +/// The cylinder: the arc along u, z = v linear over [0, 1]. +fn cylinder() -> Surface { + let net: Vec> = arc_net() + .into_iter() + .map(|(x, y, w)| { + [0.0, 1.0] + .iter() + .map(|&z| CPoint::new(x * w, y * w, z * w, w)) + .collect() + }) + .collect(); + let nurb = NurbSurface::new(net, 2, 1, arc_knots(), vec![0.0, 0.0, 1.0, 1.0]).unwrap(); + Surface::from(BSplineSurface::from_nurb(nurb, false)) +} + +fn line(from: Point3, to: Point3) -> Curve { + let interval = Extent1d::from_min_max(0.0, 1.0); + Curve::from(Line::create_line_segment(3, from, to, Some(&interval)).unwrap()) +} + +/// The tight box of the cylinder trimmed to `0 <= z <= top`, with the trim search on or off. +fn trimmed_cylinder_box(top: f64, searches_trim: bool) -> Extent3d { + let mut k = Kernel::default(); + k.fixes.face_tight_box_searches_trim = searches_trim; + let curves = vec![ + arc(0.0, false), + line(Point3::new(0.0, -1.0, 0.0), Point3::new(0.0, -1.0, top)), + arc(top, true), + line(Point3::new(1.0, 0.0, top), Point3::new(1.0, 0.0, 0.0)), + ]; + let b = trim_surface_with_3d_curves(&mut k, cylinder(), &[4], curves).unwrap(); + let fs = faces(&b, None).unwrap(); + assert_eq!(fs.len(), 1); + calculate_tight_bounding_box(&k, &b, fs[0]).unwrap() +} + +fn assert_close(what: &str, got: f64, want: f64) { + assert!((got - want).abs() <= 1e-9, "{what}: {got}, want {want}"); +} + +/// With the search on, the cells over the knots divide there and are read monotone: the box +/// is the face's, exactly. Halved at middles only, the cells over u = 1/3 along the trim at +/// z = 0.3 stay undecided to the depth limit and the box reaches above 0.3. +#[test] +fn trimmed_rational_cylinder_box_is_exact_with_the_search() { + let bx = trimmed_cylinder_box(0.3, true); + assert_close("z max", bx.max().z, 0.3); + assert_close("z min", bx.min().z, 0.0); + assert_close("x max", bx.max().x, 1.0); + assert_close("x min", bx.min().x, -1.0); + assert_close("y max", bx.max().y, 1.0); + assert_close("y min", bx.min().y, -1.0); +} + +/// Flag off (upstream): the surface's global solves classify outside the face, so the box +/// is the vertices' and edges', which here is the face's too. +#[test] +fn trimmed_rational_cylinder_box_without_the_search_is_the_edges() { + let bx = trimmed_cylinder_box(0.3, false); + assert_close("z max", bx.max().z, 0.3); + assert_close("z min", bx.min().z, 0.0); +} diff --git a/plan/upstream-issues/WR-645--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md b/plan/upstream-issues/WR-645--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md new file mode 100644 index 0000000000..7093860a56 --- /dev/null +++ b/plan/upstream-issues/WR-645--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md @@ -0,0 +1,10 @@ +#### WR-645. The trimmed-face tight box search (WR-588) leaves cells over a knot off the domain's dyadic points undecided, so the box overshoots +- **Severity:** Wrong result (a trimmed face's tight box is too large; opt-in path only) +- **Area:** Faces / tight bounding box +- **Location:** `source/SMLib/src/SmFace.cpp`, `sm_FaceDirectedMax` (the SolidCraft fork's, tangency-labs/smlib@beb6061, ported for WR-588): an undecided cell is halved at its middle. Not in upstream SMLib, which has no such search; this completes WR-588's fix. +- **What happens:** `sm_CellMonotone` reads a rational patch's monotonicity only over one knot span. Halved at middles alone, a cell over a knot that is not a dyadic point of the face's UV domain never comes to lie within one span, so every such cell along the trim stays undecided down to the depth limit (16) or the cell limit (4096) and counts with its own greatest value. +- **Trigger:** SolidCraft's threads/1250: an M10x1.5 thread groove cut from a shaft of radius 5 and length 12 boxes to z 12.46875 (expected 12); the screw sweep's walls are rational quadratic with a knot every eighth of a turn. In the port: the 270-degree rational cylinder of radius 1 (knots at u = 1/3, 2/3) trimmed to 0 <= z <= 0.3 boxes to z 0.30859375 with the search on (expected 0.3). +- **How found:** SolidCraft (the fork, tangency-labs/smlib@326de73); 15 SolidCraft thread cases overshoot without it. +- **Confirmed:** in the port (`crates/solidcore/tests/issue_1799_tight_box_divides_at_knots.rs` fails with z max 0.30859375 when the knot division is removed). The pinned C++ has no such search, so there is nothing upstream to confirm. +- **Fix (fork):** tangency-labs/smlib@326de73 divides a cell each way at the surface's knot strictly inside it (by more than `CopySubPatch`'s 1e-8 snap) nearest its middle, at its middle only where it holds none (`sm_CellDivision`), the knots read by `GetKnots` when the surface is a kind of `SmBSplineSurface`. +- **Rust port:** under the same opt-in `Kernel::fixes.face_tight_box_searches_trim` (WR-588; default `false`, upstream's behaviour): `cell_division` in `face_directed_max` (`crates/solidcore/src/query/face/p3.rs`). It is the rest of the fork's fix for one defect, and the flag has not yet been turned on by any caller. Tests: `trimmed_rational_cylinder_box_is_exact_with_the_search` (flag on: box (-1, -1, 0) to (1, 1, 0.3)) and `trimmed_rational_cylinder_box_without_the_search_is_the_edges` (flag off). From 1f85b355b049bd689a2f8dbdacce3303efe51b5f Mon Sep 17 00:00:00 2001 From: chris Date: Fri, 30 Oct 2026 23:20:08 -0700 Subject: [PATCH 08/13] test: reproduce bottle sweep validity failures after UB-170 Record native and Rust evidence and failed repair experiments. The bottle validity repair remains unresolved. Co-Authored-By: Codex --- .../tests/issue_1937_bottle_sweep.rs | 219 ++++++++++++++++++ ...ep.topology_sweep--bottle-validity-1937.md | 92 ++++++++ ...ndum-2026-10-31-bottle-validity-details.md | 29 +++ 3 files changed, 340 insertions(+) create mode 100644 crates/solidcore/tests/issue_1937_bottle_sweep.rs create mode 100644 plan/findings/sweep.topology_sweep--bottle-validity-1937.md create mode 100644 plan/upstream-issues/UB-170--addendum-2026-10-31-bottle-validity-details.md diff --git a/crates/solidcore/tests/issue_1937_bottle_sweep.rs b/crates/solidcore/tests/issue_1937_bottle_sweep.rs new file mode 100644 index 0000000000..13d4ba8f33 --- /dev/null +++ b/crates/solidcore/tests/issue_1937_bottle_sweep.rs @@ -0,0 +1,219 @@ +//! Characterization of the unresolved bottle sweep defect (#1937). +//! These tests pin the invalid default output; they do not claim a repair. +use solidcore::geom::attribute::ObjectKey; +use solidcore::sweep::topology_sweep::store::SweepIdentities; +use solidcore::topology::{BrepId, EdgeId, RegionId}; +use solidcore::{Brep, Result}; + +use solidcore::geom::attribute::AttributeStore; +use solidcore::sweep::topology_sweep::{ + access::SweepAccess, engine::SweepTargets, geometry::SweepGeometry, store::SweepStore, + TopologySweep, +}; +use solidcore::sweep::trans_sweep_geometry::TranslationalSweepGeometry; +use solidcore::topology::api_files::read_brep_from_file; +use solidcore::topology::brep::p10::TopologyEnv; +use solidcore::topology::brep::validation::{check_brep, CheckLevel}; +use solidcore::topology::topology::NotifyRecords; +use solidcore::{Kernel, Vec3}; +#[test] +#[ignore = "requires the pinned upstream bottle.smb fixture"] +fn bottle_sweep_reproduces_native_failures() { + let mut k = Kernel::default(); + let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../upstream/TestFiles/pt_TestFiles/Fillet/bottle.smb"); + let bytes = std::fs::read(path).unwrap(); + let mut attributes = AttributeStore::new(); + let mut records = NotifyRecords::new(); + let key_of = |e| WholeIdentities.topology(BrepId::from_index(0), e); + let mut env = TopologyEnv { + all_attributes: &[], + attributes: &mut attributes, + key_of: &key_of, + records: &mut records, + next_key: 1 << 50, + }; + let mut b = read_brep_from_file(&mut k, Some(bytes), true, false, &mut env).unwrap(); + b.refresh_cached_topology(); + let before = check_brep(&mut k, &mut b, CheckLevel::Level0).unwrap(); + eprintln!( + "BEFORE valid={} lines={:?}", + before.is_valid(), + before.lines() + ); + b.set_key(key_of(solidcore::error::EntityRef::Brep(BrepId::from_index(0))).key); + let identities = WholeIdentities; + let mut next = 0; + let mut store = SweepStore::new(&identities, &mut records, &mut attributes, &mut next); + let context = store.register_kernel(&mut k).unwrap(); + let id = store.register_brep(context, &mut b).unwrap(); + let region = store.infinite_region(id).unwrap(); + let g = SweepGeometry::Translation(TranslationalSweepGeometry::new(Vec3::new(0., 0., 4.))); + let mut sweep = TopologySweep { + do_stitching: false, + do_merge: false, + repetitions: 1, + ..TopologySweep::default() + }; + let result = sweep.do_sweep( + &mut store, + &g, + Some(id), + Some((id, region)), + &mut SweepTargets::default(), + ); + eprintln!("SWEEP {result:?}"); + result.unwrap(); + drop(store); + let after = check_brep(&mut k, &mut b, CheckLevel::Level0).unwrap(); + eprintln!("AFTER valid={} lines={:?}", after.is_valid(), after.lines()); + assert_eq!(before.pointers_status, 1000); + assert_eq!(before.assert_valid, Some(true)); + assert_eq!(before.reports.reports().len(), 8); + assert_eq!(after.pointers_status, 1000); + assert_eq!(after.assert_valid, Some(false)); + assert_eq!(after.reports.reports().len(), 6); + let introduced: Vec<_> = after + .reports + .reports() + .iter() + .filter(|r| r.reporting_type != 16011) + .map(|r| (r.reporting_type, r.test_index)) + .collect(); + assert_eq!(introduced, [(15001, 1), (15001, 6), (12225, 2)]); + // Validation populates derived UV curves. A second check of the unchanged body + // exposes an additional loop-orientation failure in pointer validation. + let repeated = check_brep(&mut k, &mut b, CheckLevel::Level0).unwrap(); + eprintln!("REPEATED {:?}", repeated.lines()); + assert_eq!(repeated.pointers_status, 1001); + assert_eq!(repeated.assert_valid, None); + assert_eq!( + repeated + .pointer_reports + .reports() + .iter() + .map(|r| (r.reporting_type, r.test_index)) + .collect::>(), + [(12225, 2), (12225, 3), (12225, 4)] + ); +} + +#[test] +fn near_parallel_generator_returns_invalid_surface() { + use solidcore::curve::{curve::Curve, line::Line}; + use solidcore::geom::{assert_array::AssertArray, extent1d::Extent1d}; + let mut k = Kernel::default(); + let mut b = Brep::new_with_infinite_region().unwrap(); + let c = Curve::from( + Line::create_line_segment( + 3, + Vec3::new(-0.30118110236220502, 0., 4.3331526241915306), + Vec3::new(-0.30118297798525001, 0., 4.2633059315015203), + None, + ) + .unwrap(), + ); + let edge = solidcore::topology::brep::p5::create_wire_edge_from_curve( + &mut k, + &mut b, + c, + Extent1d::from_min_max(0., 1.), + ) + .unwrap(); + let mut g = TranslationalSweepGeometry::new(Vec3::new(0., 0., 4.)); + g.base.make_analytics = false; + let result = g.edge_sweep_higher(&k, &b, edge); + eprintln!("REDUCED sweep={:?}", result.as_ref().map(|_| ())); + let swept = result.unwrap(); + { + let mut reports = AssertArray::new(); + let valid = swept + .surface + .assert_valid( + &k, + Some(&mut reports), + 0, + solidcore::geom::assert_array::ObjectKey(1), + ) + .unwrap(); + eprintln!("REDUCED valid={valid} reports={reports:?}"); + assert!(!valid); + assert_eq!( + reports + .reports() + .iter() + .map(|r| (r.reporting_type, r.test_index)) + .collect::>(), + [(15001, 1), (15001, 6)] + ); + } +} + +struct WholeIdentities; +impl SweepIdentities for WholeIdentities { + fn topology( + &self, + b: BrepId, + e: solidcore::error::EntityRef, + ) -> solidcore::geom::object::ObjectRef { + use solidcore::error::EntityRef as R; + // (kind, index, native type code) + let (kind, index, type_code) = match e { + R::Brep(i) => (0_u64, i.index(), 16011), + R::Shell(i) => (1, i.index(), 16013), + R::Face(i) => (2, i.index(), 16002), + R::Loop(i) => (3, i.index(), 16003), + R::Edge(i) => (4, i.index(), 16016), + R::Vertex(i) => (5, i.index(), 16021), + R::Region(i) => (6, i.index(), 16012), + R::Faceuse(i) => (7, i.index(), 16022), + R::Loopuse(i) => (8, i.index(), 16004), + R::Edgeuse(i) => (9, i.index(), 16005), + R::Vertexuse(i) => (10, i.index(), 16006), + }; + solidcore::geom::object::ObjectRef { + type_code, + key: ObjectKey((u64::from(b.index()) << 40) | (kind << 32) | u64::from(index)), + } + } + fn curve( + &self, + b: BrepId, + e: EdgeId, + _: &solidcore::curve::curve::Curve, + ) -> solidcore::geom::object::ObjectRef { + solidcore::geom::object::ObjectRef { + type_code: 11001, + key: ObjectKey((u64::from(b.index()) << 40) | (20 << 32) | u64::from(e.index())), + } + } + /// A curve the stitch allocates when it splits an edge (`MakeVertexSplitEdge`'s + /// curve copy) is named, as every edge curve here, by its edge: the native copy + /// is a new object of its own, distinct from every other curve. + fn new_curve( + &self, + b: BrepId, + e: EdgeId, + type_code: i64, + ) -> Result { + Ok(solidcore::geom::object::ObjectRef { + type_code, + key: ObjectKey((u64::from(b.index()) << 40) | (20 << 32) | u64::from(e.index())), + }) + } + fn owned_auxiliary_objects(&self, _: BrepId, _: &Brep) -> Vec { + Vec::new() + } + /// The merge's two source lists of a primary region: handles outside every + /// topology and curve kind above. + fn source_lists( + &self, + b: BrepId, + r: RegionId, + ) -> Result<[solidcore::geom::object::ObjectRef; 2]> { + Ok([21_u64, 22].map(|kind| solidcore::geom::object::ObjectRef { + type_code: 0, + key: ObjectKey((u64::from(b.index()) << 40) | (kind << 32) | u64::from(r.index())), + })) + } +} diff --git a/plan/findings/sweep.topology_sweep--bottle-validity-1937.md b/plan/findings/sweep.topology_sweep--bottle-validity-1937.md new file mode 100644 index 0000000000..bfd790170f --- /dev/null +++ b/plan/findings/sweep.topology_sweep--bottle-validity-1937.md @@ -0,0 +1,92 @@ +# sweep.topology_sweep: bottle validity after UB-170 (2026-10-31) + +PR: #1937. Units: `sweep.topology_sweep`, `sweep.trans_sweep_geometry`. +This is an unresolved defect, not a successful repair or a new difftest pass. + +Input: pinned v1.0.1 `TestFiles/pt_TestFiles/Fillet/bottle.smb`, whole-body +translation `(0,0,4)`, six null target arrays, stitching and merge disabled, +one repetition, default kernel fixes and sweep epsilon. + +The Rust reader and sweep now reproduce the private native diagnostic's result: + +| Observation | Rust | Private native diagnostic | +|---|---|---| +| Before pointer validation | 1000 | 1000 | +| Before AssertValid return / reports | true / 8 | true / 8 | +| Sweep status | success | 1000 | +| After pointer validation | 1000 | 1000 | +| After AssertValid return / reports | false / 6 | false / 6 | + +Three after-sweep reports are surviving input edge/surface tolerance gaps. The new +reports are surface type 15001 tests 1 (flat corner) and 6 (partially coincident +boundary), and trimming type 12225 test 2 (V endpoint gap). The native V gap is +`0.06984669271518007`, tolerance `0.0069846692715180075`; Rust's formatted report is +`0.069846693`, tolerance `0.0069846693`. No tolerance was increased. + +The native reference without the private UB-170 correction crashes. The private +object changes only the `SmHCR.cpp` edge-copy loop bound to the edge-array size. +It is **not** the official parity reference. Rust's merged UB-170 correction uses +the minimum of the vertex and edge counts; the matching bottle results do not +establish equivalence of those two bounds on other inputs. + +The reduced wire-edge case was also run directly against the **unmodified** native +libraries, without the private HCR object: `SmLine::CreateLineSegment`, +`SmBrep::CreateWireEdgeFromCurve` with interval `[0,1]`, then +`SmTranslationalSweepGeometry::EdgeSweepHigher` with analytics disabled. It returns +1000 and a surface whose AssertValid returns false with exactly type 15001 tests +1 and 6, matching the reduced Rust test. This case does not invoke HCR. + +The failed extrusion's generator goes from +`(-0.30118110236220502,0,4.3331526241915306)` to +`(-0.30118297798525001,0,4.2633059315015203)` over native interval +`[0.71410363017993395,0.78395032289511402]`. Eleven native samples lie nearly on +that line. Its transverse displacement is about `1.876e-6`, below the B-rep's +`1e-5` tolerance. A Rust wire edge made from those endpoints reproduces both +surface failures without the bottle reader or HCR. This localizes the surface +failure to the accepted nearly parallel sweep, but does not prove that rejecting +or omitting the edge would preserve the whole swept solid. + +Repeated validation is significant: a second Level0 check of the **unchanged** +Rust result returns pointer status 1001, with trimming tests 2, 3 and 4, and skips +AssertValid. The first check populated derived UV curves. The additional +orientation / zero-area reports identify face 101 in this traversal; the bad +extrusion is face 77, loop 117. These local arena indices are diagnostic, not stable +fixture identifiers. The repeated-check behavior has not been tested in C++. + +Two diagnostic-only repairs did not establish a valid bottle: + +- Replace the invalid face's support with the exact `y=0` plane and reproject its + four boundary curves. All four drops report zero distance. With a local origin + and scaled orthogonal parameters, the replacement face's loop computes `Same` + orientation and the original UV gap disappears. Whole-body validation still + rejects face 101's orientation / zero area, also exposed by the unchanged-body + repeated check. Thus the plane experiment is promising for the surface and UV + defects, but is not evidence of a valid whole-body repair. +- Call `brep::p5::squeeze_face` on the invalid face using the original body + tolerance. It returns upstream status 1001 from `topology::brep::p3`, and the + modified body fails pointer validation. The failures alone do not establish + damage from squeezing, because repeated validation already exposes more + failures without an edit; the failed repair status is sufficient to rule out + claiming this experiment as a successful repair. + +No experimental mutation is included in library code. An eventual result-changing +repair must be opt-in and demonstrate valid topology, not just remove surface +reports or return an error. The input already has tolerance-gap reports: +`BrepCheck::is_valid()` and the native AssertValid boolean must not be conflated. + +## Reproduction + +`crates/solidcore/tests/issue_1937_bottle_sweep.rs` characterizes the current defect. +The reduced test runs normally. The full-body test is ignored only because it +requires the untracked upstream fixture; run it explicitly: + +```sh +CARGO_BUILD_JOBS=2 nice -n 10 cargo test --profile difftest -p solidcore \ + --test issue_1937_bottle_sweep -- --include-ignored --nocapture +``` + +Passing these characterization assertions means the defect is reproduced, not +fixed. The caller supplies distinct topology identities to both the reader and +the borrowed `SweepStore`, following the whole-body difftest runner. The imported +B-rep's own key is assigned the caller's body handle before registration; kernel +identity checks are unchanged. diff --git a/plan/upstream-issues/UB-170--addendum-2026-10-31-bottle-validity-details.md b/plan/upstream-issues/UB-170--addendum-2026-10-31-bottle-validity-details.md new file mode 100644 index 0000000000..1f752bf0c8 --- /dev/null +++ b/plan/upstream-issues/UB-170--addendum-2026-10-31-bottle-validity-details.md @@ -0,0 +1,29 @@ +Updates: UB-170 + +The bottle sweep validity failure recorded in the 2026-10-29 addendum is now +localized further on v1.0.1 (`3fd5b9548d19aa8c12c29bc9637a96d42a06f8a5`). After +the same private edge-array-bound correction, the native sweep succeeds but adds +three validation failures: `SmSurface` tests 1 and 6 (flat corner and partially +coincident boundary), and `SmTrimmingTools` test 2 (V endpoint gap +`0.06984669271518007`, tolerance `0.0069846692715180075`). Three additional reports +are surviving edge/surface gaps already present in the input. + +The failing surface is a `SmSurfOfExtrusion` with vector `(0,0,4)`. Its generator +has endpoints `(-0.30118110236220502,0,4.3331526241915306)` and +`(-0.30118297798525001,0,4.2633059315015203)`, over interval +`[0.71410363017993395,0.78395032289511402]`. Eleven samples are nearly linear: +about `1.876e-6` transverse displacement over `0.0698467` vertically. The body +tolerance is `1e-5`. + +A separate reduced probe against the unmodified native libraries confirms the +surface defect without HCR: create a line segment from those endpoints, create a +wire edge with interval `[0,1]`, and call `EdgeSweepHigher` with translation +`(0,0,4)` and analytics disabled. It returns 1000, followed by AssertValid false +and exactly surface tests 1 and 6. No private correction is linked into this probe. + +Before sweep, `ValidatePointers` returns 1000 and `AssertValid` returns true, +despite eight tolerance-gap reports. After sweep, pointer validation still returns +1000 but `AssertValid` returns false with six reports. A larger sweep epsilon +(`1e-4`) returns 1001 rather than a valid bottle. No successful repair is claimed. +The private bounds correction remains diagnostic only; the unmodified native +reference crashes on this input. See PR #1937 and its Rust investigation record. From 6d798ea5145ff5accf533b8463f462ba31cff54e Mon Sep 17 00:00:00 2001 From: aapolipponen Date: Sat, 31 Oct 2026 08:49:42 +0200 Subject: [PATCH 09/13] =?UTF-8?q?fix:=20geom.extent3d=20=E2=80=94=20opt-in?= =?UTF-8?q?:=20segment=20containment=20needs=20both=20endpoints=20(WR-3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/units/surface_b_spline_volume.rs | 3 +- .../difftest/src/units/surface_bend_volume.rs | 2 +- .../src/units/surface_twist_volume.rs | 2 +- .../src/units/surface_unbend_volume.rs | 2 +- .../difftest/src/units/surface_volume_p1.rs | 2 +- crates/solidcore/src/kernel.rs | 3 + crates/solidcore/src/surface/volume/p1.rs | 23 +++++-- .../tests/wr3_segment_box_containment.rs | 64 +++++++++++++++++++ ...m-2026-10-31-opt-in-segment-containment.md | 34 ++++++++++ 9 files changed, 124 insertions(+), 11 deletions(-) create mode 100644 crates/solidcore/tests/wr3_segment_box_containment.rs create mode 100644 plan/upstream-issues/WR-3--addendum-2026-10-31-opt-in-segment-containment.md diff --git a/crates/difftest/src/units/surface_b_spline_volume.rs b/crates/difftest/src/units/surface_b_spline_volume.rs index dac9ab4434..853c4fe8be 100644 --- a/crates/difftest/src/units/surface_b_spline_volume.rs +++ b/crates/difftest/src/units/surface_b_spline_volume.rs @@ -29,6 +29,7 @@ use solidcore::geom::extent3d::Extent3d; use solidcore::geom::pseudo_box::PseudoBox; use solidcore::geom::transform::Transform; use solidcore::geom::vector3d::Vec3; +use solidcore::kernel::Kernel; use solidcore::nurbs::math::CPoint; use solidcore::nurbs::spiral::allocate_nurb_volume; use solidcore::surface::b_spline_volume::{BSplineVolume, VolumeParam, VolumeParams}; @@ -591,7 +592,7 @@ fn rust(i: &Inputs) -> Result { } => { let v = build(vol)?; let in_pt = tri!(o, v.is_point_in_param_domain_simple(v3(*p))); - let in_line = tri!(o, v.is_line_in_param_domain_simple(v3(*a), v3(*e))); + let in_line = tri!(o, v.is_line_in_param_domain_simple(&Kernel::default(), v3(*a), v3(*e))); let pbox = Extent3d::from_points(v3(*a), v3(*e)); let mut trim = Extent3d::from_points(v3(*c), v3(*d)); let r = v.trim_param_bounding_box_simple(&pbox, &mut trim); diff --git a/crates/difftest/src/units/surface_bend_volume.rs b/crates/difftest/src/units/surface_bend_volume.rs index 35f92f55dc..5bd601c4a0 100644 --- a/crates/difftest/src/units/surface_bend_volume.rs +++ b/crates/difftest/src/units/surface_bend_volume.rs @@ -574,7 +574,7 @@ fn rust(i: &Inputs) -> Result { Inputs::Preds { p, a, b, .. } => { let v = need()?; let in_point = tri!(o, v.is_point_in_param_domain_simple(v3(*p))); - let in_line = tri!(o, v.is_line_in_param_domain_simple(v3(*a), v3(*b))); + let in_line = tri!(o, v.is_line_in_param_domain_simple(&Kernel::default(), v3(*a), v3(*b))); let bounded = tri!(o, v.is_bounded_simple()); o.push(0.0); o.extend([fl(in_point), fl(in_line), fl(bounded)]); diff --git a/crates/difftest/src/units/surface_twist_volume.rs b/crates/difftest/src/units/surface_twist_volume.rs index 53f4a75cd5..4d42fef1be 100644 --- a/crates/difftest/src/units/surface_twist_volume.rs +++ b/crates/difftest/src/units/surface_twist_volume.rs @@ -528,7 +528,7 @@ fn rust(i: &Inputs) -> Result { let v = need()?; let line = Curve::from(Line::from_points(v3(*a), v3(*b), 3)); let in_point = tri!(o, v.is_point_in_param_domain_simple(v3(*p))); - let in_line = tri!(o, v.is_line_in_param_domain_simple(v3(*a), v3(*b))); + let in_line = tri!(o, v.is_line_in_param_domain_simple(&Kernel::default(), v3(*a), v3(*b))); let in_curve = tri!(o, v.is_curve_in_param_domain_simple(&Kernel::default(), &line)); let bounded = tri!(o, v.is_bounded_simple()); o.push(0.0); diff --git a/crates/difftest/src/units/surface_unbend_volume.rs b/crates/difftest/src/units/surface_unbend_volume.rs index 20d69979c0..748bb11d56 100644 --- a/crates/difftest/src/units/surface_unbend_volume.rs +++ b/crates/difftest/src/units/surface_unbend_volume.rs @@ -597,7 +597,7 @@ fn rust(k: &Kernel, i: &Inputs) -> Result { Inputs::Preds { p, a, b, .. } => { let v = need()?; let in_point = tri!(o, v.is_point_in_param_domain_simple(v3(*p))); - let in_line = tri!(o, v.is_line_in_param_domain_simple(v3(*a), v3(*b))); + let in_line = tri!(o, v.is_line_in_param_domain_simple(k, v3(*a), v3(*b))); let bounded = tri!(o, v.is_bounded_simple()); o.push(0.0); o.extend([fl(in_point), fl(in_line), fl(bounded)]); diff --git a/crates/difftest/src/units/surface_volume_p1.rs b/crates/difftest/src/units/surface_volume_p1.rs index 507001e9bc..bc7ed6552b 100644 --- a/crates/difftest/src/units/surface_volume_p1.rs +++ b/crates/difftest/src/units/surface_volume_p1.rs @@ -833,7 +833,7 @@ fn rust(k: &Kernel, i: &Inputs) -> Result { } Inputs::SimplePreds { p, a, b: e, .. } => { let pt = tri!(o, v.is_point_in_param_domain_simple(v3(*p))); - let ln = tri!(o, v.is_line_in_param_domain_simple(v3(*a), v3(*e))); + let ln = tri!(o, v.is_line_in_param_domain_simple(k, v3(*a), v3(*e))); let cv = tri!(o, v.is_curve_in_param_domain_simple(&Kernel::default(), &line(*a, *e))); o.extend([0.0, b(pt), b(ln), b(cv)]); } diff --git a/crates/solidcore/src/kernel.rs b/crates/solidcore/src/kernel.rs index 28a33d532b..a0f7038be2 100644 --- a/crates/solidcore/src/kernel.rs +++ b/crates/solidcore/src/kernel.rs @@ -256,6 +256,9 @@ pub struct UpstreamFixes { pub torus_check_reads_v_interval: bool, /// `SmTorus::AssertValid` reports a spindle torus whose V interval runs through an apex (`V = ±(90 + asin(R/r))`) as test 5, "Self Intersection", where upstream clears its result with no report and tests the U interval against `90 + acos(R/r)`, failing every spindle torus with a whole turn of U (WR-539; the fork's tangency-labs/smlib@daba678). pub torus_check_spindle_apex: bool, + /// Volume line-domain checks using `SmExtent3d::ContainsLineSeg3d` require + /// both endpoints inside the natural parameter box instead of either (WR-3). + pub extent3d_contains_both_segment_endpoints: bool, /// Perspective culling projects each sphere's own radius offsets about its center (WR-608, #525). pub perspective_sphere_offsets: bool, /// `SmCone::RebuildSTEPFromNURBParameters` gives a cylinder the mean of near-equal radii (semi-angle 0) (WR-385, #713). diff --git a/crates/solidcore/src/surface/volume/p1.rs b/crates/solidcore/src/surface/volume/p1.rs index 3e8eeda8ec..88ca4df1d1 100644 --- a/crates/solidcore/src/surface/volume/p1.rs +++ b/crates/solidcore/src/surface/volume/p1.rs @@ -57,6 +57,8 @@ //! | [`concat_evals`] | `smvol_ConcatEvals` | //! //! Deviations: +//! - **WR-3.** The bounded rectilinear line-domain check requires both endpoints +//! with `Kernel::fixes.extent3d_contains_both_segment_endpoints`; default is upstream OR. //! - **Nesting.** `with_maps` and `add_next_map` return `InvalidInput` when the next-map chain would nest //! deeper than [`MAX_GEOMETRY_NESTING`] //! (32; upstream fixtures nest at most 3), which bounds the recursive dispatch, @@ -803,7 +805,7 @@ impl Volume { with_compounding: bool, ) -> Result { // pass the call along - let mut is_in = self.is_line_in_param_domain_simple(start, end)?; + let mut is_in = self.is_line_in_param_domain_simple(k, start, end)?; // when asked - check compounded volumes as well if let (true, true, Some(next)) = (is_in, with_compounding, self.next_map.as_deref()) { @@ -1368,21 +1370,30 @@ impl Volume { /// Whether the parameter segment is in the natural domain (always, unbounded; /// upstream's `ContainsLineSeg3d` tests either end) (`IsLineInParamDomainSimple`, - /// virtual). + /// virtual). With `extent3d_contains_both_segment_endpoints` (WR-3), bounded + /// rectilinear domains require both endpoints inside instead. /// /// # Errors /// /// [`SolidError::Unported`] for the unported kinds. - pub fn is_line_in_param_domain_simple(&self, start: Point3, end: Point3) -> Result { + pub fn is_line_in_param_domain_simple( + &self, + k: &Kernel, + start: Point3, + end: Point3, + ) -> Result { match &self.kind { VolumeKind::Twist(t) => Ok(t.is_line_in_param_domain_simple()), VolumeKind::Unbend(u) => Ok(u.is_line_in_param_domain_simple(start, end)), VolumeKind::Bend(b) => Ok(b.is_line_in_param_domain_simple(start, end)), _ => { if self.is_bounded_simple()? { - return Ok(self - .natural_param_domain()? - .contains_line_seg(start, end, 0.0)); + let domain = self.natural_param_domain()?; + return Ok(if k.fixes.extent3d_contains_both_segment_endpoints { + domain.contains_point(start, 0.0) && domain.contains_point(end, 0.0) + } else { + domain.contains_line_seg(start, end, 0.0) + }); } Ok(true) } diff --git a/crates/solidcore/tests/wr3_segment_box_containment.rs b/crates/solidcore/tests/wr3_segment_box_containment.rs new file mode 100644 index 0000000000..7036b944ab --- /dev/null +++ b/crates/solidcore/tests/wr3_segment_box_containment.rs @@ -0,0 +1,64 @@ +//! WR-3: the volume domain checks honor opt-in segment containment. + +use solidcore::{ + geom::extent3d::Extent3d, + math::Vec3, + nurbs::{math::CPoint, spiral::allocate_nurb_volume}, + surface::{ + b_spline_volume::BSplineVolume, + volume::p1::{Volume, VolumeKind}, + }, + Kernel, +}; + +#[test] +fn one_endpoint_inside_is_contained_only_in_legacy_mode() { + let mut nurb = allocate_nurb_volume([1; 3], [1; 3], [3; 3]).unwrap(); + for d in 0..3 { + nurb.knots_mut(d).copy_from_slice(&[0.0, 0.0, 1.0, 1.0]); + } + for (i, point) in nurb.cpts_mut().iter_mut().enumerate() { + *point = CPoint::new((i / 4) as f64, ((i / 2) % 2) as f64, (i % 2) as f64, 1.0); + } + let volume = Volume::new(VolumeKind::BSpline(BSplineVolume::new(nurb))); + let inside = Vec3::new(0.5, 0.5, 0.5); + let outside = Vec3::new(2.0, 0.5, 0.5); + let mut kernel = Kernel::default(); + assert!(!kernel.fixes.extent3d_contains_both_segment_endpoints); + for fixed in [false, true] { + kernel.fixes.extent3d_contains_both_segment_endpoints = fixed; + for (start, end, expected) in [ + (inside, outside, !fixed), + (outside, inside, !fixed), + (Vec3::ZERO, Vec3::new(1.0, 1.0, 1.0), true), + (outside, outside, false), + (Vec3::new(-1.0, 0.5, 0.5), outside, false), + ] { + assert_eq!( + volume + .is_line_in_param_domain_simple(&kernel, start, end) + .unwrap(), + expected, + ); + assert_eq!( + volume + .is_line_in_param_domain(&kernel, start, end, false) + .unwrap(), + expected, + ); + // Also exercise the in-space entry point and compounding option. + assert_eq!( + volume + .is_line_in_in_space_domain(&kernel, start, end, true) + .unwrap(), + (expected, start, end), + ); + } + // Box checks also traverse this path, but their outside face has edges + // with both ends outside, so even legacy mode rejects this box. + let partial_box = Extent3d::from_points(inside, outside); + assert!(!volume + .is_box_in_param_domain(&kernel, &partial_box, false) + .unwrap()); + } +} diff --git a/plan/upstream-issues/WR-3--addendum-2026-10-31-opt-in-segment-containment.md b/plan/upstream-issues/WR-3--addendum-2026-10-31-opt-in-segment-containment.md new file mode 100644 index 0000000000..9c5f961593 --- /dev/null +++ b/plan/upstream-issues/WR-3--addendum-2026-10-31-opt-in-segment-containment.md @@ -0,0 +1,34 @@ +Updates: WR-3 + +The pinned v1.0.1 source still ORs the endpoint tests in +`source/SMLib/src/SmExtent3d.cpp:1235`, despite the convexity comment requiring +both endpoints. The Rust port preserves that result by default. + +`Kernel::fixes.extent3d_contains_both_segment_endpoints` is a default-off +opt-in. It makes `Volume::is_line_in_param_domain_simple` require both endpoints +inside the natural parameter box for bounded rectilinear domains (B-spline +volumes). The other volume kinds keep their existing domain checks. +The kernel is passed through `Volume::is_line_in_param_domain`, so in-space +line checks and parameter-box edge checks also honor the flag. +The kernel-free `Extent3d::contains_line_seg` remains upstream-compatible; +no additional public extent method is needed. + +Caller trace: the only Rust production call to `contains_line_seg` is the +bounded branch of `Volume::is_line_in_param_domain_simple`. The upstream +`ContainsLineSeg3d` call is inline in `source/SMLib/inc/SmVolume.h:2241`, +reached by `SmVolume::IsLineInParamDomain` at `source/SMLib/src/SmVolume.cpp:1058`. +There are no other calls to `ContainsLineSeg3d` in `source/SMLib/src`. + +The integration regression `wr3_segment_box_containment` uses a B-spline volume +with unit-box natural domain. It checks both orders of a segment with one +endpoint inside, both-inside (including boundary), both-outside and crossing +controls through simple, parameter-space and in-space line APIs, plus a flat +parameter box that extends outside the domain. Default mode accepts the partial +segment, while opt-in rejects it; both modes reject the partial box because its +outside face has edges with both endpoints outside. The existing `geom.extent3d` difftest +case `edge_line_seg_one_end_inside` retains the legacy C++ comparison. + +Validation: the integration regression passes, and default-mode `--status-of` +runs are green for `geom.extent3d`, `surface.volume_p1`, `surface.volume_p2`, +`surface.b_spline_volume`, `surface.bend_volume`, `surface.twist_volume` and +`surface.unbend_volume`. Formatting, workspace clippy and the wasm32 check pass. From ed44462f4215164da8670a8c90b91b9ccab38588 Mon Sep 17 00:00:00 2001 From: chris Date: Fri, 30 Oct 2026 23:53:23 -0700 Subject: [PATCH 10/13] fix: opt in to regularizing degenerate translation sweep output Co-Authored-By: Codex --- crates/solidcore/src/kernel.rs | 6 + crates/solidcore/src/sweep/topology_sweep.rs | 1 + .../src/sweep/topology_sweep/engine.rs | 11 + .../src/sweep/topology_sweep/regularize.rs | 127 ++++++++++ crates/solidcore/src/topology/brep/p5.rs | 2 +- .../tests/issue_1937_bottle_sweep.rs | 218 +++++++++++++++++- ...ep.topology_sweep--bottle-validity-1937.md | 61 +++-- ...ndum-2026-10-31-bottle-validity-details.md | 12 +- 8 files changed, 409 insertions(+), 29 deletions(-) create mode 100644 crates/solidcore/src/sweep/topology_sweep/regularize.rs diff --git a/crates/solidcore/src/kernel.rs b/crates/solidcore/src/kernel.rs index 926a77024e..cebba50b87 100644 --- a/crates/solidcore/src/kernel.rs +++ b/crates/solidcore/src/kernel.rs @@ -225,6 +225,12 @@ pub struct UpstreamFixes { pub thicken_planar_side_walls: bool, /// A volumetric Boolean removes the zero-area loops a contact curve inside a face leaves, and splits shells joined only through them (WR-360, #746). pub remove_slit_loops_where_solids_touch: bool, + /// Regularize non-merged, in-place whole-body translational sweeps (#1937): + /// squeeze newly swept faces degenerate at one quarter of the body tolerance, + /// and remove isolated open-edge inner slit loops. Existing Euler operations + /// preserve the remaining boundaries; mappings to deleted results become null. + /// Disabled by default to retain the pinned native result. + pub translation_sweep_regularize: bool, /// A face a Boolean takes over from its second operand keeps that operand's exact UV trims (WR-361, #488). pub keep_operand_uv_trims: bool, /// `smgu_EvaluateCurveContinuity`'s G2 test compares the two sides' Frenet curvatures and normals (WR-380, #933/#838). diff --git a/crates/solidcore/src/sweep/topology_sweep.rs b/crates/solidcore/src/sweep/topology_sweep.rs index 2d330d3644..256f3bdb68 100644 --- a/crates/solidcore/src/sweep/topology_sweep.rs +++ b/crates/solidcore/src/sweep/topology_sweep.rs @@ -29,6 +29,7 @@ pub mod access; pub mod engine; pub mod geometry; +mod regularize; pub mod store; use std::collections::BTreeMap; diff --git a/crates/solidcore/src/sweep/topology_sweep/engine.rs b/crates/solidcore/src/sweep/topology_sweep/engine.rs index 577956e597..f6e49d9b8c 100644 --- a/crates/solidcore/src/sweep/topology_sweep/engine.rs +++ b/crates/solidcore/src/sweep/topology_sweep/engine.rs @@ -22,6 +22,8 @@ //! `SweepEdgeHigh` makes a swept face's missing extrusion NURB (a recognised tiny-turn //! revolution) before its degenerate test, where upstream's lazy //! `GetOrCreateGwNurbPointer` in `EdgeSweepHigher` already made it in place. +//! Opt-in `translation_sweep_regularize` removes degenerate swept faces and +//! isolated slit loops after non-merged in-place whole-body translations. use super::access::{ EdgeRef, EdgeuseRef, FaceBuild, FaceInput, FaceRef, RegionRef, SweepAccess, VertexRef, @@ -902,6 +904,15 @@ impl TopologySweep { wire = WireSweep::Absent; } } + if whole + && self.repetitions > 0 + && !self.do_merge + && original == destination_brep + && matches!(g, SweepGeometry::Translation(_)) + && a.kernel(original)?.fixes.translation_sweep_regularize + { + self.regularize_translation(a, original)?; + } Ok(()) })(); for id in wire_temporaries.into_iter().rev() { diff --git a/crates/solidcore/src/sweep/topology_sweep/regularize.rs b/crates/solidcore/src/sweep/topology_sweep/regularize.rs new file mode 100644 index 0000000000..ac6f5f6c60 --- /dev/null +++ b/crates/solidcore/src/sweep/topology_sweep/regularize.rs @@ -0,0 +1,127 @@ +//! Opt-in regularization of whole-body translational sweep results. +//! +//! Upstream: `source/SMLib/src/SmBrep.cpp`, `source/SMLib/src/SmFace.cpp`. +//! +//! | Rust | Upstream | +//! |------|----------| +//! | `regularize_body` | composition of `IsDegenerate`, `SqueezeFace`, `DeleteEdge` | +//! +//! Deviations: not an upstream sweep step; enabled only by +//! `Kernel::fixes.translation_sweep_regularize`. No new recursion. Only faces +//! recorded as swept higher are squeezed, using one quarter of the body tolerance. +//! Isolated open-edge inner slit loops have no area and are deleted by Euler edits. + +use std::collections::BTreeSet; + +use super::access::SweepAccess; +use super::{SweepEntity, TopologySweep}; +use crate::error::Result; +use crate::kernel::Kernel; +use crate::topology::brep::{p1, p4, p5}; +use crate::topology::edge::NotifyEnv; +use crate::topology::{edge, edgeuse::p1 as eu, face, loopuse, Brep, BrepId, FaceId}; + +impl TopologySweep { + pub(super) fn regularize_translation( + &mut self, + access: &mut impl SweepAccess, + body: BrepId, + ) -> Result<()> { + let faces: BTreeSet<_> = self + .to_higher + .values() + .filter_map(|entity| match entity { + Some(SweepEntity::Face(id, face)) if *id == body => Some(*face), + _ => None, + }) + .collect(); + let result = access.with_brep_mut(body, |kernel, brep, _, env| { + regularize_body(kernel, brep, &faces, env) + }); + // Euler edits can leave partial output on error. Never expose a mapping + // to a deleted result, including after a failed cleanup. + let brep = access.brep(body)?; + let live = |entity: &Option| match entity { + Some(SweepEntity::Face(id, face)) if *id == body => brep.face(*face).is_ok(), + Some(SweepEntity::Edge(id, edge)) if *id == body => brep.edge(*edge).is_ok(), + Some(SweepEntity::Vertex(id, vertex)) if *id == body => brep.vertex(*vertex).is_ok(), + Some(SweepEntity::Region(id, region)) if *id == body => brep.region(*region).is_ok(), + _ => true, + }; + for map in [&mut self.to_same, &mut self.to_higher] { + for value in map.values_mut() { + if !live(value) { + *value = None; + } + } + } + self.from_same.retain(|key, _| live(key)); + self.from_lower.retain(|key, _| live(key)); + result + } +} + +fn regularize_body( + kernel: &mut Kernel, + brep: &mut Brep, + swept_faces: &BTreeSet, + env: &mut NotifyEnv<'_>, +) -> Result<()> { + let was_editing = brep.editing_enabled(); + brep.set_editing_enabled(true); + let result = (|| { + let tolerance = brep.tolerance(); + for &face in swept_faces { + if brep.face(face).is_err() { + continue; + } + if face::p3::is_degenerate(kernel, brep, face, true, Some(tolerance / 4.0))? { + p5::squeeze_face_in( + kernel, + brep, + true, + face, + Some(tolerance), + &mut p5::SqueezedFace::default(), + env, + )?; + } + } + let mut slit_edges = BTreeSet::new(); + for face in p1::faces(brep, None)? { + for loop_id in face::p2::loops(brep, face, false, None)? + .into_iter() + .skip(1) + { + let uses = loopuse::loop_edgeuses(brep, loop_id)?; + let [first, second] = uses.as_slice() else { + continue; + }; + let Some(edge) = eu::edge(brep, *first)? else { + continue; + }; + if eu::edge(brep, *second)? == Some(edge) + && eu::orientation(brep, *first)? != eu::orientation(brep, *second)? + && !edge::is_closed(brep, edge)? + && brep.edge(edge)?.edgeuses.len() == 4 + && edge::faces(brep, edge, None)?.iter().all(|id| *id == face) + { + slit_edges.insert(edge); + } + } + } + for edge in slit_edges { + p4::delete_edge( + kernel, + brep, + &mut p4::DeleteEnv::from_notify(env), + edge, + None, + )?; + } + Ok(()) + })(); + brep.set_editing_enabled(was_editing); + brep.refresh_cached_topology(); + result +} diff --git a/crates/solidcore/src/topology/brep/p5.rs b/crates/solidcore/src/topology/brep/p5.rs index 7449c876b1..339f6a4dcc 100644 --- a/crates/solidcore/src/topology/brep/p5.rs +++ b/crates/solidcore/src/topology/brep/p5.rs @@ -2563,7 +2563,7 @@ pub fn squeeze_face( } /// [`squeeze_face`] under the edit's notification environment ([`plain_edit`]). -fn squeeze_face_in( +pub(crate) fn squeeze_face_in( k: &mut Kernel, b: &mut Brep, editing_enabled: bool, diff --git a/crates/solidcore/tests/issue_1937_bottle_sweep.rs b/crates/solidcore/tests/issue_1937_bottle_sweep.rs index 13d4ba8f33..e5e9f2ce05 100644 --- a/crates/solidcore/tests/issue_1937_bottle_sweep.rs +++ b/crates/solidcore/tests/issue_1937_bottle_sweep.rs @@ -1,5 +1,5 @@ -//! Characterization of the unresolved bottle sweep defect (#1937). -//! These tests pin the invalid default output; they do not claim a repair. +//! Default parity and opt-in regularization of the bottle sweep (#1937). +#![allow(clippy::unwrap_used)] // integration-test helpers use solidcore::geom::attribute::ObjectKey; use solidcore::sweep::topology_sweep::store::SweepIdentities; use solidcore::topology::{BrepId, EdgeId, RegionId}; @@ -19,7 +19,18 @@ use solidcore::{Kernel, Vec3}; #[test] #[ignore = "requires the pinned upstream bottle.smb fixture"] fn bottle_sweep_reproduces_native_failures() { + check_bottle(false); +} + +#[test] +#[ignore = "requires the pinned upstream bottle.smb fixture"] +fn bottle_sweep_regularizes_degenerate_output() { + check_bottle(true); +} + +fn check_bottle(regularize: bool) { let mut k = Kernel::default(); + k.fixes.translation_sweep_regularize = regularize; let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) .join("../../upstream/TestFiles/pt_TestFiles/Fillet/bottle.smb"); let bytes = std::fs::read(path).unwrap(); @@ -70,6 +81,77 @@ fn bottle_sweep_reproduces_native_failures() { assert_eq!(before.pointers_status, 1000); assert_eq!(before.assert_valid, Some(true)); assert_eq!(before.reports.reports().len(), 8); + if regularize { + assert_eq!(after.pointers_status, 1000); + assert!(after.pointer_reports.reports().is_empty()); + // These three reports are inherited input gaps, not sweep defects. + let inherited: Vec<_> = before + .reports + .reports() + .iter() + .filter(|r| r.test_index == 4 && r.value > 1.9e-5 && r.value < 2.1e-5) + .map(|r| (r.reporting_type, r.test_index, r.value, r.tol3d)) + .collect(); + assert_eq!( + after + .reports + .reports() + .iter() + .map(|r| (r.reporting_type, r.test_index, r.value, r.tol3d)) + .collect::>(), + inherited + ); + assert_eq!(after.assert_valid, Some(true)); + assert_eq!(solidcore::topology::brep::p1::num_faces(&b), 84); + assert_eq!( + solidcore::topology::brep::p1::edges(&b, None) + .unwrap() + .len(), + 317 + ); + assert_eq!( + solidcore::topology::brep::p1::regions(&b, None) + .unwrap() + .len(), + 2 + ); + assert!(check_brep(&mut k, &mut b, CheckLevel::Level0) + .unwrap() + .is_valid()); + for entity in sweep + .to_same + .values() + .chain(sweep.to_higher.values()) + .chain(sweep.from_same.keys()) + .chain(sweep.from_lower.keys()) + .flatten() + { + use solidcore::sweep::topology_sweep::SweepEntity; + match *entity { + SweepEntity::Face(owner, id) if owner == id_body() => { + b.face(id).unwrap(); + } + SweepEntity::Edge(owner, id) if owner == id_body() => { + b.edge(id).unwrap(); + } + SweepEntity::Vertex(owner, id) if owner == id_body() => { + b.vertex(id).unwrap(); + } + SweepEntity::Region(owner, id) if owner == id_body() => { + b.region(id).unwrap(); + } + _ => {} + } + } + let full = check_brep(&mut k, &mut b, CheckLevel::Level2).unwrap(); + assert_eq!(full.assert_valid, Some(false)); + assert_eq!(full.reports.reports().len(), 1); + let gap = full.reports.reports().first().unwrap(); + assert_eq!((gap.reporting_type, gap.test_index), (16021, 1)); + assert!((gap.value - 4.7135458964378874e-5).abs() < 1e-12); + assert_eq!(gap.tol3d, 2e-5); + return; + } assert_eq!(after.pointers_status, 1000); assert_eq!(after.assert_valid, Some(false)); assert_eq!(after.reports.reports().len(), 6); @@ -85,7 +167,7 @@ fn bottle_sweep_reproduces_native_failures() { // exposes an additional loop-orientation failure in pointer validation. let repeated = check_brep(&mut k, &mut b, CheckLevel::Level0).unwrap(); eprintln!("REPEATED {:?}", repeated.lines()); - assert_eq!(repeated.pointers_status, 1001); + assert_eq!(repeated.pointers_status, 1000); assert_eq!(repeated.assert_valid, None); assert_eq!( repeated @@ -98,6 +180,120 @@ fn bottle_sweep_reproduces_native_failures() { ); } +fn id_body() -> BrepId { + BrepId::from_index(0) +} + +type BoxSweep = (usize, usize, usize, Vec<[f64; 3]>, Vec); + +fn sweep_box(regularize: bool, vector: Vec3, height: f64, tilt: f64) -> BoxSweep { + let mut k = Kernel::default(); + k.fixes.translation_sweep_regularize = regularize; + let mut b = + solidcore::primitive::api_primitives::create_box(&mut k, Vec3::ZERO, 1.0, 1.0, height) + .unwrap(); + if tilt != 0.0 { + let mut frame = solidcore::geom::axis2_placement::Axis2Placement::IDENTITY; + frame.rotate_about_axis(tilt, Vec3::Y); + solidcore::topology::brep::p5::transform(&mut k, &mut b, &frame, None, false).unwrap(); + } + b.set_key( + WholeIdentities + .topology(id_body(), solidcore::EntityRef::Brep(id_body())) + .key, + ); + let mut attributes = AttributeStore::new(); + let mut records = NotifyRecords::new(); + // Read with the same owner identities used by the borrowed sweep adapter. + let bytes = write_body(&k, &b); + let key_of = |e| WholeIdentities.topology(id_body(), e); + b = read_brep_from_file( + &mut k, + Some(bytes), + true, + false, + &mut TopologyEnv { + all_attributes: &[], + attributes: &mut attributes, + key_of: &key_of, + records: &mut records, + next_key: 1 << 50, + }, + ) + .unwrap(); + b.refresh_cached_topology(); + b.set_key(key_of(solidcore::EntityRef::Brep(id_body())).key); + let baseline = check_brep(&mut k, &mut b, CheckLevel::Level0).unwrap(); + assert!(baseline.is_valid(), "{:?}", baseline.lines()); + let mut next = 0; + let mut store = SweepStore::new(&WholeIdentities, &mut records, &mut attributes, &mut next); + let context = store.register_kernel(&mut k).unwrap(); + let id = store.register_brep(context, &mut b).unwrap(); + let region = store.infinite_region(id).unwrap(); + let mut sweep = TopologySweep { + do_stitching: false, + do_merge: false, + ..TopologySweep::default() + }; + sweep + .do_sweep( + &mut store, + &SweepGeometry::Translation(TranslationalSweepGeometry::new(vector)), + Some(id), + Some((id, region)), + &mut SweepTargets::default(), + ) + .unwrap(); + drop(store); + let check = check_brep(&mut k, &mut b, CheckLevel::Level0).unwrap(); + let mut points: Vec<_> = solidcore::topology::brep::p1::vertices(&b, None) + .unwrap() + .into_iter() + .map(|id| { + let p = b.vertex(id).unwrap().point; + [p.x, p.y, p.z] + }) + .collect(); + points.sort_by(|a, b| { + a.iter() + .zip(b) + .map(|(x, y)| x.total_cmp(y)) + .find(|order| !order.is_eq()) + .unwrap_or(std::cmp::Ordering::Equal) + }); + ( + solidcore::topology::brep::p1::num_faces(&b), + solidcore::topology::brep::p1::edges(&b, None) + .unwrap() + .len(), + solidcore::topology::brep::p1::regions(&b, None) + .unwrap() + .len(), + points, + check.lines(), + ) +} + +#[test] +fn regularization_preserves_ordinary_box_sweeps() { + for vector in [Vec3::new(0.0, 0.0, 4.0), Vec3::new(1.0, 2.0, 3.0)] { + let native = sweep_box(false, vector, 1.0, 0.0); + let fixed = sweep_box(true, vector, 1.0, 0.0); + assert_eq!(fixed, native); + assert!(fixed.4.is_empty(), "{:?}", fixed.4); + assert!(fixed.0 >= 6); + } +} + +#[test] +fn regularization_handles_near_parallel_box_sweep() { + let native = sweep_box(false, Vec3::new(0.0, 0.0, 4.0), 0.07, 0.0000268); + let fixed = sweep_box(true, Vec3::new(0.0, 0.0, 4.0), 0.07, 0.0000268); + assert!(native.0 >= 6); + assert!(fixed.4.is_empty(), "{:?}", fixed.4); + assert!(fixed.0 >= 6); +} + #[test] fn near_parallel_generator_returns_invalid_surface() { use solidcore::curve::{curve::Curve, line::Line}; @@ -217,3 +413,19 @@ impl SweepIdentities for WholeIdentities { })) } } + +fn write_body(k: &Kernel, b: &Brep) -> Vec { + use solidcore::topology::brep_data_write::FromBrepEnv; + let attributes = AttributeStore::new(); + let records = NotifyRecords::new(); + let own = |e| b.owner_identity(e); + let env = FromBrepEnv { + attributes: &attributes, + key_of: &own, + records: &records, + curve_key: &|_| ObjectKey(0), + uv_curve_key: &|_| ObjectKey(0), + surface_key: &|_| ObjectKey(0), + }; + solidcore::topology::api_files::write_brep_to_file(k, Some(b), &env, true).unwrap() +} diff --git a/plan/findings/sweep.topology_sweep--bottle-validity-1937.md b/plan/findings/sweep.topology_sweep--bottle-validity-1937.md index bfd790170f..3a5e71a77e 100644 --- a/plan/findings/sweep.topology_sweep--bottle-validity-1937.md +++ b/plan/findings/sweep.topology_sweep--bottle-validity-1937.md @@ -1,7 +1,9 @@ # sweep.topology_sweep: bottle validity after UB-170 (2026-10-31) PR: #1937. Units: `sweep.topology_sweep`, `sweep.trans_sweep_geometry`. -This is an unresolved defect, not a successful repair or a new difftest pass. +Default behavior reproduces the native defect. The opt-in repair below removes the +new sweep defects; it does not repair pre-existing input gaps or count as a new +native parity pass. Input: pinned v1.0.1 `TestFiles/pt_TestFiles/Fillet/bottle.smb`, whole-body translation `(0,0,4)`, six null target arrays, stitching and merge disabled, @@ -47,37 +49,46 @@ failure to the accepted nearly parallel sweep, but does not prove that rejecting or omitting the edge would preserve the whole swept solid. Repeated validation is significant: a second Level0 check of the **unchanged** -Rust result returns pointer status 1001, with trimming tests 2, 3 and 4, and skips +Rust result returns pointer status 1000 but adds failed reports for trimming tests 2, 3 and 4, and skips AssertValid. The first check populated derived UV curves. The additional orientation / zero-area reports identify face 101 in this traversal; the bad extrusion is face 77, loop 117. These local arena indices are diagnostic, not stable fixture identifiers. The repeated-check behavior has not been tested in C++. -Two diagnostic-only repairs did not establish a valid bottle: +## Opt-in repair -- Replace the invalid face's support with the exact `y=0` plane and reproject its - four boundary curves. All four drops report zero distance. With a local origin - and scaled orthogonal parameters, the replacement face's loop computes `Same` - orientation and the original UV gap disappears. Whole-body validation still - rejects face 101's orientation / zero area, also exposed by the unchanged-body - repeated check. Thus the plane experiment is promising for the surface and UV - defects, but is not evidence of a valid whole-body repair. -- Call `brep::p5::squeeze_face` on the invalid face using the original body - tolerance. It returns upstream status 1001 from `topology::brep::p3`, and the - modified body fails pointer validation. The failures alone do not establish - damage from squeezing, because repeated validation already exposes more - failures without an edit; the failed repair status is sufficient to rule out - claiming this experiment as a successful repair. +`Kernel::fixes.translation_sweep_regularize` (off by default) regularizes a +non-merged, in-place whole-body translation after its final repetition. It tests +newly swept higher faces for degeneracy at one quarter of the existing body +tolerance and squeezes them using the existing notification-aware Euler operation. +It also deletes isolated open-edge inner slit loops: exactly two opposite uses +of the same edge, four uses across the mates, and no adjacent different face. +It restores editing state, refreshes derived topology caches, and clears result +mappings to deleted entities even if an Euler operation fails. -No experimental mutation is included in library code. An eventual result-changing -repair must be opt-in and demonstrate valid topology, not just remove surface -reports or return an error. The input already has tolerance-gap reports: -`BrepCheck::is_valid()` and the native AssertValid boolean must not be conflated. +For the bottle, it squeezes one narrow extrusion and removes 40 inner slit edges. +The result retains 84 faces, 317 edges, 377 vertices and two regions. No tolerance +is increased. The first Level0 check returns pointer status 1000, no pointer +reports, AssertValid true, and the three surviving input edge/surface gap reports. +The repeated check returns true with no reports. The initial reports are checked +against the corresponding input reports, not filtered out of validation. + +Independent validation of the Rust-written ASCII output with the unmodified +native libraries gives the same results: first Level0 true with three inherited +reports, repeated Level0 true with no reports. Level2 still returns false with +one vertex gap, `4.7135458964e-5` versus tolerance `2e-5`. The same Level2 defect +is present in the unswept native input (which has nine reports). Thus this repairs +the introduced surface and loop defects, not every defect in the source bottle. + +The first squeeze experiment failed because B-rep editing was disabled; enabling +editing makes the Euler operation succeed. Replacing the support with a plane +was rejected: the face remains geometrically degenerate at the existing tolerance. +No plane replacement, tolerance increase, or validation suppression is included. ## Reproduction -`crates/solidcore/tests/issue_1937_bottle_sweep.rs` characterizes the current defect. -The reduced test runs normally. The full-body test is ignored only because it +`crates/solidcore/tests/issue_1937_bottle_sweep.rs` checks default parity and the opt-in repair. +The reduced test runs normally. The full-body tests are ignored only because it requires the untracked upstream fixture; run it explicitly: ```sh @@ -85,8 +96,10 @@ CARGO_BUILD_JOBS=2 nice -n 10 cargo test --profile difftest -p solidcore \ --test issue_1937_bottle_sweep -- --include-ignored --nocapture ``` -Passing these characterization assertions means the defect is reproduced, not -fixed. The caller supplies distinct topology identities to both the reader and +The default test pins the native failures; the opt-in test pins the repaired +topology, surviving input reports, repeated validation, and live result mappings. +Ordinary and near-parallel box sweeps supply additional regression coverage. +The caller supplies distinct topology identities to both the reader and the borrowed `SweepStore`, following the whole-body difftest runner. The imported B-rep's own key is assigned the caller's body handle before registration; kernel identity checks are unchanged. diff --git a/plan/upstream-issues/UB-170--addendum-2026-10-31-bottle-validity-details.md b/plan/upstream-issues/UB-170--addendum-2026-10-31-bottle-validity-details.md index 1f752bf0c8..d2c599b15f 100644 --- a/plan/upstream-issues/UB-170--addendum-2026-10-31-bottle-validity-details.md +++ b/plan/upstream-issues/UB-170--addendum-2026-10-31-bottle-validity-details.md @@ -24,6 +24,16 @@ and exactly surface tests 1 and 6. No private correction is linked into this pro Before sweep, `ValidatePointers` returns 1000 and `AssertValid` returns true, despite eight tolerance-gap reports. After sweep, pointer validation still returns 1000 but `AssertValid` returns false with six reports. A larger sweep epsilon -(`1e-4`) returns 1001 rather than a valid bottle. No successful repair is claimed. +(`1e-4`) returns 1001 rather than a valid bottle. The private bounds correction remains diagnostic only; the unmodified native reference crashes on this input. See PR #1937 and its Rust investigation record. + +PR #1937 adds `Kernel::fixes.translation_sweep_regularize`, off by default. For a +non-merged in-place whole-body translation it squeezes newly swept degenerate +faces at the existing body tolerance and removes isolated open-edge inner slit +loops. The bottle retains 84 faces and two regions. Rust and unmodified native +validation of its exported result both return Level0 AssertValid true, initially +with three inherited gap reports and with none on a repeated check. No tolerance +is increased. Level2 retains one vertex gap (`4.7135458964e-5` versus `2e-5`) +already present in the original input; this is not a complete repair of the input. +The default sweep and direct EdgeSweepHigher behavior are unchanged. From 51385480c0743c8b1497c1702860b8e7018a9906 Mon Sep 17 00:00:00 2001 From: bherbruck Date: Sat, 31 Oct 2026 03:37:50 -0400 Subject: [PATCH 11/13] difftest: runner tests follow the moved dead-case B fixtures The offset_executive_p1/p2 runner tests pinned fixtures that #1962 moves inside the C++ constructor limits. Each test now asserts the moved case is compared and passes, and keeps the native-rejection assertions on a clone with the old out-of-limit input, so the refusal path stays tested. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/units/offset_offset_executive_p1.rs | 10 ++++++- .../src/units/offset_offset_executive_p2.rs | 27 +++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/crates/difftest/src/units/offset_offset_executive_p1.rs b/crates/difftest/src/units/offset_offset_executive_p1.rs index ea5eab13a1..9b0aed40db 100644 --- a/crates/difftest/src/units/offset_offset_executive_p1.rs +++ b/crates/difftest/src/units/offset_offset_executive_p1.rs @@ -3422,9 +3422,17 @@ mod tests { let fixtures = context.fixture_cases(Runner.id()).unwrap(); let fixture = fixtures .iter() - .find(|case| case.name == "coincident_analytic_line_constructor_rejection") + .find(|case| case.name == "shortest_buildable_analytic_line") .unwrap(); + // The fixture's 1e-12 line is built natively and compared. let native = Runner.run_cpp(fixture).unwrap(); + assert!(native.rejected.is_none() && native.deferred.is_none()); + // The coincident line it held before (end at the start) is still refused by the + // native constructor, not compared. + let mut coincident = fixture.clone(); + coincident.name = "coincident_analytic_line_constructor_rejection".into(); + coincident.inputs["first"][0]["curve"]["end"] = serde_json::json!([0.0, 0.0, 0.0]); + let native = Runner.run_cpp(&coincident).unwrap(); assert_eq!( native.rejected.as_deref(), Some("native fixture constructor status_1007"), diff --git a/crates/difftest/src/units/offset_offset_executive_p2.rs b/crates/difftest/src/units/offset_offset_executive_p2.rs index 1c0964f4a3..f1e7c3598e 100644 --- a/crates/difftest/src/units/offset_offset_executive_p2.rs +++ b/crates/difftest/src/units/offset_offset_executive_p2.rs @@ -3089,7 +3089,30 @@ mod tests { } compared += 1; } else if case.name.ends_with("_selected") { - // The tiny box is an empty B-rep: no face 0 to select. + // The smallest box the native constructor builds here has a face 0 to + // select: compared. + let rust = Runner.run_rust(&case).unwrap(); + for output in [&cpp, &rust] { + assert!(output.rejected.is_none() && output.error.is_none()); + } + let comparison = crate::compare::compare(&cpp, &rust, Runner.abs_floors()); + assert!(comparison.pass(), "{}: {comparison:?}", case.name); + compared += 1; + // The tiny box the case held before is an empty B-rep: no face 0 to + // select, a native fixture rejection. + let mut tiny = case.clone(); + let side = if case.name.contains("foreign") { + "foreign" + } else { + "source" + }; + let size = if case.name.contains("translated") { + 1e-3 + } else { + 1e-6 + }; + tiny.inputs[side]["size"] = serde_json::json!([size, size, size]); + let cpp = Runner.run_cpp(&tiny).unwrap(); assert_eq!( cpp.rejected.as_deref(), Some("native fixture constructor status_1001") @@ -3109,7 +3132,7 @@ mod tests { compared += 1; } } - assert_eq!((rejected, compared), (4, 6)); + assert_eq!((rejected, compared), (4, 10)); } fn assert_native_constructor_control(case: &Case, cpp: &Output, counts: [i64; 5]) { From d0e2cd2bcb86a88239c703b2c37153d38fa6b1bb Mon Sep 17 00:00:00 2001 From: bherbruck Date: Sat, 31 Oct 2026 03:47:19 -0400 Subject: [PATCH 12/13] docs: WR-617 addendum: the cause at the tangent corner, parked (#1795) Adds the unmerged-offset difftest cases for the #1795 plate shell (the raw offset equals C++ for y = 20 and for the top-open shell), an ignored probe, and a WR-617 addendum recording the cause and why the fix is parked. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../issue_1795_shell_hole_through_arc.rs | 65 +++++++++++++++ .../offset.offset_executive_p2/issue1795.json | 80 ++++++++++++++++++- ...0-31-cause-at-the-tangent-corner-parked.md | 20 +++++ 3 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 plan/upstream-issues/WR-617--addendum-2026-10-31-cause-at-the-tangent-corner-parked.md diff --git a/crates/solidcore/tests/issue_1795_shell_hole_through_arc.rs b/crates/solidcore/tests/issue_1795_shell_hole_through_arc.rs index e16ab56853..eaca41713a 100644 --- a/crates/solidcore/tests/issue_1795_shell_hole_through_arc.rs +++ b/crates/solidcore/tests/issue_1795_shell_hole_through_arc.rs @@ -476,3 +476,68 @@ fn issue_1795_y20_read_back() { } } } + +/// WR-617: the shell's offset faces before the merge (`merge_results` off, the executive's +/// offset B-rep as `ShellBrep` returns it unmerged), open at y = 20 and, for comparison, at +/// the top. Prints each face's surface kind and the point at the middle of its domain, then +/// every edge with an end near the plane x = 0 (the arc's tangent junction with y = 20, +/// where the merge's vertex relation fails: vertices at (0, 18, 2), (0, 18.5, 2) and +/// (0, 19.5, 2)), with its end points and faces. Run with `--ignored --nocapture`. +#[test] +#[ignore = "probe: WR-617, the unmerged offset faces at the tangent junction"] +fn issue_1795_unmerged_near_x0() { + let kind = |d: String| d.split(['(', ' ', '{']).next().unwrap_or("").to_string(); + for (open, cos) in [(Open::Y20, false), (Open::Y20, true), (Open::Z27, true)] { + let mut k = Kernel::default(); + let mut b = drilled(&mut k); + let faces = open.faces(&k, &b); + let mut unmerged = options(cos); + unmerged.offset.merge_results = false; + let shelled = api_shell(&mut k, &mut b, unmerged, &faces); + let Ok(r) = shelled + .inspect_err(|e| eprintln!("PROBE 1795 unmerged open={open:?} cos={cos}: Err({e:?})")) + else { + continue; + }; + eprintln!( + "PROBE 1795 unmerged open={open:?} cos={cos}: {} faces, {} edges", + r.face_list().len(), + r.edge_list().len() + ); + for &f in r.face_list() { + let face = r.face(f).unwrap(); + let s = face.surface.as_ref(); + let mid = s.map(|s| s.evaluate_point(&k, face.uv_domain.mid()).unwrap()); + eprintln!( + "PROBE 1795 unmerged open={open:?} cos={cos} face {}: {:?} mid {mid:?}", + f.index(), + s.map(|s| kind(format!("{:?}", s.kind()))) + ); + } + for &e in r.edge_list() { + let points: Vec = solidcore::topology::edge::vertices(&r, e, None) + .unwrap() + .into_iter() + .flatten() + .map(|v| r.vertex(v).unwrap().point) + .collect(); + if !points.iter().any(|p| p.x.abs() < 0.05) { + continue; + } + let on: Vec = solidcore::topology::edge::faces(&r, e, None) + .unwrap() + .into_iter() + .map(FaceId::index) + .collect(); + eprintln!( + "PROBE 1795 unmerged open={open:?} cos={cos} edge {}: curve {:?} vertices {points:?} faces {on:?}", + e.index(), + r.edge(e) + .unwrap() + .curve + .as_ref() + .map(|c| kind(format!("{:?}", c.kind()))) + ); + } + } +} diff --git a/fixtures/offset.offset_executive_p2/issue1795.json b/fixtures/offset.offset_executive_p2/issue1795.json index dccb56fedf..c067bb9e3b 100644 --- a/fixtures/offset.offset_executive_p2/issue1795.json +++ b/fixtures/offset.offset_executive_p2/issue1795.json @@ -1,5 +1,5 @@ { - "coverage": "tangency-labs/solidcore#1795: SmApiShellBrepFull 2 inward (extended convex edges, merged results) on the #1528 drilled plate (data/issue1528_plate.smb; see issue1528.json for how it is built), open at its top face z = 27.5 (GetFaces index 5 of 8) or at its face y = 20 (the plane the arc meets tangentially at (0, 20), which the issue's tangent-gap bridge at x = 0, y 18..20 points to; index 2 of 8), each with create_offset_solid false and true (smlib_shell passes TRUE); the indices as crates/solidcore/tests/issue_1795_shell_hole_through_arc.rs, ignored test issue1528_plate_file_top_face_index, prints). The hole r 5 opens through the arc side r 10; inset by 2 the hole's tube (r 7) passes out through the arc's offset (r 8). Pinned SMLib 1.0.1 shells the top-open cases (closed solids) and fails the y = 20 cases with SM_ERR (1001; WR-617); the port agrees on both; SolidCraft's smlib_shell (the fork, its own plate) reports 1001. The static_shell cases run SmOffsetExecutive::ShellBrep on the read body without the API's SmBrep copy, with the same outcomes (plan/findings/offset.offset_executive_p2--1795-y20-shell-fails-in-both.md).", + "coverage": "tangency-labs/solidcore#1795: SmApiShellBrepFull 2 inward (extended convex edges, merged results) on the #1528 drilled plate (data/issue1528_plate.smb; see issue1528.json for how it is built), open at its top face z = 27.5 (GetFaces index 5 of 8) or at its face y = 20 (the plane the arc meets tangentially at (0, 20), which the issue's tangent-gap bridge at x = 0, y 18..20 points to; index 2 of 8), each with create_offset_solid false and true (smlib_shell passes TRUE); the indices as crates/solidcore/tests/issue_1795_shell_hole_through_arc.rs, ignored test issue1528_plate_file_top_face_index, prints). The hole r 5 opens through the arc side r 10; inset by 2 the hole's tube (r 7) passes out through the arc's offset (r 8). Pinned SMLib 1.0.1 shells the top-open cases (closed solids) and fails the y = 20 cases with SM_ERR (1001; WR-617); the port agrees on both; SolidCraft's smlib_shell (the fork, its own plate) reports 1001. The static_shell cases run SmOffsetExecutive::ShellBrep on the read body without the API's SmBrep copy, with the same outcomes (plan/findings/offset.offset_executive_p2--1795-y20-shell-fails-in-both.md). The _unmerged cases shell with merge_results false: issue1795_plate_api_shell_2_y20_unmerged (no offset solid) returns the raw offset B-rep, compared before any merge; the _offset_solid_unmerged ones still merge the offset (y = 20 fails there with 1001, WR-617).", "cases": [ { "name": "issue1795_plate_api_shell_2_top", @@ -208,6 +208,84 @@ "create_offset_solid": true } } + }, + { + "name": "issue1795_plate_api_shell_2_y20_offset_solid_unmerged", + "upstream_ref": "tangency-labs/solidcore#1795 (operand: issue_1528_write_operand.rs); WR-617: the offset before PiecewiseMerge (bMergeResults FALSE)", + "inputs": { + "operation": "api_shell", + "source": { + "kind": "file", + "path": { + "$repo_file": "fixtures/offset.offset_executive_p2/data/issue1528_plate.smb" + } + }, + "faces": [ + { + "origin": "source", + "index": 2 + } + ], + "tolerance": 1e-06, + "config": { + "offset_distance": -2.0, + "extend_convex": true, + "merge_results": false, + "create_offset_solid": true + } + } + }, + { + "name": "issue1795_plate_api_shell_2_top_offset_solid_unmerged", + "upstream_ref": "tangency-labs/solidcore#1795 (operand: issue_1528_write_operand.rs); WR-617: the offset before PiecewiseMerge (bMergeResults FALSE)", + "inputs": { + "operation": "api_shell", + "source": { + "kind": "file", + "path": { + "$repo_file": "fixtures/offset.offset_executive_p2/data/issue1528_plate.smb" + } + }, + "faces": [ + { + "origin": "source", + "index": 5 + } + ], + "tolerance": 1e-06, + "config": { + "offset_distance": -2.0, + "extend_convex": true, + "merge_results": false, + "create_offset_solid": true + } + } + }, + { + "name": "issue1795_plate_api_shell_2_y20_unmerged", + "upstream_ref": "tangency-labs/solidcore#1795 (operand: issue_1528_write_operand.rs); WR-617: the y = 20 offset before PiecewiseMerge (bMergeResults FALSE, no offset solid: the raw offset B-rep is returned)", + "inputs": { + "operation": "api_shell", + "source": { + "kind": "file", + "path": { + "$repo_file": "fixtures/offset.offset_executive_p2/data/issue1528_plate.smb" + } + }, + "faces": [ + { + "origin": "source", + "index": 2 + } + ], + "tolerance": 1e-06, + "config": { + "offset_distance": -2.0, + "extend_convex": true, + "merge_results": false, + "create_offset_solid": false + } + } } ] } diff --git a/plan/upstream-issues/WR-617--addendum-2026-10-31-cause-at-the-tangent-corner-parked.md b/plan/upstream-issues/WR-617--addendum-2026-10-31-cause-at-the-tangent-corner-parked.md new file mode 100644 index 0000000000..36b9374f7b --- /dev/null +++ b/plan/upstream-issues/WR-617--addendum-2026-10-31-cause-at-the-tangent-corner-parked.md @@ -0,0 +1,20 @@ +Updates: WR-617 + +#### WR-617 (update, 2026-10-31). Cause: the offset faces' loops cut the tangent corner the bridge wall covers; parked + +- **Where it fails:** + - The `SM_ERR` comes from `SmMerge::MakeFaceuse`'s vertex-orientation exit (`SmMerge.cpp`, the `pVBRel` test after `MergeEdge`): an other-operand edge's vertex is related to a primary vertex at neither end of the primary edge. + - The merge is `SmOffsetExecutive::PiecewiseMerge`'s, on the shell's offset B-rep. + - The disagreeing vertices lie at x ≈ 0 on the inset floor z = 2, at y = 18, 18.5 and 19.5 (port trace). +- **Cause (read from upstream and the port, not traced statement by statement):** + - The open face y = 20 meets the arc side (r 10) tangentially along x = 0. + - That tangent edge between a kept face and a shell face takes `EdgeuseOffset`'s shell-tangent-gap branch (`bShellTangentGap`), which bridges it with a wall: the plane x = 0, y 18..20. + - The floor's and the top's offsets keep the original loop topology. Their rim edge along y = 20 and their arc-offset circle (r 8, reaching only y = 18) share one offset vertex, placed at the circle's end (0, 18, z). So each loop cuts the corner from (40.5, 20, z) to (0, 18, z) and has no edge on the bridge wall. The port's unmerged offset (`merge_results` off, no offset solid) shows this. + - The merge then makes the inconsistent vertex relation. A correct offset needs a second corner vertex at (0, 20, z) and an edge along the wall to (0, 18, z) in each of the two loops. +- **Ruled out:** + - A `MateEdges` split. The unmated hole/arc edge is present in the top-open shell too, and that shell closes. + - The file round trip, the API's copy and the test harness. +- **The offset before the merge against pinned C++:** + - Equal (every value, max relative error 0) for the top-open shell: `issue1795_plate_api_shell_2_top_offset_solid_unmerged`. + - For y = 20, equal too (every value, max relative error 0): `issue1795_plate_api_shell_2_y20_unmerged` (no offset solid, the raw offset). The corner cut is upstream behaviour. +- **Fix:** sized L: new edge construction in offset face building (split the corner vertex and add the bridge-wall edge, as an opt-in). An opt-in design exists and is parked pending the reporter's confirmation, on tangency-labs/solidcore#1795, that the face their plate opens is the one tangent to the arc. If they confirm it, it comes back as a scheduled design task. Until then the port keeps upstream's `SM_ERR` (pin `shell_open_at_y20_fails_as_upstream`). From 968313dd59ab771caa549f1c5598ddb800f5e154 Mon Sep 17 00:00:00 2001 From: Brett Lawson Date: Sat, 31 Oct 2026 08:48:03 +0100 Subject: [PATCH 13/13] Renumber the tight-box knot-division record to WR-651 WR-645 is taken on main by the drafted-pocket corner record, so check_record_ids fails. Rename this PR's record and its references. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MNXLXj2sJf943y2dDF36Vy --- crates/solidcore/src/kernel.rs | 2 +- crates/solidcore/src/query/face/p3.rs | 8 ++++---- .../tests/issue_1799_tight_box_divides_at_knots.rs | 2 +- ...ght-box-cells-over-off-dyadic-knots-stay-undecided.md} | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) rename plan/upstream-issues/{WR-645--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md => WR-651--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md} (97%) diff --git a/crates/solidcore/src/kernel.rs b/crates/solidcore/src/kernel.rs index 2761701b60..5e23c23f99 100644 --- a/crates/solidcore/src/kernel.rs +++ b/crates/solidcore/src/kernel.rs @@ -531,7 +531,7 @@ pub struct UpstreamFixes { pub fillet_trace_rejects_short_step: bool, /// `SmCurve::CalculateTightBoundingBox` adds only the B-spline knots strictly inside the queried interval (the interval's ends are already in the box). Upstream adds every tangent-discontinuous knot and always the last knot whatever the interval, so the tight box of an edge on part of a NURBS curve (an imported sphere cap's seam, 30 deg of a quarter-circle NURBS) reaches points the edge never does (WR-474, #1054; the fork's \"a curve's tight box holds the knots inside its interval only\"). pub curve_tight_box_interval_knots: bool, - /// `SmFace::CalculateTightBoundingBox` searches a trimmed face's own extreme inside its trim (subdividing the UV domain from the vertex and edge extremes, bounded at depth 16 / 4096 cells, an undecided cell divided at the surface's knot inside it nearest its middle, else at its middle), not only the surface's global extreme, so a face whose surface peaks outside the trim is not boxed too small (WR-588, #1799; tangency-labs/smlib@9d44c1b), nor one whose rational surface has knots off the domain's dyadic points too large (WR-645; tangency-labs/smlib@326de73). + /// `SmFace::CalculateTightBoundingBox` searches a trimmed face's own extreme inside its trim (subdividing the UV domain from the vertex and edge extremes, bounded at depth 16 / 4096 cells, an undecided cell divided at the surface's knot inside it nearest its middle, else at its middle), not only the surface's global extreme, so a face whose surface peaks outside the trim is not boxed too small (WR-588, #1799; tangency-labs/smlib@9d44c1b), nor one whose rational surface has knots off the domain's dyadic points too large (WR-651; tangency-labs/smlib@326de73). pub face_tight_box_searches_trim: bool, /// `SmTopologyIntersector::IntersectTopology` skips a face pair whose face boxes lie farther apart than the intersection tolerance (a box that cannot be computed, or holds nothing, rules nothing out), and `SmBrep::MakeEdgeInFaceWithVU`, on a plane, leaves in place without classifying an inner loop whose test point lies outside the box of the new face's outer loop (grown by the edges' tolerances, at the retry tolerance). Upstream pairs every face of one sheet with every face of the other through whole-surface checks and classifies every inner loop against each imprinted face, so a 2D boolean of a sheet with n holes takes time in n^2 (WR-527, #1526/#1527; the fork's tangency-labs/smlib@60b1c6a). pub boolean_skips_faces_apart: bool, diff --git a/crates/solidcore/src/query/face/p3.rs b/crates/solidcore/src/query/face/p3.rs index 22d476cbd9..08f92af035 100644 --- a/crates/solidcore/src/query/face/p3.rs +++ b/crates/solidcore/src/query/face/p3.rs @@ -11,7 +11,7 @@ //! | Rust | Upstream | //! |-----------------------------------------------|---------------------------------------------| //! | [`calculate_tight_bounding_box`], [`calculate_tight_bounding_box_into`] | `SmFace::CalculateTightBoundingBox` | -//! | `face_directed_max` (opt-in, WR-588), `cell_directed_max`, `cell_monotone`, `cell_division` (WR-645) | `sm_FaceDirectedMax`, `sm_CellDirectedMax`, `sm_CellMonotone` (the fork tangency-labs/smlib@9d44c1b), `sm_CellDivision` (tangency-labs/smlib@326de73) | +//! | `face_directed_max` (opt-in, WR-588), `cell_directed_max`, `cell_monotone`, `cell_division` (WR-651) | `sm_FaceDirectedMax`, `sm_CellDirectedMax`, `sm_CellMonotone` (the fork tangency-labs/smlib@9d44c1b), `sm_CellDivision` (tangency-labs/smlib@326de73) | //! | [`face_classify_uv_line`] | `sm_FaceClassifyUVLine` (file static) | //! | [`face_classify_uv_line_mut`] | `sm_FaceClassifyUVLine` (in place) | //! | [`find_and_classify_loop_bisector`], [`LoopBisector`] | `sms_FindAndClassifyLoopBisector` | @@ -596,7 +596,7 @@ const FACE_EXTREME_MAX_DEPTH: u32 = 16; /// this distance, so a part nearer than that is no cell of its own. const FACE_EXTREME_KNOT_SNAP: f64 = 1.0e-08; -/// Where a cell of `[lo, hi]` is divided along one way (`sm_CellDivision`, WR-645, +/// Where a cell of `[lo, hi]` is divided along one way (`sm_CellDivision`, WR-651, /// tangency-labs/smlib@326de73): at the knot strictly inside it (by more than /// [`FACE_EXTREME_KNOT_SNAP`]) nearest its middle, the first such in `knots` on a tie, /// else at its middle. Divided at knots, a cell comes to lie within one knot span that @@ -877,7 +877,7 @@ fn cell_monotone(k: &Kernel, surface: &Surface, cell: &Extent2d, dir: Vec3) -> R /// cells hold; /// /// otherwise the cell is divided each way ([`cell_division`]: at a knot inside it, else at -/// its middle; WR-645). A cell left at [`FACE_EXTREME_MAX_DEPTH`] or past +/// its middle; WR-651). A cell left at [`FACE_EXTREME_MAX_DEPTH`] or past /// [`FACE_EXTREME_MAX_CELLS`] counts with its greatest value. The cells are an explicit /// stack (no recursion), taken last first, as the fork's. fn face_directed_max( @@ -936,7 +936,7 @@ fn face_directed_max( continue; } let (u0, v0, u1, v1) = (cell.min().x, cell.min().y, cell.max().x, cell.max().y); - // WR-645 (tangency-labs/smlib@326de73): divided at a knot inside the cell, else at + // WR-651 (tangency-labs/smlib@326de73): divided at a knot inside the cell, else at // its middle (`sm_CellDivision`). let at = Point2::new( cell_division(&knots_u, u0, u1), diff --git a/crates/solidcore/tests/issue_1799_tight_box_divides_at_knots.rs b/crates/solidcore/tests/issue_1799_tight_box_divides_at_knots.rs index eaa479e546..0af92b9a91 100644 --- a/crates/solidcore/tests/issue_1799_tight_box_divides_at_knots.rs +++ b/crates/solidcore/tests/issue_1799_tight_box_divides_at_knots.rs @@ -1,4 +1,4 @@ -//! WR-645 (tangency-labs/smlib@326de73), the second half of #1799's trimmed-face tight box +//! WR-651 (tangency-labs/smlib@326de73), the second half of #1799's trimmed-face tight box //! (`Kernel::fixes.face_tight_box_searches_trim`, WR-588): the search divides an undecided //! cell at the surface's knot inside it, not only at its middle. //! diff --git a/plan/upstream-issues/WR-645--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md b/plan/upstream-issues/WR-651--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md similarity index 97% rename from plan/upstream-issues/WR-645--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md rename to plan/upstream-issues/WR-651--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md index 7093860a56..6c3a42a139 100644 --- a/plan/upstream-issues/WR-645--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md +++ b/plan/upstream-issues/WR-651--face-tight-box-cells-over-off-dyadic-knots-stay-undecided.md @@ -1,4 +1,4 @@ -#### WR-645. The trimmed-face tight box search (WR-588) leaves cells over a knot off the domain's dyadic points undecided, so the box overshoots +#### WR-651. The trimmed-face tight box search (WR-588) leaves cells over a knot off the domain's dyadic points undecided, so the box overshoots - **Severity:** Wrong result (a trimmed face's tight box is too large; opt-in path only) - **Area:** Faces / tight bounding box - **Location:** `source/SMLib/src/SmFace.cpp`, `sm_FaceDirectedMax` (the SolidCraft fork's, tangency-labs/smlib@beb6061, ported for WR-588): an undecided cell is halved at its middle. Not in upstream SMLib, which has no such search; this completes WR-588's fix.