engine icon indicating copy to clipboard operation
engine copied to clipboard

Collision contact normal is inconsistent through re-enabling the 'other' entity.

Open Ri0ee opened this issue 2 years ago • 11 comments

Description

  • An entity, with dynamic rigidbody and collision components, has entity.collision.on('contact') event handler attached.
  • Consider it falling onto a ground, a static rigidbody. As expected, the contact result .normal for the falling entity is -1 on Y.
  • Disable and then enable back the ground entity, the same entity drops onto it, but this time the value for the .normal of a contact is unexpected 1 on Y.
  • Shortly, the contact .normal result does not persist after re-enabling the entity it falls on to.

Engine ver 1.55.4

Steps to Reproduce

  1. Explore the https://playcanvas.com/project/970566 project, fork it
  2. Run it and check the console output

Ri0ee avatar Aug 17 '22 08:08 Ri0ee

This is how Ammo reports these normals. It is not a PlayCanvas issue.

Edit: You can use a debug drawer to see the physics world that Ammo sees. If you set the render mode to 8, you should see the contacts data. https://playcanvas.com/project/744419/overview/ammo-debug-draw

LeXXik avatar Aug 17 '22 11:08 LeXXik

I wonder if there's some issue with re-adding the collider to Ammo (for example rotated 180deg), which could something like this perhaps? Maybe something is not converted to correct coordinate system.

mvaligursky avatar Aug 17 '22 11:08 mvaligursky

No, not really. Just Ammo quirks and how it calculates the normal of a contact point between two bodies. It mostly relates to the order that the bodies entered the physics world, which affects the order the solvers solve the collisions. For example, here, you can just change the order of the Floor and Cylinder in the Editor hierarchy to see the effect:

https://playcanvas.com/project/970930/overview/ammo-quirks

There isn't much you can do, I'm afraid. If the normal is needed for generating a force, since we know the position of the body, one can invert it, if it points the other way.

LeXXik avatar Aug 17 '22 21:08 LeXXik

But don't you at least get two colliders that have contact in different order? That would allow you to use that order to invert the normal.

mvaligursky avatar Aug 17 '22 22:08 mvaligursky

@mvaligursky not sure what you meant, but yes, as I mentioned, if the normal is pointing the other way, it is trivial to invert it in user script, before consuming. We don't even need 2 bodies, since we have a contact point and a body position, it should be enough.

Unless, you are suggesting PC should handle tracking the normal orientation? I would suggest not to.

LeXXik avatar Aug 18 '22 09:08 LeXXik

You seem to suggest there possibly is a quirk in Ammo. I'm saying that there is a way this is correct behaviour, without any quirk, in case the contact information specifies two bodies that have contact. It could be that they list these two contacts:

  • Floor to object, normal is 1
  • Object to floor, normal is -1.

What I mean is that the normal is relative to the order of the colliders?

mvaligursky avatar Aug 18 '22 09:08 mvaligursky

What I mean is that the normal is relative to the order of the colliders?

I just tested that and other is always the floor, since the collider is registered on the box.

After some testing I figured the normal can be calculated from the local collision points: https://playcanvas.com/project/971123/overview/rigidbody-collision-test

Basically:

var Test = pc.createScript('test');
Test.attributes.add('groundEntity', { type: 'entity' });
Test.prototype.initialize = function() {
    this.entity.collision.on('contact', this.onContact, this);
    setTimeout(() => {
        this.groundEntity.enabled = false;
        this.entity.rigidbody.linearVelocity = pc.Vec3.ZERO;
        this.entity.rigidbody.teleport(0, 5, 0);
    }, 3000);
    setTimeout(() => {
        this.groundEntity.enabled = true;
        this.entity.rigidbody.linearVelocity = pc.Vec3.ZERO;
        this.entity.rigidbody.teleport(0, 5, 0);
    }, 6000);
};
Test.prototype.onContact = function(hit) {
    function replacer(key, value) {
        if (typeof value === "number") {
            return value.toFixed(2);
        }
        if (value instanceof pc.Vec3) {
            const {x, z, y} = value;
            return `[${x.toFixed(2)}, ${y.toFixed(2)}, ${z.toFixed(2)}]`;
        }
        return value;
    }
    console.log("this", this);
    console.log('hit.other.name', hit.other.name);
    if (hit.other.name !== 'Floor') {
        debugger;
    }
    this.lastContacts = hit.contacts;
    for (const contact of hit.contacts) {
        console.log(JSON.stringify(contact, replacer, 2));
        //console.log('point.y', contact.point.y.toFixed(2), 'pointOther.y', contact.pointOther.y.toFixed(2), 'normal.y', contact.normal.y);
    }
};
Test.prototype.update = function(dt) {
    if (!this.lastContacts) {
        return;
    }
    for (const contact of this.lastContacts) {
        const normalFromLocalPoints = new pc.Vec3().sub2(contact.localPoint, contact.localPointOther);
        //const localPoint      = this.entity.getPosition().clone().add(contact.localPoint);
        //const localPointOther = this.entity.getPosition().clone().add(contact.localPointOther);
        //this.app.drawWireSphere(localPoint     , 0.1, pc.Color.GREEN , 20, false);
        //this.app.drawWireSphere(localPointOther, 0.1, pc.Color.YELLOW, 20, false);
        // point/pointOther
        const point      = contact.point;
        const pointOther = contact.pointOther;
        this.app.drawWireSphere(point     , 0.1, pc.Color.GREEN , 20, false);
        this.app.drawWireSphere(pointOther, 0.1, pc.Color.YELLOW, 20, false);
        this.app.drawLine(pc.Vec3.ZERO, contact.normal, pc.Color.RED, false);
        this.app.drawLine(pc.Vec3.ZERO, normalFromLocalPoints, pc.Color.CYAN, false);
    }
}

normalFromLocalPoints will render like this:

image

The red line is contact.normal which is first "right" and then "wrong":

image

kungfooman avatar Aug 18 '22 11:08 kungfooman

Nice! Should we create a PR with some helper function here or something similar?

mvaligursky avatar Aug 18 '22 11:08 mvaligursky

Well, potentially we could change the way we construct the normal. Instead of using whatever Ammo is giving us directly, we could add additional test for checking if it needs an inverse:

https://github.com/playcanvas/engine/blob/91811cf100e10b3fb44078673ec4b6c0ed41bda7/src/framework/components/rigid-body/system.js#L577 https://github.com/playcanvas/engine/blob/91811cf100e10b3fb44078673ec4b6c0ed41bda7/src/framework/components/rigid-body/system.js#L594

However, this is a critical code, that runs every frame. The use of a contact normal is not as common and can be easily inversed by user, if need be. Unless, you are proposing some kind of utility function, like the procedural stuff, that user would run manually?

LeXXik avatar Aug 18 '22 11:08 LeXXik

Maybe it could be part of the getter of normal on the contact object? So it would only do the check and cache the result if the user access contact.normal?

yaustar avatar Aug 18 '22 11:08 yaustar

The use of a contact normal is not as common and can be easily inversed by user, if need be.

I don't get it, how can you easily tell that a normal is "wrong"?

I guess best case would be someone takes the time to compile Ammo with debug symbols and trying to figure out why the normals don't work as expected in devtools and then fix Ammo itself.

kungfooman avatar Aug 19 '22 11:08 kungfooman