API refactoring (was ugliness)

SteveBaker · Post by **SteveBaker** » Sat Sep 23, 2006 3:17 pm

There are some annoying uglinesses in the Bullet API that really need fixing.

FIRSTLY:

The one that upsets me the most is that global variables and classes don't have a single prefix (eg OpenGL functions all start 'gl') - which means that Bullet pollutes the namespace HORRIBLY. In large applications, this can be a serious pain in the butt.

SECONDLY:
The second one is that there is no consistancy of member function naming. For example:
class CollisionShape has: 'IsConvex' and 'CalculateLocalInertia' - but 'setLocalScaling' - it's really hard to write code when there is no standard about whether functions start with uppercase or lowercase. Worse still is 'GetAabb' - where 'AABB' is an acronym and would usually be capitalised. We even have 'setLocalScaling' and 'SetMargin'!!

This also makes it very tiresome to write code that uses Bullet. The library needs some standards about function and class naming - and it needs to be MUCH more careful about namespace pollution.

THIRDLY:
In order to use Bullet, I have to include dozens and dozens of header files (sometimes these have to be included in a very specific order - which is *NASTY*) and I have to link to half a dozen separate libraries. I think we should have:

#include <Bullet/Bullet.h>

...and be able to link to libBullet.so and nothing else.

Erwin Coumans · Post by **Erwin Coumans** » Sat Sep 23, 2006 3:28 pm

SteveBaker wrote:There are some annoying uglinesses in the Bullet API that really need fixing.

FIRSTLY:

The one that upsets me the most is that global variables and classes don't have a single prefix (eg OpenGL functions all start 'gl') - which means that Bullet pollutes the namespace HORRIBLY. In large applications, this can be a serious pain in the butt.

SECONDLY:
The second one is that there is no consistancy of member function naming. For example:
class CollisionShape has: 'IsConvex' and 'CalculateLocalInertia' - but 'setLocalScaling' - it's really hard to write code when there is no standard about whether functions start with uppercase or lowercase. Worse still is 'GetAabb' - where 'AABB' is an acronym and would usually be capitalised. We even have 'setLocalScaling' and 'SetMargin'!!

This also makes it very tiresome to write code that uses Bullet. The library needs some standards about function and class naming - and it needs to be MUCH more careful about namespace pollution.

THIRDLY:
In order to use Bullet, I have to include dozens and dozens of header files (sometimes these have to be included in a very specific order - which is *NASTY*) and I have to link to half a dozen separate libraries. I think we should have:

#include <Bullet/Bullet.h>
...and be able to link to libBullet.so and nothing else.

I agree with you, and I wanted to fix this for a while, but want to choose a good point in time. Because changing the API requires changings other peoples code-bases.

So to reply on FIRST: indeed, a prefix for the classes should be great. I want to avoid global variables if possible, it's pure lazyness in most cases (or for quick debugging). Preferable we use different prefixes for Collision Detection and Dynamics, the collision detection can be used independent of the dynamics. We could go for namespaces too, but lots of people have troubles using them properly, and I don't want to keep on doing support for that. In Havok we used lower-case prefix 'hk' for all classes, like hkConvexShape, so would could use blConvexShape.

On the naming, I prefer using capitals for the method and class names, to avoid confusion with variable names. So small 'setMargin' is indeed bad. It became a mixture, because parts came from different libraries (Solid's math library for example). Needs to be fixed, just like the class prefixes.

Ideally I want a C interface-API going along with the library. That hides most internals, and focusses on the usage. We can add a C-layer (bullet.h) on top of Bullet similar to ODE or Solid (see http://www.dtecta.com).

I put it on the TODO list. Probably best if I do that in one go at some stage, we can call it Bullet 2.5

http://www.continuousphysics.com/mediaw ... BulletTodo

Thanks,
Erwin

Jack · Post by **Jack** » Sat Sep 23, 2006 3:58 pm

Perhaps it would be great to wrap BULLET into COM (at least for Windows platform)......

Similar to DirectX

Erwin Coumans · Post by **Erwin Coumans** » Sat Sep 23, 2006 5:18 pm

It would be great to add some wrappers, in the Extras folder, it's just a matter of time and priorities.

Python bindings, Java, C#/XNA (already started), and perhaps COM.

SteveBaker · Post by **SteveBaker** » Sat Sep 23, 2006 6:16 pm

Jack wrote:Perhaps it would be great to wrap BULLET into COM (at least for Windows platform)...... :roll: Similar to DirectX :roll:

The trouble with that is that it makes applications code non-portable. I think it's a bad thing.

SteveBaker · Post by **SteveBaker** » Sat Sep 23, 2006 6:18 pm

Erwin Coumans wrote:It would be great to add some wrappers, in the Extras folder, it's just a matter of time and priorities.

Python bindings, Java, C#/XNA (already started), and perhaps COM.

For my own sanity, I have a wrapper that pulls all of the Bullet libraries into one library and all of the headers into one header. It's probably not much good in general - but the name I picked for it ("Sabot") is something I'm kinda pleased with! :-)

SteveBaker · Post by **SteveBaker** » Sat Sep 23, 2006 6:35 pm

Erwin Coumans wrote: I agree with you, and I wanted to fix this for a while, but want to choose a good point in time. Because changing the API requires changings other peoples code-bases.

The problem is that the longer you leave it, the more programs exist that use it and the harder it becomes to change. It's better to grasp the nettle right now than to wait.

So to reply on FIRST: indeed, a prefix for the classes should be great. I want to avoid global variables if possible, it's pure lazyness in most cases (or for quick debugging). Preferable we use different prefixes for Collision Detection and Dynamics, the collision detection can be used independent of the dynamics. We could go for namespaces too, but lots of people have troubles using them properly, and I don't want to keep on doing support for that. In Havok we used lower-case prefix 'hk' for all classes, like hkConvexShape, so would could use blConvexShape.

Yeah - namespaces are nice - but the widespread adoption of the 'two letter prefix' thing has become something of an expected thing. What's nice about it is that you can look at a simple, unqualified name and know what it means.

On the naming, I prefer using capitals for the method and class names, to avoid confusion with variable names.

Actually, I prefer initial-capitals for class names, initial-lowercase for class members and all caps for constants and enumeration tags - that way you can see which is which at a glance. I'd prefer ANY kind of standard to no standard!

Ideally I want a C interface-API going along with the library. That hides most internals, and focusses on the usage. We can add a C-layer (bullet.h) on top of Bullet similar to ODE or Solid (see http://www.dtecta.com).

Those things can generally be generated automatically these days. Packages like SWIG can generate wrappers for almost any language from C++ headers.

The trick for manageing the transition period is to have something like:

Code: Select all

  class blBoxShape
  {
  #ifndef MODERN_BULLET_API
    inline void setLocalScaling ( const SimdVector3& scaling) { SetLocalScaling ( scaling ) ; }
  #endif
  } ;
  #ifndef MODERN_BULLET_API
    typedef blBoxShape BoxShape ;
  #endif

...then to #define MODERN_BULLET_API in whatever new, unified header file we chose. Then, new applications using the new header file would automatically disallow old style API but old style applications would get all of the legacy junk.

It would be quite a lot of editing - but kinda brainless work that can be done while watching StarTrek reruns!

Erwin Coumans · Post by **Erwin Coumans** » Sat Sep 23, 2006 7:10 pm

Let me think about the consistent naming and prefixes for a bit. I agree with you we shouldn't wait too long.

Those things can generally be generated automatically these days. Packages like SWIG can generate wrappers for almost any language from C++ headers.

No, I don't want to expose all internal C++ classes. I know swig and other tools, but that's not the direction I want to go. Please look at Solid library for what I mean, or ODE. You can download Solid at http://www.dtecta.com . This C-API only exposes the necessary bits for the user, it is the 'high level api' that is enough for 95% of the users. I don't want to expose them to C++ classes or concepts like PersistentManifold, Dispatcher etc.

Developers who want to work with the low level C++ classes don't need this C-wrapper. We can provide one header BulletLowlevelCppAPI.h, which includes most common header files. I organized Bullet's dependencies fairly careful, and a lot of functionality can be customized using C++.

So ideally there is this high-level single C-api headerfile, 'bullet.h', and a more low-level existing C++ API, with most commonly used C++ headers grouped into BulletLowlevelCppAPI.h.

Still, I prefer developers to know what they include, in most cases the depencies are very minimal. If possible don't include a headerfile, but just use a forward declaration. Also, such future grouping headerfile should NEVER be used in Bullet internally. It completely messes up the dependencies

Please click on 'directories' tab in the doxygen generated docs:
http://www.continuousphysics.com/Bullet ... index.html

If you click on either Bullet, BulletDynamics, Extras, LinearMath, Demos you can see the one-way dependencies (no circularities!). Even within Bullet Collision Detection, you will notices that the seperate folders have one-way dependencies. For example, files in Bullet/CollisionShapes never include a header from Bullet/NarrowPhase/*

There is a lot more information that would be useful to know, when we start talking about high level API. The Extras/PhysicsInterface is physics engine independent, with CcdPhysicsEnvironment a Bullet implementation. It would be good to be able to use Bullet without requiring those 'abstraction'. In other words, there is some work needed to add some mainloop/container/DynamicsWorld in BulletDynamics.

Thanks,
Erwin

Dragonlord · Post by **Dragonlord** » Sat Sep 23, 2006 7:40 pm

It's really best to cleanup the API right now as the longer you wait the messier it gets to repair it. Furthermore you tend to push it away and away every time ( god how I know this myself ).

What goes for naming don't use namespaces. They are a pain in the ass and not good to work with. a prefix like "bl" is the best thing to do as most of the GPL stuff I know works like that and it's very good to work with it.

The last thing is this header file thing. I don't really care what you do but don't "hide" things from developers using multiple "sabot" include files. If it's an open source type project always make all headers open and provide one huge include file for the lazy bums out there. Anything else makes it trickier for the more advanced users to work with. Furthermore there is nothing wrong with cross references from a .cpp to another directory as long as the .h files always work with base interfaces.

- Pl?ss Roland

Erwin Coumans · Post by **Erwin Coumans** » Sat Sep 23, 2006 7:57 pm

Dragonlord wrote:It's really best to cleanup the API right now as the longer you wait the messier it gets to repair it. Furthermore you tend to push it away and away every time ( god how I know this myself ).

What goes for naming don't use namespaces. They are a pain in the ass and not good to work with. a prefix like "bl" is the best thing to do as most of the GPL stuff I know works like that and it's very good to work with it.

The last thing is this header file thing. I don't really care what you do but don't "hide" things from developers using multiple "sabot" include files. If it's an open source type project always make all headers open and provide one huge include file for the lazy bums out there. Anything else makes it trickier for the more advanced users to work with. Furthermore there is nothing wrong with cross references from a .cpp to another directory as long as the .h files always work with base interfaces.

- Pl?ss Roland

Ok, I got the message, I'll add a prefix soon. I also want to rename Bullet/Bullet to Bullet/BulletCollision, to make it clear where the collision detection is. The dynamics is already in Bullet/BulletDynamics.

There will be no things hidden for C++ developers. C developers who want to use a future C-API won't (obviously) have all internal classes exposed. Adding a basic C API is already planned a while ago, to make a more tight integration with Blender modeler (C-sources) possible. Right now, Bullet is only integrated in Blender's C++ game engine.

I try to prevent cross-references wherever possible. For example, right now you can take Bullet's GjkPairDetector or Broadphase fairly easy, without having to worry about all the other stuff. In too many libraries it is a pain to just take the bits you want, and leave the other.

Steve: thanks for the cleanup, I double checked all changes, and they look reasonable ( http://svn.sourceforge.net/viewvc/bulle ... vision=280 )
Erwin

Dirk Gregorius · Post by **Dirk Gregorius** » Sun Sep 24, 2006 11:05 am

IMO the API has also to many unnecessary indirections (layers). I totally understand that you implemtented it this way in order to try different things out or in order to compare BULLET e.g. against SOLID. Maybe you consider a 2.0 version of BULLET which uses a more slender API. While using a wrapper like SOLID makes it easier for clients to use BULLET it will become very difficult to maintain the core API for you. Honestly I only see three modules/layers for a physic system, namely a math, collision and physic library. You maybe want to consider this as well.

Something I find very usefull is to give filenames prefixes as well, since it works nicely with SlickEdit or VisualAssist "OpenFrom Workspace" option, e.g. Math_Vector3.hpp. You do this using the directories, but you maybe want to consider this option as well.

Cheers,
-Dirk

SteveBaker · Post by **SteveBaker** » Sun Sep 24, 2006 2:05 pm

Erwin Coumans wrote:
Those things can generally be generated automatically these days. Packages like SWIG can generate wrappers for almost any language from C++ headers.
No, I don't want to expose all internal C++ classes. I know swig and other tools, but that's not the direction I want to go. Please look at Solid library for what I mean, or ODE. You can download Solid at http://www.dtecta.com . This C-API only exposes the necessary bits for the user, it is the 'high level api' that is enough for 95% of the users. I don't want to expose them to C++ classes or concepts like PersistentManifold, Dispatcher etc.

I strongly disagree.

What you do or do not expose shouldn't depend on the programming language your users happen to be writing in. That's just silly. If there is a 'simplified bullet' subset API and a 'deep guru with access to everything' API then reflect those in two separate C++ header files that each contains only what that set of users needs access to. Then you can 'SWIG' (or whatever) each of those to generate a 'simple bullet' and a 'guru bullet' in any language your end users happen to want to write code in. The language used and the API accessed are orthogonal matters. It's just not reasonable to assume that just because someone has chosen to write in (say) Python that they need a really simple API - or that someone who is using C++ automatically wants to see the whole complexity of the system.

Still, I prefer developers to know what they include, in most cases the depencies are very minimal. If possible don't include a headerfile, but just use a forward declaration.

You prefer it because you live and breathe the internals of this library. The vast majority of end users most definitely won't prefer it. They want it to be a neat little black box that quietly gets on and does the physics - with the minimum possible amount of interface.

As an end user in your target demographics, I can tell you that Bullet was a real pain to deal with. I had no easy way to know what I do or don't want. For my VERY simple first effort at putting Bullet physics into my game, I needed to include TWENTY EIGHT header files spread across SEVEN internal Bullet directories! It was hard to figure out which ones I needed and even harder to get them all in the right order so it would compile. Worse still, if I add another feature into my code, I have to go through that pain all over again. Header files are scattered all over Bullet's internal heirarcy - so I'm including files from seven different directories that reflect only some arcane internal structure of Bullet that your end users frankly don't give a damn about. When Bullet installs itself, there should be at most two or three header files and two or three '.so'/'.DLL' libraries - not some huge heirarchy of directories with headers and libraries scattered all over.

I shouldn't have to do any of that. I should be able to say:

Code: Select all

   #include <Bullet/Bullet.h>

...or at the very most:

Code: Select all

  #include <Bullet/BulletMath.h>
  #include <Bullet/BulletCollision.h>
  #include <Bullet/BulletPhysics.h>

...and even then, it shouldn't matter which order I include them - it should always 'just work'. Then at link time:

Code: Select all

  -l bullet

...or at most:

Code: Select all

  -l bulletmath  -l bulletcollision  -l bulletphysics

Also, such future grouping headerfile should NEVER be used in Bullet internally. It completely messes up the dependencies :)

Whatever.

Other really complex libraries manage to do this - Bullet isn't special in that regard. It's just a matter of some cleanup.

The end user shouldn't have to care about it - or have to suffer the consequences of it.

Erwin Coumans · Post by **Erwin Coumans** » Sun Sep 24, 2006 2:18 pm

You prefer it because you live and breathe the internals of this library. The vast majority of end users most definitely won't prefer it. They want it to be a neat little black box that quietly gets on and does the physics - with the minimum possible amount of interface.

Due to the fact that Bullet is all open source C++ code, developers can easily get lost in all folders and headerfiles of its internal code.

What I want is to provide a BulletCApi.h that provide the includes for most users (high level, not exposing internal details). C++, Python, Java, C# bindings can all use this API. Then a package will be distributed with just the precompiled library .a/.lib and such headerfiles.

Developers who want to extend and modify internal workings of the library can ignore this API and precompiled library and use the library as it is right now. It is open source after all.

A rough idea for API methods:

Code: Select all

//create/destroy methods
btCollisionShape btCreateCollisionShape(..)
btDestroyCollisionShape(btCollisionShape);
btCollisionObject CreateCollisionObject(btCollisionShape, btMotionState)
btDestroyCollisionObject(btCollisionObject);
btPhysicsWorld btCreatePhysicsWorld();
btDestroyPhysicsWorld(btPhysicsWorld world);
btRigidBody btCreateRigidBody();
btRigidBody btDestroyRigidBody(btRigidBody);

//queries and stepping the world
btStepSimulation(btPhysicsWorld world,btScalar timestep);
/ray cast
btTestRay(btCollisionWorld world,btVector3 from,btVector3 to);
//single object-object collision query
btGetDistance(btCollisionObject obj0,btCollisionObject obj1);
//some stuff for callbacks etc.
btGetTimeOfImpact(btCollisionObject obj0,btCollisionObject obj1);

I will work on such a solution, and provide packages for several platforms, with just the library in a lib folder, and the API headers in the include folder.

Does that sound reasonable?

Erwin Coumans · Post by **Erwin Coumans** » Sun Sep 24, 2006 2:57 pm

Dirk wrote: IMO the API has also to many unnecessary indirections (layers). I totally understand that you implemtented it this way in order to try different things out or in order to compare BULLET e.g. against SOLID. Maybe you consider a 2.0 version of BULLET which uses a more slender API. While using a wrapper like SOLID makes it easier for clients to use BULLET it will become very difficult to maintain the core API for you. Honestly I only see three modules/layers for a physic system, namely a math, collision and physic library. You maybe want to consider this as well.

It would have been easier, if I would just have done like most others do in this field, and just provide a C interface/API. Providing C++ headerfiles/interface is asking for troubles

As I mentioned before in this thread, there is one PhysicsInterface module, which is the CcdPhysicsEnvironment/PhysicsInterface. Once that is removed, you only have 3 modules: LinearMath, BulletCollision and BulletDynamics.

Unfortunaty, the interaction between dynamics and collision detection is more complicated then just passing contact points (like ODE does).
Following aspects make the dependency/bridge between collison detection and physics more complicated:
* parallel dispatching of both island that contain both collision detection and physics
* maintain persistent contacts
* deal with continuous collision detection
* dynamics mainloop that can call the collision detector for timewarp or backstepping.

Those are some of the reasons that the extra CcdPhysicsEnvironment layer is introduced. It also allows some flexibility to experiment with different stepping strategies. One aspect is parallelism, and the other is handling time-of-impact queue (like Brian Mirtichs TimeWarp). This makes the interaction between collision detection and physics non-trivial.
Also, this additional layer (PHY_IPhysicsEnvironment/PHY_IPhysicsController, with CcdPhysicsEnvironment Bullet's implementation, ODEPhysicsEnvironment, PhysXEnvironment other implementations) provides abstraction of the physics engine (so using such API would allow switching engine easier). Probably a bit too ambituous, but it allows you to compare and switch physics engine.

There are a lot of different issues being discussed here, but mainly:

* Issue0: there is no easy distribution, with a a single API include file (and precompiled library). Right now, developers have to include zillions of headerfiles. I've seen this issue in other physics libraries too...
* Issue1: the naming of internal source code, use prefixes and consistent method capitulization

Both need and will be address. I had them in the planning, but didn't expect it was needed so urgently. There is a lot of other work needed in the library:

* constraint motors and limits
* memory management/referencing
* etc etc etc, see the Bullet todo list (sticky in this forum)

Hope this helps. Please don't get to excited about this matter, I will address it soon!
Erwin

SteveBaker · Post by **SteveBaker** » Sun Sep 24, 2006 8:30 pm

Erwin Coumans wrote: * Issue0: there is no easy distribution, with a a single API include file (and precompiled library). Right now, developers have to include zillions of headerfiles. I've seen this issue in other physics libraries too...
* Issue1: the naming of internal source code, use prefixes and consistent method capitulization

Yeah - that'll solve my issues

Both need and will be address. I had them in the planning, but didn't expect it was needed so urgently. There is a lot of other work needed in the library:

* constraint motors and limits
* memory management/referencing
* etc etc etc, see the Bullet todo list (sticky in this forum)

Well, feel free to delegate! I'm happy to help with the cleanup...let you get on with the stuff I can't do (constraint motors, etc).

Dumb editing is something I can manage!

Real-Time Physics Simulation Forum

API refactoring (was ugliness)

API refactoring (was ugliness)

Re: API ugliness.

Re: API ugliness.