From 810f944cdd4426263d3709a68e181de48a92a2ba Mon Sep 17 00:00:00 2001 From: Fran Sanchez Date: Sat, 28 Mar 2026 12:55:07 +0100 Subject: [PATCH 1/4] Add design spec for polygon-based triangle splitter (#51) Replaces CDTTriangleSplitter with a polygon-aware approach that groups split sub-triangles into regions for bulk classification, reducing raycasts and enabling symmetric edge matching across CSG boundaries. --- .../2026-03-28-polygon-splitter-design.md | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 docs/superpowers/specs/2026-03-28-polygon-splitter-design.md diff --git a/docs/superpowers/specs/2026-03-28-polygon-splitter-design.md b/docs/superpowers/specs/2026-03-28-polygon-splitter-design.md new file mode 100644 index 00000000..f50e94d9 --- /dev/null +++ b/docs/superpowers/specs/2026-03-28-polygon-splitter-design.md @@ -0,0 +1,122 @@ +# Polygon-Based Triangle Splitter — Design Spec + +**Issue:** #51 — TriangleSplitter: Enable symmetrical clipping along connected edges +**Replaces:** CDTTriangleSplitter +**Date:** 2026-03-28 + +## Problem + +When triangles are split at CSG intersection boundaries, the resulting edges on geometry A and geometry B don't match symmetrically. Floating-point divergence in independent per-side computations causes split vertices to differ slightly, breaking half-edge connectivity. As a result: + +1. Every split sub-triangle requires an individual raycast for inside/outside classification (expensive). +2. Half-edge connectivity cannot be restored across split boundaries, degrading performance of subsequent CSG operations. +3. The CDT splitter's `triangleConnectivity` traversal is disabled (operations.js line 345) due to these precision issues. + +## Solution + +Replace `CDTTriangleSplitter` with a new `PolygonSplitter` that: + +1. **Triangulates** using cdt2d with constraint edges (same as CDT splitter). +2. **Groups** resulting sub-triangles into polygon regions via flood-fill over CDT adjacency, where a region boundary is any constraint edge. +3. **Classifies** each polygon region with a single raycast from one representative sub-triangle's midpoint. +4. **Ensures symmetric edges** by using canonical intersection edge vertices computed once and shared between both geometries. + +### Why group-then-classify instead of extract-polygons-then-triangulate + +The issue describes constructing explicit polygon loops, then triangulating only kept polygons. We instead triangulate first via cdt2d and group by adjacency. This achieves the same outcome — fewer raycasts, symmetric constraint edges preserved in output — with better numerical robustness and less code. The polygon never needs to exist as an explicit data structure. + +## Design + +### 1. Canonical Edge Vertices + +**Problem:** Currently, intersection edges are computed in `collectIntersectingTriangles` and cached in `IntersectionMap.edgeSet` per-triangle. When A and B each project to 2D independently for CDT, floating-point drift causes shared boundary vertices to diverge. + +**Fix:** Intersection edges are computed once in geometry A's local frame (already the case). When processing B's split triangles, the same canonical edge is retrieved and transformed from A's frame to B's frame via the known A→B matrix, rather than being recomputed independently. Both sides split at geometrically identical points. + +Vertex snapping within CDT (merging vertices within `VERTEX_MERGE_EPSILON`) is preserved from the existing CDT splitter. + +### 2. PolygonSplitter Class + +Replaces `CDTTriangleSplitter`. Same public interface for constraint edge addition and initialization. + +**New method: `getPolygonRegions()`** + +After `triangulate()`, returns polygon regions: groups of sub-triangle indices that share non-constraint edges. + +**Algorithm:** +1. Build adjacency from cdt2d output: for each pair of sub-triangles sharing an edge, record the connection unless that edge is a constraint edge. +2. Flood-fill connected components. Each component is a polygon region. +3. For each region, compute a representative point (midpoint of the first sub-triangle) for raycasting. + +**Data returned per region:** +``` +{ + triangleIndices: number[], // indices into splitter's triangle output + midpoint: Vector3, // representative point for classification +} +``` + +### 3. Integration with performSplitTriangleOperations + +**Current flow per split triangle:** +``` +initialize splitter → add constraint edges → triangulate +for each sub-triangle: + raycast → classify → interpolate → append to builder +``` + +**New flow per split triangle:** +``` +initialize splitter → add constraint edges → triangulate +get polygon regions (flood-fill over CDT adjacency) +for each region: + raycast ONCE from region midpoint → classify + for each sub-triangle in region: + interpolate → append to builder +``` + +The commented-out connectivity traversal (operations.js line 345-359) is removed. Polygon grouping handles bulk classification; cross-boundary half-edge connectivity is restored by the HalfEdgeMap after geometry is built, which now works because symmetric edges match. + +### 4. Coplanar Triangle Handling + +No change. Coplanar triangles are already classified by checking if the midpoint lies inside a coplanar B triangle (`COPLANAR_ALIGNED` / `COPLANAR_OPPOSITE`). The polygon splitter's representative midpoint is used for this check the same way individual sub-triangle midpoints are used today. + +### 5. Attribute Interpolation + +No change. Barycentric interpolation from the base triangle's vertices is used for all split sub-triangles, handled by `GeometryBuilder.appendInterpolatedAttributeData()`. + +## Files Changed + +| File | Change | +|---|---| +| `src/core/CDTTriangleSplitter.js` | Rename to `PolygonSplitter.js`. Add flood-fill polygon grouping via `getPolygonRegions()`. | +| `src/core/operations/operations.js` | Update `performSplitTriangleOperations` to iterate polygon regions with one raycast per region. Remove dead connectivity traversal code (lines 345-359). | +| `src/core/operations/operationsUtils.js` | Ensure canonical edge storage in `collectIntersectingTriangles` so B reuses A's edge data via transform. | +| `src/core/Evaluator.js` | Update import: CDTTriangleSplitter → PolygonSplitter. | +| `src/core/IntersectionMap.js` | Add method to retrieve edges by triangle pair key for canonical lookup. | +| `src/index.js` | Update export. | +| `src/index.d.ts` | Update type declaration. | +| `tests/PolygonSplitter.test.js` | New: test polygon region extraction, symmetric edge matching, region classification. | +| Existing evaluator tests | Verify all pass with new splitter as default. | + +## Files NOT Changed + +| File | Reason | +|---|---| +| `src/core/LegacyTriangleSplitter.js` | Stays as fallback when `useCDTClipping = false`. | +| `src/core/HalfEdgeMap.js` | No changes needed — symmetric edges mean existing matching works. | +| `src/core/operations/GeometryBuilder.js` | Interpolation logic unchanged. | +| `src/core/Brush.js` | No changes to brush preparation. | + +## Risks + +1. **Flood-fill correctness:** If cdt2d produces unexpected adjacency (e.g., due to degenerate constraint edges), polygon regions could be wrong. Mitigated by the existing vertex merge/degenerate-edge filtering. +2. **Canonical edge precision:** The A→B matrix transform introduces one multiply of floating-point error. This is strictly less error than the current approach (two independent intersection computations). If issues arise, vertex snapping in CDT handles the remainder. +3. **Backward compatibility:** Replacing CDT splitter changes default behavior for `useCDTClipping = true`. All existing tests must pass. Legacy splitter remains as fallback. + +## Success Criteria + +1. All existing tests pass with the new splitter as default. +2. Split triangles produce symmetric edges across A/B boundaries (verifiable by checking HalfEdgeMap connectivity on result geometry). +3. Number of raycasts during split triangle processing is reduced (one per polygon region vs one per sub-triangle). +4. No regression in benchmark performance; ideally measurable improvement on complex CSG operations. From 86c7d04e020bda9f22ba02c1d6d4e83105cd4406 Mon Sep 17 00:00:00 2001 From: Fran Sanchez Date: Sat, 28 Mar 2026 12:58:47 +0100 Subject: [PATCH 2/4] Add implementation plan for PolygonSplitter (#51) 5-task TDD plan: create PolygonSplitter with flood-fill polygon grouping, wire into Evaluator, update split triangle operations for bulk classification, add canonical edge sharing, verify with benchmarks. --- .../plans/2026-03-28-polygon-splitter.md | 689 ++++++++++++++++++ 1 file changed, 689 insertions(+) create mode 100644 docs/superpowers/plans/2026-03-28-polygon-splitter.md diff --git a/docs/superpowers/plans/2026-03-28-polygon-splitter.md b/docs/superpowers/plans/2026-03-28-polygon-splitter.md new file mode 100644 index 00000000..1861375f --- /dev/null +++ b/docs/superpowers/plans/2026-03-28-polygon-splitter.md @@ -0,0 +1,689 @@ +# Polygon Splitter Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace CDTTriangleSplitter with a PolygonSplitter that groups sub-triangles into polygon regions for bulk classification, reducing raycasts and enabling symmetric edge matching across CSG boundaries. + +**Architecture:** The PolygonSplitter reuses the existing cdt2d triangulation with constraint edges, then adds a flood-fill grouping step that clusters sub-triangles into polygon regions separated by constraint edges. Each region is classified with a single raycast instead of one per sub-triangle. The integration point is `performSplitTriangleOperations` in operations.js. + +**Tech Stack:** three.js, three-mesh-bvh, cdt2d, vitest + +--- + +## File Structure + +| File | Responsibility | +|---|---| +| `src/core/PolygonSplitter.js` | New file (based on CDTTriangleSplitter). Triangulates via cdt2d, groups sub-triangles into polygon regions via flood-fill. | +| `src/core/CDTTriangleSplitter.js` | Deleted — replaced by PolygonSplitter. | +| `src/core/Evaluator.js` | Import swap: CDTTriangleSplitter → PolygonSplitter. | +| `src/core/operations/operations.js` | Update `performSplitTriangleOperations` to use polygon regions for bulk classification. | +| `src/index.js` | Export swap. | +| `src/index.d.ts` | Type declaration swap. | +| `tests/PolygonSplitter.test.js` | New test file for polygon region grouping. | + +--- + +### Task 1: Create PolygonSplitter with polygon region grouping + +**Files:** +- Create: `src/core/PolygonSplitter.js` +- Create: `tests/PolygonSplitter.test.js` + +This task creates the core new class. It's a copy of CDTTriangleSplitter with the addition of `getPolygonRegions()` which flood-fills the CDT adjacency graph to group sub-triangles by non-constraint edges. + +- [ ] **Step 1: Write the failing test for polygon region grouping** + +```js +import { Vector3, Triangle } from 'three'; +import { PolygonSplitter } from '../src/core/PolygonSplitter.js'; + +describe( 'PolygonSplitter', () => { + + let splitter; + beforeEach( () => { + + splitter = new PolygonSplitter(); + + } ); + + describe( 'getPolygonRegions', () => { + + it( 'should return a single region when no constraint edges split the triangle.', () => { + + const tri = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 1, 0, 0 ), + new Vector3( 0, 1, 0 ), + ); + + splitter.initialize( tri, 0, 1, 2 ); + splitter.triangulate(); + + const regions = splitter.getPolygonRegions(); + expect( regions ).toHaveLength( 1 ); + expect( regions[ 0 ].triangleIndices ).toHaveLength( splitter.triangles.length ); + + } ); + + it( 'should return two regions when a constraint edge bisects the triangle.', () => { + + const tri = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 1, 0, 0 ), + new Vector3( 0, 1, 0 ), + ); + + splitter.initialize( tri, 0, 1, 2 ); + + // add a constraint edge from the midpoint of edge AB to the midpoint of edge AC + const edge = { + start: new Vector3( 0.5, 0, 0 ), + end: new Vector3( 0, 0.5, 0 ), + }; + splitter.addConstraintEdge( edge ); + splitter.triangulate(); + + const regions = splitter.getPolygonRegions(); + expect( regions.length ).toBe( 2 ); + + // every sub-triangle should be in exactly one region + const allIndices = regions.flatMap( r => r.triangleIndices ); + expect( allIndices ).toHaveLength( splitter.triangles.length ); + expect( new Set( allIndices ).size ).toBe( splitter.triangles.length ); + + } ); + + it( 'should provide a midpoint for each region.', () => { + + const tri = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 1, 0, 0 ), + new Vector3( 0, 1, 0 ), + ); + + splitter.initialize( tri, 0, 1, 2 ); + + const edge = { + start: new Vector3( 0.5, 0, 0 ), + end: new Vector3( 0, 0.5, 0 ), + }; + splitter.addConstraintEdge( edge ); + splitter.triangulate(); + + const regions = splitter.getPolygonRegions(); + for ( const region of regions ) { + + expect( region.midpoint ).toBeDefined(); + expect( region.midpoint.x ).toBeDefined(); + expect( region.midpoint.y ).toBeDefined(); + expect( region.midpoint.z ).toBeDefined(); + + } + + } ); + + } ); + +} ); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run tests/PolygonSplitter.test.js` +Expected: FAIL — cannot resolve `../src/core/PolygonSplitter.js` + +- [ ] **Step 3: Create PolygonSplitter by copying CDTTriangleSplitter and adding getPolygonRegions** + +Create `src/core/PolygonSplitter.js` — this is a copy of `src/core/CDTTriangleSplitter.js` with: +- Class renamed from `CDTTriangleSplitter` to `PolygonSplitter` +- New `getPolygonRegions()` method added after `triangulate()` +- New `polygonRegions` array field in constructor and `reset()` + +The key addition — the `getPolygonRegions` method: + +```js +getPolygonRegions() { + + const { triangles, triangleConnectivity } = this; + const regions = []; + const visited = new Set(); + + for ( let i = 0, l = triangles.length; i < l; i ++ ) { + + if ( visited.has( i ) ) continue; + + const region = { + triangleIndices: [], + midpoint: new Vector3(), + }; + + // flood-fill connected sub-triangles via non-constraint edges + const stack = [ i ]; + while ( stack.length > 0 ) { + + const idx = stack.pop(); + if ( visited.has( idx ) ) continue; + visited.add( idx ); + + region.triangleIndices.push( idx ); + + const connected = triangleConnectivity[ idx ]; + if ( connected ) { + + for ( let c = 0, cl = connected.length; c < cl; c ++ ) { + + if ( ! visited.has( connected[ c ] ) ) { + + stack.push( connected[ c ] ); + + } + + } + + } + + } + + // use the first sub-triangle's midpoint as the representative point + triangles[ region.triangleIndices[ 0 ] ].getMidpoint( region.midpoint ); + regions.push( region ); + + } + + return regions; + +} +``` + +The full file is CDTTriangleSplitter.js with: +1. Class name `CDTTriangleSplitter` → `PolygonSplitter` +2. The `getPolygonRegions()` method above added before `reset()` +3. `import { Vector3, Line3 } from 'three';` — Vector3 already imported + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run tests/PolygonSplitter.test.js` +Expected: PASS — all 3 tests green + +- [ ] **Step 5: Commit** + +```bash +git add src/core/PolygonSplitter.js tests/PolygonSplitter.test.js +git commit -m "Add PolygonSplitter with polygon region grouping via flood-fill" +``` + +--- + +### Task 2: Wire PolygonSplitter into Evaluator and exports + +**Files:** +- Modify: `src/core/Evaluator.js:1-2,12-26` +- Modify: `src/index.js:5` +- Modify: `src/index.d.ts` +- Delete: `src/core/CDTTriangleSplitter.js` + +- [ ] **Step 1: Update Evaluator.js imports and useCDTClipping getter/setter** + +In `src/core/Evaluator.js`, replace: +```js +import { CDTTriangleSplitter } from './CDTTriangleSplitter.js'; +``` +with: +```js +import { PolygonSplitter } from './PolygonSplitter.js'; +``` + +Replace the `useCDTClipping` getter: +```js +get useCDTClipping() { + + return this.triangleSplitter instanceof CDTTriangleSplitter; + +} +``` +with: +```js +get useCDTClipping() { + + return this.triangleSplitter instanceof PolygonSplitter; + +} +``` + +Replace the `useCDTClipping` setter: +```js +set useCDTClipping( v ) { + + if ( v !== this.useCDTClipping ) { + + this.triangleSplitter = v ? new CDTTriangleSplitter() : new LegacyTriangleSplitter(); + + } + +} +``` +with: +```js +set useCDTClipping( v ) { + + if ( v !== this.useCDTClipping ) { + + this.triangleSplitter = v ? new PolygonSplitter() : new LegacyTriangleSplitter(); + + } + +} +``` + +- [ ] **Step 2: Update src/index.js export** + +Replace: +```js +export * from './core/CDTTriangleSplitter.js'; +``` +with: +```js +export * from './core/PolygonSplitter.js'; +``` + +- [ ] **Step 3: Update src/index.d.ts type declaration** + +There is no `CDTTriangleSplitter` type currently exported in index.d.ts, so no removal needed. Add a `PolygonSplitter` type after the `LegacyTriangleSplitter` declaration: + +```ts +export class PolygonSplitter { + + trianglePool: TrianglePool; + triangles: Triangle[]; + triangleIndices: Array>; + triangleConnectivity: Array>; + normal: Vector3; + + initialize( tri: Triangle, i0?: number, i1?: number, i2?: number ): void; + addConstraintEdge( edge: Line3 ): void; + triangulate(): void; + getPolygonRegions(): Array<{ triangleIndices: number[], midpoint: Vector3 }>; + reset(): void; + +} +``` + +- [ ] **Step 4: Delete CDTTriangleSplitter.js** + +```bash +rm src/core/CDTTriangleSplitter.js +``` + +- [ ] **Step 5: Run all existing tests to verify nothing breaks** + +Run: `npx vitest run` +Expected: All tests PASS — the evaluator uses LegacyTriangleSplitter by default (constructor line 30), so CDT deletion doesn't affect default behavior. + +- [ ] **Step 6: Commit** + +```bash +git add src/core/Evaluator.js src/index.js src/index.d.ts src/core/PolygonSplitter.js +git rm src/core/CDTTriangleSplitter.js +git commit -m "Replace CDTTriangleSplitter with PolygonSplitter in exports and Evaluator" +``` + +--- + +### Task 3: Update performSplitTriangleOperations for polygon-level classification + +**Files:** +- Modify: `src/core/operations/operations.js:264-406` + +This is the core integration. The loop over split sub-triangles changes from classifying each individually to grouping by polygon region first. + +- [ ] **Step 1: Write a failing integration test for polygon-level classification** + +Add to `tests/PolygonSplitter.test.js`: + +```js +import { BoxGeometry } from 'three'; +import { Brush, Evaluator, SUBTRACTION, INTERSECTION, ADDITION, computeMeshVolume } from '../src'; + +describe( 'PolygonSplitter integration', () => { + + it( 'should produce correct volume with useCDTClipping enabled.', () => { + + const evaluator = new Evaluator(); + evaluator.useCDTClipping = true; + + const brush1 = new Brush( new BoxGeometry() ); + brush1.updateMatrixWorld(); + + const brush2 = new Brush( new BoxGeometry() ); + brush2.rotation.set( Math.PI / 4, 0, Math.PI / 4 ); + brush2.updateMatrixWorld(); + + const result1 = new Brush(); + const result2 = new Brush(); + evaluator.evaluate( brush1, brush2, [ SUBTRACTION, INTERSECTION ], [ result1, result2 ] ); + + const vol1 = computeMeshVolume( result1 ); + const vol2 = computeMeshVolume( result2 ); + expect( vol1 + vol2 ).toBeCloseTo( 1, 7 ); + + } ); + + it( 'should produce correct volume for ADDITION with useCDTClipping.', () => { + + const evaluator = new Evaluator(); + evaluator.useCDTClipping = true; + + const brush1 = new Brush( new BoxGeometry() ); + brush1.updateMatrixWorld(); + + const brush2 = new Brush( new BoxGeometry() ); + brush2.position.set( 0.5, 0.5, 0.5 ); + brush2.updateMatrixWorld(); + + const result = evaluator.evaluate( brush1, brush2, ADDITION ); + const vol = computeMeshVolume( result ); + + // two overlapping unit boxes offset by 0.5 in each axis + // overlap volume = 0.5 * 0.5 * 0.5 = 0.125 + // union volume = 1 + 1 - 0.125 = 1.875 + expect( vol ).toBeCloseTo( 1.875, 5 ); + + } ); + +} ); +``` + +- [ ] **Step 2: Run test to verify it fails or passes (baseline)** + +Run: `npx vitest run tests/PolygonSplitter.test.js` + +These tests may already pass since PolygonSplitter still works like CDTTriangleSplitter. That's fine — they serve as regression tests for the next step. + +- [ ] **Step 3: Update performSplitTriangleOperations to use polygon regions** + +In `src/core/operations/operations.js`, replace the inner loop (lines 264-405). The current code iterates `triangles` one by one with `_traversed` tracking. Replace with polygon-region-based iteration. + +Replace the block starting at line 264 (`const { triangles, triangleIndices = [], triangleConnectivity = [] } = splitter;`) through line 405 (end of the outer `for` body closing the split triangle iteration): + +```js + // cache all the attribute data in origA's local frame + const { triangles, triangleIndices = [] } = splitter; + for ( let i = 0, l = builders.length; i < l; i ++ ) { + + builders[ i ].initInterpolatedAttributeData( a.geometry, _builderMatrix, _normalMatrix, ia0, ia1, ia2 ); + + } + + // get polygon regions from the splitter if it supports them, + // otherwise treat each triangle as its own region + let regions; + if ( splitter.getPolygonRegions ) { + + regions = splitter.getPolygonRegions(); + + } else { + + regions = triangles.map( ( tri, idx ) => { + + const mp = new Vector3(); + tri.getMidpoint( mp ); + return { + triangleIndices: [ idx ], + midpoint: mp, + }; + + } ); + + } + + // classify and add triangles per polygon region + for ( let ri = 0, rl = regions.length; ri < rl; ri ++ ) { + + const region = regions[ ri ]; + const { triangleIndices: regionTriIndices, midpoint: regionMidpoint } = region; + + // determine the hit side for this entire region using the representative midpoint + const raycastMatrix = invert ? null : _matrix; + let hitSide = null; + + // check coplanar triangles first + for ( let cp = 0, cpl = _coplanarTriangles.length; cp < cpl; cp ++ ) { + + const cpt = _coplanarTriangles[ cp ]; + if ( cpt.containsPoint( regionMidpoint ) ) { + + cpt.getNormal( _coplanarNormal ); + hitSide = _normal.dot( _coplanarNormal ) > 0 ? COPLANAR_ALIGNED : COPLANAR_OPPOSITE; + break; + + } + + } + + if ( hitSide === null ) { + + // build a temporary triangle for raycasting using the representative midpoint + const firstTriIdx = regionTriIndices[ 0 ]; + hitSide = getHitSide( triangles[ firstTriIdx ], bBVH, raycastMatrix ); + + } + + // determine actions for each builder + _actions.length = 0; + _builders.length = 0; + + for ( let o = 0, lo = operations.length; o < lo; o ++ ) { + + const op = getOperationAction( operations[ o ], hitSide, invert ); + if ( op !== SKIP_TRI ) { + + _actions.push( op ); + _builders.push( builders[ o ] ); + + } + + } + + if ( _builders.length === 0 ) continue; + + // add all triangles in this region to the geometry + for ( let ti = 0, tl = regionTriIndices.length; ti < tl; ti ++ ) { + + const index = regionTriIndices[ ti ]; + const tri = triangles[ index ]; + + // get the triangle indices + const indices = triangleIndices[ index ]; + let t0 = null, t1 = null, t2 = null; + if ( indices ) { + + t0 = indices[ 0 ]; + t1 = indices[ 1 ]; + t2 = indices[ 2 ]; + + } + + // get the barycentric coordinates relative to the base triangle + _triA.getBarycoord( tri.a, _barycoordTri.a ); + _triA.getBarycoord( tri.b, _barycoordTri.b ); + _triA.getBarycoord( tri.c, _barycoordTri.c ); + + // append the triangle to all builders + for ( let k = 0, lk = _builders.length; k < lk; k ++ ) { + + const builder = _builders[ k ]; + const action = _actions[ k ]; + const invertTri = action === INVERT_TRI; + const invert = invertedGeometry !== invertTri; + + builder.appendInterpolatedAttributeData( groupIndex, _barycoordTri.a, t0, invert ); + if ( invert ) { + + builder.appendInterpolatedAttributeData( groupIndex, _barycoordTri.c, t2, invert ); + builder.appendInterpolatedAttributeData( groupIndex, _barycoordTri.b, t1, invert ); + + } else { + + builder.appendInterpolatedAttributeData( groupIndex, _barycoordTri.b, t1, invert ); + builder.appendInterpolatedAttributeData( groupIndex, _barycoordTri.c, t2, invert ); + + } + + } + + } + + } +``` + +Also remove the `_traversed` Set declaration (line 25) and `_traversed.clear()` since it's no longer needed. + +Remove from module-level: +```js +const _traversed = new Set(); +``` + +- [ ] **Step 4: Run all tests** + +Run: `npx vitest run` +Expected: All tests PASS — both PolygonSplitter unit tests and all existing evaluator tests. + +- [ ] **Step 5: Commit** + +```bash +git add src/core/operations/operations.js tests/PolygonSplitter.test.js +git commit -m "Use polygon regions for bulk classification in split triangle operations" +``` + +--- + +### Task 4: Ensure canonical edge sharing between A and B intersection maps + +**Files:** +- Modify: `src/core/operations/operationsUtils.js:58-131` +- Modify: `src/core/IntersectionMap.js` + +Currently, `collectIntersectingTriangles` stores independent `Line3` copies for both aIntersections and bIntersections. For symmetric edges, B should reference the same edge data as A (just transformed). + +- [ ] **Step 1: Write a test to verify edge symmetry** + +Add to `tests/PolygonSplitter.test.js`: + +```js +import { Line3 } from 'three'; + +describe( 'Canonical edge sharing', () => { + + it( 'should store the same edge object for both intersection maps.', () => { + + const evaluator = new Evaluator(); + evaluator.useCDTClipping = true; + + const brush1 = new Brush( new BoxGeometry() ); + brush1.updateMatrixWorld(); + + const brush2 = new Brush( new BoxGeometry() ); + brush2.rotation.set( Math.PI / 4, 0, Math.PI / 4 ); + brush2.updateMatrixWorld(); + + // the result should still be correct + const result = evaluator.evaluate( brush1, brush2, SUBTRACTION ); + expect( computeMeshVolume( result ) ).toBeGreaterThan( 0 ); + + } ); + +} ); +``` + +- [ ] **Step 2: Update collectIntersectingTriangles for shared edge references** + +In `src/core/operations/operationsUtils.js`, in the `intersectsTriangles` callback, change the edge caching so both maps share the same `Line3` instance for non-coplanar intersections: + +Replace lines 101-108: +```js +} else { + + // non-coplanar + const ea = _edgePool.getInstance().copy( _edge ); + const eb = _edgePool.getInstance().copy( _edge ); + aIntersections.addIntersectionEdge( va, ea ); + bIntersections.addIntersectionEdge( vb, eb ); + +} +``` +with: +```js +} else { + + // non-coplanar — share the same edge instance for symmetric splitting + const e = _edgePool.getInstance().copy( _edge ); + aIntersections.addIntersectionEdge( va, e ); + bIntersections.addIntersectionEdge( vb, e ); + +} +``` + +For coplanar edges, do the same — share instances: + +Replace lines 90-100: +```js +// coplanar +const count = getCoplanarIntersectionEdges( triangleA, triangleB, _coplanarEdges ); +for ( let i = 0; i < count; i ++ ) { + + const e = _edgePool.getInstance().copy( _coplanarEdges[ i ] ); + aIntersections.addIntersectionEdge( va, e ); + bIntersections.addIntersectionEdge( vb, e ); + +} +``` +This part is already sharing — no change needed for coplanar case. + +- [ ] **Step 3: Run all tests** + +Run: `npx vitest run` +Expected: All tests PASS. + +- [ ] **Step 4: Commit** + +```bash +git add src/core/operations/operationsUtils.js tests/PolygonSplitter.test.js +git commit -m "Share canonical edge instances between intersection maps for symmetric splitting" +``` + +--- + +### Task 5: Run full test suite and benchmarks + +**Files:** None modified — verification only. + +- [ ] **Step 1: Run full test suite** + +Run: `npx vitest run` +Expected: All tests PASS. + +- [ ] **Step 2: Run lint** + +Run: `npm run lint` +Expected: No errors. + +- [ ] **Step 3: Run benchmarks** + +Run: `npm run benchmark` + +Record results. No specific threshold — this establishes the baseline for the new splitter. Performance should be comparable or better than before. + +- [ ] **Step 4: Verify build succeeds** + +Run: `npm run build` +Expected: Build completes without errors. Output in `build/` directory. + +- [ ] **Step 5: Commit any lint fixes if needed** + +```bash +git add -A +git commit -m "Fix lint issues" +``` + +Only run this step if lint found issues that needed fixing. From de0c16c6e91686dd3dddf2fcdc4177fd2ec1d1e6 Mon Sep 17 00:00:00 2001 From: Fran Sanchez Date: Sat, 28 Mar 2026 15:06:37 +0100 Subject: [PATCH 3/4] Add comprehensive triangle splitting tests (#73) Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/TriangleSplitter.test.js | 313 +++++++++++++++++++++++++++++++++ 1 file changed, 313 insertions(+) create mode 100644 tests/TriangleSplitter.test.js diff --git a/tests/TriangleSplitter.test.js b/tests/TriangleSplitter.test.js new file mode 100644 index 00000000..f1b81f03 --- /dev/null +++ b/tests/TriangleSplitter.test.js @@ -0,0 +1,313 @@ +import { Vector3, Triangle, Line3, Plane } from 'three'; +import { ExtendedTriangle } from 'three-mesh-bvh'; +import { LegacyTriangleSplitter, CDTTriangleSplitter } from '../src'; +import { isTriangleCoplanar, getCoplanarIntersectionEdges } from '../src/core/utils/intersectionUtils.js'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function mulberry32( seed ) { + + return function () { + + seed |= 0; seed = seed + 0x6D2B79F5 | 0; + let t = Math.imul( seed ^ seed >>> 15, 1 | seed ); + t = t + Math.imul( t ^ t >>> 7, 61 | t ) ^ t; + return ( ( t ^ t >>> 14 ) >>> 0 ) / 4294967296; + + }; + +} + +function triangleArea( tri ) { + + const ab = new Vector3().subVectors( tri.b, tri.a ); + const ac = new Vector3().subVectors( tri.c, tri.a ); + return new Vector3().crossVectors( ab, ac ).length() * 0.5; + +} + +function validateSplitResult( originalTri, resultTriangles ) { + + // At least 1 sub-triangle produced + expect( resultTriangles.length ).toBeGreaterThanOrEqual( 1 ); + + const originalArea = triangleArea( originalTri ); + + // Area conservation + let totalArea = 0; + for ( let i = 0, l = resultTriangles.length; i < l; i ++ ) { + + const subArea = triangleArea( resultTriangles[ i ] ); + + // No degenerate sub-triangles + expect( subArea ).toBeGreaterThan( 1e-10 ); + + totalArea += subArea; + + } + + const relError = Math.abs( totalArea - originalArea ) / Math.max( originalArea, 1e-15 ); + expect( relError ).toBeLessThan( 1e-6 ); + + // All vertices coplanar with original + const plane = new Plane(); + originalTri.getPlane( plane ); + + for ( let i = 0, l = resultTriangles.length; i < l; i ++ ) { + + const t = resultTriangles[ i ]; + expect( Math.abs( plane.distanceToPoint( t.a ) ) ).toBeLessThan( 1e-6 ); + expect( Math.abs( plane.distanceToPoint( t.b ) ) ).toBeLessThan( 1e-6 ); + expect( Math.abs( plane.distanceToPoint( t.c ) ) ).toBeLessThan( 1e-6 ); + + } + +} + +function splitTriangle( SplitterClass, triA, triB ) { + + const splitter = new SplitterClass(); + + if ( SplitterClass === LegacyTriangleSplitter ) { + + splitter.initialize( triA ); + + const coplanar = isTriangleCoplanar( triA, triB ); + splitter.splitByTriangle( triB, coplanar ); + + } else { + + // CDTTriangleSplitter (PolygonSplitter) + splitter.initialize( triA, 0, 1, 2 ); + + const coplanar = isTriangleCoplanar( triA, triB ); + if ( coplanar ) { + + const edges = []; + const count = getCoplanarIntersectionEdges( triA, triB, edges ); + for ( let i = 0; i < count; i ++ ) { + + splitter.addConstraintEdge( edges[ i ] ); + + } + + } else { + + const edge = new Line3(); + const extA = new ExtendedTriangle(); + extA.copy( triA ); + extA.update(); + const extB = new ExtendedTriangle(); + extB.copy( triB ); + extB.update(); + + if ( extA.intersectsTriangle( extB, edge, true ) ) { + + splitter.addConstraintEdge( edge ); + + } + + } + + splitter.triangulate(); + + } + + return splitter.triangles; + +} + +// --------------------------------------------------------------------------- +// Parameterized test suite +// --------------------------------------------------------------------------- + +const splitters = [ + [ 'LegacyTriangleSplitter', LegacyTriangleSplitter ], + [ 'CDTTriangleSplitter', CDTTriangleSplitter ], +]; + +describe.each( splitters )( '%s', ( _name, SplitterClass ) => { + + it( 'should not split non-intersecting triangles.', () => { + + const triA = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 1, 0, 0 ), + new Vector3( 0, 1, 0 ), + ); + const triB = new Triangle( + new Vector3( 0, 0, 2 ), + new Vector3( 1, 0, 2 ), + new Vector3( 0, 1, 2 ), + ); + + const results = splitTriangle( SplitterClass, triA, triB ); + validateSplitResult( triA, results ); + expect( results ).toHaveLength( 1 ); + + } ); + + it( 'should split coplanar overlapping triangles.', () => { + + const triA = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 2, 0, 0 ), + new Vector3( 0, 2, 0 ), + ); + const triB = new Triangle( + new Vector3( 1, 0, 0 ), + new Vector3( 3, 0, 0 ), + new Vector3( 1, 2, 0 ), + ); + + const results = splitTriangle( SplitterClass, triA, triB ); + validateSplitResult( triA, results ); + expect( results.length ).toBeGreaterThanOrEqual( 2 ); + + } ); + + it( 'should handle coplanar identical triangles.', () => { + + const triA = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 1, 0, 0 ), + new Vector3( 0, 1, 0 ), + ); + const triB = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 1, 0, 0 ), + new Vector3( 0, 1, 0 ), + ); + + const results = splitTriangle( SplitterClass, triA, triB ); + validateSplitResult( triA, results ); + expect( results.length ).toBeGreaterThanOrEqual( 1 ); + + } ); + + it( 'should handle vertex-on-edge intersection.', () => { + + const triA = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 2, 0, 0 ), + new Vector3( 1, 2, 0 ), + ); + const triB = new Triangle( + new Vector3( 1, 0, - 1 ), + new Vector3( 1, 0, 1 ), + new Vector3( 1, 2, 1 ), + ); + + const results = splitTriangle( SplitterClass, triA, triB ); + validateSplitResult( triA, results ); + expect( results.length ).toBeGreaterThanOrEqual( 1 ); + + } ); + + it( 'should handle vertex-on-vertex (shared vertex).', () => { + + const triA = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 1, 0, 0 ), + new Vector3( 0, 1, 0 ), + ); + const triB = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 0, 0, 1 ), + new Vector3( 0, 1, 0 ), + ); + + const results = splitTriangle( SplitterClass, triA, triB ); + validateSplitResult( triA, results ); + expect( results.length ).toBeGreaterThanOrEqual( 1 ); + + } ); + + it( 'should handle edge-on-edge overlap (shared collinear edge).', () => { + + const triA = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 1, 0, 0 ), + new Vector3( 0.5, 1, 0 ), + ); + const triB = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 1, 0, 0 ), + new Vector3( 0.5, - 1, 0 ), + ); + + const results = splitTriangle( SplitterClass, triA, triB ); + validateSplitResult( triA, results ); + expect( results.length ).toBeGreaterThanOrEqual( 1 ); + + } ); + + it( 'should split on clean bisection.', () => { + + const triA = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 2, 0, 0 ), + new Vector3( 1, 2, 0 ), + ); + const triB = new Triangle( + new Vector3( 1, - 1, - 1 ), + new Vector3( 1, - 1, 1 ), + new Vector3( 1, 3, 0 ), + ); + + const results = splitTriangle( SplitterClass, triA, triB ); + validateSplitResult( triA, results ); + expect( results.length ).toBeGreaterThanOrEqual( 2 ); + + } ); + + it( 'should not split near-miss triangles.', () => { + + const triA = new Triangle( + new Vector3( 0, 0, 0 ), + new Vector3( 1, 0, 0 ), + new Vector3( 0, 1, 0 ), + ); + const triB = new Triangle( + new Vector3( 0, 0, 0.001 ), + new Vector3( 1, 0, 0.001 ), + new Vector3( 0, 1, 0.001 ), + ); + + const results = splitTriangle( SplitterClass, triA, triB ); + validateSplitResult( triA, results ); + expect( results ).toHaveLength( 1 ); + + } ); + + describe( 'fuzz tests', () => { + + it( 'should satisfy invariants for 100 random triangle pairs.', () => { + + const random = mulberry32( 42 ); + const randomVec = () => new Vector3( + ( random() - 0.5 ) * 2, + ( random() - 0.5 ) * 2, + ( random() - 0.5 ) * 2, + ); + + for ( let i = 0; i < 100; i ++ ) { + + const triA = new Triangle( randomVec(), randomVec(), randomVec() ); + const triB = new Triangle( randomVec(), randomVec(), randomVec() ); + + // Skip degenerate input triangles + if ( triangleArea( triA ) < 1e-10 || triangleArea( triB ) < 1e-10 ) continue; + + const results = splitTriangle( SplitterClass, triA, triB ); + validateSplitResult( triA, results ); + + } + + } ); + + } ); + +} ); From c86d617a69abb136fad5a3abf59b26a95c1484c0 Mon Sep 17 00:00:00 2001 From: Fran Sanchez Date: Sat, 28 Mar 2026 15:07:49 +0100 Subject: [PATCH 4/4] Add comprehensive triangle splitting tests (#73) Parameterized test suite validates both LegacyTriangleSplitter and CDTTriangleSplitter against 8 edge cases (coplanar, identical, vertex-on-edge, clean bisection, etc.) plus a seeded fuzz test with 100 random triangle pairs checking area conservation, coplanarity, and degenerate triangle invariants. --- .../plans/2026-03-28-polygon-splitter.md | 689 ------------------ .../2026-03-28-polygon-splitter-design.md | 122 ---- tests/TriangleSplitter.test.js | 4 + 3 files changed, 4 insertions(+), 811 deletions(-) delete mode 100644 docs/superpowers/plans/2026-03-28-polygon-splitter.md delete mode 100644 docs/superpowers/specs/2026-03-28-polygon-splitter-design.md diff --git a/docs/superpowers/plans/2026-03-28-polygon-splitter.md b/docs/superpowers/plans/2026-03-28-polygon-splitter.md deleted file mode 100644 index 1861375f..00000000 --- a/docs/superpowers/plans/2026-03-28-polygon-splitter.md +++ /dev/null @@ -1,689 +0,0 @@ -# Polygon Splitter Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Replace CDTTriangleSplitter with a PolygonSplitter that groups sub-triangles into polygon regions for bulk classification, reducing raycasts and enabling symmetric edge matching across CSG boundaries. - -**Architecture:** The PolygonSplitter reuses the existing cdt2d triangulation with constraint edges, then adds a flood-fill grouping step that clusters sub-triangles into polygon regions separated by constraint edges. Each region is classified with a single raycast instead of one per sub-triangle. The integration point is `performSplitTriangleOperations` in operations.js. - -**Tech Stack:** three.js, three-mesh-bvh, cdt2d, vitest - ---- - -## File Structure - -| File | Responsibility | -|---|---| -| `src/core/PolygonSplitter.js` | New file (based on CDTTriangleSplitter). Triangulates via cdt2d, groups sub-triangles into polygon regions via flood-fill. | -| `src/core/CDTTriangleSplitter.js` | Deleted — replaced by PolygonSplitter. | -| `src/core/Evaluator.js` | Import swap: CDTTriangleSplitter → PolygonSplitter. | -| `src/core/operations/operations.js` | Update `performSplitTriangleOperations` to use polygon regions for bulk classification. | -| `src/index.js` | Export swap. | -| `src/index.d.ts` | Type declaration swap. | -| `tests/PolygonSplitter.test.js` | New test file for polygon region grouping. | - ---- - -### Task 1: Create PolygonSplitter with polygon region grouping - -**Files:** -- Create: `src/core/PolygonSplitter.js` -- Create: `tests/PolygonSplitter.test.js` - -This task creates the core new class. It's a copy of CDTTriangleSplitter with the addition of `getPolygonRegions()` which flood-fills the CDT adjacency graph to group sub-triangles by non-constraint edges. - -- [ ] **Step 1: Write the failing test for polygon region grouping** - -```js -import { Vector3, Triangle } from 'three'; -import { PolygonSplitter } from '../src/core/PolygonSplitter.js'; - -describe( 'PolygonSplitter', () => { - - let splitter; - beforeEach( () => { - - splitter = new PolygonSplitter(); - - } ); - - describe( 'getPolygonRegions', () => { - - it( 'should return a single region when no constraint edges split the triangle.', () => { - - const tri = new Triangle( - new Vector3( 0, 0, 0 ), - new Vector3( 1, 0, 0 ), - new Vector3( 0, 1, 0 ), - ); - - splitter.initialize( tri, 0, 1, 2 ); - splitter.triangulate(); - - const regions = splitter.getPolygonRegions(); - expect( regions ).toHaveLength( 1 ); - expect( regions[ 0 ].triangleIndices ).toHaveLength( splitter.triangles.length ); - - } ); - - it( 'should return two regions when a constraint edge bisects the triangle.', () => { - - const tri = new Triangle( - new Vector3( 0, 0, 0 ), - new Vector3( 1, 0, 0 ), - new Vector3( 0, 1, 0 ), - ); - - splitter.initialize( tri, 0, 1, 2 ); - - // add a constraint edge from the midpoint of edge AB to the midpoint of edge AC - const edge = { - start: new Vector3( 0.5, 0, 0 ), - end: new Vector3( 0, 0.5, 0 ), - }; - splitter.addConstraintEdge( edge ); - splitter.triangulate(); - - const regions = splitter.getPolygonRegions(); - expect( regions.length ).toBe( 2 ); - - // every sub-triangle should be in exactly one region - const allIndices = regions.flatMap( r => r.triangleIndices ); - expect( allIndices ).toHaveLength( splitter.triangles.length ); - expect( new Set( allIndices ).size ).toBe( splitter.triangles.length ); - - } ); - - it( 'should provide a midpoint for each region.', () => { - - const tri = new Triangle( - new Vector3( 0, 0, 0 ), - new Vector3( 1, 0, 0 ), - new Vector3( 0, 1, 0 ), - ); - - splitter.initialize( tri, 0, 1, 2 ); - - const edge = { - start: new Vector3( 0.5, 0, 0 ), - end: new Vector3( 0, 0.5, 0 ), - }; - splitter.addConstraintEdge( edge ); - splitter.triangulate(); - - const regions = splitter.getPolygonRegions(); - for ( const region of regions ) { - - expect( region.midpoint ).toBeDefined(); - expect( region.midpoint.x ).toBeDefined(); - expect( region.midpoint.y ).toBeDefined(); - expect( region.midpoint.z ).toBeDefined(); - - } - - } ); - - } ); - -} ); -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `npx vitest run tests/PolygonSplitter.test.js` -Expected: FAIL — cannot resolve `../src/core/PolygonSplitter.js` - -- [ ] **Step 3: Create PolygonSplitter by copying CDTTriangleSplitter and adding getPolygonRegions** - -Create `src/core/PolygonSplitter.js` — this is a copy of `src/core/CDTTriangleSplitter.js` with: -- Class renamed from `CDTTriangleSplitter` to `PolygonSplitter` -- New `getPolygonRegions()` method added after `triangulate()` -- New `polygonRegions` array field in constructor and `reset()` - -The key addition — the `getPolygonRegions` method: - -```js -getPolygonRegions() { - - const { triangles, triangleConnectivity } = this; - const regions = []; - const visited = new Set(); - - for ( let i = 0, l = triangles.length; i < l; i ++ ) { - - if ( visited.has( i ) ) continue; - - const region = { - triangleIndices: [], - midpoint: new Vector3(), - }; - - // flood-fill connected sub-triangles via non-constraint edges - const stack = [ i ]; - while ( stack.length > 0 ) { - - const idx = stack.pop(); - if ( visited.has( idx ) ) continue; - visited.add( idx ); - - region.triangleIndices.push( idx ); - - const connected = triangleConnectivity[ idx ]; - if ( connected ) { - - for ( let c = 0, cl = connected.length; c < cl; c ++ ) { - - if ( ! visited.has( connected[ c ] ) ) { - - stack.push( connected[ c ] ); - - } - - } - - } - - } - - // use the first sub-triangle's midpoint as the representative point - triangles[ region.triangleIndices[ 0 ] ].getMidpoint( region.midpoint ); - regions.push( region ); - - } - - return regions; - -} -``` - -The full file is CDTTriangleSplitter.js with: -1. Class name `CDTTriangleSplitter` → `PolygonSplitter` -2. The `getPolygonRegions()` method above added before `reset()` -3. `import { Vector3, Line3 } from 'three';` — Vector3 already imported - -- [ ] **Step 4: Run test to verify it passes** - -Run: `npx vitest run tests/PolygonSplitter.test.js` -Expected: PASS — all 3 tests green - -- [ ] **Step 5: Commit** - -```bash -git add src/core/PolygonSplitter.js tests/PolygonSplitter.test.js -git commit -m "Add PolygonSplitter with polygon region grouping via flood-fill" -``` - ---- - -### Task 2: Wire PolygonSplitter into Evaluator and exports - -**Files:** -- Modify: `src/core/Evaluator.js:1-2,12-26` -- Modify: `src/index.js:5` -- Modify: `src/index.d.ts` -- Delete: `src/core/CDTTriangleSplitter.js` - -- [ ] **Step 1: Update Evaluator.js imports and useCDTClipping getter/setter** - -In `src/core/Evaluator.js`, replace: -```js -import { CDTTriangleSplitter } from './CDTTriangleSplitter.js'; -``` -with: -```js -import { PolygonSplitter } from './PolygonSplitter.js'; -``` - -Replace the `useCDTClipping` getter: -```js -get useCDTClipping() { - - return this.triangleSplitter instanceof CDTTriangleSplitter; - -} -``` -with: -```js -get useCDTClipping() { - - return this.triangleSplitter instanceof PolygonSplitter; - -} -``` - -Replace the `useCDTClipping` setter: -```js -set useCDTClipping( v ) { - - if ( v !== this.useCDTClipping ) { - - this.triangleSplitter = v ? new CDTTriangleSplitter() : new LegacyTriangleSplitter(); - - } - -} -``` -with: -```js -set useCDTClipping( v ) { - - if ( v !== this.useCDTClipping ) { - - this.triangleSplitter = v ? new PolygonSplitter() : new LegacyTriangleSplitter(); - - } - -} -``` - -- [ ] **Step 2: Update src/index.js export** - -Replace: -```js -export * from './core/CDTTriangleSplitter.js'; -``` -with: -```js -export * from './core/PolygonSplitter.js'; -``` - -- [ ] **Step 3: Update src/index.d.ts type declaration** - -There is no `CDTTriangleSplitter` type currently exported in index.d.ts, so no removal needed. Add a `PolygonSplitter` type after the `LegacyTriangleSplitter` declaration: - -```ts -export class PolygonSplitter { - - trianglePool: TrianglePool; - triangles: Triangle[]; - triangleIndices: Array>; - triangleConnectivity: Array>; - normal: Vector3; - - initialize( tri: Triangle, i0?: number, i1?: number, i2?: number ): void; - addConstraintEdge( edge: Line3 ): void; - triangulate(): void; - getPolygonRegions(): Array<{ triangleIndices: number[], midpoint: Vector3 }>; - reset(): void; - -} -``` - -- [ ] **Step 4: Delete CDTTriangleSplitter.js** - -```bash -rm src/core/CDTTriangleSplitter.js -``` - -- [ ] **Step 5: Run all existing tests to verify nothing breaks** - -Run: `npx vitest run` -Expected: All tests PASS — the evaluator uses LegacyTriangleSplitter by default (constructor line 30), so CDT deletion doesn't affect default behavior. - -- [ ] **Step 6: Commit** - -```bash -git add src/core/Evaluator.js src/index.js src/index.d.ts src/core/PolygonSplitter.js -git rm src/core/CDTTriangleSplitter.js -git commit -m "Replace CDTTriangleSplitter with PolygonSplitter in exports and Evaluator" -``` - ---- - -### Task 3: Update performSplitTriangleOperations for polygon-level classification - -**Files:** -- Modify: `src/core/operations/operations.js:264-406` - -This is the core integration. The loop over split sub-triangles changes from classifying each individually to grouping by polygon region first. - -- [ ] **Step 1: Write a failing integration test for polygon-level classification** - -Add to `tests/PolygonSplitter.test.js`: - -```js -import { BoxGeometry } from 'three'; -import { Brush, Evaluator, SUBTRACTION, INTERSECTION, ADDITION, computeMeshVolume } from '../src'; - -describe( 'PolygonSplitter integration', () => { - - it( 'should produce correct volume with useCDTClipping enabled.', () => { - - const evaluator = new Evaluator(); - evaluator.useCDTClipping = true; - - const brush1 = new Brush( new BoxGeometry() ); - brush1.updateMatrixWorld(); - - const brush2 = new Brush( new BoxGeometry() ); - brush2.rotation.set( Math.PI / 4, 0, Math.PI / 4 ); - brush2.updateMatrixWorld(); - - const result1 = new Brush(); - const result2 = new Brush(); - evaluator.evaluate( brush1, brush2, [ SUBTRACTION, INTERSECTION ], [ result1, result2 ] ); - - const vol1 = computeMeshVolume( result1 ); - const vol2 = computeMeshVolume( result2 ); - expect( vol1 + vol2 ).toBeCloseTo( 1, 7 ); - - } ); - - it( 'should produce correct volume for ADDITION with useCDTClipping.', () => { - - const evaluator = new Evaluator(); - evaluator.useCDTClipping = true; - - const brush1 = new Brush( new BoxGeometry() ); - brush1.updateMatrixWorld(); - - const brush2 = new Brush( new BoxGeometry() ); - brush2.position.set( 0.5, 0.5, 0.5 ); - brush2.updateMatrixWorld(); - - const result = evaluator.evaluate( brush1, brush2, ADDITION ); - const vol = computeMeshVolume( result ); - - // two overlapping unit boxes offset by 0.5 in each axis - // overlap volume = 0.5 * 0.5 * 0.5 = 0.125 - // union volume = 1 + 1 - 0.125 = 1.875 - expect( vol ).toBeCloseTo( 1.875, 5 ); - - } ); - -} ); -``` - -- [ ] **Step 2: Run test to verify it fails or passes (baseline)** - -Run: `npx vitest run tests/PolygonSplitter.test.js` - -These tests may already pass since PolygonSplitter still works like CDTTriangleSplitter. That's fine — they serve as regression tests for the next step. - -- [ ] **Step 3: Update performSplitTriangleOperations to use polygon regions** - -In `src/core/operations/operations.js`, replace the inner loop (lines 264-405). The current code iterates `triangles` one by one with `_traversed` tracking. Replace with polygon-region-based iteration. - -Replace the block starting at line 264 (`const { triangles, triangleIndices = [], triangleConnectivity = [] } = splitter;`) through line 405 (end of the outer `for` body closing the split triangle iteration): - -```js - // cache all the attribute data in origA's local frame - const { triangles, triangleIndices = [] } = splitter; - for ( let i = 0, l = builders.length; i < l; i ++ ) { - - builders[ i ].initInterpolatedAttributeData( a.geometry, _builderMatrix, _normalMatrix, ia0, ia1, ia2 ); - - } - - // get polygon regions from the splitter if it supports them, - // otherwise treat each triangle as its own region - let regions; - if ( splitter.getPolygonRegions ) { - - regions = splitter.getPolygonRegions(); - - } else { - - regions = triangles.map( ( tri, idx ) => { - - const mp = new Vector3(); - tri.getMidpoint( mp ); - return { - triangleIndices: [ idx ], - midpoint: mp, - }; - - } ); - - } - - // classify and add triangles per polygon region - for ( let ri = 0, rl = regions.length; ri < rl; ri ++ ) { - - const region = regions[ ri ]; - const { triangleIndices: regionTriIndices, midpoint: regionMidpoint } = region; - - // determine the hit side for this entire region using the representative midpoint - const raycastMatrix = invert ? null : _matrix; - let hitSide = null; - - // check coplanar triangles first - for ( let cp = 0, cpl = _coplanarTriangles.length; cp < cpl; cp ++ ) { - - const cpt = _coplanarTriangles[ cp ]; - if ( cpt.containsPoint( regionMidpoint ) ) { - - cpt.getNormal( _coplanarNormal ); - hitSide = _normal.dot( _coplanarNormal ) > 0 ? COPLANAR_ALIGNED : COPLANAR_OPPOSITE; - break; - - } - - } - - if ( hitSide === null ) { - - // build a temporary triangle for raycasting using the representative midpoint - const firstTriIdx = regionTriIndices[ 0 ]; - hitSide = getHitSide( triangles[ firstTriIdx ], bBVH, raycastMatrix ); - - } - - // determine actions for each builder - _actions.length = 0; - _builders.length = 0; - - for ( let o = 0, lo = operations.length; o < lo; o ++ ) { - - const op = getOperationAction( operations[ o ], hitSide, invert ); - if ( op !== SKIP_TRI ) { - - _actions.push( op ); - _builders.push( builders[ o ] ); - - } - - } - - if ( _builders.length === 0 ) continue; - - // add all triangles in this region to the geometry - for ( let ti = 0, tl = regionTriIndices.length; ti < tl; ti ++ ) { - - const index = regionTriIndices[ ti ]; - const tri = triangles[ index ]; - - // get the triangle indices - const indices = triangleIndices[ index ]; - let t0 = null, t1 = null, t2 = null; - if ( indices ) { - - t0 = indices[ 0 ]; - t1 = indices[ 1 ]; - t2 = indices[ 2 ]; - - } - - // get the barycentric coordinates relative to the base triangle - _triA.getBarycoord( tri.a, _barycoordTri.a ); - _triA.getBarycoord( tri.b, _barycoordTri.b ); - _triA.getBarycoord( tri.c, _barycoordTri.c ); - - // append the triangle to all builders - for ( let k = 0, lk = _builders.length; k < lk; k ++ ) { - - const builder = _builders[ k ]; - const action = _actions[ k ]; - const invertTri = action === INVERT_TRI; - const invert = invertedGeometry !== invertTri; - - builder.appendInterpolatedAttributeData( groupIndex, _barycoordTri.a, t0, invert ); - if ( invert ) { - - builder.appendInterpolatedAttributeData( groupIndex, _barycoordTri.c, t2, invert ); - builder.appendInterpolatedAttributeData( groupIndex, _barycoordTri.b, t1, invert ); - - } else { - - builder.appendInterpolatedAttributeData( groupIndex, _barycoordTri.b, t1, invert ); - builder.appendInterpolatedAttributeData( groupIndex, _barycoordTri.c, t2, invert ); - - } - - } - - } - - } -``` - -Also remove the `_traversed` Set declaration (line 25) and `_traversed.clear()` since it's no longer needed. - -Remove from module-level: -```js -const _traversed = new Set(); -``` - -- [ ] **Step 4: Run all tests** - -Run: `npx vitest run` -Expected: All tests PASS — both PolygonSplitter unit tests and all existing evaluator tests. - -- [ ] **Step 5: Commit** - -```bash -git add src/core/operations/operations.js tests/PolygonSplitter.test.js -git commit -m "Use polygon regions for bulk classification in split triangle operations" -``` - ---- - -### Task 4: Ensure canonical edge sharing between A and B intersection maps - -**Files:** -- Modify: `src/core/operations/operationsUtils.js:58-131` -- Modify: `src/core/IntersectionMap.js` - -Currently, `collectIntersectingTriangles` stores independent `Line3` copies for both aIntersections and bIntersections. For symmetric edges, B should reference the same edge data as A (just transformed). - -- [ ] **Step 1: Write a test to verify edge symmetry** - -Add to `tests/PolygonSplitter.test.js`: - -```js -import { Line3 } from 'three'; - -describe( 'Canonical edge sharing', () => { - - it( 'should store the same edge object for both intersection maps.', () => { - - const evaluator = new Evaluator(); - evaluator.useCDTClipping = true; - - const brush1 = new Brush( new BoxGeometry() ); - brush1.updateMatrixWorld(); - - const brush2 = new Brush( new BoxGeometry() ); - brush2.rotation.set( Math.PI / 4, 0, Math.PI / 4 ); - brush2.updateMatrixWorld(); - - // the result should still be correct - const result = evaluator.evaluate( brush1, brush2, SUBTRACTION ); - expect( computeMeshVolume( result ) ).toBeGreaterThan( 0 ); - - } ); - -} ); -``` - -- [ ] **Step 2: Update collectIntersectingTriangles for shared edge references** - -In `src/core/operations/operationsUtils.js`, in the `intersectsTriangles` callback, change the edge caching so both maps share the same `Line3` instance for non-coplanar intersections: - -Replace lines 101-108: -```js -} else { - - // non-coplanar - const ea = _edgePool.getInstance().copy( _edge ); - const eb = _edgePool.getInstance().copy( _edge ); - aIntersections.addIntersectionEdge( va, ea ); - bIntersections.addIntersectionEdge( vb, eb ); - -} -``` -with: -```js -} else { - - // non-coplanar — share the same edge instance for symmetric splitting - const e = _edgePool.getInstance().copy( _edge ); - aIntersections.addIntersectionEdge( va, e ); - bIntersections.addIntersectionEdge( vb, e ); - -} -``` - -For coplanar edges, do the same — share instances: - -Replace lines 90-100: -```js -// coplanar -const count = getCoplanarIntersectionEdges( triangleA, triangleB, _coplanarEdges ); -for ( let i = 0; i < count; i ++ ) { - - const e = _edgePool.getInstance().copy( _coplanarEdges[ i ] ); - aIntersections.addIntersectionEdge( va, e ); - bIntersections.addIntersectionEdge( vb, e ); - -} -``` -This part is already sharing — no change needed for coplanar case. - -- [ ] **Step 3: Run all tests** - -Run: `npx vitest run` -Expected: All tests PASS. - -- [ ] **Step 4: Commit** - -```bash -git add src/core/operations/operationsUtils.js tests/PolygonSplitter.test.js -git commit -m "Share canonical edge instances between intersection maps for symmetric splitting" -``` - ---- - -### Task 5: Run full test suite and benchmarks - -**Files:** None modified — verification only. - -- [ ] **Step 1: Run full test suite** - -Run: `npx vitest run` -Expected: All tests PASS. - -- [ ] **Step 2: Run lint** - -Run: `npm run lint` -Expected: No errors. - -- [ ] **Step 3: Run benchmarks** - -Run: `npm run benchmark` - -Record results. No specific threshold — this establishes the baseline for the new splitter. Performance should be comparable or better than before. - -- [ ] **Step 4: Verify build succeeds** - -Run: `npm run build` -Expected: Build completes without errors. Output in `build/` directory. - -- [ ] **Step 5: Commit any lint fixes if needed** - -```bash -git add -A -git commit -m "Fix lint issues" -``` - -Only run this step if lint found issues that needed fixing. diff --git a/docs/superpowers/specs/2026-03-28-polygon-splitter-design.md b/docs/superpowers/specs/2026-03-28-polygon-splitter-design.md deleted file mode 100644 index f50e94d9..00000000 --- a/docs/superpowers/specs/2026-03-28-polygon-splitter-design.md +++ /dev/null @@ -1,122 +0,0 @@ -# Polygon-Based Triangle Splitter — Design Spec - -**Issue:** #51 — TriangleSplitter: Enable symmetrical clipping along connected edges -**Replaces:** CDTTriangleSplitter -**Date:** 2026-03-28 - -## Problem - -When triangles are split at CSG intersection boundaries, the resulting edges on geometry A and geometry B don't match symmetrically. Floating-point divergence in independent per-side computations causes split vertices to differ slightly, breaking half-edge connectivity. As a result: - -1. Every split sub-triangle requires an individual raycast for inside/outside classification (expensive). -2. Half-edge connectivity cannot be restored across split boundaries, degrading performance of subsequent CSG operations. -3. The CDT splitter's `triangleConnectivity` traversal is disabled (operations.js line 345) due to these precision issues. - -## Solution - -Replace `CDTTriangleSplitter` with a new `PolygonSplitter` that: - -1. **Triangulates** using cdt2d with constraint edges (same as CDT splitter). -2. **Groups** resulting sub-triangles into polygon regions via flood-fill over CDT adjacency, where a region boundary is any constraint edge. -3. **Classifies** each polygon region with a single raycast from one representative sub-triangle's midpoint. -4. **Ensures symmetric edges** by using canonical intersection edge vertices computed once and shared between both geometries. - -### Why group-then-classify instead of extract-polygons-then-triangulate - -The issue describes constructing explicit polygon loops, then triangulating only kept polygons. We instead triangulate first via cdt2d and group by adjacency. This achieves the same outcome — fewer raycasts, symmetric constraint edges preserved in output — with better numerical robustness and less code. The polygon never needs to exist as an explicit data structure. - -## Design - -### 1. Canonical Edge Vertices - -**Problem:** Currently, intersection edges are computed in `collectIntersectingTriangles` and cached in `IntersectionMap.edgeSet` per-triangle. When A and B each project to 2D independently for CDT, floating-point drift causes shared boundary vertices to diverge. - -**Fix:** Intersection edges are computed once in geometry A's local frame (already the case). When processing B's split triangles, the same canonical edge is retrieved and transformed from A's frame to B's frame via the known A→B matrix, rather than being recomputed independently. Both sides split at geometrically identical points. - -Vertex snapping within CDT (merging vertices within `VERTEX_MERGE_EPSILON`) is preserved from the existing CDT splitter. - -### 2. PolygonSplitter Class - -Replaces `CDTTriangleSplitter`. Same public interface for constraint edge addition and initialization. - -**New method: `getPolygonRegions()`** - -After `triangulate()`, returns polygon regions: groups of sub-triangle indices that share non-constraint edges. - -**Algorithm:** -1. Build adjacency from cdt2d output: for each pair of sub-triangles sharing an edge, record the connection unless that edge is a constraint edge. -2. Flood-fill connected components. Each component is a polygon region. -3. For each region, compute a representative point (midpoint of the first sub-triangle) for raycasting. - -**Data returned per region:** -``` -{ - triangleIndices: number[], // indices into splitter's triangle output - midpoint: Vector3, // representative point for classification -} -``` - -### 3. Integration with performSplitTriangleOperations - -**Current flow per split triangle:** -``` -initialize splitter → add constraint edges → triangulate -for each sub-triangle: - raycast → classify → interpolate → append to builder -``` - -**New flow per split triangle:** -``` -initialize splitter → add constraint edges → triangulate -get polygon regions (flood-fill over CDT adjacency) -for each region: - raycast ONCE from region midpoint → classify - for each sub-triangle in region: - interpolate → append to builder -``` - -The commented-out connectivity traversal (operations.js line 345-359) is removed. Polygon grouping handles bulk classification; cross-boundary half-edge connectivity is restored by the HalfEdgeMap after geometry is built, which now works because symmetric edges match. - -### 4. Coplanar Triangle Handling - -No change. Coplanar triangles are already classified by checking if the midpoint lies inside a coplanar B triangle (`COPLANAR_ALIGNED` / `COPLANAR_OPPOSITE`). The polygon splitter's representative midpoint is used for this check the same way individual sub-triangle midpoints are used today. - -### 5. Attribute Interpolation - -No change. Barycentric interpolation from the base triangle's vertices is used for all split sub-triangles, handled by `GeometryBuilder.appendInterpolatedAttributeData()`. - -## Files Changed - -| File | Change | -|---|---| -| `src/core/CDTTriangleSplitter.js` | Rename to `PolygonSplitter.js`. Add flood-fill polygon grouping via `getPolygonRegions()`. | -| `src/core/operations/operations.js` | Update `performSplitTriangleOperations` to iterate polygon regions with one raycast per region. Remove dead connectivity traversal code (lines 345-359). | -| `src/core/operations/operationsUtils.js` | Ensure canonical edge storage in `collectIntersectingTriangles` so B reuses A's edge data via transform. | -| `src/core/Evaluator.js` | Update import: CDTTriangleSplitter → PolygonSplitter. | -| `src/core/IntersectionMap.js` | Add method to retrieve edges by triangle pair key for canonical lookup. | -| `src/index.js` | Update export. | -| `src/index.d.ts` | Update type declaration. | -| `tests/PolygonSplitter.test.js` | New: test polygon region extraction, symmetric edge matching, region classification. | -| Existing evaluator tests | Verify all pass with new splitter as default. | - -## Files NOT Changed - -| File | Reason | -|---|---| -| `src/core/LegacyTriangleSplitter.js` | Stays as fallback when `useCDTClipping = false`. | -| `src/core/HalfEdgeMap.js` | No changes needed — symmetric edges mean existing matching works. | -| `src/core/operations/GeometryBuilder.js` | Interpolation logic unchanged. | -| `src/core/Brush.js` | No changes to brush preparation. | - -## Risks - -1. **Flood-fill correctness:** If cdt2d produces unexpected adjacency (e.g., due to degenerate constraint edges), polygon regions could be wrong. Mitigated by the existing vertex merge/degenerate-edge filtering. -2. **Canonical edge precision:** The A→B matrix transform introduces one multiply of floating-point error. This is strictly less error than the current approach (two independent intersection computations). If issues arise, vertex snapping in CDT handles the remainder. -3. **Backward compatibility:** Replacing CDT splitter changes default behavior for `useCDTClipping = true`. All existing tests must pass. Legacy splitter remains as fallback. - -## Success Criteria - -1. All existing tests pass with the new splitter as default. -2. Split triangles produce symmetric edges across A/B boundaries (verifiable by checking HalfEdgeMap connectivity on result geometry). -3. Number of raycasts during split triangle processing is reduced (one per polygon region vs one per sub-triangle). -4. No regression in benchmark performance; ideally measurable improvement on complex CSG operations. diff --git a/tests/TriangleSplitter.test.js b/tests/TriangleSplitter.test.js index f1b81f03..89f6a1a8 100644 --- a/tests/TriangleSplitter.test.js +++ b/tests/TriangleSplitter.test.js @@ -286,6 +286,7 @@ describe.each( splitters )( '%s', ( _name, SplitterClass ) => { it( 'should satisfy invariants for 100 random triangle pairs.', () => { + let testedCount = 0; const random = mulberry32( 42 ); const randomVec = () => new Vector3( ( random() - 0.5 ) * 2, @@ -303,9 +304,12 @@ describe.each( splitters )( '%s', ( _name, SplitterClass ) => { const results = splitTriangle( SplitterClass, triA, triB ); validateSplitResult( triA, results ); + testedCount ++; } + expect( testedCount ).toBeGreaterThan( 0 ); + } ); } );