API refactoring (was ugliness)

[Erwin Coumans]

Thanks Steve!

I will make some plan/preparation very soon, and you can pick some task you if you got time, that is appreciated!

Also, I leave the difficult work on motors/limits up to Dirk
I prefer working on (continuous) collision detection and improving the performance and overal programmability etc. Adding the current sequential impulse solver with constraints was enough fun in that area.

Thanks,
Erwin

[Erwin Coumans]

The first step in refactoring is committed.

http://continuousphysics.com/Bullet/php ... .php?t=553

Source code moved to src, 2 new headerfiles for convenience, only the src folder needs to be included in projects (instead of 3 path before, LinearMath, Bullet, BulletDynamics now becomes src).

The files in BulletCollision and BulletDynamics have bt prefix. Other files in Demos and Extras folder should not be touched, they are not part of the library. LinearMath still has prefix Simd and Gen, but some files have been moved away etc.

Erwin

[SteveBaker]

I'm surprised that the "Extras/PhysicsInterface" stuff isn't a part of the 'main' package (and hence will be missing the 'bt' prefix).

Those functions aren't demos - they are actually needed - and as far as I can tell, the code doesn't come from someone else's library like the COLLADA and LibXML stuff.

Is there a case for moving them into the new 'src' area?

[Erwin Coumans]

I'm surprised that the "Extras/PhysicsInterface" stuff isn't a part of the 'main' package (and hence will be missing the 'bt' prefix).

Those functions aren't demos - they are actually needed - and as far as I can tell, the code doesn't come from someone else's library like the COLLADA and LibXML stuff.

Is there a case for moving them into the new 'src' area?

This CcdPhysicsEnvironment and CcdPhysicsController are experimental mainloops. I am hoping to add TimeWarp or other timestepping algorithms that take time of impact into account. There is also a ParallelPhysicsEnvironment, heading for multi-core. The history of those files is a physics-engine-independent interface. It was used abstracting Havok, Ageia, Mathengine, ODE, Dynamo, Sumo, Bullet etc. Please ignore it for now, and focus on BulletCollision and BulletDynamics.

I will add a DiscreteDynamicsWorld in BulletDynamics very soon. Then all demos will use this, except for some the 'continuuos collision detection, research demo. Once I'm happy with the quality of time-of-impact based physics, I will add a ContinuousDynamicsWorld to BulletDynamics.

So please ignore Demos and Extras in the refactoring, apart from getting it to compile obviously , just tackle the src folder. I will get in touch about the details, it would be useful if you can help.

Thanks,
Erwin

[Erwin Coumans]

I started working on replacing CcdPhysicsEnvironment with a direct version inside BulletDynamics.

Its objects are just RigidBody, and CollisionShapes.

See http://svn.sourceforge.net/viewvc/bulle ... iew=markup for a sneak preview.

Next to btSimpleDynamicsWorld, a btDiscreteDynamicsWorld and byContinuousDynamicsWorld will be added.

[Erwin Coumans]

ok, most of the issues have been address hopefully.

Here is a new release with lots of refactoring...
http://www.continuousphysics.com/Bullet ... .php?t=560

The CcdPhysicsEnvironment is now obsolete, and replaced by btDynamicsWorld derived classes.

[cippyboy]

I thought of starting another topic but maybe it should be adressed here.

How about deallocation of stuff ? I don't think I've seen a demo that deallocates anything. Miraculously I see that I first have to remove the ccdphysics controller from the enviroment in order to delete it's shape and then the object and even so, VC8 shows me a ton of memory leaks and it's not from my code (i'm having cases when I don't use it). Could there (ever) be 2 easy functions like : Enviroment->ClearPhysicalObjects and DeleteEnviroment ? or something like that.

[Erwin Coumans]

I thought of starting another topic but maybe it should be adressed here.

How about deallocation of stuff ? I don't think I've seen a demo that deallocates anything. Miraculously I see that I first have to remove the ccdphysics controller from the enviroment in order to delete it's shape and then the object and even so, VC8 shows me a ton of memory leaks and it's not from my code (i'm having cases when I don't use it). Could there (ever) be 2 easy functions like : Enviroment->ClearPhysicalObjects and DeleteEnviroment ? or something like that.

The CcdPhysicsEnvironment and CcdPhysicsController are obsolete and will be erased from the project. They have been replaced by btDiscreteDynamicsWorld. It's just that not all demos are updated yet (I preferred to get the release out first).

Proper memory handling, including custom memory allocators and reference counting is on the todo list. I will add a demo with a custom memory allocator that keeps track of all allocations, to make sure no leaks happen.

Bullet uses this memory rule: you are responsible to destroy what you construct (*).
Do you have any trace of a memory leak, which violate this rule?

Thanks!
Erwin

(*) Bullet has no reference counting yet, but for reference counted objects are an exception to above rule: reference-counted objects will self-destruct as soon as there are no refences to it (its reference count becomes zero)

[cippyboy]

With this initialization

Code: Select all

 CollisionDispatcher*	  dispatcher =new CollisionDispatcher();
	//
	SimdVector3 worldAabbMin(-30000,-30000,-30000);
	SimdVector3 worldAabbMax(30000,30000,30000);
	//
	OverlappingPairCache *broadphase=new SimpleBroadphase();
	//
	physicsEnvironmentPtr = new CcdPhysicsEnvironment(dispatcher,broadphase);
	physicsEnvironmentPtr->setDeactivationTime(2.f);
	//
	physicsEnvironmentPtr->setGravity(0,-10,0);
 

and just one deallocation

Code: Select all

delete physicsEnvironmentPtr;

 

gives this

Code: Select all

Detected memory leaks!
Dumping objects ->
{2303} normal block at 0x01D04FD8, 4 bytes long.
 Data: <xn  > 78 6E DD 00 
{2301} normal block at 0x01DAEE70, 16384 bytes long.
 Data: <                > CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD CD 
{2300} normal block at 0x01DAAE30, 16384 bytes long.
 Data: <                > 00 00 00 00 01 00 00 00 02 00 00 00 03 00 00 00 
{2299} normal block at 0x01D82DF0, 163840 bytes long.
 Data: <                > 00 00 00 00 CD CD CD CD 00 00 00 00 00 00 00 00 
{2298} normal block at 0x01D62DB0, 131072 bytes long.
 Data: <                > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
{2297} normal block at 0x01D62D48, 44 bytes long.
 Data: <8d   -          > 38 64 DD 00 B0 2D D6 01 00 00 00 00 00 20 00 00 
{2296} normal block at 0x00875EA0, 1508 bytes long.
 Data: < d              > C0 64 DD 00 CD CD CD CD 00 00 00 00 00 00 00 00 
Object dump complete.
 

I guessed that the enviroment self deallocates the Broadphases and all that stuff inside it, guess not ?

EDIT:cleaned unused code

[B_old]

Bullet uses this memory rule: you are responsible to destroy what you construct (*).

This can be misleading in my oppinion.
When you construct the DynamicsWorld you provide pointers to different stuff that YOU have allocated, yet the destructor of DynamicsWorld deletes it by itself.
At least that is how I understand it.

[Dirk Gregorius]

As Erwin stated. You are responsible for what you allocate. Normally you simply deallocate in opposite order and IMO this is a very practical solution Erwin uses here. Maybe Erwin should consider to explecitely point out that the Environment class doesn't take the ownership of the passed objects.

[Dirk Gregorius]

This can be misleading in my oppinion.
When you construct the DynamicsWorld you provide pointers to different stuff that YOU have allocated, yet the destructor of DynamicsWorld deletes it by itself.
At least that is how I understand it.

Then you understand it wrong. If I create the Dispatcher with placement new and use a special allocator, how should the Envirnment know about this. You can't simply call delete since you don't know how the object was constructed.

[B_old]

Code: Select all

btDiscreteDynamicsWorld::~btDiscreteDynamicsWorld()
{
	delete m_islandManager ;

	delete m_constraintSolver;

	//delete the dispatcher and paircache
	delete m_dispatcher1;
	m_dispatcher1 = 0;
	delete m_pairCache;
	m_pairCache = 0;
}

Am I looking in the wrong place?

[Dirk Gregorius]

This is indeed misleading and IMO this isn't correct. Bullet doesn't know how the object was created and therefore shouldn't destroy the object. Actually this is totally opposite to what Erwin wrote before. Maybe he will comment this himself. The rule of thump is: "Who creates the object should also destroy it".

Erwin:
For the referenced counted objects you should consider to pass an optional deleter function.

[Erwin Coumans]

Am I looking in the wrong place?

You are looking in the wrong place. On my harddrive is Bullet 2.11 which checks a boolean for ownership.

If the user passes it in, it is false and the user needs to delete it.
Otherwise, the btDiscreteDynamicsWorld calls a different constructor, and that ownership boolean is true, in which case the destructor deletes it.

There are no reference-counted object yet. As I mentioned before, memory management, custom allocators and reference counting is on the todo/work in progress. So don't make conclusions about memory management, until this is done