three-to-cannon
three-to-cannon copied to clipboard
Use connected faces from generated convex hull
- Use connected faces from generated convex hull
- Fixes #76
- Generated convex hulls did not have connected faces, which is required for convex-convex collision in cannon-es
- Using the generated faces in ConvexHull addresses this
- Inspiration taken from discussion in cannon-es repo
- https://github.com/pmndrs/cannon-es/issues/103
Codecov Report
Merging #80 (3a13f24) into main (144fd6b) will increase coverage by
1.80%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## main #80 +/- ##
==========================================
+ Coverage 77.94% 79.74% +1.80%
==========================================
Files 3 3
Lines 1904 1940 +36
Branches 46 45 -1
==========================================
+ Hits 1484 1547 +63
+ Misses 420 393 -27
Impacted Files | Coverage Δ | |
---|---|---|
lib/ConvexHull.js | 86.04% <100.00%> (+2.55%) |
:arrow_up: |
src/index.ts | 76.34% <100.00%> (-0.66%) |
:arrow_down: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Updated to address comments. This approach reverts back to using setFromObject
and now removes vertices not included in the convex hull (interior vertices).
I'll give this another pass shortly - and any feedback would be appreciated 🙂
Sorry, was busy for a sec! 🙂
Marking as ready for review now. I've done some testing and this PR looks good to go.
As an aside, the generated shapes are now returning less vertices. Take this snippet as an example:
const cylinderGeometry = new CylinderGeometry(0.8, 1, 1, 32, 32);
const cylinderMesh = new Mesh(cylinderGeometry, meshBasicMaterial);
const cylinderMeshResult = threeToCannon(cylinderMesh, {
type: ShapeType.HULL,
});
With the existing implementation, the generated hull had 1170 vertices. With the new implementation, the generated hull had 213 vertices.
Also, something appears to be indeterministic - the number of faces and vertices vary in both the existing and new implementations on each run. I'm not going to address that in this PR though.
Thank you! Will do another review pass soon.
Also, something appears to be indeterministic - the number of faces and vertices vary in both the existing and new implementations on each run
The vertex positions displaced by small random amounts, here:
https://github.com/donmccurdy/three-to-cannon/blob/f570a1c8391269636c045672b6258490d624137a/src/index.ts#L246-L255
This may be the cause. If I remember correctly, it's a simple way to avoid edge cases we would otherwise need to deal with in the convex hull algorithm, related to co-planar points. Not a problem, for our purposes!
Ah that looks like it, thanks. And yep, not really a big issue when cannon itself isn't deterministic!
Thanks so much @isaac-mason! I've merged the changes and published v4.3.0.
Thanks for the help @donmccurdy!