Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No need for c++ exceptions or rtti (15% smaller) and allow growth in wasm #93

Merged
merged 1 commit into from
Jul 19, 2018

Conversation

kripken
Copy link
Owner

@kripken kripken commented Apr 3, 2018

Some improvements we should add, first to shrink the size by disabling c++ features we don't need, and second to enable growth in wasm where it has almost no overhead and is useful in allowing arbitrary size data usage.

I'm not able to build box2d currently, though, due to errors like

In file included from glue_stub.cpp:27:
./box2d_glue.cpp:1124:10: error: no viable overloaded '+='
  (*self += arg0);
   ~~~~~ ^  ~~~~
Box2D_v2.2.1/Box2D/Common/b2Math.h:94:7: note: candidate function not viable: no known conversion from 'const b2Vec2 *' to 'const b2Vec2' for
      1st argument; dereference the argument with *
        void operator += (const b2Vec2& v)
             ^

Perhaps we introduced a bug in the idl file? Or maybe an emscripten update broke things? If no one has an answer, we should go back in the history in the two repos to when things worked and bisect to see what went wrong.

…se size by 15% based on the emscripten test suite), and enable memory growth in wasm where it has almost no overhead
@MrSonicMaster
Copy link

@kripken I need to build Box2D and I'm running into this issue too with no modifications to box2d or emscripten. Have you figured out any fix to this, or any workarounds? I'm experiencing the same issue reported here: #90 and was hoping to implement his fix and build a new wasm js to see if that fixed the memory leak.

kripken added a commit to emscripten-core/emscripten that referenced this pull request Jul 19, 2018
…that function handles attributes like Ref etc., without which operator+=(T& other) would not work. fixes a bug noticed in kripken/box2d.js#93
@kripken
Copy link
Owner Author

kripken commented Jul 19, 2018

Ok, I found some time now to look into this, and it was a regression in upstream emscripten. Fix in emscripten-core/emscripten#6879

@kripken
Copy link
Owner Author

kripken commented Jul 19, 2018

So let's merge this in.

@kripken kripken merged commit de6186e into master Jul 19, 2018
@kripken kripken deleted the better branch July 19, 2018 23:47
@kripken kripken mentioned this pull request Jul 20, 2018
kripken added a commit to emscripten-core/emscripten that referenced this pull request Jul 23, 2018
Properly use the right helper function in WebIDL binding of operators. That function handles attributes like Ref etc., without which operator+=(T& other) would not work.

Fixes a bug noticed in kripken/box2d.js#93
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants