-
Notifications
You must be signed in to change notification settings - Fork 36
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
Mesh closest point #420
base: master
Are you sure you want to change the base?
Mesh closest point #420
Conversation
2) Added a loop in the CloudClosestPoint instead of the IndexOf(Min()) method 3) Added an if statement to distiguish triangular faces and/or quad faces in the Mesh class
- from lists to arrays - code refinements - addition of polygon option (fan strategy)
Ngon doesn't get detected
…3 class) - added Ngon implementation (stellate method) (Notice: the vertices of the Ngon are being converted to Point3. Can't index a IEnumerable instead of a list)
Added Docstrings for all methods. Quad analysed as Ngon. Cloud closest point now static method.
Co-authored-by: iltabe <64929000+iltabe@users.noreply.github.com>
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.
Thank you for this contribution! Please check comments and update code accordingly. Mostly cosmetic and readibility comments.
Please also add the relevant unit tests in the appropriate test classes for the new methods. For closest point methods edge and point coincidence edge cases will be important to include.
There is also another PR open for PointInPolygon which may or may not be useful. #419
/// <param name="trianglePoints">Triangle vertices.</param> | ||
/// <returns>Boolean result (In - True , Out - False).</returns> | ||
/// <exception cref="ArgumentOutOfRangeException">Thrown when trianglePoints is not 3 points.</exception> | ||
private bool IsPointInTriangle(Point3 projection, Point3[] trianglePoints) |
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.
- Could take in a Polygon and check it has three vertices instead of an array of Point3.
- If you keep array as input, do you need to check the three points for collinearity?
- Rename input args to point and triangle for clarity
- What happens with coincident points with an edge? Are they in or out?
@@ -547,6 +547,122 @@ private FaceData CountFaceEdges() | |||
return data; | |||
} | |||
|
|||
/// <summary> | |||
/// Checks if point is in or out a triangle. |
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.
inside or outside of a triangle
/// <param name="point">Test point.</param> | ||
/// <returns>Closest point to the triangle.</returns> | ||
/// <exception cref="ArgumentOutOfRangeException">Thrown when trianglePoints is not 3 points.</exception> | ||
private Point3 ClosestPointToTriangle(Point3[] trianglePoints, Point3 point) |
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.
- ClosestPoint is fine as method name on Mesh, trianglular polygon implied by input args
- Same comments regarding Polygon input vs point array as above
- Same comments regarding naming
} | ||
|
||
/// <summary> | ||
/// Finds the closest point to a mesh. |
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.
A bit more detail to differentiate the different ClosestPoint methods would be helpful in the summary docstring
Point3 ClosestPointToNgon = ClosestPointToTriangle(trianglePoints, point); | ||
TrianglesCP.Add(ClosestPointToNgon); | ||
} | ||
Point3 ClosestPoint = Point3.CloudClosestPoint(TrianglesCP, point); |
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.
lowercase for vars
{ | ||
// Ngon triangulation using the vertices' centroid and consecutive vertices to create the triangles | ||
Point3 ngonCentre = Point3.AveragePoint(faceVertices); | ||
var TrianglesCP = new List<Point3>(); |
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.
lowercase for vars
List<MeshVertex> meshVertices = Vertices; | ||
// All faces check | ||
IEnumerable<MeshFace> allFaces = this.Faces; | ||
var closerPointToFaceList = new List<Point3>(); |
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.
closer or closest?
for (int i = 0; i < faceVertices .Count; i++) | ||
{ | ||
Point3[] trianglePoints = new Point3[] { ngonCentre, faceVertices[i], faceVertices[(i + 1)% faceVertices.Count] }; | ||
Point3 ClosestPointToNgon = ClosestPointToTriangle(trianglePoints, point); |
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.
lowercase for vars
/// </summary> | ||
/// <param name="points"></param> | ||
/// <returns>The average point.</returns> | ||
public static Point3 AveragePoint (IEnumerable<Point3> points) |
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.
Already implemented as Centroid() method in Point3
G-Shark/src/GShark/Geometry/Point3.cs
Line 520 in 76819e0
public static Point3 Centroid(IEnumerable<Point3> points) |
} | ||
|
||
/// <summary> | ||
/// Finds closest point to a test point from a cloud of points. |
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 moved to Point3 class as Point3.ClosestPoint(Point3[] points, Point3 testPoint)
. I don't think the "cloud" terminology is useful here as we do not have a specialized PointCloud class.
What type of PR is this? (check all applicable)
Description
This PR adds a ClosestPoint() implementation for meshes. The method takes a mesh and a test point and returns the closest point to the test point on the mesh. The algorithm finds the closest point to all the faces, then the absolute closest. This point can be either on a mesh vertex, mesh edge, or inside a mesh face. The method can be used with triangle meshes, quad meshes, and Ngon meshes. Any Ngons faces are triangulated using vertices' centroid and consecutive vertices. Quads are treated as Ngons. I did not work on the tests yet, I'd like some feedback first.
Related Tickets & Documents
Please use this format link issue numbers: Fixes #416
#416 (comment)
Added tests?
Added to documentation?