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

Wasm #146

Closed
wants to merge 2 commits into from
Closed

Wasm #146

wants to merge 2 commits into from

Conversation

makc
Copy link
Contributor

@makc makc commented Jun 30, 2022

This is not a real pull request but rather a simple test for #73

makc added 2 commits June 30, 2022 10:14
minimal bindings that are doing something useful
wasm build and small three.js test
@pca006132
Copy link
Collaborator

It's great that there is a three.js demo (?). I think you can add the generated files to .gitignore so they won't be added to the commit.

@makc
Copy link
Contributor Author

makc commented Jun 30, 2022

ok, noted, but how would I integrate it into the build? e g how to run this long ass emcc command during/after make?

@pca006132
Copy link
Collaborator

You can modify this and test it in GitHub Action:

build_wasm:
runs-on: ubuntu-20.04
if: github.event.pull_request.draft == false
steps:
- name: Install dependencies
run: |
sudo apt-get -y update
DEBIAN_FRONTEND=noninteractive sudo apt install -y python3 nodejs
- uses: actions/checkout@v3
with:
submodules: true
- name: Setup WASM
run: |
# setup emscripten
git clone https://github.com/emscripten-core/emsdk.git
cd emsdk
./emsdk install latest
./emsdk activate latest
- uses: jwlawson/actions-setup-cmake@v1.12
- name: Build WASM
run: |
source ./emsdk/emsdk_env.sh
# patch assimp
patch third_party/assimp/code/CMakeLists.txt < assimp.diff
mkdir build
cd build
emcmake cmake -DCMAKE_BUILD_TYPE=Release .. && emmake make
- name: Test WASM
run: |
cd build/test
node ./manifold_test.js

Copy link
Owner

@elalish elalish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great demo, thank you! Let's see if we can get this building in the CI. I wonder what the easiest way to test it end-to-end would be?

a -= b;
}

EMSCRIPTEN_BINDINGS(whatever) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it; does this name even show up anywhere in the actual WASM blob?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is not in manifold.js at least; emscripten docs say this:

a label to mark a group of related bindings (for example EMSCRIPTEN_BINDINGS(physics), EMSCRIPTEN_BINDINGS(components), etc.)

return manifold;
}
catch(const std::runtime_error& e) {
puts(e.what());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting, and gets at some of the discussion around removing or changing exception handling: #141. Is there no way for C++ exceptions to automatically throw JS exceptions with WASM?

Copy link
Contributor Author

@makc makc Jul 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elalish it does throw all the way to js but the message you get is just some random number (maybe a pointer to exception object somewhere in wasm memory, Idk) - so, with puts(e.what()) you can at least see what was actually wrong on c++ side

Copy link
Contributor Author

@makc makc Jul 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elalish you can see what I mean by uncommenting new Module.Manifold(....) line in js instead - and passing the original geometry (without calling simplify(...) on it)

return geometry;
}

// most of three.js geometries arent manifolds, so...
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

<script>
var Module = {
onRuntimeInitialized: function() {
// we have manifold module, let's do some three.js
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@elalish
Copy link
Owner

elalish commented Jul 1, 2022

The CI isn't running because this is a draft PR. Feel free to mark it as ready for review so we can play with the automated builds. Don't worry, I'm not going to merge it in it's current state :)

@makc
Copy link
Contributor Author

makc commented Jul 1, 2022

@elalish yea I just posted this to get some early feedback, I will do another one maybe this weekend or so

@makc makc closed this Jul 1, 2022
@makc makc mentioned this pull request Jul 10, 2022
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.

None yet

3 participants