Open
Conversation
peterstace
changed the title
The previous spanning tree approach connected arbitrary points from each component, which could create ghost lines that interfered with the geometry in problematic ways. The new algorithm: - Identifies connected components and their rightmost points - Casts horizontal rays rightward to find the closest intersection - Connects components that don't hit anything via a common vertical line This fixes several edge cases in set operations where ghost lines caused invalid DCEL construction.
peterstace
force-pushed
the
revamp_ghost_construction
branch
from
a8b5db0 to
8ac15d6
Compare
There was a problem hiding this comment.
Pull request overview
This PR revamps the DCEL ghost edge algorithm by replacing the previous spanning tree approach with a new ray-casting algorithm. The new implementation is more robust and prevents ghost lines from interfering with geometries in problematic ways.
Key Changes
- Replaced spanning tree ghost line construction with a ray-casting approach that finds connected components and links them using horizontal rays
- Updated test infrastructure to support pre-noded geometries with explicit ghost lines
- Fixed several geometry operation test cases that now produce different (more accurate) results due to the improved algorithm
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| geom/dcel_ghosts.go | Complete rewrite of ghost line generation using ray-casting instead of spanning tree |
| geom/dcel.go | Refactored to separate geometry preparation from DCEL construction |
| geom/dcel_internal_test.go | Updated test infrastructure to accept explicit ghost lines via new input structure |
| geom/dcel_ghosts_internal_test.go | Replaced spanning tree tests with new component representative and geometry preparation tests |
| geom/internal_test.go | Added new test helper functions for internal testing |
| geom/alg_set_op_test.go | Updated expected results for set operations and added new test cases |
| internal/cmprefimpl/cmpgeos/checks.go | Removed workarounds for previously failing cases |
| CHANGELOG.md | Documented the improvement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
peterstace
changed the title
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.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.
Description
Improves robustness of set operations (
Union,Intersection,Difference,SymmetricDifference) by revamping ghost line construction. The previousspanning tree approach could produce ghost lines that interfered with the
geometry in problematic ways. The new ray casting approach is more robust.