Skip to content

boolean.topology_intersector_p1: follow-ups to #146 - #227

Merged
bherbruck merged 3 commits into
tangency-labs:mainfrom
bluecmd:followup/boolean.topology_intersector_p1
Oct 19, 2026
Merged

bherbruck merged 3 commits into
tangency-labs:mainfrom
bluecmd:followup/boolean.topology_intersector_p1

Conversation

@bluecmd

@bluecmd bluecmd commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Follow-ups to #146 (boolean.topology_intersector_p1), as announced in #146 (comment). One commit on main (a28e0f0). No port claim and no change to the unit's case outcomes.

Also fixes main's current FUZZ-FAIL. --fuzz 2000 on main 154853c has 3 differs (sm_SetEdgeClass with tolerance -1e300, fixed by item 2) and 5 C++-runner subtract-overflow panics at boolean_topology_intersector_p1.rs:406/407 (empty knot vectors, fixed by item 3). It passes at this PR's head.

What changes

  1. Exact enum decoding (lesson 22). The runner and FFI decode SmPointClassificationType and SmTsectCurveType codes exactly. pc_from no longer falls back to Unknown; an unknown code is a harness error, and the FFI bounds curve types to 0..=6.

  2. SmZoneTol3d in SetEdgeClass. The runner's new_classification passes the tolerance through zone_tol3d before the > 0 check, as the C++ constructor does via SmZoneTol3d(double) (SmTol.cpp:107 / SmTol.h:146: fabs, USE_DEFAULT). This fixes main's 3 --fuzz differs on sm_SetEdgeClass with tolerance -1e300 (C++ VERTEX, Rust EDGE).

  3. Empty knot vectors. The surface encoder's knot counts use checked_sub. An empty knot vector is now a harness error, where it used to panic (fuzz00058, fuzz00396).

  4. CalculateUVDomainFromUVTrimCurves call sites. At the three sites, SmExtent2d::Intersect's SM_ERR_INVALID_INPUT (Extent2d::intersect's InvalidInput) keeps the face domain, as upstream's == SM_SUCCESS test does. Before, it was reported as undefined behaviour. This is latent: no case reaches it until topology.face_p3's CreateUVTrimCurves.

  5. Stale stub entry. plan/modules.toml drops the topology.face_p1 stub entry (wired since port: topology.face_p1 #148).

  6. Lesson-27 audit of p1.rs:

    • stub signatures carry every upstream argument, with defaults passed explicitly (SmBrep.h / SmFace.h);
    • ignored statuses keep upstream's default outputs;
    • the split tolerance is re-read;
    • the glue check runs on both vertices;
    • the face bounding box persists across the face loop.

    The brep_edges / brep_vertices stubs are kept exactly as on main for port: topology.brep_p1 #145's IntersectorBrep methods.

Minors from the swarm review

  • zone_tol3d defaults from the kernel's zone_tol_3d, which is the Rust equivalent of upstream's thread-local s_ZoneTol3d; the two are equal in the harness.
  • EdgeFaceIntersect.new_verts/new_edges and SplitTwoEdges.new_edges1/2 change from Vec<O> to Vec<Option<O>>, a public field type change; port: boolean.topology_intersector_p2 #203 and port: heal.stitch_p2 #205 read these fields.
  • This PR widens the private brep_delete_face stub, which port: topology.brep_p3 #204 replaces with IntersectorBrep::brep_delete_face. Whichever merges second resolves it; see COORD.

Verification at the head e14da886 (main merged through #155; sc-builder-1):

PARTIAL boolean.topology_intersector_p1 cases=827 passed=690 failed=0 deferred=137 max_rel_err=0e0 with_values=687 no_values=3 blocked_by=query.brep(16),query.face_p1(7),topology.brep_p2(21),topology.face_p2(82),topology.face_p4(11)
FUZZ-PASS boolean.topology_intersector_p1 cases=2000 rejected=213 malformed=615 ran=1172 differs=0 crashes=0 panics=0 timeouts=0
  • Case outcomes are identical to main's report, case by case (port: topology.brep_p1 #145 moved the unit from 653/174 to 690/137 on main itself).
  • Fuzz passes; main had 3 differs before this PR.
  • fmt (including the include!-ed unit files), cargo check --workspace (plain and with difftest/cpp), clippy --features cpp and wasm32 are clean; cargo test -p solidcore passes 1378 tests.

At the first push (77f74fd0, before the main merges): --perturb SENSITIVE 653/653, green-par 200 units with 0 failed, and valgrind clean on 84 sampled shim programs (about 10 per op family).

🤖 Generated with Claude Code

https://claude.ai/code/session_01HBgcxze64BBVRNjGewaEuN

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

- Runner/FFI: decode SmPointClassificationType and SmTsectCurveType codes
  exactly (unknown codes are a harness error, no Unknown default); pc_from
  returns Result.
- new_classification applies SmZoneTol3d (fabs, USE_DEFAULT) before the > 0
  check, as the C++ constructor does: fixes the 3 fuzz differs of
  sm_SetEdgeClass with tolerance -1e300.
- Surface encoder: empty knot vectors are a harness error, not a panic
  (fuzz00058, fuzz00396).
- CalculateUVDomainFromUVTrimCurves call sites: SmExtent2d::Intersect's
  SM_ERR_INVALID_INPUT keeps the face domain (upstream's == SM_SUCCESS test).
- Lesson-27 audit of p1.rs: full stub signatures, explicit defaults, ignored
  statuses keep upstream defaults, split_tol re-read.
- plan/modules.toml: drop the stale topology.face_p1 stub entry.

difftest: PARTIAL 827 cases, 653 pass, 0 fail, 174 deferred (unchanged);
SENSITIVE 653/653; --fuzz 2000 FUZZ-PASS; green-par 200/0; valgrind clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBgcxze64BBVRNjGewaEuN
bluecmd added a commit to bluecmd/solidcore that referenced this pull request Oct 18, 2026
…into topology.brep_p7

AGENTS.md 4a, following tangency-labs#145/tangency-labs#204's adapter pattern:
- IntersectorBrep<O> gains brep_glue_edges_general(k, brep, surviving,
  relative_orientation, edge_gap, do_region_nesting, to_delete,
  inside_faceuses) with every upstream argument (pInsideFaceuses NULL
  passed explicitly); the private UNPORTED(topology.brep_p7) stub is
  removed and the SmTopologyIntersector.cpp:8338 call site uses it.
- face_face_intersect takes the kernel mutably (glue_edges_general
  needs it for IsInnerShell).
- The intersector runner's Two adapter implements it with
  topology::brep::p7::glue_edges_general on the handle's B-rep.
- The intersector_p1 report is byte-identical: no case reaches the
  glue yet (earlier stubs defer first). COORD.md records the trait
  addition and its overlap with tangency-labs#145, tangency-labs#204 and tangency-labs#227.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
@bluecmd bluecmd mentioned this pull request Oct 18, 2026
bluecmd and others added 2 commits October 19, 2026 06:51
…ncy-labs#196) into followup/boolean.topology_intersector_p1

p1.rs: main's IntersectorBrep::brep_edges / brep_vertices (the GetEdges /
GetVertices stubs removed) with this branch's MakeVertexLoop signature.
modules.toml: main's note kept verbatim (its tangency-labs#145 UPDATE included); this
branch's follow-up text appended after it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBgcxze64BBVRNjGewaEuN
…followup/boolean.topology_intersector_p1

p1.rs: main's IntersectorBrep::brep_glue_edges_general call (this branch's
GlueEdgesGeneral stub dropped) with this branch's DeleteFace signature.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBgcxze64BBVRNjGewaEuN
@bherbruck

Copy link
Copy Markdown
Collaborator

Thanks @bluecmd for these follow-ups. I reviewed head e14da886 (which includes the merge of main at 6cca30b9). It's good to merge once CI is green.

What passed (local run, fresh worktree):

  • cargo fmt --all --check, clippy (plain and --features difftest/cpp), cargo test -p solidcore (1378 tests) and cargo check --target wasm32-unknown-unknown are all clean.
  • The unit has the same 827 cases as before: 690 pass bit-identical, 0 fail, 137 deferred. This matches main's report.
  • --fuzz 2000: FUZZ-PASS with 0 differs, 0 crashes, 0 panics and 0 timeouts. That resolves main's 3 sm_SetEdgeClass differs.
  • --green: 209 units, 0 failed.
PARTIAL boolean.topology_intersector_p1 cases=827 passed=690 failed=0 deferred=137 max_rel_err=0e0 with_values=687 no_values=3 blocked_by=query.brep(16),query.face_p1(7),topology.brep_p2(21),topology.face_p2(82),topology.face_p4(11)
FUZZ-PASS boolean.topology_intersector_p1 cases=2000 rejected=213 malformed=615 ran=1172 differs=0 crashes=0 panics=0 timeouts=0
difftest: 209 unit(s), 113 passed, 96 partial, 0 failed

Checked against upstream:

  • zone_tol3d in new_classification matches SmZoneTol3d::SmZoneTol3d(double) (SmTol.cpp:107).
  • The FFI's rd.index(6) is an inclusive bound, so SM_TC_REGION_BOUNDARY (6) is still accepted. This matches SmTsectCurveType (SmTypes.h:862).
  • Hoisting sFBBox out of the face loop matches SmTopologyIntersector.cpp:7415, where it is declared together with sEBBox.
  • The DeleteFace defaults are FALSE, TRUE, NULL, NULL (SmBrep.h:589), and the call at SmTopologyIntersector.cpp:8499 uses them.
  • Point3DClassify's pOptGuessUV is only read: it is passed as const SmPoint2d& to LocalPointSolve (SmFace.cpp ~12806). Passing Some(uv) by value therefore loses nothing.
  • The MakeVertexLoop default is SM_OT_OPPOSITE (SmBrep.h:1138).
  • No .github/, build script, Cargo.toml or dependency changes, and nothing touches licensing or branding.

Nits (optional):

  • The "Verification" block in the PR description still shows the pre-port: topology.brep_p1 #145 counts (653/174). The status section and the run above both give 690/137.
  • The plan/modules.toml note now appends a long follow-up paragraph. A short Follow-ups (PR #227): ... prefix would make it easier to scan, though the content is accurate as it stands.

🤖 Generated with Claude Code

@bluecmd

bluecmd commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the review and the upstream checks. I'll take both nits (the stale 653/174 verification counts and a shorter Follow-ups (PR #227): ... prefix for the notes) in a small follow-up, so this head stays as reviewed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HBgcxze64BBVRNjGewaEuN

@bherbruck
bherbruck merged commit 9147045 into tangency-labs:main Oct 19, 2026
8 checks passed
@bherbruck

Copy link
Copy Markdown
Collaborator

Landed in batch #259 together with other reviewed PRs; thanks @bluecmd!

🤖 Generated with Claude Code

bluecmd added a commit to bluecmd/solidcore that referenced this pull request Oct 19, 2026
…differs

The three --fuzz 2000 differs on main 0cf3e29 (sm_SetEdgeClass with
tolerance -1e300; fixed in tangency-labs#227 by passing it through SmZoneTol3d) become
value-comparing fixtures. 830 cases, 693 pass, 0 failed, 137 deferred.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBgcxze64BBVRNjGewaEuN
denelson1-dot pushed a commit to denelson1-dot/solidcore that referenced this pull request Oct 20, 2026
… follow-ups (review nit)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HBgcxze64BBVRNjGewaEuN
denelson1-dot pushed a commit to denelson1-dot/solidcore that referenced this pull request Oct 20, 2026
…ency-labs#227 follow-ups (notes prefix, fuzz regression fixtures))

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pfe7dodPG5rpuN9B8QcnuX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants