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

PhysX Emscripten WebAssembly build target #238

Open
wants to merge 2 commits into
base: 4.1
Choose a base branch
from

Conversation

prestomation
Copy link

  • Created new 'emscripten' platform, using the linux build as a template
  • emscripten build does not support snippets/samples(although this
    support should be possible to add)
  • after configuring emscripten in the local environment, running a standard generate_projects.sh, selecting emscripten, and
    make in a build configuration directory builds a wasm/js file for
    loading in the browser.
  • a new module has been added PhysxWebBindings which only exists in the
    emscripten platform. This includes(for now) a single cpp file describing
    th embind bindings.
  • This build has only been tested on mac and WSL. This is not expected
    to work in windows.

The bindings include what is used today by Amazon Sumerian, it is expected that other consumers will pull this down and augment the bindings as desired, although I suppose it would be possible to bind everything.

* Created new 'emscripten' platform, using the linux build  as a template
* emscripten build does not support snippets/samples(although this
support should be possible to add)
* running a standard generate_projects.sh, selecting emscripten, and
`make` in a build configuration directory builds a wasm/js file for
loading in the browser.
* a new module has been added PhysxWebBindings which only exists in the
emscripten platform. This includes(for now) a single cpp file describing
th embind bindings.
* This build has only been tested on mac and WSL. This is not expected
to work in windows.
@ashconnell
Copy link

@prestomation

This is great! I made some notes along the way:

  1. The readme file says python 2.7 minimum. I had to upgrade from 2.7.10 (OSX default) to 2.7.12 due to an error about an unsupported piece of functionality
  2. The new versioning in the emsdk repo treats 1.38.31 as legacy (i think) but i was able to switch to this version by running ./emsdk install sdk-fastcomp-1.38.31-64bit and activating it with ./emsdk activate sdk-fastcomp-1.38.31-64bit.
  3. I wasn't able to make the emscripten-debug build due to an NDEBUG/_DEBUG error but this may be an issue outside of the emscripten target

Either way, I now have an emscripten-release build that should be usable, which is EPIC to say the least!

I do have a few questions about the future, maybe you have some ideas:

  • How should we deal with publishing this to npm? Do we need a repo to house the package.json file, readme, etc?
  • Where should communications happen for this emscripten target? Again, this might be best handled in a separate repo, although it's definitely a benefit to have this code here...

@prestomation
Copy link
Author

Thanks for taking a look at this @ashconnell!

  1. Hm, this seems like a PhysX-wide issue unrelated to this change. There is only a handful of lines of Python in this change. That said, our team had no issues with whatever system python we had on OSX and Win10 WSL.

  2. My team hasn't made changes to the emscripten build in some time, as we have been integrating it into our product and testing, which is why this is an older build. That said, I would expect the newest version of emscripten to work. Did you try it? I simply wanted to record the version I was using.

  3. I did look at this. Adding add_definitions(-DNDEBUG) anywhere in physx/source/compiler/cmake/emscripten/PhysXWebBindings.cmake will fix this. I'm not familiar enough with PhysX's build processes to know if this is the best fix though.

  4. This is a good question. At Sumerian we package this up as part of our product build, and we do not distribute via NPM, so I haven't thought too much about this. It does seem we need a repo to do this, for folks who don't want to build from source. It seems unlikely the PhysX team will want to own this.

  5. Similarly to the above, I'd personally like to keep as much in this repo. One problem with ammo.js is that it is a fork of ammojs as it was several years ago and there is no community keeping this up to date. A hard fork of PhysX will have the same problems. A major reason I chose to invest this work in PhysX is that PhysX is one of the only open source physics libraries that is actively innovating with a strong team behind it, and I like to keep the pipeline of getting changes to the webassembly build as efficient as possible. Of course this is all subject to what Nvidia wants to do. For now, feel free to comment on my github fork, to keep the chatter out of the Nvidia repo: https://github.com/prestomation/PhysX/tree/emscripten

@ashconnell
Copy link

ashconnell commented Jan 8, 2020

  1. Makes sense, just putting it out there in case anyone has issues

  2. I did try using the latest emsdk but the configure script threw errors. I didn't take note of what they were as I wanted to mirror a working environment and get it running first.

  3. Nice to know there's a solution to this! I'm not sure we need debug targets for wasm builds but i may not fully understand how these translate.

  4. (and 5) Do you guys have any need for this as an NPM library? I'm happy with either of us creating a small github repo with a package.json, a readme with build notes etc and then having npm publish it from that repo. I don't want to step on toes if you already have plans, especially since it will consume an npm package name. I think it makes sense to keep the emscripten target here, assuming Nvidia are interested in this.

@jtoy
Copy link

jtoy commented Feb 20, 2020

signed?

@CLAassistant
Copy link

CLAassistant commented Apr 20, 2020

CLA assistant check
All committers have signed the CLA.

* Fix an error generated by -Wmisleading-indentation

Signed-off-by: Squareys <[email protected]>

* Allow building emscripten target on win32

Signed-off-by: Squareys <[email protected]>

* Add -Wno-alloca and Wno-anon-enum-enum-conversion to clang flags

Signed-off-by: Squareys <[email protected]>

* Avoid casting const ref to non-const ref

Signed-off-by: Squareys <[email protected]>

* Add -Wno-dtor-name for emscripten target

Signed-off-by: Squareys <[email protected]>

* Also add compile definitions to PhysXWebBindings target

Signed-off-by: Squareys <[email protected]>

* Add -fPIC to avoid linker error

Specifically:
wasm-ld: warning: unexpected existing value for R_WASM_TABLE_INDEX_REL_SLEB

Signed-off-by: Squareys <[email protected]>

* Avoid duplicate symbols caused by transitively linking libs multiple times

Signed-off-by: Squareys <[email protected]>
@Squareys
Copy link

Squareys commented Jul 1, 2020

@jtoy @ashconnell All signed :)

@yosmo78
Copy link

yosmo78 commented Dec 14, 2021

Has anyone been successful with linking this into their emscripten project recently and has any pointers in doing so with the modern releases of emscripten?

@ashconnell
Copy link

@yosmo78 it works fine but currently uses an older version of emscripten with embind. The instructions here should get you up and running: https://github.com/ashconnell/physx-js

@yosmo78
Copy link

yosmo78 commented Dec 15, 2021

Thanks @ashconnell , your build scripts were really helpful to learning on what to do

Does anyone know what the .bc files are? I want to statically link in PhysX with my code in emscripten so I can access it in c++ and have it compile down into the generated wasm of emscripten. Are the .bc file equivalent to .a files? Do I just include the headers of PhysX to access the functions inside my c++ code?

Sorry for the questions, I am a new to this wasm stuff. But having physX would help reduce dev time of implementing everything myself. Thank!



# We need to output .bc files instead of .a files for our emscripten build
SET(CMAKE_STATIC_LIBRARY_SUFFIX ".bc")

Choose a reason for hiding this comment

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

this is not required anymore since 2.0.x emscripten


SET(PHYSX_PLATFORM_LINKED_LIBS dl)

SET(CMAKE_STATIC_LIBRARY_SUFFIX ".bc")

Choose a reason for hiding this comment

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

same, can be removed

@shrinktofit
Copy link

What's the status of this PR?

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.

8 participants