feat: Add compound shape and convex decomposition support#361
feat: Add compound shape and convex decomposition support#361hsheth2 wants to merge 7 commits intodimforge:masterfrom
Conversation
|
Hey @sebcrozet, let me know if there's any blockers for getting this merged in |
sebcrozet
left a comment
sebcrozet
left a comment
There was a problem hiding this comment.
Hey! Took me some time to get to it, but thank you for this PR.
src.ts/geometry/collider.ts
Outdated
| return null; | ||
| } | ||
|
|
||
| // Create a wrapper - note: we can't deserialize compound shapes yet, |
There was a problem hiding this comment.
The comment is misleading here. The issue isn’t related to deserialization, it’s related to the fact that we don’t have any accessors in rawShape to retrieve the list of its primitives. I think we should:
- Add the relevant accessors on the rust side so we can get the primitive count, list, and transforms, from a compound shape.
- Add explicitely a
class RawCompound extends Shapeinstead of creating an anonymous class here. - Add a method to
RawCompoundfor converting it to aCompound(by calling the accessors from (1)). - This should also allow us to provide an actual implementation of deserialization of a Compound shape instead of throwing an exception.
| const shape1 = new RAPIER.Cuboid(0.8, 0.4, 0.5); | ||
| const shape2 = new RAPIER.Cuboid(0.8, 0.4, 0.5); | ||
| const shape3 = new RAPIER.Cuboid(0.8, 0.4, 0.5); |
There was a problem hiding this comment.
Instead of redefining te same shape three times, it can be defined just once and appear three times in the array shapes = [shape, shape, shape].
src/geometry/shape.rs
Outdated
| #[cfg(feature = "dim3")] | ||
| pub fn convexDecompositionWithParams( | ||
| vertices: Vec<f32>, | ||
| indices: Vec<u32>, | ||
| params: &RawVHACDParameters, | ||
| ) -> Option<RawShape> { | ||
| let vertices: Vec<_> = vertices.chunks(DIM).map(|v| Point::from_slice(v)).collect(); | ||
| let indices: Vec<_> = indices.chunks(3).map(|v| [v[0], v[1], v[2]]).collect(); | ||
|
|
||
| let shape = SharedShape::convex_decomposition_with_params(&vertices, &indices, ¶ms.0); | ||
| Some(Self(shape)) | ||
| } | ||
|
|
||
| #[cfg(feature = "dim2")] | ||
| pub fn convexDecomposition(vertices: Vec<f32>, indices: Vec<u32>) -> Option<RawShape> { | ||
| let vertices: Vec<_> = vertices.chunks(DIM).map(|v| Point::from_slice(v)).collect(); | ||
| let indices: Vec<_> = indices.chunks(2).map(|v| [v[0], v[1]]).collect(); | ||
|
|
||
| let shape = | ||
| SharedShape::convex_decomposition_with_params(&vertices, &indices, &Default::default()); | ||
| Some(Self(shape)) | ||
| } | ||
|
|
||
| #[cfg(feature = "dim2")] | ||
| pub fn convexDecompositionWithParams( | ||
| vertices: Vec<f32>, | ||
| indices: Vec<u32>, | ||
| params: &RawVHACDParameters, | ||
| ) -> Option<RawShape> { | ||
| let vertices: Vec<_> = vertices.chunks(DIM).map(|v| Point::from_slice(v)).collect(); | ||
| let indices: Vec<_> = indices.chunks(2).map(|v| [v[0], v[1]]).collect(); | ||
|
|
||
| let shape = SharedShape::convex_decomposition_with_params(&vertices, &indices, ¶ms.0); | ||
| Some(Self(shape)) | ||
| } | ||
|
|
There was a problem hiding this comment.
The only difference between the 2D and 3D versions here is the chunking of indices.
So instead of duplicating the functions for 2D and 3D, there should only be one version of each but with something like that inside:
#[cfg(feature = "dim2")]
let indices: Vec<_> = indices.chunks(2).map(|v| [v[0], v[1]]).collect();
#[cfg(feature = "dim3")]
let indices: Vec<_> = indices.chunks(3).map(|v| [v[0], v[1], v[2]]).collect();
src/geometry/shape.rs
Outdated
| #[cfg(feature = "dim2")] | ||
| let pos_offset = i * 2; | ||
| #[cfg(feature = "dim3")] | ||
| let pos_offset = i * 3; |
There was a problem hiding this comment.
| #[cfg(feature = "dim2")] | |
| let pos_offset = i * 2; | |
| #[cfg(feature = "dim3")] | |
| let pos_offset = i * 3; | |
| let pos_offset = i * DIM; |
src/geometry/shape.rs
Outdated
| use rapier::math::{Isometry, Point, Real, Vector, DIM}; | ||
| use rapier::parry::query; | ||
| use rapier::parry::query::{Ray, ShapeCastOptions}; | ||
| #[cfg(any(feature = "dim2", feature = "dim3"))] |
There was a problem hiding this comment.
This isn’t needed, we are always supposed to compile with either dim2 or dim3 enabled.
| #[cfg(any(feature = "dim2", feature = "dim3"))] |
|
Thanks for the review @sebcrozet . I've largely addressed the comments, with exceptions below. I ended up not adding a I did run into some issues with the round-tripping of convex decomposition compounds - the comment on ConvexPolyhedron's intoRaw explains my workaround for it that preserves the RawShape + shape data so that the round-trip encode/decode works fine, but it does feel a bit hacky. The core problem is that for some deserialized convex-decomposition children (e.g. the ones coming out of VHACD), reconstructing a fresh raw child with |
|
Managed to work around that issue where compound shapes weren't round tripping properly - see the latest commit for how. That removes some of the hacks I needed to put in earlier. |
2 participants
Closes #44 and closes #226 and supersedes #303.
Isometryclass)Known limitation: compound shapes cannot be fully deserialized from snapshots (the Rust/WASM layer doesn't expose sub-shape query APIs). This only affects save/load workflows that need to inspect compound internals - normal creation and physics simulation work fine.
Disclaimer: I used Claude Code to produce this PR, but have manually reviewed the code myself already.