port: boolean.topology_intersector_p2

[bluecmd]

Port of boolean.topology_intersector_p2: part 2 of SmTopologyIntersector.cpp in boolean::topology_intersector::p2. It covers 12 functions: RegisterExistingSSI, CheckForSqueezeVertices, DeleteFaces, FixGaps, RemoveRelationship, IntersectInsertRelate, IntersectTopology, SetRelationships, FixupClassMates, FixupSplitVertexPair, EdgeEdgeIntersect and EdgeFaceCoincidence. The two constructors were co-ported in #146.

Builds on main's #146 (part 1). Calls into units with open PRs by others go through UNPORTED stubs: topology.face_p4 #134, topology.brep_p1 #145, brep_p3, brep_p4 #173, brep_p5 #194, brep_p7 #155, query.face_p1 #105 and face_p3 #139. Nothing of theirs is re-implemented here. #211 (boolean.topology_intersector_p3, oursland) builds on this branch. His check_for_squeeze_vertices_into is taken here, re-implemented, with him as co-author.

Difftest

PARTIAL boolean.topology_intersector_p2 cases=841 passed=659 failed=0 deferred=182 max_rel_err=0e0 with_values=657 no_values=2 blocked_by=topology.brep_p1(126),topology.brep_p5(15),topology.brep_p9(7),topology.face_p3(3),topology.face_p4(31)
PARTIAL boolean.topology_intersector_p1 cases=827 passed=653 failed=0 deferred=174 max_rel_err=0e0 with_values=650 no_values=3 (unchanged)
PARTIAL topology.proto_topology_p1 cases=1857 passed=1765 failed=0 deferred=92 max_rel_err=0e0 (unchanged)
PARTIAL topology.proto_topology_p3 cases=3013 passed=2638 failed=0 deferred=375 max_rel_err=0e0 (unchanged)
FUZZ-PASS boolean.topology_intersector_p2 cases=2000 rejected=161 malformed=574 ran=1265 differs=0 crashes=0 panics=0 timeouts=0
sc-remote green: 201 units, failed=0

• Cases: 841 in all: 9 upstream-derived fixtures, 32 edge cases built in code and 800 generated scripts on part 1's two-B-rep script. The 2 no_values cases are scripts cut at op 0 (cases_both_errored), so there are 657 real passes.
• Memory (correction). At 4a6bcc8 this said "valgrind 75 cases clean". That was wrong: the sample had no IntersectInsertRelate program that builds proto edges. The pre-review found an upstream use-after-free in ~SmProtoTopologyManager (UB-175 [PSIRT], below). After the fix, valgrind runs clean (0 errors) on 19 programs: 12 generated op-47 programs, including the three that had failed, plus the iir*, intersect_topology and new edge cases. Both the C++ and the Rust side run in the same process.
• Mostly no-ops: these passes complete no real work. Per op occurrence:
  • CheckForSqueezeVertices: 76 successes have no conflicting mate (empty lists). A conflict that combines vertices defers.
  • DeleteFaces: all 68 successes have an empty face list.
  • FixGaps: 119 successes find no gap pair. A pair that combines defers.
  • SetRelationships: 39 successes, all on managers without definitions.
  • EdgeEdgeIntersect: 129 successes, none of which splits. A split defers on brep_p9.
  • EdgeFaceCoincidence: 128 successes, none of which imprints.
• Passes that compare real results:
  • RemoveRelationship: 28 successes, 36 SM_ERR.
  • CheckForSqueezeVertices: 25 SM_ERR (the existing relationship is better).
  • FixGaps: 14 SM_ERR_NULL_POINTER (a gap pair with an unmated vertex).
  • FixupClassMates: 54 successes on 0 to 3 intervals, 19 SM_ERR (sizes differ).
  • FixupSplitVertexPair: 56 successes, 6 of which remove the interval (one with a shorter other classification, split_oob).
• No passing case at all: IntersectTopology and IntersectInsertRelate both start with SmBrep::GetFaces (topology.brep_p1). Their tails are ported in full behind the stub.

The ignored test units::boolean_topology_intersector_p2::tests::census reproduces these counts.

Changes outside the unit

• Part 1's harness is shared. The p2 shim includes p1's through an SC_TI_EXTRA_OPS hook (ops 41 to 51). The p2 FFI binding extends p1's validation through ExtraOps. p1's runner interprets part 2's ops. p1's own build and difftest are unchanged.
• RegisterExistingSSI (op 16) in p1's script now runs the port instead of the runner filling the table directly.
• SmTsectCurveType is bounded to 0–6 in the FFI. An unknown code is now a harness error; it used to map silently to Unknown.
• p1 runner panic fixed: encoding a surface without knots no longer panics (found by fuzz).
• Back-edge wired: RemoveRelationship in topology::proto_topology::p3 (GlueCoincidentBrepVertices). The other_mate closure is replaced by a Mates trait. TopologyIntersector passes its maps (MapMates); NoMates is for no maps.
• Guards.
  • IntersectInsertRelate (op 47) needs a nonzero tolerance (HANG-25: 0 and -0 hang; negative and tiny positive ones run) and coordinates below 1e154 (HANG-16's class). The port defers op 47 in every case, so the guard removes no compared case.
  • The CheckForSqueezeVertices and SetRelationships (no maps) guards now refuse only where upstream dereferences.
• UB-175 handling. The shim runs a verbatim copy of IntersectInsertRelate. It unlinks the stale proto-edge entries before the delete and answers -2; the port detects the same condition and returns InvalidInput. The shim's own manager teardown unlinks them as well.
• Upstream issues:
  • UB-169: a failed RegisterExistingSSI leaves the table out of step. The text is rewritten with its three outcomes.
  • UB-175 [PSIRT]: use-after-free in the proto manager's destructor, reachable from file data through the boolean merge.
  • HANG-25: IntersectInsertRelate at tolerance 0.
  • WR-170: leaks.

UNPORTED tags

• Added: topology.brep_p1 (SmBrep::GetFaces), topology.brep_p3 (DeleteFace), topology.brep_p4 (DeleteEdge/DeleteVertex), topology.brep_p5 (CombineCoincidentVertices) and query.face_p1 (SmFace::Point3DClassify). All their units are todo with open PRs.
• Reused from p1: topology.face_p4 (GetEdges/GetVertices/GetBrep), topology.face_p3 (FaceIntersect), topology.brep_p5 (MakeEdgeInFace) and topology.brep_p7 (GlueEdgesGeneral).
• Removed: UNPORTED(boolean.topology_intersector_p2) RemoveRelationship in proto_topology p3, now wired.

Status (updated 2026-10-19 08:19 UTC)

• Phase: ready for review. The pre-review is CLEAN at 118041e6; 9286f181 adds the 3 comment minors. The merge of main@86f2d3bd (ffi: share the validate Reader through solidcore_ffi::reader #186), checked CLEAN at 29b12286, is pushed as 0d8b1028.
• Functions ported: 12/12. Difftest: 841 cases: 657 pass (655 real, 2 both-errored) / 0 fail / 184 deferred; perturb SENSITIVE 657/657; FUZZ-PASS 2000; green 204 units, 0 failed.
• Count shift at the merge (659 → 657 pass, brep_p9 7 → 9): main's topology.shell fixture changes (a richer shell-state import, and the GetEdges-first walk that renumbers handles) changed 10 generated cases on both sides; no p2 code changed.
• Upstream issues registered: UB-175 [PSIRT] (use-after-free in ~SmProtoTopologyManager, guarded) and HANG-25 (IntersectInsertRelate at tolerance 0).
• Dependencies: builds on port: boolean.topology_intersector_p1 #146 (merged). topology.brep_p1 (port: topology.brep_p1 #145), topology.face_p4 (port: topology.face_p4 #134), topology.brep_p5 (port: topology.brep_p5 #194) and topology.brep_p9 (port: topology.brep_p9 #183) are UNPORTED stubs. port: boolean.topology_intersector_p3 #211 builds on this branch.
• Next: approved by the maintainer; main is merged in at 19c58b4c, with one required compile adaptation explained in the PR comments (p2 now calls port: topology.brep_p7 #155's brep_glue_edges_general). The optional nits, plus wiring the now-merged topology.brep_p1 stubs, go into a follow-up PR after this merges.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VhgnDJqMfisAk17esFqRL5

[oursland]

Heads-up from #211 (boolean.topology_intersector_p3), which builds on this branch. I made two small changes to p2.rs there that you may want to take here:

1. check_for_squeeze_vertices_into(brep, v1, v2, &mut SqueezeVertices): appends to the caller's lists, with check_for_squeeze_vertices kept as a wrapper around it.
   • Upstream's CheckForSqueezeVertices can append in its first conflict block and then return SM_ERR from the second ("previous one is better").
   • MergeCurveClasses (part 3) passes the same three lists on every iteration and keeps going after a failure. So it needs the partial appends, which the Result<SqueezeVertices> form drops.
2. let ... else at the fi.cc1/fi.cc2 pair in the classification loop (around line 938). Clippy 1.98 flags the match form with -D warnings.

No behaviour change for existing callers. Happy to drop both from #211 if you'd rather do them differently; I'll re-merge your branch as it moves.

🤖 Generated with Claude Code

[bluecmd]

@oursland thanks, I've taken both in ecd4ebd. check_for_squeeze_vertices_into is re-implemented with you credited as co-author, since your edits sat inside the p3 port commit. You're right about upstream: the first conflict block appends and the second can still return SM_ERR. The let … else was already there. I won't rewrite this branch's history.

🤖 Generated with Claude Code

[bluecmd]

Ready for review at 9286f18. The independent pre-review is CLEAN; 657 real passes, 0 failing; UB-175 [PSIRT] and HANG-25 are registered. Downstream: @oursland's #211 builds on this branch.

[bherbruck]

Thanks @bluecmd for this port. I reviewed head 0d8b1028 against SmTopologyIntersector.cpp and found nothing that needs to change before merge. Once CI is green, it's good to merge.

What I checked

• All 12 functions, side by side with upstream: RegisterExistingSSI, CheckForSqueezeVertices, DeleteFaces, FixGaps, RemoveRelationship, IntersectInsertRelate, IntersectTopology, SetRelationships, FixupClassMates, FixupSplitVertexPair, EdgeEdgeIntersect and EdgeFaceCoincidence.
  • Defaults and parameters match upstream: DeleteFace(.., FALSE) with the remaining defaults, UpdateObjectTolerance(.., 0, 0, FALSE), IsClosed(ivl, 0.0), SetTParam(.., FALSE), DropPoint with SM_SO_MINIMIZE and the SM_USE_OLDTOL deviation, SimpleCoincidenceChecker with partial coincidence FALSE, and the manager built with dOverrideTol = 0.
  • Statuses upstream ignores are ignored, and the ones it checks are checked.
  • The operation order matches, including:
    • FixupClassMates restarts at 0;
    • EdgeEdgeIntersect re-reads the list lengths and does not recompute the e1 box after a split;
    • the second Point3DClassify reuses sPtClass and runs at the approximation tolerance;
    • FaceIntersect's SmObjsDelete runs before the empty-result continue.
• UNPORTED stubs: each target is todo on main (brep_p1/p3/p4/p5/p9, face_p4, query.face_p1) and carries its owner tag from units.json. The code after each stub is ported.
• Back-edge: the back-edge on main into this unit (proto_topology/p3.rs RemoveRelationship) is now wired through Mates/MapMates. git merge-tree against origin/main is clean. port: boolean.topology_intersector_p3 #211 builds on this branch, so nothing is ported twice.
• Safety: no unwrap, no unchecked indexing and no recursion. EdgeEdgeIntersect growth is bounded by MAX_EDGE_LIST. Nothing allocates from an input-declared size.

Gates (all pass): fmt, clippy -D warnings (plain and --features solidcore-ffi/cpp,difftest/cpp), cargo test -p solidcore, the wasm32 check, and the difftest runs below. Nothing crashed or panicked.

Extra adversarial cases. I ran 542 hand-written cases, kept locally and not pushed. 343 of them compare values.

• Planar sheets at scales from 1e-8 to 1e8. At 1e-8 and from 1e5 up, the faces don't build on either side, and both sides agree on that.
• FixGaps with no mates and with duplicate vertices in the list.
• FixupSplitVertexPair / FixupClassMates at the first, middle and last index, on 1-, 2- and 3-interval pairs, with vertex deviations of 1e-6, 0, -1 and 1e300.
• The same fix-ups on the first three edge curves of plane_circ, disk, cone, sphere and ellips_ts2 (circle, ellipse and B-spline curves).
• EdgeEdgeIntersect with nulls and duplicates in the lists, strict and ignore modes, and a 1e-6 near miss.
• Squeeze with conflicting mates, with a worse new mate, and RemoveRelationship called twice.

Results:

PARTIAL boolean.topology_intersector_p2 cases=841 passed=657 failed=0 deferred=184 max_rel_err=0e0 with_values=655 no_values=2 blocked_by=topology.brep_p1(126),topology.brep_p5(15),topology.brep_p9(9),topology.face_p3(3),topology.face_p4(31)
PARTIAL boolean.topology_intersector_p2 (review cases) cases=542 passed=527 failed=0 deferred=15 max_rel_err=0e0 with_values=343
PARTIAL boolean.topology_intersector_p2 (--seeds 3000) cases=3041 passed=2385 failed=0 deferred=656 max_rel_err=0e0
FUZZ-PASS boolean.topology_intersector_p2 cases=1500 rejected=122 malformed=436 ran=942 differs=0 crashes=0 panics=0 timeouts=0
SENSITIVE boolean.topology_intersector_p2 perturbed=657 detected=657 unperturbed=0 deferred=184
PARTIAL boolean.topology_intersector_p1 cases=827 passed=653 failed=0 deferred=174 max_rel_err=0e0
PARTIAL topology.proto_topology_p3 cases=3013 passed=2638 failed=0 deferred=375 max_rel_err=0e0
PARTIAL topology.proto_topology_p1 cases=1857 passed=1765 failed=0 deferred=92 max_rel_err=0e0
difftest --green: 204 unit(s), 113 passed, 91 partial, 0 failed

Nits (optional)

• crates/difftest/src/units/topology_proto_topology_p1.rs:3703 adds a new rustfmt --edition 2021 --check diff: the glue_coincident_brep_vertices(.., &mut solidcore::topology::proto_topology::p3::NoMates) line is over the width limit. cargo fmt doesn't see include!d files, so CI won't catch it. A use for NoMates, or a wrapped call, would fix it.
• Errors built in p2 with dangling(..) use p1's operation path ("boolean::topology_intersector::p1"). A p2-local helper that takes the OP_* constants already defined in p2.rs would make error paths point at the right function.

🤖 Generated with Claude Code

[bluecmd]

Thanks @bherbruck for the thorough review and the extra adversarial cases. I'll leave this head as approved and send both nits as a small follow-up PR after merge: the NoMates import and wrapping in the proto_topology_p1 runner, and a p2-local error helper so the errors carry p2's OP_* paths.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VhgnDJqMfisAk17esFqRL5

[bluecmd]

Merged main to resolve a conflict. The only other change is the compile adaptation it needs: edge_face_coincidence now calls #155's IntersectorBrep::brep_glue_edges_general (it's behind the still-stubbed SmFace::GetBrep, so no results change).

🤖 Generated with Claude Code

https://claude.ai/code/session_01VhgnDJqMfisAk17esFqRL5