Add in new 3D mesh SDF implementation #12
Conversation
There was a problem hiding this comment.
Thank you so much for putting the time to write this PR! I've made a couple suggestions. The main talking point is that this file should be under the helpers/sdfexp package since I'd like to give users and myself time to test it before setting the API in stone.
sdf3-v2.go
Outdated
| Bb r3.Box | ||
| } | ||
|
|
||
| func NewMesh(model []r3.Triangle, vertTol float64) Mesh { |
There was a problem hiding this comment.
I'm wondering if this should return an error? i.e. negative vertTol, vertTol that is too large. See https://github.com/soypat/sdf/blob/main/helpers/sdfexp/import.go#L63
sdf3-v2.go
Outdated
| } | ||
| } | ||
|
|
||
| func minVec(a r3.Vec, b r3.Vec) r3.Vec { |
There was a problem hiding this comment.
Could you use the internal/d3 functions for both minVec and maxVec and delete both these and the min max from this file?
sdf3-v2.go
Outdated
| // pick the smaller magnitude number, effectively the distance from | ||
| // the closest triangle | ||
| func minAbs(a float64, b float64) float64 { | ||
| if math.Abs(a) < math.Abs(b) { |
There was a problem hiding this comment.
Can we rewrite this as
math.Min(math.Abs(a), math.Abs(b))?
There was a problem hiding this comment.
I just removed that function, it was used before when the BIH traversal code was a little different, but it's now unused
sdf3-v2.go
Outdated
| } | ||
| } | ||
|
|
||
| // TODO figure out what to make public |
There was a problem hiding this comment.
For now its OK to leave stuff exported since it will be living in the experimental sdfexp package. That will give us time to figure out what works as an exported field and what should be private.
There was a problem hiding this comment.
Sounds good to me! For further reference the edge adjacency list, edge and vertex normals are all currently unused
|
PTAL, I added fixes for all the suggestions |
|
Looks great to me! Tested it and works like a charm. Thanks a million @cactorium |
PTAL for #7
I used a slightly older version of the code I was talking about before, so it looks a little different. I'm also not entirely sure how to test against a dev version of a Go package without dealing with a lot of Go module stuff, so I'll take any advice you guys have on that. It compiles at least!
It currently generates normals for vertices, faces, and edges, but effectively only uses the face normals right now. It'd involve tagging the distance measurements with their type and tracking that through all the distance comparison logic in case anyone's up for dealing with that