fix(polygonize): deduplicate segments in planar graph#3045
Open
Traksewt wants to merge 2 commits intoTurfjs:masterfrom
Open
fix(polygonize): deduplicate segments in planar graph#3045Traksewt wants to merge 2 commits intoTurfjs:masterfrom
Traksewt wants to merge 2 commits intoTurfjs:masterfrom
Conversation
Author
|
Hi @NickCis and @DenisCarriere, I have a few fixes to make this more robust, I can do them 1 at a time. Let me know if it is of interest and if there is anything else you need. Thanks. |
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.
contributorsfield ofpackage.json- you've earned it!Summary
When input geometry contains duplicate line segments (the same segment appearing more than once),
@turf/polygonizesilently returns 0 polygons instead of the expected result.Bug
Root cause
Graph.addEdge()creates a new pair of directed edges for every input segment, including duplicates. The duplicate edges corrupt the planar graph topology, causing edge-ring traversal to fail.Fix
Track seen edge IDs in
Graphand skip segments that have already been added. Two lines inaddEdge()plus cleanup inremoveEdge().Test
Added a regression test with the above scenario — confirms polygonize correctly returns 1 polygon when a duplicate segment is present.
This is not a breaking change. Inputs that previously worked continue to produce the same output. Only the failure case (duplicate segments -> 0 polygons) is fixed.
Relates to #1714.
Note: This is a focused subset of #1714, which was opened in 2019 but covered multiple fixes in a single large PR (1000+ lines). This PR intentionally targets just the duplicate segment issue to keep the change small and reviewable per the contributing guidelines. Happy to follow up with separate PRs for the other polygonize issues if this lands.