Converge react native integration layers across platforms #135
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… include (since it will contain source)
…ngine view back on and re-entering XR
Fix an issue with xr.mm
…3+, but is not needed for touch (button id is always 0 for touch inputs)
bghgary
reviewed
Dec 21, 2020
Contributor
bghgary
left a comment
bghgary
left a comment
There was a problem hiding this comment.
Did a pass on it, but I can't say I understand everything. Maybe good to go over in person (virtually).
Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp
Outdated
Show resolved
Hide resolved
Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp
Outdated
Show resolved
Hide resolved
Small fix for CR feedback on caching prop name as std::string
chrisfromwork
approved these changes
Dec 22, 2020
…oker is wrapped and not directly exposed
bghgary
approved these changes
Jan 4, 2021
Update to merged changes in BabylonNative
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change significantly overhauls the react native integration layers. The goal is to reduce duplication across platforms and simplify the code. The main shared integration code is in
BabylonNative.cpp. This is similar to what we had for both Android and iOS, but is platform agnostic. Android and iOS have relatively thin platform specific layers on top of this. Additionally, the class defined inBabylonNative.cppis now a JSIHostObjectwhich is injected into the JavaScript engine so that JS code can call directly into it in a platform agnostic way (so we don't have to expose native functionality to JS through platform specific legacy bridge based modules). This object exposes a JS promise that JS Babylon React Native initialization code can await, which allowed me to remove all the platform specific native promise/callback related code. I was also able to remove all traces of the old usages of loopers, since we fully rely on the React NativeCallInvokerto get onto the JS thread.NOTE: This change requires a change in BabylonNative that is not merged yet, so I'll need to update that once BabylonJS/BabylonNative#550 is merged.
This also resolves #24