-
-
Notifications
You must be signed in to change notification settings - Fork 38
Adds constrained delaunay triangulation #15
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.
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
Conversation
1. Provides constrained delaunay triangulation & tests of small/simple polygons. No testing has been done with large/complex polygons. 2. This does not handle overlapping edges or vertices.
1. Haven't run any real world data through it yet, but the test cases are all passing as expected.
1. General house keeping in the constrained triangulation code.
Pull Request Test Coverage Report for Build 74
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I skimmed over the files in the triangulate directory, as they appeared to be the same as the fields in #10. The comment there would apply here as well.
Look over things, I wondering if a better package layout would be:
planer/triangulate/delaunay
for the Delaunay package. and planer/triangulate/delaunay/constrained
for the constrained Delaunay package?
|
||
/* | ||
locateEdgeByVertex finds a quad edge that has this vertex as Orig(). This will | ||
not be a unique edge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be for LocateSegment
not locateEdgeByVertex
return nil | ||
} | ||
|
||
// TriangulatePseudoPolygon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be:
// triangulatePseudoPolygon
1. Modify comments to start with function name 2. Note in comments that calling with a nil object will cause a panic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is just a minor style thing, infinite loops in go are generally written as:
for {
…
}
for !done { | ||
|
||
for true { | ||
base = idt.subdiv.Connect(e, base.Sym()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change this to:
for {
For infinite loops in go
you don't specify the conditional value.
This pull request has been merge with f174354 |
No description provided.