Skip to content

Conversation

@bghgary
Copy link
Contributor

@bghgary bghgary commented Jul 10, 2020

This change switches Babylon Native to use NAPI over JSI.

Notes:

  • There are outstanding issues with the JSI implementation over JSC and thus we currently have a custom JSCRuntime.h and JSCRuntime.cpp to compensate. These changes will need to be upstreamed to Facebook in the future.
  • Not all of the NAPI contract is implemented yet, but enough is implemented so that the default app runs with XR support.
  • Returning NAPI types that don't match exactly with the return type (e.g. returning a Napi::Object for a function that returns Napi::Value will result in a C++ warning only when using NAPI/JSI). You must add an std::move in this situation as will be indicated by a note following the warning.
  • Performance is slower with JSI due to various reasons. This will be documented in the future along with measurements.
  • Code has been tested with Hermes which adds additional slowness.

TODO

  • Test on iOS (@ryantrem Can you help with this?)
  • Merge Babylon Native counterpart

@bghgary bghgary requested a review from ryantrem July 10, 2020 23:57
@bghgary bghgary marked this pull request as draft July 10, 2020 23:58
@bghgary bghgary marked this pull request as ready for review July 15, 2020 22:08
@bghgary bghgary merged commit ef7293e into BabylonJS:master Jul 16, 2020
@bghgary bghgary deleted the napi-jsi branch November 2, 2021 22:18
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