Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…gy_intersector_p3 Resolve against main: keep main's merge_intervals_with_partial (PR tangency-labs#164) and thread tangency-labs#85's k/geometry through it; update main's newer callers (face_p1 SplitAtSeam/MoveSeam, heal_data_p3) to the new MergeIntervals signature. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ogy_intersector_p3 Brings the full SmCurveClassification constructor (with_curve), which MergeCurveOnSurfaces needs; its ReSet comes from tangency-labs#85. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…Surfaces, MergeCoincidentSurfaces (WIP, no runner yet) Full bodies with a stub at each unported call site (query.face_p1, topology.face_p4, topology.brep_p4). IntersectorBrepP3 adds the surface, edge, loopuse, edgeuse and faceuse walks part 3 needs. Part 2: add check_for_squeeze_vertices_into, which keeps upstream's partial appends when its second conflict check fails (MergeCurveClasses carries on with them); let-else for clippy 1.98. curve_class_p9 test: with tangency-labs#85's ReSet merged the deferral moves to the edge branch of sm_InsertSolutions (UNPORTED(topology.edge)). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…, exact guards, fuzz fixes - check_for_squeeze_vertices_into appends to the caller's lists as upstream's out-params do (the first conflict block appends, the second can still return SM_ERR), with check_for_squeeze_vertices as a wrapper; suggested by oursland for tangency-labs#211 (MergeCurveClasses keeps going after a failure), re-implemented here. - Stubs carry every upstream argument with the defaults passed explicitly (DeleteFace, DeleteEdge, GetFaces, Point3DClassify). - FixGaps: NER of both mates before the vertex casts, as upstream. - Shim/runner: the B-rep guards of FixGaps and CheckForSqueezeVertices are exact (only where a combination would run); IntersectInsertRelate needs a positive tolerance and coordinates below 1e154 (the full C++ intersection does not finish otherwise: 3 fuzz timeouts; the port defers it anyway). - p1 runner: no panic on a surface without knots (fuzz). - UB-169, WR-170; proto_topology_p3 notes: RemoveRelationship wired. difftest 835 cases, 655 pass, 0 fail, 180 deferred; FUZZ-PASS 2000. Co-authored-by: Jacob Oursland <jacob.oursland@gmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VhgnDJqMfisAk17esFqRL5
…curve_class_p2 into p4; port sm_MakeConsistentClassifications - Runner/shim/FFI for MergeCurveOnSurfaces, MergeCurveClasses and MergeCoincidentSurfaces on two real B-reps (reusing the p1 runner's two-B-rep adapter via pub(super); p1 FFI gains blocks_len). The shim also reports each vertex's use-list order, which the shared import does not carry; the runner reorders the imported lists so GetEdges matches upstream. - intersect.curve_class_p4: ReSet / ConnectIntervals / GetMinUV / GetMaxUV stubs wired to part 2 (tangency-labs#85), now merged here. - intersect.curve_class_p1: sm_MakeConsistentClassifications ported in full through the ClassGeometry topology hooks (was a stub on topology.edge, which is partial); ClassGeometry gains topology_handle_in for adapters over several B-reps. - face_p2 test: the trim now reaches the face classification (SM_ERR). difftest: PARTIAL boolean.topology_intersector_p3 cases=128 passed=61 failed=0 deferred=67. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… edge-pair tolerance through the topology hooks topology.edge is partial; the part-7 calls with classification geometry in scope (AreCoincidentMaybe, SetDoFilletingGeometry, the partial-coincidence test) now read the edge through ClassGeometry::topology_entity. An adapter that cannot resolve a handle (no hooks, or a stand-in edge without a B-rep, as in the curve_class_p7 runner) keeps the previous Unported deferral; the same applies to sm_MakeConsistentClassifications in part 1. difftest: PARTIAL boolean.topology_intersector_p3 cases=128 passed=65 failed=0; intersect.curve_class_p7 1077 passed, 0 failed (unchanged); intersect.curve_class_p1 2048 passed, 0 failed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… cases, plan notes and reports - Runner: at least 256 generated cases plus 11 inline ones (coincident edges, conflicting relations, the wire network, a refused null curve, tiny tolerance, coincident surfaces); vertex uses the export walk never registers are skipped when reordering. - Shim guards (fuzzed, 2000 cases, 0 crashes): classification specs are checked before construction, on the raw class/object pairs (SetClassObject turns a vertex/edge/face class without object into UNKNOWN with its parameter set, which the merge then crashes on); the FFI rejects doing_fillets (the fillet path needs a fillet context). - intersect.curve_class_p4 shim: refuse GetLoopCrossings on an empty classification (reachable after a squeeze now that ConnectIntervals is wired; upstream reads GetAt(0) unchecked). - plan/modules.toml: boolean.topology_intersector_p3 partial; notes on curve_class_p1 / _p4 / _p7 and topology_intersector_p2. - plan/findings.md: the shared import's vertex-use order; units failing in a Debian/GCC 12 container also on main; this unit's section. - Reports regenerated for p3 (new), curve_class_p4 (2222 passed, was 1318), curve_class_p6 (4216, was 4097), query.face_p2 and topology.face_p2 (deferrals re-attributed). PARTIAL boolean.topology_intersector_p3 cases=267 passed=140 failed=0 deferred=127; --perturb SENSITIVE 140/140; --fuzz 2000 FUZZ-PASS. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
main brings topology.edgeuse_p3 (tangency-labs#117), surface.offset_surface (tangency-labs#62), topology.brep_p6 (tangency-labs#169) and tangency-labs#146 itself. edge.rs takes main's wired GetOrCreateUVTrimCurve; the EvaluateBinormal stub tangency-labs#137 called through topology::edge moves into curve_class_p4 (the real edgeuse_p3 function needs &mut Brep, which part 4 cannot reach through ClassGeometry). Conflicting reports take main's copies (regenerated next). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…to boolean.topology_intersector_p3 tangency-labs#203 adopted check_for_squeeze_vertices_into; its side is taken for p2.rs, its notes for the unit, and its additions to the p1 runner's shared items. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ean.topology_intersector_p3 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tor) and wire query.face_p2's CurveOnClassify comparison - IntersectorBrepP3::edge_find_radial_sector takes &mut self: main's find_radial_sector_between can build missing UV trim curves. - query.face_p2 runner: with the classification constructor's ReSet available (tangency-labs#85 via tangency-labs#137), CurveOnClassify no longer defers; the Rust side now builds the line (CreateLineSegment) and the classification (full constructor) as the shim does and compares instead of asserting a deferral. - Reports regenerated for the merged state. --green (Linux container, GCC 12): 210 units, 112 passed, 94 partial, 4 failed; the 4 (heal.heal_data_p1 3, _p2 104, _p3 7, geom.hash_table 435) fail identically on origin/main d045b29 in the same container. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…o unit/boolean.topology_intersector_p3 curve_class p4 takes tangency-labs#137's EvaluateBinormal wiring (status ignored as upstream); the header drops the now-empty "not ported yet" bullet and keeps the part-2 callee note. modules.toml combines both notes; reports are regenerated after the follow-up merges. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ersector_p3 tangency-labs#85's own merges of main resolved the same spots as tangency-labs#211's earlier merge; take tangency-labs#85's side in each conflict hunk (it lands first): the query.face_p2 runner's CurveOnClassify comparison, SplitAtSeam's merges passing the caller's geometry (NoClassGeometry dropped), the p9 test assertion. Reports are regenerated after merging main. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…into unit/boolean.topology_intersector_p3 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…and main boolean.topology_intersector_p3 unchanged (267 / 139 / 0 / 128); every touched unit 0 failed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- findings/notes: say outright what op 52 leaves uncovered (no pass reaches BrepCurveSolve, so the coincidence branch, IIREdgeBrep and new topology have no compared case) and why the drop tolerance is unobservable here (under SM_SO_MINIMIZE it does not gate bSuccess and no snap vector is passed, so a 0.5x mutant survives and a near-tolerance case would not tell them apart). - merge_curve_classes stub: note that wiring tangency-labs#211's method changes iir_edge_brep's call site and widens its bound to IntersectorBrepP3. - runner vertex_common_edges: a vertex of the other B-rep (or no vertex) gives no common edge, as upstream, instead of matching edges with a missing end. No case changes (905/725/0/180, identical case by case). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VhgnDJqMfisAk17esFqRL5
…; wire query.face_p1 Merge resolutions: main's runner helpers kept beside part 1's pub(super) entity_h; doc comments combined (part 4's ReSet / ConnectIntervals / GetMinUV / GetMaxUV stay wired to part 2 here, still stubs on main); main's notes for other units, with tangency-labs#211's own sentences carried; reports from main (regenerated next). Adapting to main: - CheckForSqueezeVertices now combines coincident vertices (tangency-labs#274, brep_p5): it takes &mut Kernel and IntersectorBrepP2, so IntersectorBrepP3 extends IntersectorBrepP2 and the merges take &mut Kernel. - p1's face_edges takes GetEdges' optional arguments: (None, false, None), upstream's defaults at every call here. query.face_p1 is partial on main, so its two stubs are wired: SmFace::Point3DClassify and SmFace::CalculateBoundingBox (bTight FALSE) are IntersectorBrepP3 hooks the runner implements with query::face::p1. dCurveTolerance reaches the point classifications as SmZoneTol3d (zone_tol3d), as upstream's implicit conversion does. Back-edge: blend.fillet_geom_p4's MergeCurveOnSurfaces stub is retagged blend.fillet_executive_p1 (it runs on the executive's intersector, like its RegisterExistingSSI stub). The p3 runner is rustfmt-clean. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…7's B-spline cast - runner: the merges take &mut Kernel. - intersect.curve_class_p7 TestForPartialCoincidentEdges: copy the B-spline base of a coincident edge curve of any kind derived from SmBSplineCurve (SmLine, SmEllipse/SmCircle, SmHyperbola, SmParabola), as SM_CAST_PTR does; it rejected all but a plain B-spline. Found by this unit's fuzz (3 inputs: C++ SM_ERR, Rust InvalidInput); 2000 fuzz cases now 0 differs. curve_class_p7's own difftest is unchanged (1516 / 1077 / 0 / 439). - difftest: 267 cases, 143 passed, 0 failed, 124 deferred; --perturb 143/143; --fuzz 2000 FUZZ-PASS. - plan: notes, findings (query.face_p1 wiring, the p7 cast, back-edges; the container failures are arm64, not GCC 12). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…aceholder handles CI merges tangency-labs#211 into main, where topology.brep_p2 (new on main) failed one fixture case: merge_plane10_line deferred at part 4's ReSet stub, which tangency-labs#211 wires; it then reached a classification needing a face's geometry, and the runner's MergeObjects names only the case's curves (every face, edge and vertex is one placeholder without geometry). The runner now defers that error under harness:merge-objects, a gap of the runner, not of the port: topology.brep_p2 1709 / 1437 / 0 / 272. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Thanks @oursland, this is a big, careful port. I reviewed head dfa56606 against SmTopologyIntersector.cpp (MergeCurveClasses 3001-3889, MergeCurveOnSurfaces 3915-4329, MergeCoincidentSurfaces 4347-4891) and sm_MakeConsistentClassifications (SmCurveClass.cpp 3048-3275).
What passed
-
cargo fmt --all --check, clippy-D warnings(plain and withsolidcore-ffi/cpp,difftest/cpp),cargo test -p solidcore(1526 passed), wasm32 check. The branch merges cleanly into currentmain(0a676a04, 83 commits ahead). -
The port follows upstream closely in every branch I traced. Examples:
eSqueezeStatcarries across iterations, the start-vertex block only checks deleted edges while the end block also checks deleted vertices, and the edge repair keeps upstream'sGetBrepMate(pE1)in the second branch. The deleted lists stay untouched on the earlySM_SUCCESSreturns and are replaced on eachMergeCurveClasses. The twosEdgesB/lOEdgesCountedge-count baselines inMergeCoincidentSurfacesdiffer, as upstream's do. TheSM_CAST_PTR(SmBSplineCurve, ..)fix in p7 is correct:SmLine,SmEllipse/SmCircle,SmHyperbolaandSmParaboladerive fromSmBSplineCurve, andSmConicdoes not. -
488 extra adversarial cases through difftest. All 8 fixture files were covered, with these inputs:
- surfaces without edges, so the
CurveOnClassify/SetIntersectDatapath runs; - one edge plus one surface, and both surfaces plus both edges;
- tolerances
1e-15,1e-6,1e-2and1e3, so the degeneratePoint3DClassify/MergeIntoObjectpath runs; - mismatched edges with
ignore_tangency; - cross-file coincident surfaces;
- short sub-range classifications that hit the
/ 20.0skip; - vertex/edge end mixes with and without
merge_singularities; - face-mid classifications.
487 ran: 242 passed, 245 deferred, 0 failed. One crashed the C++ side (must-fix 1).
- surfaces without edges, so the
PARTIAL boolean.topology_intersector_p3 cases=267 passed=143 failed=0 deferred=124 max_rel_err=0e0 with_values=143 no_values=0 blocked_by=intersect.curve_class_p6(44),topology.brep_p1(4),topology.brep_p3(4),topology.edge(36),topology.face_p4(28),topology.vertex(8)
difftest: 235 unit(s), 114 passed, 121 partial, 0 failed (--green, x86-64)
extra cases (local, by file group): 487 ran, 242 passed, 245 deferred, 0 failed; 1 SIGSEGV in C++ (below)
Must-fix
-
The shim segfaults on a classification that the FFI and the runner accept.
- Location:
harness/cpp/shim/boolean_topology_intersector_p3.cpp:263(spec_ok) andcrates/difftest/src/units/boolean_topology_intersector_p3.rs:509(class_ok). - Input:
box_with_large_fillet.smbtwice,merge: curve_classes. Each side hascurve = [b,3,0],f0 = 0.25,f1 = 0.75, start and endSM_PC_UNKNOWNwith a null object, and midSM_PC_FACEon[b,2,0]. - Result:
difftestdies with SIGSEGV inSmCurveClassification::MergeInterval, called fromMergeClassificationsandMergeCurveClasses. The whole--unit/--greenrun is lost. - Cause: upstream
SmCurveClass.cpp:17320triessm_ComputeFacePointClassfor the Unknown end of a face-mid interval. When the point does not project onto that face, the end stays Unknown with no object.pStartVis then NULL, and it is dereferenced at:17659(pStartV->GetPoint(), where both ends are NULL) or at:17672(pStartV->GetBrep()). - Fix: in both
spec_okandclass_ok, refuse anSM_PC_UNKNOWNstart or end whose interval mid isSM_PC_FACEorSM_PC_REGION. Report it asNULL_DEREF, as for the other refused inputs, and add this case as an inline edge case. If you want those inputs kept, the exact guard is to run the same face projection first. A face-mid class with vertex ends ran fine in all 8 files. - Why the fuzz missed it:
gen_classonly generatesSM_PC_EDGEmids.
- Location:
-
rustfmt --edition 2021 --checkfails on the newinclude!d FFI file.crates/solidcore-ffi/src/units/boolean_topology_intersector_p3.rshas diffs at lines 161, 318 and 339.cargo fmtdoes not seeinclude!d files, so CI passes. Fix: runrustfmt --edition 2021on that file. The runner file is already clean.
Should-fix (short)
boolean.merge_p1still hasUNPORTED(boolean.topology_intersector_p3)(crates/solidcore/src/boolean/merge/p1.rs:186) now that this unit ispartial. You documented why, and every path throughMergeCoincidentSurfacesdefers ontopology.face_p4(GetEdges) today, so no compared case is lost. Please open a tracking issue (or reference one in thenotes) so that wiring theArenaPairadapter is not forgotten.MergeCurveOnSurfacesis never compared with UV curves. The shim passesnullptrforpUVCurve1/pUVCurve2, and the runner passesNone, souv_curve1/uv_curve2reach the classifications untested. One fixture case with a dropped UV curve would cover it.- Owner of the curve copies in
MergeCoincidentSurfaces. Upstream sets the owner (the edge) onpECopy(SmTopologyIntersector.cpp:4484,:4662/:4666) but not on the per-UVpECopy2.SmTol::GetZoneTol3d(curve)andSmCurve::GetEdgeread that owner. The port usesedge_curve.clone()for both (p3.rs:1522,:1616). Whether the clone carries the arena curve's owner decides which of the two it matches. This is unreachable untiltopology.face_p4lands; worth a line in the Deviations entry so the next porter checks it.
Notes
- #316 (opened later) rewires the same p7 helpers (
edge_pair_xsect_tol_3d,as_b_spline,edge_interval).git merge-treeshows conflicts inp7.rs,plan/modules.tomland thecurve_class_p4report, so whichever lands second will need to reconcile. No action is needed here. - The new
UNPORTED(topology.edge)/UNPORTED(topology.vertex)fallbacks in p1/p7 cover adapters that cannot resolve a handle, the same pattern asobject_zone_tol_3donmain. That is fine.
🤖 Generated with Claude Code
…REdgeBrep
SmTopologyIntersector::CloseIntersectionLoop (SmTopologyIntersector.cpp:6108-6354,
called from SmMerge.cpp:2958) and its only callee IIREdgeBrep (:2836-2971)
are listed by no unit in harness/callgraph/units.json; per the overseer they
are co-ported into this unit as TopologyIntersector::{close_intersection_loop,
iir_edge_brep}. IIREdgeBrep's MergeCurveClasses stays
UNPORTED(boolean.topology_intersector_p3) (draft tangency-labs#211 ports it).
IntersectorBrepP2 gains edge_normalized_evaluate (topology.edgeuse_p3).
Script op 52 runs CloseIntersectionLoop and, on success, reports the other
mate of each edge of the vertex. It is covered by 16 edge cases on a planar
face and its copy plus 48 separately seeded scripts, so the generated cases
are unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VhgnDJqMfisAk17esFqRL5
Port of
boolean.topology_intersector_p3:SmTopologyIntersector::MergeCurveClasses,MergeCurveOnSurfacesandMergeCoincidentSurfaces, with shim, FFI, difftest runner and report. Statuspartial.Dependencies
All on
mainnow: #146 and #203 (parts 1 and 2), #85 (intersect.curve_class_p2) and #137 (intersect.curve_class_p4). The branch is up to date withmainate4dd81c4.The port
All three functions are ported in full. There is a stub at each call into a unit that is still
todo, with the upstream logic after it:topology.face_p4:GetEdges,GetUpwardLoopuses;topology.brep_p4:DeleteEdge.query.face_p1ispartial, so its calls are wired.SmFace::Point3DClassifyandCalculateBoundingBox(bTight = FALSE) areIntersectorBrepP3hooks, and the difftest adapter implements them withquery::face::p1. Upstream passes thedoubledCurveTolerancewhere anSmZoneTol3dis expected, in the classify call and theSmPointClassificationconstructor. The port applieszone_tol3dat both, as that implicit conversion does.IntersectorBrepP3extendsIntersectorBrepP2and adds the surface, edge, loop-use, edge-use and face-use walks. Out-params become return values. Temporary curves are stored for a handle and reported for the caller to free.Changes outside the unit
curve_class_p4: itsReSet,ConnectIntervalsandGetMinUV/GetMaxUVstubs are wired to part 2. They are still stubs onmain, because port: intersect.curve_class_p2 #85 and port: intersect.curve_class_p4 #137 merged separately.curve_class_p1:sm_MakeConsistentClassificationswas a stub ontopology.edge, which is already partial. It is now ported in full, throughClassGeometry::topology_entityand a new default methodClassGeometry::topology_handle_in.curve_class_p7:SmEdge::GetCurve/GetIntervaland the edge-pair tolerance are wired the same way where classification geometry is in scope.TestForPartialCoincidentEdgesnow copies the B-spline base of a coincident edge curve of any type derived fromSmBSplineCurve(SmLine,SmEllipse/SmCircle,SmHyperbola,SmParabola), asSM_CAST_PTRdoes. Before, it rejected everything but a plain B-spline. This unit's fuzz found it: 3 inputs where C++ returnedSM_ERRand Rust a null-dereference error.curve_class_p7's own difftest is unchanged.curve_class_p7runner's stand-in edges) keep the previousUnporteddeferral, so no runner's outcome flips to a failure.curve_class_p4shim: refusesGetLoopCrossingson an empty classification. A squeeze can now empty one, and upstream then readsGetAt(0)unchecked.topology.brep_p2runner: itsMergecases classify throughMergeObjects, which names only the case's curves: every face, edge and vertex is one placeholder handle without geometry. WithReSetwired, the fixture casemerge_plane10_line(smTopoTest.cpp:1664) gets past theReSetstub it deferred on and needs a face's geometry. It now defers underharness:merge-objects, a gap of that runner, not of the port. That unit is at 1709 / 1437 / 0 / 272.pub(super)), plus the two topology hooks on itsTwoadapter andblocks_len.Back-edges
blend.fillet_geom_p4'sMergeCurveOnSurfacesstub is retaggedblend.fillet_executive_p1. It runs on the fillet executive's intersector, which that unit provides; itsRegisterExistingSSIstub is already tagged the same way.boolean.merge_p1'sMergeCoincidentSurfacesstaysUNPORTED(boolean.topology_intersector_p3). Wiring it needs two things: anIntersectorBrepP3implementation forboolean.merge_p4'sArenaPair, andfix_face_coincidencetaking the kernels and notification context. That is cross-unit work, so it is left for a follow-up PR (noted inplan/modules.tomlandplan/findings.md).Harness note
The shared B-rep import keeps vertex uses in arena order, not upstream's list order, so
GetEdgescan come out in a different order. This unit's runner reads the list order from its shim and reorders the imported lists. Details are inplan/findings.md; exporting the order insc_topology_fixture.hwould fix every runner at once.Difftest (linux/arm64 container)
The
topology.brep_p1/brep_p3deferrals aretopology.brep_p5's own stubs (m_bEditingEnabled,SqueezeEdge), reached throughCombineCoincidentVerticeswhen squeezing.The 4
--greenfailures are the arm64 conversion difference already inplan/findings.md, not a regression. They areheal.heal_data_p13,_p2123,_p37 andgeom.hash_table435. CI's x86-64 run of this branch merged intomainpassed all four. The container runslinux/arm64, and upstream's out-of-rangedouble→ULONGconversions differ from x86-64, which the port follows.main(bb137d7d) gives 3 / 104 / 7 / 435 in the same container.heal_data_p2cases thatmaindefers now run (gen0070,gen0211,gen0544,gen0549).heal.heal_data_p2passes every case it runs (452, 0 failed). So these 4 pass on x86-64 CI.fmt (including
rustfmt --edition 2021on theinclude!d runner), clippy (--workspace --all-targets, with and withoutdifftest/cpp), the 1526 kernel tests and the wasm32 check pass.🤖 Generated with Claude Code