Adding Greiner Hormann Method for polygon Clipping #1125
Conversation
There was a problem hiding this comment.
Thank you @souma4 ! These are very nice, non-trivial contributions! ❤️
A few comments:
- Perhaps we can reuse existing algorithms like
point in polyhere, given that our current implementation is extremely efficient and reasonably tested. - If you find out that a function is missing that is independent of this clipping algorithm, consider adding it in a separate self-contained PR before this one.
A few questions:
- Are you aware of the software license of the reference code? We need to be careful to not base our implementation in GPL for example.
- Can you comment on the time complexity of the algorithm used to find segment-segment intersections? If we could rely on an efficient sweep-line algorithm, that would be perfect. This video series provides a really nice exposition of the algorithm: https://youtu.be/nNtiZM-j3Pk?si=eCsujt3J6EkHy8tS
Regarding the sweep-line algorithm, I drafted an initial version but got stuck due to DataStructures.jl limitations. I will open a separate issue with this partial implementation, and will cc you there.
| struct GreinerHormannClipping <: ClippingMethod end | ||
|
|
||
| mutable struct GreinerVertex | ||
| coords::Point |
There was a problem hiding this comment.
if this field is a point, better rename it accordingly
| ring[next_index].prev = prev_index | ||
|
|
||
| deleteat!(ring, index) | ||
| return ring |
There was a problem hiding this comment.
we only use return in premature termination
| return ring | ||
| end | ||
|
|
||
| function pointInPoly(R::Point, poly::Ring) |
There was a problem hiding this comment.
here you mean point \in poly maybe? we already have a super efficient (multi-threaded) implementation for this
|
Honestly, I need to do a mass check of all functions in the GeoStats ecosystem because I keep finding existing functions that can help. Point in poly is an example. Like duh it exists, but brain be weird. The current implementation for the intersection phase is O(nm), so a sweep line algorithm would be great. They mention that using such an approach is a natural extention. I'll give the draft some checks when I get the chance. That's true that the MIT license needs to be preserved. The reference code actually doesn't seem to have a specific license tied to it. The paper is CC BY-NC-ND 4.0 and the supplementals are CC BY 4.0. 🤷. With a plane sweep algorithm, a labeling phase that can interface with that, all in Julia, it should be fine, but let me know what makes you feel most comfortable. |
souma4
force-pushed
the
JRC_addGreinerHormann
branch
from
993586b
to
41ad8d4
Compare
This is a pull request to add the up-to-date Greiner Hormann method for clipping polygons as part of Issue#578.
Currently, it is under a HEAVY DRAFT. All three phases are in need of heavy work, and there is ghost code from false starts. This draft is simply to put this code on the open pull requests so that others may contribute if they want, or to correct false starts.
There may be a need to update the
src/intersections.jlIntersectionTypes. The existing reference C++ code includes different IntersectionTypes. However, the currentMeshes.jlimplementation for overlapping intersections may not necessitate a change to these types.Additionally, I created an internal struct for this method because tracking neighbors across polygons and adjacent vertices is critical, and keeping it under one struct is much more readable.
IMO, the first goal is to get this to run. Second goal is to match with the codebase. Then we can include type definitions/safety.