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
Enable NAPI-JSI build on top of V8JSI #832
Merged
Merged
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
ba251cc
Working NAPI-JSI
rgerd b41356c
Use original port
rgerd 9b57f89
Cleanup
rgerd ca6eefe
Back out CallbackInfo change
rgerd d3b6cab
Easier promise type fix
rgerd 4fc8cfe
Simpler NativeCamera function fix
rgerd a809aeb
Clean up all the napi imports
rgerd f901c78
Add link to source
rgerd 537e4bc
Final PR feedback
rgerd File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,37 +1,34 @@ | ||
if(NOT NAPI_JAVASCRIPT_ENGINE STREQUAL "JSI") | ||
|
||
set(SOURCES | ||
"Include/Babylon/AppRuntime.h" | ||
"Source/AppRuntime.cpp" | ||
"Source/AppRuntime${NAPI_JAVASCRIPT_ENGINE}.cpp" | ||
"Source/WorkQueue.cpp" | ||
"Source/WorkQueue.h") | ||
|
||
if(APPLE) | ||
set(SOURCES ${SOURCES} "Source/AppRuntime${BABYLON_NATIVE_PLATFORM}.mm") | ||
else() | ||
set(SOURCES ${SOURCES} "Source/AppRuntime${BABYLON_NATIVE_PLATFORM}.cpp") | ||
endif() | ||
set(SOURCES | ||
"Include/Babylon/AppRuntime.h" | ||
"Source/AppRuntime.cpp" | ||
"Source/AppRuntime${NAPI_JAVASCRIPT_ENGINE}.cpp" | ||
"Source/WorkQueue.cpp" | ||
"Source/WorkQueue.h") | ||
|
||
if(APPLE) | ||
set(SOURCES ${SOURCES} "Source/AppRuntime${BABYLON_NATIVE_PLATFORM}.mm") | ||
else() | ||
set(SOURCES ${SOURCES} "Source/AppRuntime${BABYLON_NATIVE_PLATFORM}.cpp") | ||
endif() | ||
|
||
add_library(AppRuntime ${SOURCES}) | ||
warnings_as_errors(AppRuntime) | ||
add_library(AppRuntime ${SOURCES}) | ||
warnings_as_errors(AppRuntime) | ||
|
||
target_include_directories(AppRuntime | ||
PRIVATE "Include/Babylon" | ||
INTERFACE "Include") | ||
target_include_directories(AppRuntime | ||
PRIVATE "Include/Babylon" | ||
INTERFACE "Include") | ||
|
||
if(UNIX AND NOT APPLE AND NOT ANDROID) | ||
target_include_directories(AppRuntime INTERFACE "/usr/include/webkitgtk-4.0/") | ||
endif() | ||
if(UNIX AND NOT APPLE AND NOT ANDROID) | ||
target_include_directories(AppRuntime INTERFACE "/usr/include/webkitgtk-4.0/") | ||
endif() | ||
|
||
target_link_to_dependencies(AppRuntime | ||
PRIVATE arcana | ||
PUBLIC JsRuntime) | ||
target_link_to_dependencies(AppRuntime | ||
PRIVATE arcana | ||
PUBLIC JsRuntime) | ||
|
||
target_compile_definitions(AppRuntime | ||
PRIVATE NOMINMAX) | ||
target_compile_definitions(AppRuntime | ||
PRIVATE NOMINMAX) | ||
|
||
set_property(TARGET AppRuntime PROPERTY FOLDER Core) | ||
source_group(TREE ${CMAKE_CURRENT_SOURCE_DIR} FILES ${SOURCES}) | ||
set_property(TARGET AppRuntime PROPERTY FOLDER Core) | ||
source_group(TREE ${CMAKE_CURRENT_SOURCE_DIR} FILES ${SOURCES}) | ||
|
||
endif() |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
#include "AppRuntime.h" | ||
#include "WorkQueue.h" | ||
|
||
#include <napi/env.h> | ||
#include <V8JsiRuntime.h> | ||
#include <ScriptStore.h> | ||
|
||
namespace | ||
{ | ||
class TaskRunnerAdapter : public v8runtime::JSITaskRunner | ||
{ | ||
public: | ||
TaskRunnerAdapter(Babylon::WorkQueue& workQueue) | ||
: m_workQueue(workQueue) | ||
{ | ||
} | ||
|
||
void postTask(std::unique_ptr<v8runtime::JSITask> task) override | ||
{ | ||
std::shared_ptr<v8runtime::JSITask> shared_task(task.release()); | ||
m_workQueue.Append([shared_task2 = std::move(shared_task)](Napi::Env) { | ||
shared_task2->run(); | ||
}); | ||
} | ||
|
||
private: | ||
TaskRunnerAdapter(const TaskRunnerAdapter&) = delete; | ||
TaskRunnerAdapter& operator=(const TaskRunnerAdapter&) = delete; | ||
|
||
Babylon::WorkQueue& m_workQueue; | ||
}; | ||
} | ||
|
||
namespace Babylon | ||
{ | ||
void AppRuntime::RunEnvironmentTier(const char*) | ||
{ | ||
v8runtime::V8RuntimeArgs args{}; | ||
args.inspectorPort = 5643; | ||
args.foreground_task_runner = std::make_shared<TaskRunnerAdapter>(*m_workQueue); | ||
|
||
const auto runtime{v8runtime::makeV8Runtime(std::move(args))}; | ||
const auto env{Napi::Attach<facebook::jsi::Runtime&>(*runtime)}; | ||
Dispatch([&runtime](Napi::Env env) { | ||
JsRuntime::NativeObject::GetFromJavaScript(env) | ||
.Set("_JSIRuntime", Napi::External<facebook::jsi::Runtime>::New(env, runtime.get())); | ||
}); | ||
Run(env); | ||
Napi::Detach(env); | ||
} | ||
} |
This file contains 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
This file contains 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
24 changes: 14 additions & 10 deletions
24
...pi/napi-direct/OnLinkedAsDependency.cmake → Dependencies/napi/OnLinkedAsDependency.cmake
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,27 +1,31 @@ | ||
# Callback to perform custom behavior -- in this case, copying runtime output artifacts like DLLs -- when | ||
# linked from an executable target as a library. | ||
function(on_linked_as_dependency target) | ||
# We only have to do anything if the JavaScript engine is V8. | ||
if (NAPI_JAVASCRIPT_ENGINE STREQUAL "V8") | ||
|
||
# Propagate this file to the target so that it will be transitively available to targets that | ||
# link to that one, too. | ||
propagate_on_linked_as_dependency_cmake_file(napi ${target}) | ||
|
||
# Propagate this file to the target so that it will be transitively available to targets that | ||
# link to that one, too. | ||
propagate_on_linked_as_dependency_cmake_file(napi ${target}) | ||
if (DEFINED NAPI_JAVASCRIPT_RUNTIME_OUTPUT_ARTIFACTS) | ||
# We only need to actually copy files if we're being linked from an executable. | ||
get_target_property(type ${target} TYPE) | ||
if(${type} STREQUAL "EXECUTABLE") | ||
if (WINDOWS_STORE) | ||
# WINDOWS_STORE allows us to use the VS_DEPLOYMENT_CONTENT property. | ||
target_sources(${target} PRIVATE ${NAPI_JAVASCRIPT_RUNTIME_OUTPUT_ARTIFACTS}) | ||
set_property(SOURCE ${NAPI_JAVASCRIPT_RUNTIME_OUTPUT_ARTIFACTS} PROPERTY VS_DEPLOYMENT_CONTENT 1) | ||
|
||
if ((DEFINED NAPI_JAVASCRIPT_RUNTIME_OUTPUT_ARTIFACTS_DEBUG) AND (DEFINED NAPI_JAVASCRIPT_RUNTIME_OUTPUT_ARTIFACTS_RELEASE)) | ||
set_property(SOURCE ${NAPI_JAVASCRIPT_RUNTIME_OUTPUT_ARTIFACTS_DEBUG} PROPERTY VS_DEPLOYMENT_CONTENT $<CONFIG:Debug>) | ||
set_property(SOURCE ${NAPI_JAVASCRIPT_RUNTIME_OUTPUT_ARTIFACTS_RELEASE} PROPERTY VS_DEPLOYMENT_CONTENT $<NOT:$<CONFIG:Debug>>) | ||
else() | ||
set_property(SOURCE ${NAPI_JAVASCRIPT_RUNTIME_OUTPUT_ARTIFACTS} PROPERTY VS_DEPLOYMENT_CONTENT 1) | ||
endif() | ||
|
||
set_property(SOURCE ${NAPI_JAVASCRIPT_RUNTIME_OUTPUT_ARTIFACTS} PROPERTY VS_DEPLOYMENT_LOCATION ".") | ||
else() | ||
# Without the VS_DEPLOYMENT_CONTENT property, create custom rules to copy the artifacts. | ||
foreach(artifact ${NAPI_JAVASCRIPT_RUNTIME_OUTPUT_ARTIFACTS}) | ||
add_custom_command(TARGET ${target} POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy_if_different ${artifact} $<TARGET_FILE_DIR:${target}>) | ||
endforeach(artifact) | ||
endforeach() | ||
endif() | ||
endif() | ||
endif() | ||
endfunction() | ||
endfunction() |
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<configuration> | ||
<config> | ||
<add key="repositoryPath" value="packages" /> | ||
</config> | ||
<packageSources> | ||
<clear /> | ||
<add key="Nuget.org" value="https://api.nuget.org/v3/index.json" /> | ||
</packageSources> | ||
<disabledPackageSources> | ||
<clear /> | ||
</disabledPackageSources> | ||
</configuration> | ||
rgerd marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is mostly unmodified from the node repo. If this is actually necessary, then we should try to upstream it to node. We typically don't change the N-API contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed a PR here: nodejs/node-addon-api#1026