Issues using the Box2D implementation #629

Closed
iforce2d opened this Issue Oct 10, 2012 · 4 comments

Comments

Projects
None yet
2 participants

I was very excited to discover Emscripten recently, and really pleased to see that a Box2D build was already listed as an example. I have been wanting to use Box2D in an HTML5 canvas for a while but the two main options are both ported from Flash and are somewhat ageing. Being able to keep up with the C++ source code will be a huge advantage.

For some reason this Emscripten port seems to be still not that well known, but I expect we will see a LOT of interest in it going forward. In fact, I think there is a good chance it could become the de-facto standard for HTML5 games using Box2D.

So I have been spending some time with it lately, and there are a few issues that come up when you take things beyond the simple falling boxes scenario. Hopefully this is the right place to discuss these, if not let me know where I should go. For some of them I have found a workaround, some not. I'll list them all anyway.

  1. Some classes are not named/bound, as a result of being abstract - this is easily dealt with by making their pure virtual functions non-pure (empty function body), and/or giving them a public constructor. No problem.
  2. The b2Color class has its member variables declared as:
    float32 r, g, b;
    but for some reason this results in only set_b() and get_b() being generated. The problem is solved by changing this to:
    float32 r;
    float32 g;
    float32 b;
    Again, no problem.
  3. Some functions take an array of points, eg. to define a polygon or chain shape. In the original C++ the parameter is const b2Vec2*, but I can't for the life of me figure out how I am supposed to pass that to the Emscripten generated function. I guess this could be worked around by adding a function in the C++ to take one point at a time and add it to some temporary internal buffer, then call a second function to actually construct the shape once the points were all passed in. This would be feasible but seems very clumsy and I feel I am missing something.
  4. Similar to 3. but the reverse. In some of the debug draw functions, Box2D gives you an array of points to draw. The original C++ function provides a const b2Vec2* and the number of points in the array. I have managed to use this successfully by wrapping an offset of the passed value to a b2Vec2 for every point individually:
    for( tmpI = 0; tmpI < vertexCount; tmpI++ ) { var vert = Box2D.wrapPointer(vertices+(tmpI*8), b2Vec2); if ( tmpI == 0 ) context.moveTo(vert.get_x(),vert.get_y()); else context.lineTo(vert.get_x(),vert.get_y()); }
    This is workable but probably quite inefficient, so again I feel like I am missing something.
  5. Some class members do not seem to be available in their subclasses, eg. all joint types connect two bodies, so the b2JointDef base class has two b2Body* members, but these are not visible in specific joint subclasses such as b2MouseJointDef. This can be worked around by adding SetBodyA(b2Body_) and SetBodyB(b2Body_) functions to each subclass explicitly. Again, no problem, but I wonder if this is the expected behavior.
  6. The original C++ uses a factory type function to create an instance of a concrete-class joint, for example a b2MouseJoint, but it returns a pointer to a generic b2Joint. In typical usage, the user is supposed to cast the returned value to whatever joint type they are creating, like:
    b2MouseJoint* myJoint = (b2MouseJoint*)world->CreateJoint(&jointDefinition);
    ...wherein the definition dictates that the joint should be a mouse joint. Unfortunately with this code:
    var joint = world.CreateJoint(jointDef); //joint is created correctly and behaves as expected console.log(joint); //prints 'b2Joint' var test1 = Box2D.castObject( joint, b2MouseJoint ); console.log(test1); //prints 'null'
    ...I cannot get hold of a proper b2MouseJoint variable with which to control the joint. I suspect this could be fixed by adding a dedicated function eg. CreateMouseJoint for each joint type, but I have not tried yet. It would be nice not to have to do that.
  7. Need to replace Float64Array with Float32Array to work on Safari. No problem.

Recently I've been trying to recreate the Box2D testbed in a canvas. Apart from issue 6. above it's pretty much good to go, you can see the work in progress here:
http://www.iforce2d.net/emscripten/test.html

Major kudos to all involved in making this project happen - looking forward to working with it a lot in future :)

Owner

kripken commented Oct 14, 2012

If you have fixes for some of those issues, pull requests to box2d.js are welcome of course.

In general, the path forward is the new bindings ("embind") that are now in the incoming branch. They are not quite done, but anyone that wants to see major improvements in the binding of libraries like box2d.js would get the best results from helping with embind itself and its application to box2d.js. The new bindings approach fixes a lot of limitations in the old hackish bindings (which were basically an experiment ;)

Sorry for the slow reply.

I don't think my JavaScript capabilities are enough to help with embind, but in the meantime I have made a few changes to help with the things pointed out above.

Issue 6 turned out to be simply that I had not made the constructor public in the joint subclass - I must have been getting tired that day.

As mentioned in another thread here, the conversion of Float64Array to Float32Array working at all was indeed a surprise. As you say, it sure doesn't seem like the right thing to do in the long run, but it does get things running on Safari and iOS and I have not noticed any problems. I have not committed the converted version, but I am using it in the demo on my site.

Unfortunately there are a couple of problems which remain. After making the changes in my pull request I found that running the test scenes gave me loads of errors like these, usually immediately upon starting, but sometimes after running for a half-minute or so, particularly with the falling shapes test:

Uncaught Assertion failed: 0 <= index && index < m_count, at: Box2D_v2.2.1/Box2D/Collision/b2Distance.h,103,const b2Vec2 &b2DistanceProxy::GetVertex(int32) const

Uncaught Assertion failed: m_entryCount < b2_maxStackEntries, at: Box2D_v2.2.1/Box2D/Common/b2StackAllocator.cpp,38,void *b2StackAllocator::Allocate(int32)

Uncaught Assertion failed: m_index == 0, at: Box2D_v2.2.1/Box2D/Common/b2StackAllocator.cpp,32,b2StackAllocator::~b2StackAllocator()

Uncaught Assertion failed: 0 <= proxyId && proxyId < m_nodeCapacity, at: Box2D_v2.2.1/Box2D/Collision/b2DynamicTree.h,159,const b2AABB &b2DynamicTree::GetFatAABB(int32) const

I found this could be reproduced merely by moving the constructor of certain joints (gear, prismatic) from protected to public, whereas other joints (mouse, revolute, wheel) could be changed in the same way without causing any problems. Even stranger, the problem would show up even when the test scene being run had nothing to do with joints at all. For the record, this was without any converting of Float64Array to Float32Array malarkey :)

After a good 5-6 hours of fruitless stabbing in the dark I discovered that --closure 0 avoids the problem. Thinking about it a bit more, this problem only showed up on the falling shapes scene, and that scene is the only one that uses a chain shape. Perhaps the stack allocation method used to create these is suspect? (see the createChainShape function in embox2d-helpers.js)

The only other problem I found was that b2Vec2.op_sub seems to simply negate the vector rather than subtract from it (op_add works fine).

In this commit, my changes and the recent changes from nuisanceofcats have been marked in the comments of the related class so that a 'todo list' can be found like:

> find Box2D_v2.2.1/Box2D -exec grep "emscripten" {} \;

// emscripten - b2AABB: add constructor
// emscripten - b2Color: rearrange member variables to be on separate lines
// emscripten - b2Draw: make virtual functions non-pure
// emscripten - b2ContactEdge: add constructor
...etc

This should make it easier to keep track of what has been changed from the original C++.

I hope that dumping in all this stuff is not too audacious for my first ever pull request, and hopefully I didn't screw anything up. While I'm being audacious though, I was kinda wondering if you were aware of the other javascript Box2D libraries? One of them, the first on the scene iirc, is officially named "box2d.js". It might be convenient if this one was labelled differently before it becomes more well-known... say, embox2d or something? Hm.. doesn't really matter I suppose :)

Owner

kripken commented Oct 25, 2012

Those errors are odd. If i have time i'll investigate. But my first suspicion, if disabling closure makes it work, is a closure conflict - closure renames something to a short name and that short name is used by a global var in other code in the global namespace. If you put box2d into a closure (no pun intended) so it does not use the global namespace, this might fix it.

Thanks for the pull!

About the name, I wasn't aware of another box2d.js? I only knew about box2dweb. Anyhow, we can change the name if anyone complains, I don't care strongly ;)

Yes, quite odd errors. I had noticed that you need to be careful not to forget the 'var' qualifier for variable that should not be global, but I can't see any of that type of problem in this case. For now, I am going with the non-closure result, which works great and can be minified to a tolerable 1.5Mb. The non-closure version also seems to have less stuttering on startup.

@iforce2d iforce2d closed this Nov 2, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment