Crash using collisions checks for certain objects

Bugs
1

Hi

I noticed a crash while using collision checks for some objects.
Seems that in the _testTriangle function some undefined points where passed which is causing a crash:

Screenshot 2023-07-28 at 09.38.32
Screenshot 2023-07-28 at 09.38.322032×1167 648 KB

I have reproduced my issue in this playground:

(just try to walk around with arrow keys)

2

Your model has two empty meshes (0 faces) beside root

This might be due to multiple materials on your meshes?

In the inspector you can see:

cam grav coll
cam grav coll1248×745 90.5 KB

Maybe you can remove one material per mesh in your file via an 3D Editor (Blender…)?

Or you disable those empty meshes:

mesh.setEnabled(false);

Or don’t check collision on them.

1 Like
3

Yes this sounds good to just disable it. Thank you.
But I think it should also be fixed that the check is not executed on these objects :slight_smile:

4

We would need to see what this implies but I guess is a fair point (somehow). And then, may be not. Collisions can be costly and I suppose a good practice would be to set’em by mesh or using a collision map. Nonetheless, we could eventually make it a Feature requests.
Let’s first see what the team has to say about it? cc @sebavan

5

A workaround would be to handle them as root mesh by converting to unindexed mesh when the mesh has no indices:

1 Like
6

Smart thinking :smile:. Thinking of it, this could also help eliminating some other potential issues that might occur with these meshes.

7

Hmmm we always have to be cautious when adding checks to checkCollisions because it needs to be as light as possible to have good performance. One option would be to change the check from

for (let i = 0; i < pts.length; i += 3) {

to

for (let i = 0; i+2 < pts.length; i += 3) {

but that’s still an extra op per loop. Another would be to check the mesh’s geometry at the moment checkCollisions is set to true and try to correct it if there are any problems, or just not set the collisions in this case. What do you think @Evgeni_Popov ?

8

I don’t think changing the loop is the right solution, as we shouldn’t end up in this block of code in the first place. The code tests !indices || indices.length === 0 to see if the mesh is non-indexed, but this is the wrong check: we can have indices.length === 0 for an indexed mesh (as shown in PG). We’d need to take Mesh.isUnIndexed into account, but it’s not available in this piece of code.

I would update the AbstractMesh._collideForSubMesh function like this:

public _collideForSubMesh(subMesh: SubMesh, transformMatrix: Matrix, collider: Collider): AbstractMesh {
    this._generatePointsArray();

    if (!this._positions) {
        return this;
    }

    const indices = this.getIndices();
    if (!subMesh.getMesh().isUnIndexed && (!indices || indices.length === 0)) {
        return this;
    }
   ...

(I added the last 4 lines)

But it’s not working, because isUnIndexed is a property of Mesh, not AbstractMesh… We could add isUnIndexed in AbstractMesh, but I don’t know if it’s the right thing to do.

Let’s see what @sebavan thinks when he comes back from vacation.

9

What decides if isUnIndexed is true?
Maybe those parts exist on AbstractMesh?

Edit;
console.log on original PG shows isUnIndexed is false for all meshes.

To be honest this kinda just looks like a faulty mesh… the array of points should always be divisible by 3 in a triangular mesh… needs to be cleaned up in your DCC / CAD program of choice.

10

I believe the idea of @Takemura is a manual fix the user would make himself. Not to integrate in the process (at least the way I understood it).

I agree. I thought the same but I didn’t want to answer just ‘NO’. Who am I to just say ‘NO’? :face_with_hand_over_mouth:

1 Like
11

Meshes are always indexed by default. They become unindexed when you call Mesh.convertToUnIndexedMesh.

No, not when in indexed mode. Think of a cube, which has 8 vertices for example. It’s only in the unindexed mode that the number of vertices must be divisible by 3.

A mesh with vertices but without any face is allowed, so I think we should handle it gracefully. The problem here is that the collision code always assumes that in this case the mesh is unindexed, which is wrong.

1 Like
12

I used the term “array of points” as an overall description, my bad for the misunderstanding.
Whether the mesh have indices or not, the result should divide by 3.
8 indices would be 24 “points” :smile:

13

Yes, it’s the size of the index buffer which must be dividable by 3 for an indexed mesh, not the number of vertices. In the present case, the number of vertices is 100 and the size of the index buffer is 0, so the mesh is valid :slight_smile: .