Skip to content
This repository has been archived by the owner on Nov 29, 2022. It is now read-only.

feat!: Body::TriMesh implementation #42

Closed
wants to merge 3 commits into from

Conversation

Dmajster
Copy link

@Dmajster Dmajster commented Feb 27, 2021

Opening this as a draft for #41 and as a place to ask for help with potential issues regarding it.

In it's current state i'm faced with the following errors.

thread '<unnamed>' panicked at 'index out of bounds: the len is 128 but the index is 128', C:\Users\domen\.cargo\registry\src\github.com-1ecc6299db9ec823\rapier3d-0.5.0\src\dynamics\solver\interaction_groups.rs:89:21

I noticed that this feature is under the 'parallel' feature and tried to disable it. Once I did that I got the following error.

thread 'main' panicked at 'assertion failed: proxy.aabb.mins[dim] <= self.max_bound', C:\Users\domen\.cargo\registry\src\github.com-1ecc6299db9ec823\rapier3d-0.5.0\src\geometry\broad_phase_multi_sap.rs:171:13

As of now all the code required to produce the Vec and Vec<[u32; 3]> pair from Asset lives in my project https://github.com/Dmajster/radwars. So if you wish to reproduce this errors or help out I think this would be the fastest path.

I also think this would be a great feature to move this into Heron down the road.

@Dmajster Dmajster changed the title Next bevy trimesh Body::TriMesh implementation Feb 27, 2021
@Dmajster Dmajster marked this pull request as draft February 27, 2021 15:34
@jcornaz
Copy link
Owner

jcornaz commented Mar 1, 2021

Thanks @Dmajster for adding the TriMesh, that will be a great addition.

I noticed that this feature is under the 'parallel' feature and tried to disable it. Once I did that I got the following error.

thread 'main' panicked at 'assertion failed: proxy.aabb.mins[dim] <= self.max_bound', C:\Users\domen\.cargo\registry\src\github.com-1ecc6299db9ec823\rapier3d-0.5.0\src\geometry\broad_phase_multi_sap.rs:171:13

Is your trimesh convex? AFAIK, Rapier (like many other physics engine) do not support concave shapes.

@jcornaz
Copy link
Owner

jcornaz commented Mar 1, 2021

Note, you may prefer work against the main branch rather than next-bevy. The next-bevy branch can be broken at anytime by stuff commited in the bevy repository or in bevy_prototype_lyon. And, you may find unpleasant to have to continuously catch up the state of this two repos, instead of being able to focus on the implementation.

But I say that mostly for your confort. If you prefer to work agains the next-bevy branch, it is fine for me ;-)

@Dmajster
Copy link
Author

Dmajster commented Mar 1, 2021

Thanks for reminding me about the convex limitations. What is weird about this is that when I called the same trimesh builder in bevy_rapier directly it didn't crash and from the little I tested it seemed to have accurate physics meshes. I have now played around with the physics mesh generation crash and reduced it to this:

slika

it is just 2 cubes where the upper one slightly intersects with the bottom one. I tried using convex and concave objects and that didn't seem to affect the outcome, nor did the transform of the object. It seems that the intersections are what cause the crash

As to why I chose to work against next-bevy is because my main use case was to load a .gltf file that represented all the static geometry in my game and create mesh colliders for that: Bevy only supports loading whole GLTF assets like that using their new path#Scene0 syntax for gltf loaders.

@jcornaz
Copy link
Owner

jcornaz commented Mar 1, 2021

Thanks for your investiguations @Dmajster 👍

it is just 2 cubes where the upper one slightly intersects with the bottom one.

In a single trimesh? Or two trimeshes?

If it is a single trimesh, I think it is simply a case not supported by rapier. And I would personally not expect this case to be supported.

If it is two trimeshes, that would be a bug of rapier wouldn't it? In this case, I think it should be reported in the rapier repo.

@Dmajster
Copy link
Author

Dmajster commented Mar 1, 2021

In a single trimesh? Or two trimeshes?

It is two trimeshes.

I will go and file an issue with rapier for this.

@Dmajster
Copy link
Author

Dmajster commented Mar 1, 2021

slika

@jcornaz
Copy link
Owner

jcornaz commented Mar 1, 2021

Ah ok, that makes sense.

I wanted to introduce #45 anyway. I'll try to push it (maybe tonight if I have time). It should be fairly easy to implement.

As part of #45, I'll define a "default" mass, so that every body (except sensors) always have a mass. I think a wrong mass is better than none.

If I understood well, that should basically solve your problem.

@jcornaz jcornaz self-requested a review March 1, 2021 16:16
@jcornaz
Copy link
Owner

jcornaz commented Mar 2, 2021

Finally I don't think #45 will help much. I think I mostly want do deal with densities rather than absolute mass and angular inerta. That should provide more natural defaults and should be quite natural to use in general. (the mass & angular inertia would automatically be proportional to the volume of the shape).

But density won't work out of the box for TriMesh, because it is hard to compute its the volume and unit angular inertia.

I propose to explicitly set the body MassProperties in case of a TriMesh, and compute it from the axis-aligned bounding box. It won't be perfect, but is better than a panic.

@jcornaz
Copy link
Owner

jcornaz commented Mar 2, 2021

Also, we should add a center_of_mass: Vec3 field to the Body::TriMesh variant. It is certain that any trimesh will need to have the center of mass to be user-defined. Because, AFAIK, there is no easy way to compute a good center of mass automaticaly.

EDIT: Forget it, I'll make possible to define the center of mass as part of #45

EDIT2: Finally #45 doesn't make possible to define center of mass, but only allow to set density

@jcornaz jcornaz changed the title Body::TriMesh implementation feat!: Body::TriMesh implementation Mar 3, 2021
@Dmajster
Copy link
Author

I will close this pull request as I have opened a new one that works with the main branch.

@Dmajster Dmajster closed this Mar 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants