Skip to content

port: topology.brep_p7 - #155

Merged
bherbruck merged 34 commits into
tangency-labs:mainfrom
bluecmd:unit/topology.brep_p7
Oct 19, 2026
Merged

bherbruck merged 34 commits into
tangency-labs:mainfrom
bluecmd:unit/topology.brep_p7

Conversation

@bluecmd

@bluecmd bluecmd commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Ports unit topology.brep_p7 (source/SMLib/src/SmBrep.cpp, 6 functions): ReadFromFile, MakeFace, MakeFaceWithCurves, GlueEdgesGeneral, CopyFaces, MergeSelfIntersectionCurves → crates/solidcore/src/topology/brep/p7.rs (new topology::brep module; brep_p1/p2/p10 add their pub mod lines next to p7), with shim harness/cpp/shim/topology_brep_p7.cpp, FFI wrapper and difftest runner.

Status: partial. Calls into units not ported yet are UNPORTED stubs (no co-porting): GetInfiniteRegion (topology.brep_p1); SmBrep::Notify, SetTolerance, sm_CheckAndFixUVTrimCurves, RebuildUVTrimCurves (topology.brep_p2); MakeWireEdge(Vertex) (brep_p3); DeleteEdge (brep_p4); GlueVertices (brep_p5); MakeShellVertex (brep_p9); MakeTopologyFromData, TestClosureSplitRegion(edge) (brep_p10, #130); SmFace::Notify/SetTolerance/CreateUVTrimCurves (face_p3); SmFace::GetBrep/GetEdges (face_p4); SmMerge (boolean.merge_p2); the curve constructor of SmCurveClassification (curve_class_p4). ReadFromFile takes the file's bytes (the library reads no files); the reader is BrepData::read_from_file, which bounds every count and nesting level.

Difftest

PARTIAL topology.brep_p7 cases=316 passed=197 failed=0 deferred=119 max_rel_err=0e0 with_values=197 no_values=0 blocked_by=cpp-guard:ub-165(3),curve.crv_on_surf(1),curve.curve_p6(2),topology.brep_p1(12),topology.brep_p10(30),topology.brep_p2(51),topology.brep_p9(19),topology.face_p3(1)
SENSITIVE topology.brep_p7 perturbed=197 detected=197 unperturbed=0 deferred=119
FUZZ-PASS topology.brep_p7 cases=2000 rejected=233 malformed=296 ran=1471 differs=0 crashes=0 panics=0 timeouts=0
cases_both_errored=0

What the passes cover: all 197 passes are no-ops, empty inputs or error statuses. No pass completes a path that builds topology (a successful read, a face made, a real glue, a copy, a non-empty self-intersection merge); every such path defers, mostly at topology.brep_p2's SmBrep::Notify.

  • 110 glue no-ops: null, identical, or wrapped-to-identical edges. Each compares the B-rep's counts and its sorted edge and vertex tolerances after the op, 10 to 52 values per case.
  • 64 MergeSelfIntersectionCurves calls with no curves.
  • 23 error statuses: 12 read errors, 5 face-from-curves input errors, 4 MakeFace null surfaces, 1 square too small for its lines on both sides, and 1 generated case.

no_values=0; the shim's -2 guards are deferrals, not passes.

Coverage, by op (from the report):

op pass deferred what the passes compare
ReadFromFile 12 30 read errors (status 1001) of damaged streams the topology.brep_data guard accepts, pinned by a deterministic search (an overwritten digit of an integer token); every other read defers at MakeTopologyFromData (brep_p10)
MakeFaceWithCurves 5 27 input errors (no curves, no loops, degenerate surfaces), the last two now also with curve on surface and no UV curves; 3 such calls past those checks defer to cpp-guard:ub-165; a curve on surface over a plane defers to curve.crv_on_surf; an open UV-only curve-on-surface loop on a B-spline patch defers to curve.curve_p6
MakeFace 5 5 SM_ERR_NULL_POINTER for a null surface; a square too small for its lines (-3 on both sides); square loops defer at MakeShellVertex (brep_p9)
GlueEdgesGeneral 107 43 no-ops only (null, identical, or wrapped-to-identical edges), compared as the B-rep's counts and tolerances after the op. Everything past that early return defers at SmBrep::Notify (brep_p2): non-adjacent pairs (SM_ERR), wires, real merges, and every shared-vertex pair of the files
MergeSelfIntersectionCurves 60 0 empty input (no curves): no edges
CopyFaces 0 6 - (SmBrep::SetTolerance)
generated 8 8 glue no-ops (3), empty merge-self (4), 1 status; the generator draws all six ops (face-from-curves and copies had been left out by oversight), and every orientation code including SM_OT_UNKNOWN

Every path that builds topology is deferred. partial is the right status until brep_p1/p2/p3/p6/p9/p10 land.

C++ guards.

  • Damaged streams SmBrepData accepts are not run in C++ (cpp-guard:unvalidated-topology). Both sides decide this with the C++ reader. The guard is needed: MakeTopologyFromData uses the file's cross-reference indices unchecked (UB-164, [PSIRT]). With the guard off, 11 of 600 single-byte damages crashed, and a batch of 100 crashed in 13 of the 15 files. Revisit when brep_p10 lands. The 45 overwrites of the leading comment line were dropped: C++ reads them exactly as the plain file.
  • -2 guards (null dereferences) are deferred. These cover:
    • a curve on surface without UV curves (UB-165);
    • a glue with SM_OT_SAME, equal intervals and exactly one edge without a curve (upstream's exact dereference condition; no fixture edge lacks a curve);
    • MergeSelfIntersectionCurves on a face without a surface.
  • No other guards. Shared-vertex glue runs in C++. The native read skips the stream check above only for bytes identical to one of the shipped test files; there is no caller-settable trust flag.

Upstream issues found:

  • UB-165: SmBrep::MakeFaceWithCurves with pbOptCrvOnSurf TRUE and no UV curves dereferences a null array (SmBrep.cpp:16600). Confirmed by a C++ run. Renumbered from UB-163, which port: heal.heal_data_p3 #164 took on main.
  • UB-164 [PSIRT]: SmBrep::MakeTopologyFromData indexes its section arrays with unchecked indices from the file. Confirmed by C++ runs.

Gaps in other units' ports: the port of SmCurve::DeformToEndPointTargets (curve.curve_p6) has no pOptSurfCrvOnSurf. A curve on surface with a gap target therefore defers (UNPORTED(curve.curve_p6)) instead of being approximated (case face_curves_cos_open_loop); other curves still use the port. The gap is recorded in curve.curve_p6's notes. CrvOnSurf holds only B-spline surface kinds, so other kinds defer to curve.crv_on_surf, with the same label and tag as that gap's other sites. See plan/findings.md.

Back-edges (§4a)

Wired:

  • topology.trimming_tools (TrimSurfaceWithModelSpaceCurves → make_face_with_curves).
  • boolean.topology_intersector_p1 (FaceFaceIntersect → GlueEdgesGeneral): a new IntersectorBrep::brep_glue_edges_general trait method with all upstream arguments (including the default-NULL face-use list), called with upstream's exact arguments; the runner's adapter calls p7::glue_edges_general. No case reaches the glue yet (earlier stubs defer first), so its report is byte-identical. port: topology.brep_p1 #145, port: topology.brep_p3 #204 and boolean.topology_intersector_p1: follow-ups to #146 #227 touch the same trait (noted in COORD.md).
  • This unit now calls topology.shell's SmShell::IsInnerShell (port: topology.shell #154) in MergeShells.
  • topology.brep_p6 (port: topology.brep_p6 #169): MakeFace calls MakeFaceTopology with upstream's arguments, and GlueEdgesGeneral's region nesting calls TestClosureSplitRegion(faceuse) (status ignored; each call resets rpNewRegion, as upstream). The one case that deferred at brep_p6 (face_curves_cos_no_uv_1_1) now defers inside it at SmFace::SetTolerance (topology.face_p3).

Relabelled, not wired: offset.offset_executive_p1 (#175) had an UNPORTED(topology.brep_p7) MakeFaceWithCurves stub. It still does not call this unit. Its first argument, m_pOffsetBrep->GetInfiniteRegion(), is unported (topology.brep_p1), so the stub now names that real first blocker. No case reaches it. For whoever wires it later: upstream ignores a failed MakeFaceWithCurves there (if (... == SM_SUCCESS) {...} and then falls through), so that error must not be passed up with ?.

PARTIAL topology.trimming_tools cases=3263 passed=2842 failed=0 deferred=421
PARTIAL boolean.topology_intersector_p1 cases=827 passed=653 failed=0 deferred=174
PARTIAL topology.shell cases=911 passed=892 failed=0 deferred=19
PARTIAL topology.brep_p6 cases=498 passed=450 failed=0 deferred=48
PARTIAL offset.offset_executive_p1 cases=693 passed=675 failed=0 deferred=17

Verification (at f787d107; merged with origin/main at e6414686)

  • fmt, plus rustfmt --edition 2021 --check on the include!-ed runner and FFI unit files (lesson 25: they had never been formatted; the reformat is in its own commit).
  • clippy --workspace --all-targets [--features difftest/cpp] -D warnings, cargo test -p solidcore (1369 passed), cargo check --workspace --all-targets and -p difftest --features cpp, wasm32 check.
  • --fuzz 2000: FUZZ-PASS, 0 crashes/panics/timeouts, 0 differs.
  • Full green check (sc-remote green, 207 units, failed=0).
  • valgrind (C++ and Rust in one process, unmodified generated programs, compared with a baseline run): 0 errors. 90 generated programs (face-from-curves 25, glue 15, merge-self 15, reads 15, make-face 10, copies 10), plus the 32 built-in face-from-curves and 6 copy cases.
  • The 300 built-in cases keep their outcomes; the one deferral that changed is intended: face_curves_4 (a curve on surface over a plane) now defers to curve.crv_on_surf instead of topology.brep_p9.
  • The topology.edgeuse_p1, topology.edgeuse_p2 and topology.loopuse runners now name topology.brep_p10 MakeTopologyFromData for their inexact-import deferrals (200, 225 and 87 cases); their reports differ only in those labels. topology.vertex had its own copy of the label (relabelled, 224 cases), and topology.edgeuse_p3 shares edgeuse_p2's (381 cases).
  • gen0014 changed from pass to deferred, by an input change from main. Since topology.shell (port: topology.shell #154) the shared fixture walk registers wire edges, so this generated glue now draws a wire-network edge pair instead of a pair that took the no-op early return. The C++ reaches SmBrep::Notify on it and the Rust defers there (topology.brep_p2), which is correct; the port did not regress. Hence 197 passes / 119 deferred (was 198 / 118).
  • The FFI wrapper uses ffi: share the validate Reader through solidcore_ffi::reader #186's shared reader (unit_reader!(MAX_COORD)); only the unit's signed and orient checks stay local.
  • Merging main (ffi: share the validate Reader through solidcore_ffi::reader #186, port: offset.offset_executive_p1 #175, port: topology.edgeuse_p3 #117, fix main build: offset executive passes the kernel and a mutable B-rep #230, port: topology.brep_p6 #169) and wiring changed no other case: brep_p6, offset_executive_p1, intersector_p1, trimming_tools and shell are byte-identical.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA

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

bluecmd and others added 6 commits October 16, 2026 16:13
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQThHpN1ijFAC69dfmSevR
ReadFromFile, MakeFace, MakeFaceWithCurves, GlueEdgesGeneral, CopyFaces,
MergeSelfIntersectionCurves; calls into unported units are UNPORTED stubs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQThHpN1ijFAC69dfmSevR
difftest: PARTIAL topology.brep_p7 cases=415 passed=276 failed=0 deferred=139;
--perturb SENSITIVE 276/276.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQThHpN1ijFAC69dfmSevR
Wire trimming_tools' MakeFaceWithCurves back-edge; flags of degeneracy tests do
not abort; CrvOnSurf on other surfaces is Unported(curve.crv_on_surf); FFI no
longer over-rejects; C++ guard deferrals labelled cpp-guard:*; no both-errored
cases. difftest: PARTIAL cases=310 passed=169 failed=0 deferred=141; SENSITIVE.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQThHpN1ijFAC69dfmSevR
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQThHpN1ijFAC69dfmSevR
difftest: PARTIAL topology.brep_p7 cases=310 passed=169 failed=0 deferred=141
(cpp-guard:unvalidated-topology 48, cpp-guard:glue-shared-vertices 8);
--perturb SENSITIVE 169/169; cases_both_errored 0; green 191 units, failed=0.

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

bluecmd commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

First full difftest run: topology.brep_p7 is PARTIAL with 310 cases: 169 pass, 0 fail and 141 deferred, --perturb sensitive 169/169. The topology-building paths wait on #130 and #154, so the passes so far are error paths and no-ops. Still a draft; an independent pre-review is running before it's marked ready.

🤖 Generated with Claude Code

bluecmd and others added 7 commits October 16, 2026 21:30
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MQThHpN1ijFAC69dfmSevR
…w fixes

PARTIAL topology.brep_p7 cases=330 passed=184 failed=0 deferred=146;
--perturb SENSITIVE 184/184; 0 both-errored.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
Brings in intersect.curve_class_p7 (tangency-labs#65).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
Brings in boolean.merge_p5 (tangency-labs#161) and boolean.merge_p1 (tangency-labs#163). The
plan/findings.md conflict keeps both sections, main's first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
bherbruck added a commit that referenced this pull request Oct 17, 2026
surface_p8 merged, but brep_p1/p3/p4/p5/p6/p7/p9 are still todo on main
(PRs #145, #173, #169, #155 unmerged; p3/p5/p9 have none). Unit stays blocked.
Resolves the plan/findings.md conflict by keeping both sections.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BY8UpXKuMci15rAWGXsbsm
bluecmd and others added 4 commits October 17, 2026 04:10
- Must 1: a curve on surface with a gap target defers at
  DeformToEndPointTargets (UNPORTED(curve.curve_p6), pOptSurfCrvOnSurf
  branch) instead of taking the port's approximation path; other curve kinds
  and target-less calls still use the port, as upstream dispatches them.
- crv_on_surf gap uses main's label and tag (curve.crv_on_surf).
- C++ read guard decided on both sides with the C++ SmBrepData reader; the
  45 comment-only damaged reads are dropped; revisit note and the new
  [PSIRT] UB-164 (MakeTopologyFromData uses unchecked cross-reference
  indices; 11 of 600 accepted single-byte damages crash).
- Glue guard keeps only the null-curve -2; shared-vertex pairs run in C++
  and defer at brep_p2 (pairs (0,4) and (2,5) added).
- -2 guard echoes (UB-165, null curve, null surface) are deferrals.
- MakeFace kind 1: the Rust side mirrors the shim's -3 and otherwise defers
  at MakeShellVertex (brep_p9); make_face_tiny_1e-12 renamed.
- MakeFaceWithCurves null-UV entry renumbered UB-163 -> UB-165 (tangency-labs#164 took
  UB-163 on main).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
Brings in heal.heal_data_p3 (tangency-labs#164) and the main commits since f72938d. The
plan/upstream-issues.md conflict keeps tangency-labs#164's UB-163 entry verbatim, after
this PR's UB-165 (renumbered from UB-163).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
…ew fixes

PARTIAL topology.brep_p7 cases=315 passed=197 failed=0 deferred=118;
173 passes compare values, 24 a status only; SENSITIVE 197/197.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
- curve.curve_p6 notes record the dropped pOptSurfCrvOnSurf branch.
- face_curves_cos_open_loop: an open UV-only loop with crv_on_surf TRUE
  reaches the curve.curve_p6 deferral (316 cases, 119 deferred).
- Drop the built-in damage list the brep_data guard rejected for every
  file; fix the notes wording.
- UB-164 line references :20342 / :20343.
- Glue -2 guard narrowed to upstream's dereference: SM_OT_SAME, equal
  intervals and exactly one curve missing.
- Runner refuses unknown crv_on_surf codes (lesson 22).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
@bluecmd
bluecmd marked this pull request as ready for review October 17, 2026 05:34
@bluecmd

bluecmd commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

Ready for review. topology.brep_p7 is PARTIAL with 316 cases: 197 pass, 0 fail and 119 deferred, --perturb sensitive 197/197, and 0 both-errored. Of the passes, 173 compare values and 24 compare only a status. The rest defer at brep_p2/p10/p9/p1/p6 or at C++ crash guards. The trimming_tools back-edge (MakeFaceWithCurves) is wired: 3263 cases, 2842 pass, 0 fail. Every green/partial unit's difftest passes (198 units, 0 failed), and fmt, clippy, tests and wasm are clean at bbdb8bf1.

Upstream issues: UB-165 (MakeFaceWithCurves with curves on surface and no UV curves) and a [PSIRT]-class entry UB-164 (MakeTopologyFromData). Details are in plan/upstream-issues.md.

Three independent pre-review rounds went before this. They found a faked curve-on-surface deform path, which now defers, and they measured every C++ guard for over-rejection.

CI needs a maintainer's approval to run on this fork PR.

🤖 Generated with Claude Code

bluecmd and others added 5 commits October 17, 2026 17:53
Brings in topology.face_p1 (tangency-labs#148) and intersect.curve_class_p8 (tangency-labs#152).
findings.md conflict: main's face_p1 section kept verbatim first, then
the brep_p7 section.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
The generator drew reads, glue, merge-self and MakeFace only, an
oversight. Faces from curves (closed and open square loops on planes
and patches, 3D/UV/both, every crv_on_surf code) and copies now go
through the same guards as the built-in cases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
Brings in tangency-labs#176 (difftest --fuzz, with_values/no_values).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
…report after the main merge

cargo fmt skips these include!-ed files. The report is the post-merge
run with MakeFaceWithCurves and CopyFaces in the generator: 316 cases,
198 pass (with_values=198 no_values=0), 118 deferred.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA8bchjV6Km7PFkmnvYBTA
…the extended generator

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

bluecmd commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

Merged origin/main (#148, #152, #176) and refreshed the numbers in the description. topology.brep_p7 is now 316 cases: 198 pass, 0 fail, 118 deferred, with with_values=198 and no_values=0. --fuzz 2000 gives FUZZ-PASS, valgrind is clean, and green passes on 200 units.

Two other changes since the last push. The generator now also draws MakeFaceWithCurves and CopyFaces. The include!-ed runner and FFI files have been rustfmt-ed, in a commit of their own.

🤖 Generated with Claude Code

@bluecmd

bluecmd commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

@bherbruck, this is refreshed against main at f787d107: main merged up to e6414686, so it merges cleanly again. It adds no change to this unit's review points; it only follows main:

  • topology.shell (port: topology.shell #154): MergeShells calls SmShell::IsInnerShell.
  • topology.brep_p6 (port: topology.brep_p6 #169):
    • MakeFace calls MakeFaceTopology with upstream's arguments.
    • GlueEdgesGeneral's region nesting calls TestClosureSplitRegion and ignores its status, as upstream does. Each call resets rpNewRegion.
  • Back-edge: boolean.topology_intersector_p1 reaches GlueEdgesGeneral through a new IntersectorBrep::brep_glue_edges_general method with upstream's arguments. No case reaches it yet, and its report is byte-identical.
  • Relabelled, not wired: offset.offset_executive_p1's MakeFaceWithCurves stub now names its real first blocker, GetInfiniteRegion (topology.brep_p1, its first argument). It still does not call this unit. Upstream ignores that call's failure, so the eventual wiring must not pass the error up.
  • ffi: share the validate Reader through solidcore_ffi::reader #186: the FFI wrapper uses the shared reader.
  • Relabel: topology.vertex and topology.edgeuse_p3 (from port: topology.edgeuse_p3 #117) get the same topology.brep_p10 MakeTopologyFromData label as the other three runners. Their reports differ only in labels.

Two deferrals changed, both expected:

  • gen0014 now defers at SmBrep::Notify instead of passing. Since port: topology.shell #154 the shared fixture walk registers wire edges, so this generated glue draws a wire-network pair. The C++ also goes past the early return to Notify, so this is a changed input, not a regression.
  • face_curves_cos_no_uv_1_1 moved from brep_p6 to SmFace::SetTolerance inside it (topology.face_p3).
PARTIAL topology.brep_p7 cases=316 passed=197 failed=0 deferred=119
SENSITIVE topology.brep_p7 perturbed=197 detected=197
FUZZ-PASS topology.brep_p7 cases=2000 differs=0 crashes=0 panics=0 timeouts=0

Workspace check, clippy, 1369 tests, wasm and the full green check (207 units, 0 failed) pass. The description has the details.

🤖 Generated with Claude Code

bherbruck added a commit that referenced this pull request Oct 19, 2026
brep_p6 merged as partial; brep_p1/p3/p4/p5/p7/p9 still todo on main.
Their open PRs (#145, #204, #173, #194, #155, #183) do not complete a
face construction yet, so the unit stays blocked.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W8ERxmTxLqSjFQKckiXC5a
@bherbruck

Copy link
Copy Markdown
Collaborator

Thanks for the thorough follow-up, @bluecmd. I reviewed head f787d107 and it's good to merge once CI is green (CI has already passed on this head).

Earlier review (a5359b4): every item is fixed

  • Must-fix: stand-in CrvOnSurf base (p7.rs:676). Fixed. Only a plain SurfaceKind::BSpline builds a CrvOnSurf now (p7.rs:694-705). Planes and surfaces of revolution return Unported(curve.crv_on_surf), as main does.
  • MergeSelfIntersectionCurves loop shape. Fixed. Checked against SmBrep.cpp:22701-22790: skip when size ≠ 1 before the merge, SM_ERR after it, SE + continue for a middle not on an edge, then the GlueVertices/GlueEdgesGeneral order. MergeIntervals resets pOptNewFaces (SmCurveClass.cpp:18610), so returning fresh faces per call is faithful.
  • Deferral labels in the edgeuse_p1/p2, loopuse, vertex and edgeuse_p3 runners. Fixed. They now point at topology.brep_p10 MakeTopologyFromData.

I also checked the new wiring since then. MakeFace → p6::make_face_topology restores the owner whether the build fails or succeeds (SmBrep.cpp:18870). GlueEdgesGeneral's region nesting matches :10443-10471. The trimming_tools and topology_intersector_p1 back-edges pass upstream's arguments (SmTrimmingTools.cpp:156, pInsideFaceuses NULL).

Gates (all pass)

  • cargo fmt --check, plus rustfmt --edition 2021 --check on the include!d unit files. The PR's own files are clean. The edgeuse_p1/p2, loopuse and vertex runners have the same diffs as main, so nothing new.
  • clippy -D warnings, both plain and with solidcore-ffi/cpp,difftest/cpp.
  • cargo test -p solidcore: 1369 tests pass.
  • The wasm32 check.
  • git merge-tree against origin/main is clean.

Extra adversarial cases

  • I added 497 hand-written cases (not pushed):
    • MakeFaceWithCurves at scales from 1e-7 to 1e6;
    • corner gaps from 0 to 0.5, bracketing the tolerance at 1e-6, 1e-5 and 1e-4;
    • zero-length and 1e-9 curves, including loops made only of degenerate curves;
    • odd loop-count shapes, empty and inverted domains, near-zero and tilted plane normals, loop points;
    • MakeFace sides from 1e-11 to 1e9;
    • glue with gaps of -1, 0 and 1e3 in every orientation;
    • MergeSelf and Copy with extreme tolerances and counts.
  • I also ran 1,800 generated cases (--seeds 1500) and --fuzz 3000.
  • Results: 0 real failures, 0 crashes, 0 panics.
    • The 4 "failures" are inputs I wrote whose loop counts exceed the curves. The FFI guard rejects them, which is correct, because upstream reads past cp3DCurves.
PARTIAL topology.brep_p7 cases=316 passed=197 failed=0 deferred=119 max_rel_err=0e0 with_values=197 no_values=0
extra (497 hand + 1800 generated): passed=852 failed=0 real (4 guard-rejected invalid inputs) deferred=1757
SENSITIVE topology.brep_p7 perturbed=403 detected=403
FUZZ-PASS topology.brep_p7 cases=3000 rejected=557 malformed=573 ran=1870 differs=0 crashes=0 panics=0 timeouts=0
partial topology.trimming_tools cases=3263 passed=2842 failed=0 deferred=421
partial boolean.topology_intersector_p1 cases=827 passed=653 failed=0 deferred=174
partial offset.offset_executive_p1 cases=693 passed=675 failed=0 deferred=17
partial topology.edgeuse_p1/p2/p3, loopuse, vertex: 0 failed
--green: 207 unit(s), 113 passed, 94 partial, 0 failed

Should-fix (optional, can be a follow-up)

  • p7.rs:127-133: the GetInfiniteRegion stub. Upstream's getter is an inline field read (SmBrep.h:736), and main already exposes it as Brep::infinite_region(); p6.rs:123 and shell.rs:359 use it. The stub is allowed, since topology.brep_p1 is todo. But returning b.infinite_region().ok_or_else(..) would stop 12 cases (197 in the seeded run) deferring at it. The relabelled offset stub (offset_executive/p1.rs:815) could use the same accessor and call make_face_with_curves directly.

🤖 Generated with Claude Code

@bluecmd

bluecmd commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the re-review and the extra adversarial runs, @bherbruck.

I'll leave f787d107 as it is. Your GetInfiniteRegion should-fix will come as a small follow-up PR once this merges. It reads Brep::infinite_region() (the inline m_pInfiniteRegion read) in MakeFace, MakeFaceWithCurves, CopyFaces and the glue's shell merge, and keeps a NULL region as None.

The reset after a glue removes an empty region (m_pInfiniteRegion = m_pList) stays a stub, because the B-rep's region list order isn't modelled. The offset site needs that unit's surface ownership and face identity settled, and upstream ignores the call's status there, so I've left it for its porter.

🤖 Generated with Claude Code

…o unit/topology.brep_p7

Merge only. Both sides kept: the IntersectorBrep trait gets both
brep_glue_edges_general and tangency-labs#145's brep_edges/brep_vertices, the runner both
imports, findings.md and upstream-issues.md both appended sections (UB-162
before UB-165).

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

bluecmd commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

merged main (#145/#196) to resolve conflicts, no other change

🤖 Generated with Claude Code

@bherbruck
bherbruck merged commit 9184d78 into tangency-labs:main Oct 19, 2026
7 checks passed
@bherbruck

Copy link
Copy Markdown
Collaborator

Merged, thanks @bluecmd!

🤖 Generated with Claude Code

denelson1-dot added a commit to denelson1-dot/solidcore that referenced this pull request Oct 19, 2026
…gency-labs#214, tangency-labs#234) into unit/blend.fillet_solver_p3

Conflict: plan/upstream-issues.md, where main's UB-165 and this branch's
UB-166 were added at the same place; kept both, in number order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bluecmd added a commit to bluecmd/solidcore that referenced this pull request Oct 19, 2026
…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
denelson1-dot added a commit to denelson1-dot/solidcore that referenced this pull request Oct 19, 2026
…gency-labs#214, tangency-labs#234) into unit/query.face_p1

Conflict: plan/findings.md, both new sections kept.

topology.brep_p7 is now ported, so the query.face_p1 runner no longer labels
its 31 cases on analytic geometry that the shared B-rep export flattens to
B-splines as topology.brep_p7 (SmBrep::ReadFromFile); they carry main's
harness:analytic-geometry-export label, as in topology.brep_p1 and
heal.stitch_p1. Counts unchanged.

Difftest after the merge (0 failed; --green 210 units, 113 pass, 97
partial): query.face_p1 1535/689 (SENSITIVE 1535/1535), topology.shell
896/15, topology.brep_cache 662/1, mass.integrator 1521/7,
query.api_queries 743/460.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bluecmd added a commit to bluecmd/solidcore that referenced this pull request Oct 19, 2026
, tangency-labs#234, tangency-labs#196, tangency-labs#145) into unit/topology.brep_p2

Merge-only, to resolve a plan/modules.toml conflict: the topology.edgeuse_p3
note keeps main's text and appends tangency-labs#149's sentence (lesson 20).

Plus one argument threaded at topology/brep/p7.rs:1310 for tangency-labs#149's NotifyEnv:
tangency-labs#155's glue_edges_general called p6::test_closure_split_region_attempt with
its pre-tangency-labs#149 signature; it already takes `env`, which is passed through
(overseer-approved; tangency-labs#204 resolves it identically). No other change.

Checked: cargo check (workspace, all targets, with and without cpp; wasm);
difftest case by case against the merged reports: topology.brep_p2
1629/1290/0/339 (report byte-identical), topology.brep_p7 316/197/0/119,
heal.stitch_p1 300/206/0/94, topology.brep_p6, heal.heal_data_p1/p2/p3 and
boolean.topology_intersector_p1 unchanged.

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

crates/solidcore/src/boolean/topology_intersector/p1.rs: take main's side
(tangency-labs#155 moved GlueEdgesGeneral behind IntersectorBrep and dropped the stub).

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

tangency-labs#155 (topology.brep_p7) replaced part 1's GlueEdgesGeneral stub with a trait
method. EdgeFaceCoincidence now calls it (pInsideFaceuses NULL, as upstream)
and takes the kernel mutably. The call sits behind SmFace::GetBrep
(topology.face_p4), so no case reaches it yet.

Difftest unchanged case by case apart from iir_negative_tol / iir_tiny_tol,
which now defer at SmBrep::GetMinEdgeBBoxSize (topology.brep_p2) instead of
topology.brep_p1: 841 cases, 657 pass, 0 fail, 184 deferred.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VhgnDJqMfisAk17esFqRL5
@bluecmd bluecmd mentioned this pull request Oct 19, 2026
@bluecmd bluecmd mentioned this pull request Oct 19, 2026
bluecmd added a commit to bluecmd/solidcore that referenced this pull request Oct 19, 2026
Brings in topology.brep_p7 (tangency-labs#155), tangency-labs#214 and tangency-labs#234. One conflict,
plan/upstream-issues.md, resolved keeping both sides: main's UB-165
(brep_p7) before this branch's UB-166.

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

Conflicts (lesson 20): IntersectorBrep keeps both hooks, tangency-labs#204's
brep_delete_face and tangency-labs#155's brep_glue_edges_general, and both private
stubs go (each side had wired one); the runner keeps both
implementations, with_edit_env and glue_key_of; topology.edgeuse_p3's
notes take main's line verbatim plus tangency-labs#149's re-appended UPDATE.

Lesson 32: tangency-labs#155's topology/brep/p7.rs stubs UNPORTED(topology.brep_p3)
MakeWireEdge / MakeWireEdgeVertex are wired to brep::p3 through
with_kernel_callbacks under the new shared brep::p3::default_key_of
(heal.stitch_p1's squeeze uses it too); p7::make_face_with_curves,
p7::copy_faces and trimming_tools' trim_surface_with_model_space_curves
take &mut Kernel (curve identities come from its context). Integration
fix: p7's glue_edges_general passes its env to tangency-labs#149's
p6::test_closure_split_region_attempt (which gained the env in tangency-labs#149).
Notes appended for topology.brep_p7 and topology.trimming_tools.

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

- topology.brep_p7 (tangency-labs#155): SmBrep::Notify (GlueEdgesGeneral's PRE_EDIT and
  MERGE_IN_BREP through the caller's NotifyEnv), SetTolerance (CopyFaces),
  sm_CheckAndFixUVTrimCurves and RebuildUVTrimCurves (ReadFromFile), statuses
  ignored as upstream.
- heal.stitch_p1 (tangency-labs#196): SmBrep::Notify(SM_NO_PRE_EDIT) on an empty notify
  state (no B-rep cache, no attribute state on the heal path).
- topology.brep_p10 (tangency-labs#130): IsManifoldSolid, sm_GetPointFromSol and
  GetShellVertices; MergeCurvesOnSurface retagged UNPORTED(query.brep) (no
  MergeCurvesObjects world outside query.brep's B-rep cache).
- topology.brep_p2: ConnectIntervals calls intersect.curve_class_p2 (tangency-labs#85).

No UNPORTED(topology.brep_p2) remains. Difftest case by case: brep_p10 12
deferred -> pass (201-16358 values), 2 -> topology.face_p2; brep_p7 51
deferrals relabel (face_p3 43, brep_p1 7, brep_p4 1); brep_p2, stitch_p1,
intersector p1, offset executive p1, merge p1 unchanged; 0 fails. Three unit
tests of p7/p10 updated to the newly reachable results. fmt, clippy (both),
cargo test 1400, runner tests, wasm pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xbnf2bFY8cTu4zUgAv9s85
denelson1-dot added a commit to denelson1-dot/solidcore that referenced this pull request Oct 19, 2026
…cy-labs#130 brep_p10, tangency-labs#155 brep_p7, tangency-labs#153 cone_p2, tangency-labs#179) into unit/topology.face_p4

Wire the new SmFace::GetBrep / GetVertices / GetEdges stubs of
topology.brep_p7 and topology.brep_p10 to topology::face::p4 (default
arguments; latent, both units' counts unchanged).

tangency-labs#85 un-defers the 50 periodic HasSeamProblem cases, which then reach
part 5's ClassGeometry::vertex_face_gap. The runner's classification
objects now answer it as upstream does
(GetVertexuseOfFace(face)->GetVertexFaceGap()->GetLength(), 0 without a
gap), on a copy of the B-rep since the getter fills a lazy cache; the
cases go on to defer at intersect.curve_class_p4.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

3 participants