From 1c883d10aef914f6bbbacc7a055ad4c2c76af5d1 Mon Sep 17 00:00:00 2001 From: Ryan Tremblay Date: Mon, 19 Oct 2020 15:30:51 -0700 Subject: [PATCH 1/3] Add a dispatch function wrapper that synchronously re-throws the exception through setImmediate so that logbox correctly displays it --- .../src/main/cpp/BabylonNativeInterop.cpp | 25 ++++++------- .../react-native/ios/BabylonNative.cpp | 19 ++++------ .../react-native/ios/BabylonNative.h | 2 +- .../react-native/ios/BabylonNativeInterop.mm | 2 +- .../react-native/shared/DispatchFunction.h | 36 +++++++++++++++++++ 5 files changed, 54 insertions(+), 30 deletions(-) create mode 100644 Modules/@babylonjs/react-native/shared/DispatchFunction.h diff --git a/Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp b/Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp index fe3027277..fb0262931 100644 --- a/Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp +++ b/Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp @@ -22,6 +22,10 @@ #include #include +#include "../../../../shared/DispatchFunction.h" + +using namespace facebook; + namespace Babylon { namespace @@ -36,19 +40,10 @@ namespace Babylon { public: // This class must be constructed from the JavaScript thread - Native(facebook::jsi::Runtime* jsiRuntime, std::shared_ptr callInvoker, ANativeWindow* windowPtr) - : m_env{ Napi::Attach(*jsiRuntime) } + Native(jsi::Runtime& jsiRuntime, std::shared_ptr callInvoker, ANativeWindow* windowPtr) + : m_env{ Napi::Attach(jsiRuntime) } { - JsRuntime::DispatchFunctionT dispatchFunction = - [env = m_env, callInvoker](std::function func) - { - callInvoker->invokeAsync([env, func = std::move(func)] - { - func(env); - }); - }; - - m_runtime = &JsRuntime::CreateForJavaScript(m_env, dispatchFunction); + m_runtime = &JsRuntime::CreateForJavaScript(m_env, CreateJsRuntimeDispatcher(m_env, jsiRuntime, callInvoker)); auto width = static_cast(ANativeWindow_getWidth(windowPtr)); auto height = static_cast(ANativeWindow_getHeight(windowPtr)); @@ -135,10 +130,10 @@ extern "C" JNIEXPORT void JNICALL Java_com_reactlibrary_BabylonNativeInterop_res extern "C" JNIEXPORT jlong JNICALL Java_com_reactlibrary_BabylonNativeInterop_create(JNIEnv* env, jclass obj, jlong jsiRuntimeRef, jobject jsCallInvokerHolder, jobject surface) { - auto jsiRuntime = reinterpret_cast(jsiRuntimeRef); - auto callInvoker = facebook::jni::alias_ref {reinterpret_cast(jsCallInvokerHolder)}->cthis()->getCallInvoker(); + auto jsiRuntime = reinterpret_cast(jsiRuntimeRef); + auto callInvoker = jni::alias_ref {reinterpret_cast(jsCallInvokerHolder)}->cthis()->getCallInvoker(); ANativeWindow* windowPtr = ANativeWindow_fromSurface(env, surface); - auto native = new Babylon::Native(jsiRuntime, callInvoker, windowPtr); + auto native = new Babylon::Native(*jsiRuntime, callInvoker, windowPtr); return reinterpret_cast(native); } diff --git a/Modules/@babylonjs/react-native/ios/BabylonNative.cpp b/Modules/@babylonjs/react-native/ios/BabylonNative.cpp index 7c215d190..eb41f29b9 100644 --- a/Modules/@babylonjs/react-native/ios/BabylonNative.cpp +++ b/Modules/@babylonjs/react-native/ios/BabylonNative.cpp @@ -19,6 +19,8 @@ #include #include +#include "../shared/DispatchFunction.h" + namespace Babylon { using namespace facebook; @@ -26,8 +28,8 @@ namespace Babylon class Native::Impl { public: - Impl(facebook::jsi::Runtime* jsiRuntime, std::shared_ptr callInvoker) - : env{ Napi::Attach(*jsiRuntime) } + Impl(facebook::jsi::Runtime& jsiRuntime, std::shared_ptr callInvoker) + : env{ Napi::Attach(jsiRuntime) } , jsCallInvoker{ callInvoker } { } @@ -39,23 +41,14 @@ namespace Babylon Plugins::NativeInput* nativeInput{}; }; - Native::Native(facebook::jsi::Runtime* jsiRuntime, std::shared_ptr callInvoker, void* windowPtr, size_t width, size_t height) + Native::Native(facebook::jsi::Runtime& jsiRuntime, std::shared_ptr callInvoker, void* windowPtr, size_t width, size_t height) : m_impl{ std::make_unique(jsiRuntime, callInvoker) } { dispatch_sync(dispatch_get_main_queue(), ^{ m_impl->m_graphics = Graphics::InitializeFromWindow(windowPtr, width, height); }); - JsRuntime::DispatchFunctionT dispatchFunction = - [env = m_impl->env, callInvoker = m_impl->jsCallInvoker](std::function func) - { - callInvoker->invokeAsync([env, func = std::move(func)] - { - func(env); - }); - }; - - m_impl->runtime = &JsRuntime::CreateForJavaScript(m_impl->env, std::move(dispatchFunction)); + m_impl->runtime = &JsRuntime::CreateForJavaScript(m_impl->env, CreateJsRuntimeDispatcher(m_impl->env, jsiRuntime, callInvoker)); m_impl->m_graphics->AddToJavaScript(m_impl->env); diff --git a/Modules/@babylonjs/react-native/ios/BabylonNative.h b/Modules/@babylonjs/react-native/ios/BabylonNative.h index 5b780e53d..f021f7246 100644 --- a/Modules/@babylonjs/react-native/ios/BabylonNative.h +++ b/Modules/@babylonjs/react-native/ios/BabylonNative.h @@ -9,7 +9,7 @@ namespace Babylon { public: // This class must be constructed from the JavaScript thread - Native(facebook::jsi::Runtime* jsiRuntime, std::shared_ptr callInvoker, void* windowPtr, size_t width, size_t height); + Native(facebook::jsi::Runtime& jsiRuntime, std::shared_ptr callInvoker, void* windowPtr, size_t width, size_t height); ~Native(); void Refresh(void* windowPtr, size_t width, size_t height); void Resize(size_t width, size_t height); diff --git a/Modules/@babylonjs/react-native/ios/BabylonNativeInterop.mm b/Modules/@babylonjs/react-native/ios/BabylonNativeInterop.mm index cd9338d2b..9bab58cc9 100644 --- a/Modules/@babylonjs/react-native/ios/BabylonNativeInterop.mm +++ b/Modules/@babylonjs/react-native/ios/BabylonNativeInterop.mm @@ -132,7 +132,7 @@ + (void)setCurrentNativeInstance:(RCTBridge*)bridge mtkView:(MTKView*)mtkView wi jsi::Runtime* jsiRuntime = GetJSIRuntime(currentBridge); if (jsiRuntime) { - currentNativeInstance = std::make_unique(GetJSIRuntime(currentBridge), currentBridge.jsCallInvoker, (__bridge void*)mtkView, width, height); + currentNativeInstance = std::make_unique(*jsiRuntime, currentBridge.jsCallInvoker, (__bridge void*)mtkView, width, height); } } diff --git a/Modules/@babylonjs/react-native/shared/DispatchFunction.h b/Modules/@babylonjs/react-native/shared/DispatchFunction.h new file mode 100644 index 000000000..f623a4370 --- /dev/null +++ b/Modules/@babylonjs/react-native/shared/DispatchFunction.h @@ -0,0 +1,36 @@ +#pragma once + +#include + +#include +#include + +namespace Babylon +{ + using namespace facebook; + + inline JsRuntime::DispatchFunctionT CreateJsRuntimeDispatcher(Napi::Env env, jsi::Runtime& jsiRuntime, std::shared_ptr callInvoker) + { + return [env, &jsiRuntime, callInvoker](std::function func) + { + callInvoker->invokeAsync([env, &jsiRuntime, func{std::move(func)}] + { + try + { + func(env); + } + catch (...) + { + auto ex{std::current_exception()}; + auto setImmediate{jsiRuntime.global().getPropertyAsFunction(jsiRuntime, "setImmediate")}; + auto throwFunc{jsi::Function::createFromHostFunction(jsiRuntime, jsi::PropNameID::forAscii(jsiRuntime, "throwFunc"), 0, + [ex](jsi::Runtime &, const jsi::Value &, const jsi::Value *, size_t) -> jsi::Value + { + std::rethrow_exception(ex); + })}; + setImmediate.call(jsiRuntime, {std::move(throwFunc)}); + } + }); + }; + } +} \ No newline at end of file From b54a64608af337814d5990b8f349456f03478f3e Mon Sep 17 00:00:00 2001 From: Ryan Tremblay Date: Mon, 19 Oct 2020 16:00:52 -0700 Subject: [PATCH 2/3] Add CMakeLists.txt for BabylonReactNativeShared --- Modules/@babylonjs/react-native/android/CMakeLists.txt | 4 ++++ .../android/src/main/cpp/BabylonNativeInterop.cpp | 2 +- Modules/@babylonjs/react-native/ios/BabylonNative.cpp | 2 +- Modules/@babylonjs/react-native/ios/CMakeLists.txt | 4 ++++ Modules/@babylonjs/react-native/shared/CMakeLists.txt | 2 ++ 5 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 Modules/@babylonjs/react-native/shared/CMakeLists.txt diff --git a/Modules/@babylonjs/react-native/android/CMakeLists.txt b/Modules/@babylonjs/react-native/android/CMakeLists.txt index 70a095817..b504f072a 100644 --- a/Modules/@babylonjs/react-native/android/CMakeLists.txt +++ b/Modules/@babylonjs/react-native/android/CMakeLists.txt @@ -34,6 +34,9 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/src/") set(BABYLON_NATIVE_DIR "${CMAKE_CURRENT_LIST_DIR}/../submodules/BabylonNative") add_subdirectory(${BABYLON_NATIVE_DIR} ${BABYLON_NATIVE_DIR}/build/Android_${CMAKE_ANDROID_ARCH_ABI}/) +set(BABYLON_REACT_NATIVE_SHARED_DIR "${CMAKE_CURRENT_LIST_DIR}/../shared") +add_subdirectory(${BABYLON_REACT_NATIVE_SHARED_DIR} ${CMAKE_CURRENT_BINARY_DIR}/shared) + add_library(fbjni SHARED IMPORTED) set_target_properties(fbjni PROPERTIES IMPORTED_LOCATION ${FBJNI_LIBPATH}/${ANDROID_ABI}/libfbjni.so @@ -63,6 +66,7 @@ target_link_libraries(BabylonNative fbjni jsi turbomodulejsijni + BabylonReactNativeShared AndroidExtensions Graphics JsRuntime diff --git a/Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp b/Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp index fb0262931..69e86db30 100644 --- a/Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp +++ b/Modules/@babylonjs/react-native/android/src/main/cpp/BabylonNativeInterop.cpp @@ -22,7 +22,7 @@ #include #include -#include "../../../../shared/DispatchFunction.h" +#include using namespace facebook; diff --git a/Modules/@babylonjs/react-native/ios/BabylonNative.cpp b/Modules/@babylonjs/react-native/ios/BabylonNative.cpp index eb41f29b9..4bae09ee8 100644 --- a/Modules/@babylonjs/react-native/ios/BabylonNative.cpp +++ b/Modules/@babylonjs/react-native/ios/BabylonNative.cpp @@ -19,7 +19,7 @@ #include #include -#include "../shared/DispatchFunction.h" +#include namespace Babylon { diff --git a/Modules/@babylonjs/react-native/ios/CMakeLists.txt b/Modules/@babylonjs/react-native/ios/CMakeLists.txt index ceec905dd..2fe30f790 100644 --- a/Modules/@babylonjs/react-native/ios/CMakeLists.txt +++ b/Modules/@babylonjs/react-native/ios/CMakeLists.txt @@ -23,6 +23,9 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/") set(BABYLON_NATIVE_DIR "${CMAKE_CURRENT_LIST_DIR}/../submodules/BabylonNative") add_subdirectory(${BABYLON_NATIVE_DIR} ${BABYLON_NATIVE_DIR}/build/ios/) +set(BABYLON_REACT_NATIVE_SHARED_DIR "${CMAKE_CURRENT_LIST_DIR}/../shared") +add_subdirectory(${BABYLON_REACT_NATIVE_SHARED_DIR} ${CMAKE_CURRENT_BINARY_DIR}/shared) + add_library(BabylonNative ${CMAKE_CURRENT_LIST_DIR}/BabylonNative.h ${CMAKE_CURRENT_LIST_DIR}/BabylonNative.cpp) @@ -38,6 +41,7 @@ target_link_libraries(BabylonNative Graphics jsi reactnative + BabylonReactNativeShared JsRuntime NativeWindow NativeEngine diff --git a/Modules/@babylonjs/react-native/shared/CMakeLists.txt b/Modules/@babylonjs/react-native/shared/CMakeLists.txt new file mode 100644 index 000000000..ea2151292 --- /dev/null +++ b/Modules/@babylonjs/react-native/shared/CMakeLists.txt @@ -0,0 +1,2 @@ +add_library(BabylonReactNativeShared INTERFACE) +target_include_directories(BabylonReactNativeShared INTERFACE ".") \ No newline at end of file From e1483e17ccfe4a9a9f30227a9d38c9b491fe0b7b Mon Sep 17 00:00:00 2001 From: Ryan Tremblay Date: Mon, 19 Oct 2020 16:08:56 -0700 Subject: [PATCH 3/3] Add an explanation for the dispatcher function --- .../@babylonjs/react-native/shared/DispatchFunction.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Modules/@babylonjs/react-native/shared/DispatchFunction.h b/Modules/@babylonjs/react-native/shared/DispatchFunction.h index f623a4370..b010e03bb 100644 --- a/Modules/@babylonjs/react-native/shared/DispatchFunction.h +++ b/Modules/@babylonjs/react-native/shared/DispatchFunction.h @@ -9,10 +9,20 @@ namespace Babylon { using namespace facebook; + // Creates a JsRuntime::DispatchFunctionT that integrates with the React Native execution environment. inline JsRuntime::DispatchFunctionT CreateJsRuntimeDispatcher(Napi::Env env, jsi::Runtime& jsiRuntime, std::shared_ptr callInvoker) { return [env, &jsiRuntime, callInvoker](std::function func) { + // Ideally we would just use CallInvoker::invokeAsync directly, but currently it does not seem to integrate well with the React Native logbox. + // To work around this, we wrap all functions in a try/catch, and when there is an exception, we do the following: + // 1. Call the JavaScript setImmediate function. + // 2. Have the setImmediate callback call back into native code (throwFunc). + // 3. Re-throw the exception from throwFunc. + // This works because: + // 1. setImmediate queues the callback, and that queue is drained immediately following the invocation of the function passed to CallInvoker::invokeAsync. + // 2. The immediates queue is drained as part of the class bridge, which knows how to display the logbox for unhandled exceptions. + // In the future, CallInvoker::invokeAsync likely will properly integrate with logbox, at which point we can remove the try/catch and just call func directly. callInvoker->invokeAsync([env, &jsiRuntime, func{std::move(func)}] { try