Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ll/polygon intersects #53

Merged
merged 94 commits into from
Jan 30, 2024
Merged

Ll/polygon intersects #53

merged 94 commits into from
Jan 30, 2024

Conversation

LanaLubecke
Copy link
Contributor

This branch has the most updated polygon clipping functions!

skygering and others added 30 commits October 2, 2023 16:55
@skygering
Fix up intersection point base calculation
aba090f
@skygering
Update intersects and add line tests
ab16785
@skygering
Add more tests and debug intersects
7e75e86
@skygering
Add comments to point_in_poly
a7a7367
@LanaLubecke
adds mutable struct for node
e86bfef
@LanaLubecke
Merge branch 'main' of github.com:Caltech-OCTO/GeometryOps.jl into ll…
9842146 
…/polygon_intersects
@skygering
Remove CairoMakie
b99e37d
@LanaLubecke
implements 1st draft of intersection for polygons
d65b6eb
@LanaLubecke
correctly inserts intersection points into b_list
a369c07
@LanaLubecke
takes into account alpha vaks when adding intr pts
a2a7ca8
@LanaLubecke
returns intersection of polygons
f448736
@skygering
Update equals and overlaps
319bd88
@LanaLubecke
simplifies loops, & tracking of unprocessed pts
3f28df9
@skygering
Remove use of findfirst for 1.6 compat
0b9799d
@skygering
Updated geom, multi-geom equality
90fff1c
@skygering
Merge branch 'asinghvi17:main' into sg/update_intersects
ab211f3
@skygering
Merge pull request #1 from Caltech-OCTO/sg/update_intersects
6184971 
Sg/update intersects
@LanaLubecke
adds latest changes before pulling equals func
529837a
@LanaLubecke
Merge branch 'main' into ll/polygon_intersects
b9c1e59
@LanaLubecke
adds compareGO_LG
c135df1
@LanaLubecke
expands tests for intersections
a8e0ba1
@LanaLubecke
adds tolerance when comparing polygons
025b813
@skygering
Add files for new functions
37e0fc7
@LanaLubecke
Merge branch 'main' of github.com:Caltech-OCTO/GeometryOps.jl
a224c87
@LanaLubecke
Merge branch 'main' into ll/polygon_intersects
22e8616
@LanaLubecke
deals with merge on 10_27
413b587
@skygering
Start cleaning up geom on and in geom
92778b0
@skygering
Uncomment polygon_in_polygon
bce6381
@skygering
Add more geom in geom
98af38e
@LanaLubecke
Merge branch 'sg/in_on_updates' into ll/polygon_intersects
09fd1fb
@rafaqz
Copy link
Member

rafaqz commented Jan 27, 2024

Amazing. Is this ready for code review?

@skygering
Copy link
Collaborator

Yep! Go for it! I have already reviewed it offline 😄 this is my intern @LanaLubecke !

rafaqz
rafaqz requested changes Jan 28, 2024
View reviewed changes
Copy link
Member

@rafaqz rafaqz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, a few comments on style and performance.

Also wondering if this handles degeneracies? Like intersections exactly at vertices?

src/methods/clipping/clipping_processor.jl Outdated Show resolved Hide resolved
src/methods/clipping/intersection.jl Outdated Show resolved Hide resolved
src/methods/geom_relations/geom_geom_processors.jl Outdated Show resolved Hide resolved
src/methods/clipping/clipping_processor.jl Show resolved Hide resolved
src/methods/clipping/clipping_processor.jl Outdated Show resolved Hide resolved
src/methods/clipping/union.jl Outdated Show resolved Hide resolved
src/methods/clipping/intersection.jl Outdated Show resolved Hide resolved
src/methods/clipping/difference.jl Outdated Show resolved Hide resolved
src/methods/clipping/clipping_processor.jl Outdated Show resolved Hide resolved
src/methods/clipping/difference.jl Show resolved Hide resolved
@skygering
Copy link
Collaborator

It doesn't handle degeneracies. But neither does LibGEOS (JuliaGeo/LibGEOS.jl#166) . There is an extension of the algorithm that we used that does and we are considering implementing the additional parts soon.

@skygering
Copy link
Collaborator

Just ran this on some pretty ugly polygons that @LanaLubecke designed that each have two overlapping holes.

Screenshot 2024-01-28 at 5 41 48 PM

@rafaqz
Copy link
Member

rafaqz commented Jan 29, 2024

Wild. We are so easily getting that order of magnitude faster.

@skygering
Copy link
Collaborator

I think we are good to go @rafaqz. Can you take one more look over it? Once it is merged, @LanaLubecke and I are going to have conversations about her next steps. That includes further profiling as well as implementing the degenerate cases.

rafaqz
rafaqz approved these changes Jan 30, 2024
View reviewed changes
Copy link
Member

@rafaqz rafaqz left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Degeneracy problems are fine for now if GEOS has them too 😂

We could also add a Martinez-Rueda alg at some stage to handle that
https://github.com/w8r/martinez

.gitignore Outdated
.vscode/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably revert this file

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We aren't okay with having that in gitignore? I added it so that my .vscode files wouldn't get added by mistake.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No its all the lines below, sorry doesnt show in the comment

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah lol - that should go

@skygering skygering merged commit 30e480c into JuliaGeo:main Jan 30, 2024
3 checks passed
@skygering skygering deleted the ll/polygon_intersects branch January 30, 2024 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skygering skygering skygering left review comments

@rafaqz rafaqz rafaqz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LanaLubecke @rafaqz @skygering