Runtime error about BulletSoftBodyNode.appendAngularJoint()

Hello, our friends

I’m afraid if this forum is an appropriate place to post a bug report, but
I’d like to report a runtime error about BulletSoftBodyNode.appendAngularJoint()
in the following version of Panda3D.

Version:  Development SDK Panda3D-2013.06.21-40.exe for Windows.
Language: Python

That error happened in BulletWorld.do_physics() after calling
BulletSoftBodyNode.appendAngularJoint() as follows.

    softBody.appendAngularJoint(rigidBody, axis=Vec3(0, 1, 0), erp=0.5)

The cause of the runtime error seems as follows. BulletSoftBodyNode::append_angular_joint() in
the C++ source code is declared so that the argument ‘control’ should be NULL in default.

    void append_angular_joint(BulletBodyNode *body, const LVector3 &axis,
    PN_stdfloat erp=1.0,
    PN_stdfloat cfm=1.0,
    PN_stdfloat split=1.0,
    BulletSoftBodyControl *control=NULL);

The argument ‘control’ is then stored by a btSoftBody instance. Because btSoftBody::AJoint
does no NULL check when using btSoftBody::AJoint::m_icontrol to solve constraints,
that NULL in ‘control’ causes an access violation.

One way to avoid that runtime error is to specify a BulletSoftBodyControl instance
for the argument ‘control’ explicitly, and keep the instance from getting released
while repeatedly executing doPhysics().

     self.dummyCtrl = BulletSoftBodyControl()
     softBody.appendAngularJoint(rigidBody, axis=Vec3(0, 1, 0), erp=0.5, cfm=1.0, split=0, 
control=self.dummyCtrl)

It would be nice if BulletSoftBodyNode::append_angular_joint() is changed either

  1. to do NULL check:
    void BulletSoftBodyNode::
    append_angular_joint(BulletBodyNode *body, const LVector3 &axis, PN_stdfloat erp, PN_stdfloat
    cfm, PN_stdfloat split, BulletSoftBodyControl *control) {
      // ... omitted ...
      btSoftBody::AJoint::Specs as;
      // --> The constructor initializes as.control = btSoftBody::AJoint::IControl::Default()
      // ... omitted ...
      if(control != NULL){
        as.icontrol = control;
      }
      // ... omitted ...
    }
  1. Or, to have an appropriate default instance instead of NULL for the ‘control’ argument:
    // In bulletSoftBodyNode.cxx
    void append_angular_joint(BulletBodyNode *body, const LVector3 &axis,
    PN_stdfloat erp=1.0,
    PN_stdfloat cfm=1.0,
    PN_stdfloat split=1.0,
    BulletSoftBodyControl *control=static_cast<BulletSoftBodyControl*>(BulletSoftBodyControl::Default()));
    // --> Default() is a static member function inherited from the base class btSoftBody::AJoint::IControl.
  1. Or, to do the same as above by appropriately overriding that static Default()
    member function in BulletSoftBodyControl:
    void append_angular_joint(BulletBodyNode *body, const LVector3 &axis,
    PN_stdfloat erp=1.0,
    PN_stdfloat cfm=1.0,
    PN_stdfloat split=1.0,
    BulletSoftBodyControl *control=BulletSoftBodyControl::Default());
    // --> overridden Default().

I think that the option 3 above would be better than the others for its extensibility.

Regards,

Right, this is a bug. My assumption has been that you should assign NULL to as.icontrol if there should be no control for this angular joint. However, this is not right. Bullet seems expects to have a default interface assigned to this member. I can fix it in two ways. Either the NULL-check (your first suggestion), or assigning the default:

as.icontrol = control ? control : IControl::Default();

I will check in a fix this weekend. Thank for reporting, and the good analysis why the crash happens.

Remark: The BulletSoftBodyNode::append_angular_joint method has as last parameter “BulletSoftBodyControl *control=NULL”. It is NOT a PT(BulletSoftBodyControl). This means - by Panda3D conventions - that the BulletsoftBodyNode does NOT take ownership of the control. You are responsible for managing the lifttime of the control, and thus have to keep a reference to it around (e. g. self.control) until it is no longer needed.

I did consider making the control a TypedReferenceCount and then keep references to the controls around in a private vector, but then I have to problem of how to get rid of them again. The only idea I have would be to keep them until the soft body node itself is destroyed. If you have any suggestions feel free to post them.