Skip to content
This repository was archived by the owner on Nov 22, 2023. It is now read-only.

add isinside function #165

Merged
merged 2 commits into from
Mar 2, 2020
Merged

add isinside function #165

merged 2 commits into from
Mar 2, 2020

Conversation

mkborregaard
Copy link
Contributor

@mkborregaard mkborregaard commented Feb 1, 2019

We need the definition of AbstractPoint, but not sure how you want to play that with how it's implemented here. And Polygons should specify that the Points are 2D.
Also needed - interface methods, tests, and a way to ensure that e.g. HyperRectangles{2} can also be treated as polygons.
Or - can 3D mesh triangles be polygons too? (probably not, but spatial data e.g. can have this duality - they are flat polygons on a space (the earth) which is distended along the third dimension. probably best to just ignore the 3rd dimension though in that case?)

@sjkelly
Copy link
Member

sjkelly commented Oct 18, 2019

@mkborregaard I implemented the Hao algorithm here: https://github.com/JuliaGeometry/PolygonOps.jl/blob/master/src/PolygonOps.jl

Not sure if we want to use that instead since it handles degeneracies quite well.

@mkborregaard
Copy link
Contributor Author

@sjkelly oh wow I'd forgotten about this PR. If your implementation is better, then by all means. Are they similar performance-wise?

@sjkelly
Copy link
Member

sjkelly commented Oct 22, 2019

I am not sure about the performance. It may be good to run some benchmarks. The paper is pretty bold in their claims, so it would be good to verify. It will probably be a little while before I finish up the clipping algorithm component. I am working over at: https://github.com/JuliaGeometry/PolygonOps.jl for now because these algorithms can be pretty generic (across e.g. GeometryTypes, GeometryBasics, GIS).

@mkborregaard
Copy link
Contributor Author

@SimonDanisch Is this so stale that it would be better to reimplement from scratch? Should we just go with @sjkelly 's instead and close?

@sjkelly
Copy link
Member

sjkelly commented Jan 30, 2020

I am unopposed to merging once conflicts are resolved. FWIW I implemented both the Hao Sun and Hormann Agathos algorithms in PolygonOps. It is untagged, but if you would prefer to test that package to see if it works for you, I can tag.

@@ -57,7 +57,8 @@ include("deprecated.jl")
include("center.jl")
include("convexhulls.jl")
include("gjk.jl")
include("polygons.jl")
include("polygon.jl")
include("polygon_triangulations.jl")
Copy link
Member

Choose a reason for hiding this comment

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

This file should be removed.

@SimonDanisch SimonDanisch merged commit b19b44e into master Mar 2, 2020
@SimonDanisch SimonDanisch deleted the mkb/isinside branch March 2, 2020 09:44
@mkborregaard
Copy link
Contributor Author

@SimonDanisch thanks for merging this.
So - I just realized that there are no tests here. Where would you like me to add them - just one of the toy polygons here? https://github.com/JuliaGeometry/GeometryTypes.jl/blob/master/test/polygons.jl

@SimonDanisch
Copy link
Member

That would be awesome :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants