Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sphere-sphere collisions not reported in native Bullet #1029

Closed
stephengold opened this Issue Mar 2, 2019 · 18 comments

Comments

Projects
None yet
3 participants
@stephengold
Copy link
Contributor

commented Mar 2, 2019

Native Bullet doesn't report sphere-sphere collisions to registered collision listeners. This issue affects rigid bodies, ghost objects, and combinations of the 2. It doesn't affect collisions between spheres and other shapes. It also doesn't affect JBullet. The issue is found in JME 3.2.2 and recent master branch. I haven't yet gone back to see whether it's a regression.

Here is a simple test:

package jme3test.bullet;

import com.jme3.app.Application;
import com.jme3.app.SimpleApplication;
import com.jme3.bullet.BulletAppState;
import com.jme3.bullet.PhysicsSpace;
import com.jme3.bullet.collision.PhysicsCollisionEvent;
import com.jme3.bullet.collision.PhysicsCollisionListener;
import com.jme3.bullet.collision.shapes.BoxCollisionShape;
import com.jme3.bullet.collision.shapes.CollisionShape;
import com.jme3.bullet.collision.shapes.SphereCollisionShape;
import com.jme3.bullet.objects.PhysicsGhostObject;
import com.jme3.bullet.objects.PhysicsRigidBody;
import com.jme3.math.Vector3f;

/**
 * Test sphere-sphere collision reporting via a listener.
 *
 * @author Stephen Gold sgold@sonic.net
 */
public class TestSphereSphereCollision
        extends SimpleApplication
        implements PhysicsCollisionListener {

    private double elapsedSeconds = 0.0;

    public static void main(String[] arguments) {
        Application application = new TestSphereSphereCollision();
        application.start();
    }

    @Override
    public void simpleInitApp() {
        BulletAppState bulletAppState = new BulletAppState();
        stateManager.attach(bulletAppState);
        bulletAppState.setDebugEnabled(true);

        PhysicsSpace physicsSpace = bulletAppState.getPhysicsSpace();
        physicsSpace.addCollisionListener(this);

        CollisionShape shape;
        shape = new SphereCollisionShape(1f);
        //shape = new BoxCollisionShape(new Vector3f(1f, 1f, 1f));

        PhysicsRigidBody staticBody = new PhysicsRigidBody(shape, 0f);
        physicsSpace.add(staticBody);

        PhysicsGhostObject ghost = new PhysicsGhostObject(shape);
        physicsSpace.add(ghost);
    }

    @Override
    public void simpleUpdate(float tpf) {
        super.simpleUpdate(tpf);

        elapsedSeconds += tpf;
        if (elapsedSeconds > 1.0) {
            throw new RuntimeException("No collisions reported!");
        }
    }

    @Override
    public void collision(PhysicsCollisionEvent event) {
        Class aClass = event.getObjectA().getCollisionShape().getClass();
        String aShape = aClass.getSimpleName().replace("CollisionShape", "");
        Class bClass = event.getObjectB().getCollisionShape().getClass();
        String bShape = bClass.getSimpleName().replace("CollisionShape", "");

        System.out.printf("%s-%s collision reported at t = %f sec%n",
                aShape, bShape, elapsedSeconds);
        stop();
    }
}
@stephengold

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

Looks like this was a regression introduced between JME 3.1.0-stable and 3.2.0-stable. In other words, between Bullet 2.82-r2704 and Bullet 2.86.1

@riccardobl

This comment has been minimized.

Copy link
Member

commented Mar 2, 2019

This is the commit that changed the behaviour bulletphysics/bullet3@dcd02a1

disable sphere-sphere contact cache, it is buggy (some contact point stay in the cache, when sphere penetrates more than total margins)

Removing #define CLEAR_MANIFOLD 1 from btSphereSphereCollisionAlgorithm.cpp restores it, but as you see from the commit message, according to the developer, this is broken..

@stephengold stephengold self-assigned this Mar 2, 2019

@stephengold

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

There's also a comment in the CPP file that warns not to #define CLEAR_MANIFOLD:

#ifdef CLEAR_MANIFOLD
	m_manifoldPtr->clearManifold(); //don't do this, it disables warmstarting
#endif

It's broken either way, eh?

@stephengold

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

If we implemented a btMultiSphereShape-based CollisionShape in jme3-bullet, we'd at least have a workaround.

@stephengold

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

What sort of Java changes do you anticipate?

@riccardobl

This comment has been minimized.

Copy link
Member

commented Mar 2, 2019

PhysicsCollisionEvent

    public PhysicsCollisionEvent(int type, PhysicsCollisionObject nodeA, PhysicsCollisionObject nodeB, long manifoldPointObjectId) {

manifoldPointObjectId is the address for a btManifoldPoint, we don't have it in the gContactStartedCallback, but we have btPersistentManifold, see typedefs below

What is used now

typedef bool(* ContactProcessedCallback) (btManifoldPoint &cp, void *body0, void *body1)

What we could use to fix the issue

typedef void(* ContactEndedCallback) (btPersistentManifold *const &manifold)

or

typedef void(* ContactStartedCallback) (btPersistentManifold *const &manifold)

So we would probably have to change also the methods that are used to retrieve all the infos that are expected by jme (if we can do that with this callback...).

@riccardobl

This comment has been minimized.

Copy link
Member

commented Mar 2, 2019

Ok, ignore what i said, I took a better look at the apis, and we can get the btManifoldPoints from btPersistentManifold using btManifoldPoints.getContactPoint.

@stephengold

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

You seem to have a good grasp of this issue. Shall I assign it to you?

@riccardobl

This comment has been minimized.

Copy link
Member

commented Mar 3, 2019

I have other priorities at the moment, but i tried to quicky implement the callback

jmePhysicsSpace.cpp

--- <unnamed>
+++ <unnamed>
@@ -150,8 +150,8 @@
     dynamicsWorld->getPairCache()->setOverlapFilterCallback(new jmeFilterCallback());
     dynamicsWorld->setInternalTickCallback(&jmePhysicsSpace::preTickCallback, static_cast<void *> (this), true);
     dynamicsWorld->setInternalTickCallback(&jmePhysicsSpace::postTickCallback, static_cast<void *> (this));
-    if (gContactProcessedCallback == NULL) {
-        gContactProcessedCallback = &jmePhysicsSpace::contactProcessedCallback;
+    if (gContactStartedCallback == NULL) {
+        gContactStartedCallback = &jmePhysicsSpace::contactStartedCallback;
     }
 }
 
@@ -183,11 +183,11 @@
     }
 }
 
-bool jmePhysicsSpace::contactProcessedCallback(btManifoldPoint &cp, void *body0, void *body1) {
-    //    printf("contactProcessedCallback %d %dn", body0, body1);
-    btCollisionObject* co0 = (btCollisionObject*) body0;
+void jmePhysicsSpace::contactStartedCallback(btPersistentManifold *pm) {
+    btCollisionObject* co0 = pm->getBody0();
+    btCollisionObject* co1 = pm->getBody1();
+    if(co0==NULL||co1==NULL)return;
     jmeUserPointer *up0 = (jmeUserPointer*) co0 -> getUserPointer();
-    btCollisionObject* co1 = (btCollisionObject*) body1;
     jmeUserPointer *up1 = (jmeUserPointer*) co1 -> getUserPointer();
     if (up0 != NULL) {
         jmePhysicsSpace *dynamicsWorld = (jmePhysicsSpace *)up0->space;
@@ -197,18 +197,18 @@
             if (javaPhysicsSpace != NULL) {
                 jobject javaCollisionObject0 = env->NewLocalRef(up0->javaCollisionObject);
                 jobject javaCollisionObject1 = env->NewLocalRef(up1->javaCollisionObject);
-                env->CallVoidMethod(javaPhysicsSpace, jmeClasses::PhysicsSpace_addCollisionEvent, javaCollisionObject0, javaCollisionObject1, (jlong) & cp);
+                for(int i=0;i<pm->getNumContacts();i++){
+                    env->CallVoidMethod(javaPhysicsSpace, jmeClasses::PhysicsSpace_addCollisionEvent, javaCollisionObject0, javaCollisionObject1, (jlong) & pm->getContactPoint(i));
+                    if (env->ExceptionCheck()) {
+                        env->Throw(env->ExceptionOccurred());
+                    }
+                }
                 env->DeleteLocalRef(javaPhysicsSpace);
                 env->DeleteLocalRef(javaCollisionObject0);
                 env->DeleteLocalRef(javaCollisionObject1);
-                if (env->ExceptionCheck()) {
-                    env->Throw(env->ExceptionOccurred());
-                    return true;
-                }
             }
         }
     }
-    return true;
 }
 
 btDynamicsWorld* jmePhysicsSpace::getDynamicsWorld() {

jmePhysicsSpace.h

--- <unnamed>
+++ <unnamed>
@@ -62,5 +62,5 @@
         JNIEnv* getEnv();
         static void preTickCallback(btDynamicsWorld*, btScalar);
         static void postTickCallback(btDynamicsWorld*, btScalar);
-        static bool contactProcessedCallback(btManifoldPoint &, void *, void *);
+        static void contactStartedCallback(btPersistentManifold*);
 };

But this crashes with

SIGSEGV (0xb) at pc=0x00007f0483d57bb6, pid=4564, tid=0x00007f0483ac1700
C  [libbulletjme.so+0x295bb6]  btCollisionObject::getUserPointer() const+0xc

on this line

    jmeUserPointer *up0 = (jmeUserPointer*) co0 -> getUserPointer();

No idea why.

@Ali-RS

This comment has been minimized.

Copy link
Member

commented Mar 3, 2019

I am pinging @erwincoumans just in case he might have some insight of the issue.
sphere-sphere collisions detection should really be something that provided in any physics library, It's pretty weird that bullet can not handle it.

@stephengold

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2019

Note that when 2 spherical rigid bodies collide, there's an exchange of momentum. So I know contact response works. My current approach is to trace the addCollisionEvent_native() path backward and see what's different about sphere-sphere as opposed to, say, sphere-box.

Btw, in the Minie library, I've basically undefined CLEAR_MANIFOLD. So far I haven't noticed any ill effects such as contact points stuck in the cache. However, I haven't tested it thoroughly.

@stephengold

This comment has been minimized.

Copy link
Contributor Author

commented Mar 3, 2019

This issue appears to be the root cause of JME issue #989

riccardobl added a commit to riccardobl/jmonkeyengine that referenced this issue Mar 3, 2019

@riccardobl

This comment has been minimized.

Copy link
Member

commented Mar 3, 2019

I realized that the callback signature was wrong in my previous patch, i fixed it and made a pr: #1031

It seems to be working fine so far in my game.

@stephengold

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

Looking good so far. Unless concerns arise, I plan to integrate this tomorrow.

@stephengold

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

@riccardobl I've written a test app, which I plan to integrate separately. Or you can provide your own, if you prefer.

@riccardobl

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

Go ahead, I don't have any test app since i've been testing this on your TestSphereSphereCollision and on my project.

stephengold added a commit that referenced this issue Mar 4, 2019

stephengold added a commit that referenced this issue Mar 4, 2019

fix issue #1029, integrate PR #1031
* Use gContactStartedCallback instead of gContactProcessedCallback to detect collision events. ( fix for #1029 )

* Fix  Conversion loses qualifiers
@stephengold

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

Fixed in master at 3.3-6737 (2bbf784).

@stephengold stephengold closed this Mar 4, 2019

@stephengold stephengold added this to the v3.2.3 milestone Mar 4, 2019

stephengold added a commit that referenced this issue Mar 17, 2019

stephengold added a commit that referenced this issue Mar 17, 2019

fix issue #1029, integrate PR #1031
* Use gContactStartedCallback instead of gContactProcessedCallback to detect collision events. ( fix for #1029 )

* Fix  Conversion loses qualifiers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.