search

czaloj / bullet

Automatically exported from code.google.com/p/bullet
0 stars 0 forks source link

btTransform and btMatrix3x3 identity intialization #139

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
This patch makes it easier to construct an instance with the identity.  In 
particular, if you want to 
pass the identity transform to a function, I see a lot of code do something 
like:
btTransform trans;
trans.setIdentity();
setTransform(trans);

With this patch you can do:
setTransform(btTransform().setIdentity());

I note there is a static getIdentity in btTransform, which is another way to do 
this, but there isn't 
a static getIdentity in btMatrix3x3.  You may prefer to add that.

However, an argument for supporting this style as well is if you make similar 
changes to setOrigin 
and setBasis etc., so you can chain calls such as:
    trans.setOrigin(xyz).setRotation(q)
or
    setTransform(btTransform().setIdentity().setOrigin(xyz))
this removes some need to have multiple constructors and set() permutations for 
such things

Finally, the static getIdentity calls may be more efficient to return a const 
btTransform& as such:
static const btTransform& getIdentity()
{
    static btTransform ident(btMatrix3x3().setIdentity());
    return ident;
}
(This avoids making a new construction on each call, only makes a copy when 
required)

Original issue reported on code.google.com by ejtt...@gmail.com on 10 Nov 2008 at 5:24

Attachments:

GoogleCodeExporter commented 9 years ago 

Thanks for the patch. 

We'll look into it after the Bullet 2.73 release (it has been delayed for too 
long).

Original comment by erwin.coumans on 11 Nov 2008 at 4:15

GoogleCodeExporter commented 9 years ago 
Completely agree with you on the cumbersome way of setting an identity 
transform like
this:

btTransform trans;
trans.setIdentity();
setTransform(trans);

But instead of using

setTransform(btTransform().setIdentity());

which is still rather cumbersome imo, why not use:

setTransform(btTransform::identity);

In that case you only need to keep a static member in the btTransform class.

We do that in a lot of our math classes

static const Matrix44::zero;
static const Matrix44::identity;

etc.

Furthermore I think that the identity should always be the default when 
constructing
a transform or matrix. No need to explicitly do that after it is constructed.

Original comment by mart...@twotribes.com on 9 Dec 2008 at 8:11

GoogleCodeExporter commented 9 years ago 
Ok, finally got around implementing this, but not using the 'setIdentity' with 
return 
value:
http://code.google.com/p/bullet/source/detail?r=1610

static const btTransform & btTransform::getIdentity();
static const btMatrix3x3 & btMatrix3x3::getIdentity();
static const btQuaternion& btQuaternion::getIdentity();

should be equivalent to the static ::identity

For performance reasons, all the math classes don't get initialized by default, 
just 
like the built-in C/C++ types (float, int etc). There are many places where 
those 
classes are used as temporary, and it would likely slow-down performance. We 
could,however, benchmark it, and if it really doesn't hurt performance we could 
initialize to identity by default.

Thanks,
Erwin

Original comment by erwin.coumans on 6 Feb 2009 at 7:34