Fix assert crash in crossingsRemoved for paths with duplicate intersections#142
Open
hfutrell wants to merge 3 commits into
Open
Fix assert crash in crossingsRemoved for paths with duplicate intersections#142hfutrell wants to merge 3 commits into
hfutrell wants to merge 3 commits into
Conversation
…duce mutual neighbor nodes When selfIntersections returns the same intersection twice (possible with degenerate near-coincident curves), sortAndMergeDuplicates creates two node pairs at the same IndexedPathLocation. Merging the second pair caused addNeighbor to encounter a neighbor already added during the first merge, triggering the assert. Fix replaceNeighbor to remove rather than duplicate when the replacement is already present, and fix mergeNeighbors to skip self-referential connections, guard against adding duplicates, and clean up the stale reference to the merged-away node. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A closed path whose first element is a degenerate cubic (start == end, with control points off that point) caused the bezier clipping algorithm to detect the same self-intersection twice. This produced duplicate nodes in sortAndMergeDuplicates which triggered assert(self.neighborsContain(node) == false). The test uses a 3-segment path (degenerate cubic + line + closing line) — the smallest structure that reproduces the crash without the fix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The 260-line real-world path is replaced by a 3-segment path that reliably demonstrates the same bug: a degenerate cubic (start == end) followed by a line, closed back to the start point. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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
assert(self.neighborsContain(node) == false)) inAugmentedGraphwhencrossingsRemovedis called on a path with degenerate near-coincident curves (such as a tiny loop at the start of a path)selfIntersectionsreturns the same intersection twice (or two intersections at identicalIndexedPathLocations),sortAndMergeDuplicatesmerges the duplicate node pairs in a way that creates mutual-neighbor conflictsreplaceNeighborto remove rather than duplicate an entry when the replacement node is already presentmergeNeighborsto skip self-referential connections, guard against adding duplicate neighbors, and remove the stale reference to the merged-away nodeTest plan
testCrossingsRemovedAugmentedGraphDuplicateNeighborAssert: reproduces the crash using the exact path from the bug report; also validates correctness by sampling ±normal offsets at t=0.05/0.5/0.95 on every element and assertingpath.contains(winding) matchesresult.contains(evenOdd)🤖 Generated with Claude Code