Add geometry open-boundary diagnostics#252
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces geometry diagnostics utilities to help identify open boundary edges and connected open-boundary regions (useful for debugging “missing faces” / non-watertight CSG results), exposes them through the package entry points (JS + TS), and adds debug-page controls to visualize and log the diagnostic regions.
Changes:
- Added
getGeometryDiagnostic,getOpenBoundaryEdges,getOpenTriangleSets, andgetTriangleutilities for open-boundary analysis. - Exported the new helpers via
src/index.jsand added TypeScript declarations. - Enhanced
examples/debug.jsto display open edges / triangle sets and to log triangle definitions for problematic regions.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Utils.geometryDiagnostics.test.js | Adds targeted tests covering solid vs open geometries and disjoint-edge fragment handling. |
| src/utils/geometryDiagnostics.js | Implements open-boundary edge detection, grouping into connected open-triangle sets, and a diagnostic summary object. |
| src/index.js | Exposes the new diagnostics and isWaterTight from the package entry point. |
| src/index.d.ts | Adds TS types and function declarations for the new diagnostics APIs. |
| examples/debug.js | Adds GUI toggles and rendering/logging for diagnostic open edges and open triangle sets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const parent = this.parents.get( value ); | ||
| if ( parent === value ) { | ||
|
|
||
| return value; | ||
|
|
||
| } | ||
|
|
||
| const root = this.find( parent ); | ||
| this.parents.set( value, root ); |
There was a problem hiding this comment.
Addressed in bb22602: DisjointSet.find now validates that values have been added before lookup, uses iterative root discovery/path compression, and throws a clear error if an unexpected parent is missing. This avoids recursive stack growth on large meshes and fails explicitly for invalid input.
Verification:
- npm test -- Utils.geometryDiagnostics.test.js
- npm test
- npm run lint (existing warnings only)
- npm run build-examples
- git diff --check
| const { | ||
| matchDisjointEdges = true, | ||
| useAllAttributes = false, | ||
| } = options; | ||
|
|
There was a problem hiding this comment.
Addressed in bb22602: when useAllAttributes is true, disjoint-edge matching is disabled via effectiveMatchDisjointEdges so edges intentionally separated by attribute differences are not reconnected by the position-only fallback.
Verification:
- npm test -- Utils.geometryDiagnostics.test.js
- npm test
- npm run lint (existing warnings only)
- npm run build-examples
- git diff --check
| const vertexA = line ? line.start.clone() : getTriangleVertex( geometry, triangleIndex, edgeIndex, new Vector3() ); | ||
| const vertexB = line ? line.end.clone() : getTriangleVertex( geometry, triangleIndex, nextEdgeIndex, new Vector3() ); | ||
|
|
||
| return { | ||
| triangleIndex, | ||
| edgeIndex, | ||
| vertexIndices: [ vertexIndexA, vertexIndexB ], | ||
| vertexHashes: [ hashVertex3( vertexA ), hashVertex3( vertexB ) ], | ||
| vertices: [ vertexA, vertexB ], | ||
| line: line ? line.clone() : new Line3( vertexA.clone(), vertexB.clone() ), |
There was a problem hiding this comment.
Addressed in bb22602: fragment/disjoint boundary edges now set vertexIndices to null and expose originalVertexIndices separately. The TypeScript declaration reflects this shape, and the regression test covers the fragment case.
Verification:
- npm test -- Utils.geometryDiagnostics.test.js
- npm test
- npm run lint (existing warnings only)
- npm run build-examples
- git diff --check
Summary
Fixes #68
Verification
npm cigit diff --check