Open MartinSStewart opened 4 years ago
Just looking through the code now - a few questions/comments:
nearestPointOnTriangle
should return for a point that is inside the triangle? The point itself, or the nearest point on the circumference of the triangle? That is, should a triangle generally be considered to include its interior? I leant towards 'yes', in which case the nearest point on a triangle to a point inside the triangle is that point itself, but that's not really a very useful thing to return.Triangle3d.pointClosestTo
and LineSegment3d.pointClosestTo
since those seem pretty consistent with things like startPoint
and midpoint
. (You can get the start point of a line segment, the midpoint of a line segment, or the point on a line segment closest to some other given point.) To be extra explicit in the triangle case we could consider things like Triangle3d.pointOnEdgeClosestTo
or Triangle3d.interiorPointClosestTo
; thoughts?Point2d.insideTriangle
already exists as Triangle2d.contains
; your version is almost certainly more efficient, though (top level helper function instead of one created within a let
, cross product operations inlined instead of by allocating some temporary vectors), so I'll probably replace the implementation with something based on your version.In general I try to avoid too many circular dependencies where lower-level concepts like points are aware of higher-level concepts like triangles; I think currently there are a bunch of unavoidable cyclic dependencies between what I think of as the true 'primitives' (points, vectors, directions, axes, planes, frames, sketch planes, bounding boxes) but then there's a pretty clear separation between those and higher-level actual 'geometry' types (line segments, triangles, arcs, splines etc.).
What do you think nearestPointOnTriangle should return for a point that is inside the triangle?
It currently returns the point itself if it's inside the triangle (or rather, the point projected onto the triangle).
My inclination for names would be Triangle3d.pointClosestTo and LineSegment3d.pointClosestTo
Those are good names too. I don't think it's a good idea to differentiate with Triangle3d.pointOnEdgeClosestTo
and Triangle3d.interiorPointClosestTo
though. To me it seems to imply that interiorPointClosestTo won't return an edge point even though numerical imprecision means we can't really have a good distinction between the two. Also I've personally never had a need for Triangle3d.pointOnEdgeClosestTo
. If someone does need it, it would be easy to implement by getting all the edges of the triangle and using LineSegment3d.pointClosestTo
.
Point2d.insideTriangle
already exists asTriangle2d.contains
Oops. I never even thought to look for "contains" though it seems obvious in hindsight. I suppose you can swap out implementations if you'd like, or copy over the tests I wrote to make extra certain the function does what it's supposed to.
In general I try to avoid too many circular dependencies where lower-level concepts like points are aware of higher-level concepts like triangles
That's fair and with that in mind, LineSegment3d.pointClosestTo
and Triangle3d.pointClosestTo
seem definately like the names to go with.
I've implemented
Point3d.nearestPointOnLine
andPoint3d.nearestPointOnTriangle
and would like to add them to elm-geometry. Code for them can be found here and tests can be found here.Note that the tests sometimes fail when a fuzzer manages to get nearestPointOnTriangle to return the wrong result due to numerical instability. I'm not sure yet if this is a problem in practice or if it only happens with very unusual triangles (i.e, two vertices are 0.0001 meters apart and the third vertex is 100000 meters away).
In response to this comment on Slack
I chose to place these functions in Point3d because I could come up with more intuitive names then. Additionally, to me it feels like Point3d is the "subject" of these functions and it feels more natural then that the functions are in the Point3d module.