kripken / box2d.js

Port of Box2D to JavaScript using Emscripten
1.32k stars 198 forks source link

Upgrade plans #47

Closed kripken closed 10 years ago

kripken commented 10 years ago

We should upgrade box2d like bullet is ( https://github.com/kripken/ammo.js/issues/60 ), by moving to the latest box2d code, latest emscripten, and the new bindings approach (the WebIDL binder). See details in that link, the situation is very parallel here in box2d. Basically if you want to help, start a branch with an update of box2d, and start to write the IDL stuff we need.

lzantal commented 10 years ago

Could you provide a simple example of the webidl for box2D? I like what I read over at ammo.js about the speed up and also the easy to generate missing bindings.

Thank you

kripken commented 10 years ago

Not sure what you mean by an example? Look e.g. at btVector3 in bullet and in the idl for bullet. We need the same over here for box2d classes, each needs to be described in idl.

mlogan commented 10 years ago

I'd like to help with this, partly to deal with https://github.com/kripken/box2d.js/issues/56, which I suspect is due to a bug with customizeVTable (or possibly emscripten, but the former seems more likely).

I've started on this, and 95% of the work is very easy and straightforward, but I'm running into a few problems:

kripken commented 10 years ago

I'm not sure if WebIDL supports enums or not. If it does, we should just need to connect them up.

void* is an interesting problem. Perhaps we should add a VoidPtr "class", and special-case it to turn into void*. Where does box2d need that, btw?

mlogan commented 10 years ago

WebIDL does support enums. Here's my plan - I have not written the code to do this automatically yet, I just manually edited the glue.cpp and glue.js files until I got something that worked.

In the idl file:

enum b2BodyType {
"b2_staticBody",
"b2_kinematicBody",
"b2_dynamicBody"
};

In the glue.cpp file:

b2BodyType EMSCRIPTEN_KEEPALIVE emscripten_bind_enum_b2_staticBody() {
  return b2_staticBody;
}
// and similarly for the other values

In the glue.js file

Module['b2_staticBody'] = _emscripten_bind_enum_b2_staticBody();
// etc

(I couldn't figure out how to write a global static variable such that emscripten would make it accessible in the js file, hence the use of functions.)

Regarding void*, Box2D uses it for void SetUserData(void *) and void * GetUserData() on various objects. In the current build of box2d.js, you can pass integers to and from those functions, which is handy for labeling fixtures. This isn't strictly needed in js (one can always monkey-patch the js wrapper objects or store a map from an object's address to arbitrary data), but it is fairly useful.

mlogan commented 10 years ago

Pull request filed to emscripten/incoming for enum support: https://github.com/kripken/emscripten/pull/2618

mlogan commented 10 years ago

Ok, I've got a partial set of IDL bindings over at https://github.com/artillery/box2d.js/tree/webidl-bindings - I'll send a pull after I'm a little farther along. Anybody who wants to help should find it fairly easy to jump in at this point, as there's enough bindings written to give obvious examples of how to proceed. Also see the documentation at http://kripken.github.io/emscripten-site/docs/coding/connecting_cpp_and_javascript/WebIDL-Binder.html

Compared to the previous bindings, it's a lot of boilerplate to write, but it seems to remove a lot of compiler warnings, removes the need for customizeVTable, should fix bugs like https://github.com/kripken/box2d.js/issues/38, and should make it easy to upgrade to the latest Box2D version with minimal changes to the Box2D source.

mlogan commented 10 years ago

(Oh, one important detail - that branch won't build or run correctly until at least emscripten 1.23.0)

kripken commented 10 years ago

What did we end up doing here with void* pointers?

kripken commented 10 years ago

For void*, I added VoidPtr in https://github.com/kripken/emscripten/commit/f30112c86e5595a9be52b6ebc52a1e1291acb2bf now.

mlogan commented 10 years ago

Oh, I used the any type to indicate void *. You merged that in https://github.com/kripken/emscripten/pull/2632

kripken commented 10 years ago

Oh right, I forgot about that. Well, no harm in having them both, I guess...

kripken commented 10 years ago

A difference, however, is that VoidPtr returns an actual wrapper object, like other pointers do, while any returns a raw integer. Not sure if one is better than the other, I guess. I'll document the difference.

mlogan commented 10 years ago

The bindings for any types already un-wrap pointers properly if you happen to pass in a wrapper object. On the return side of things, if you did want to use the VoidPtr return, you'd have to re-wrap (cast) it yourself as whatever class you want to use it as - the bindings can't do that automatically. So I'm not sure what you'd be gaining by getting a type-less wrapper object back instead of an integer.

kripken commented 10 years ago

Yes, good points. I was thinking of just return values, and I forgot that we cast now using castObject(x, type) instead of x.castObject(type), which means it doesn't really matter.

Still, it is a user-observable difference,

  1. Perhaps someone would do x.ptr to access the underlying pointer, but more importantly
  2. Someone can add their own properties to the JS wrapper object, using it as a true opaque blob of user data.
mlogan commented 10 years ago

Good points. Personally, I'm happy with either approach, but there should probably only be one of them. Whichever you choose is fine with me.

mlogan commented 10 years ago

This can be closed now (#58)

kripken commented 10 years ago

Closing. Thanks @mlogan !