Repository navigation
Conversation
Root cause The fuzzer's trimmed geometry was a degenerate network where the new main-polygon edge passes essentially exactly through an existing vertex of the open polygon (the intersection point lands ~3e-14 sheet-units from the existing vertex v_20, well within float noise of it). In DCELShapeIndex._registerShape, Phase 2 collects segment intersections, then Phase 4 splits existing DCEL edges at those points. Because the intersection point was almost but not exactly on v_20: 1. addVertex(split.point) minted a near-duplicate vertex v_45 instead of reusing v_20 (the exact-id check splitVId === currentOriginId || currentDestId at DCELShapeIndex.ts:1891 never fired). 2. Two adjacent existing edges (v_15→v_20 and v_20→v_25) both got split at the same phantom point, producing the same edge key v_20|v_45 twice, each time with a different half-edge pair. The second splitEdge overwrote the _edgeCache entry, orphaning the first pair while shapes still referenced it. 3. Ref counts went inconsistent, so releaseEdge/mergeEdges later culled a half-edge pair that a shape's halfEdgeIds still referenced → linkNext: half-edge does not exist on undo/redo replay. Fix In _registerShape Phase 2, snap each computed intersection point onto the closest of the four relevant vertex positions (existing edge origin/dest, candidate edge origin/dest) when within SPLIT_ENDPOINT_EPSILON = 1e-9 (snapPointToNearbyVertex, DCELShapeIndex.ts). This makes addVertex return the genuinely-shared vertex ID, so the existing endpoint-touch guard fires and no phantom vertex / duplicate edge-key is ever created. I initially also tried a Phase-5 guard, but it regressed an existing test, so I dropped it — the Phase-2 snap alone fixes the corruption at the source.
ToggleLinkDimensionsAction and OpenClosePolygonAction skipped geometries outside their target type (polygons for link-dimensions, non-polygons for open/close). When the selection contained only such shapes, the wrapped applyTransaction captured zero entries yet still pushed an empty undo frame, making undo() a silent no-op and breaking undo/redo symmetry. Use omitIfEmpty so nothing is pushed when no shape was modified.
TrimSplitTool.processCurrentTrim deduplicates the trimmed main polygon by comparing candidate polygons against the computed boundary with points.every((p, i) => equals(p, mainPoints[i])). When an existing polygon has more vertices than the boundary (e.g. an unmodified shape whose edges only partly coincide), mainPoints[i] was undefined and PolygonSegment.equals crashed on `b.type`. Compare vertex counts first so mismatched polygons are skipped instead of throwing.
walkCombinedBoundary can produce a boundary that hugs an overlapping, untouched shape instead of the union of the shapes being trimmed. Re-registering that boundary as a new polygon duplicates the untouched shape's edges, corrupting DCEL shared-edge ref counts and crashing undo/redo with linkNext: half-edge does not exist. Detect this when none of the boundary vertices belong to an affected shape and skip the trim with a console.warn instead of corrupting the document.
When a new shape's boundary edge lands a colinear u≈0 contact on an existing edge, Phase 5's final segment used to close into a zero-length self-loop edge (prevVId === destId). Those self-loop pairs orphan half-edges from the edge cache and corrupt DCEL ref counts, crashing undo/redo with linkNext: half-edge does not exist. The real prevVId -> destId edge was already added by the split loop, so the trailing self-loop is redundant. Skip it unless the split is a load-bearing shared-edge endpoint (u≈1) that colinear merges rely on.
Adds the fuzzer harness (app, ops, repro, invariants, shrink, corpus ...) referenced by the regression tests, and adapts the trim-split op to the current tool registration (trim-split now lives inside the 'edit' multi-tool rather than as a flat top-level tool). Also adds the tsx devDependency used to run fuzz/cli.ts.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch fixes a family of DCEL (Doubly-Connected Edge List) corruption bugs discovered by the fuzzer. The crashes all surface the same way —
linkNext: half-edge "he_..." does not exist,splitEdge/mergeEdges"edge not found", orTypeError— during undo/redo of trim / split operations on overlapping shapes.The root cause is a single theme: re-registering polygon boundaries into the DCEL can mint degenerate edges (zero-length self-loops, duplicate half-edge pairs under one edge key), which orphans half-edge references and corrupts the ref-counted edge cache. Later undo/redo replay then trips over the dangling entries.
A fuzzer (added in the last commit) replays randomized sequences and checks invariants after each op. End-to-end result across the 253-artifact corpus:
Full Jest suite: 1052 passing tests across 37 suites (10 skipped).
Fix 1 —
3be97aa— Address DCELShapeIndex duplicate edge causing thrown errorProblem. When a new polygon edge passes within floating-point noise of an existing vertex,
addVertex(split.point)minted a near-duplicate vertex, and two adjacent existing edges were both "split" at that phantom point — producing two half-edge pairs for the same edge key. The edge cache can only hold one pair per key, so the second overwrite orphaned the first while shapes still referenced it. On undo/redo,releaseEdge/mergeEdgesculled a live pair →linkNext: half-edge "he_..." does not exist.Fix. In
_registerShapePhase 2, snap each computed intersection point onto the closest of the four relevant vertex positions (existing origin/dest, candidate origin/dest) when withinSPLIT_ENDPOINT_EPSILON(1e-9).addVertexthen returns the genuinely-shared vertex id, so the existing endpoint-touch guard fires and no phantom vertex / duplicate edge key is created.Before
After — the minimized repro (ellipse + open polygon + trim-split) reports:
Fix 2 —
dce010a— Avoid empty undo transactions for no-op selection actionsProblem.
ToggleLinkDimensionsActionandOpenClosePolygonActioniterate the selection and silently skip geometries outside their target type. With a selection containing only such shapes,applyTransactioncaptured zero entries but still pushed an empty undo frame — soundo()became a silent no-op, trippingundo() did not change the document at all.Fix. Pass
{ omitIfEmpty: true }to both transactions (a pre-existingHistoryManageroption already used by the selection inspector).Before
After — all 35 no-op-undo artifacts report
NOT REPRODUCED.Fix 3 —
d11d8c7— Guard main-polygon dedup against vertex count mismatch in trimProblem. In
TrimSplitTool.processCurrentTrim, the "does the main polygon already exist?" dedup comparedpoints.every((p, i) => equals(p, mainPoints[i])). When an existing polygon had more vertices than the reconstructed boundary,mainPoints[i]wasundefinedandPolygonSegment.equalscrashed reading.type.Before
After — vertex counts are compared first; mismatched polygons are skipped. All 6 TypeError artifacts now report
NOT REPRODUCED.Fix 4 —
4185f6a— Skip trims whose boundary retraces an untouched shapeProblem.
walkCombinedBoundaryis a heuristic (see #63): it can produce a boundary that hugs an overlapping, untouched shape instead of the union of the shapes being trimmed. Re-registering that boundary as a new polygon duplicated the untouched shape's edges → shared-edge ref-count corruption → undo/redo crash.Example (rectangle
(5.497,25.771)-(7.409,36.34)+ closed polygon[(3.126,54.906),(5.093,40.035),(6.522,9.195),(13.767,10.635)], trim at(8.44, 24.126)):Before — the trim produced a corrupt merged region whose shape contained interleaved zero-length self-loop edges:
and crashed on undo with
linkNext: next half-edge "he_97" does not exist.After — the trim is detected as ambiguous (none of the boundary vertices belong to an affected shape) and skipped with an explanatory
console.warn; geometry is left untouched:Fix 5 —
377235f— Skip degenerate self-loop edges when registering shapesProblem. In
_registerShapePhase 5, when an intersection split point resolves to the candidate edge's own destination, the split loop already adds the realprevVId → destIdedge, but the trailing final segment closed into a zero-length self-loop edge (destId → destId). Self-loop pairs orphan half-edges from the edge cache and corrupt ref counts →linkNextcrashes on undo/redo.Fix. When
prevVId === candidate.destIdand the last split was a colinear-duplicate contact (uOnExisting < 0.5), skip the redundant final segment. Shared-edge endpoint touches (u ≈ 1— used by colinear-merges in the shared-edge removal tests) are preserved.After — the guard plus the trim-skip above leaves 182 / 253 fuzzer artifacts passing and the shared-edge tests still green.
Last commit —
800c0d4— Add the fuzzer harness, adapted to the current tool registryThe regression tests live in
fuzz/__tests__/fuzz.test.ts. They need the harness (fuzz/app.ts,fuzz/ops.ts,fuzz/repro.ts,fuzz/invariants.ts, ...), which is added here. The harness is adapted to the current architecture wheretrim-splitlives inside theeditmulti-tool (instead of a flat top-level tool):setActiveTool('edit')+changeToolSubTool('edit', 'trim-split'). Also adds thetsxdevDependency used to runfuzz/cli.ts.Testing
npm test— 37 suites, 1052 passing, 10 skipped.fuzz/cli.ts repro <artifact>→NOT REPRODUCEDfor the 182 fixed cases.fuzz/__tests__/fuzz.test.tsand existing unit suites.