-
Notifications
You must be signed in to change notification settings - Fork 62
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
NGFX CMake Nabla build system integration #789
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
option(NBL_BUILD_WITH_NGFX "Enable NGFX build" OFF) | ||
|
||
# NOTE: on windows default installation path is: | ||
# "C:/Program Files/NVIDIA Corporation/Nsight Graphics <version>/SDKs/NsightGraphicsSDK" <- define as "NGFX_SDK" environment variable | ||
# then you can pick SDK version with "NGFX_SDK_VERSION" cache variable (CMake GUI list supported) | ||
|
||
if(NBL_BUILD_WITH_NGFX) | ||
if(NOT DEFINED ENV{NGFX_SDK}) | ||
message(FATAL_ERROR "\"NGFX_SDK\" environment variable must be defined to build with NBL_BUILD_WITH_NGFX enabled!") | ||
endif() | ||
|
||
set(NGFX_SDK "$ENV{NGFX_SDK}") | ||
cmake_path(NORMAL_PATH NGFX_SDK OUTPUT_VARIABLE NGFX_SDK) | ||
|
||
if(NOT EXISTS "${NGFX_SDK}") | ||
message(FATAL_ERROR "Found \"NGFX_SDK\" environment variable but it is invalid, env:NGFX_SDK=\"${NGFX_SDK}\" doesn't exist!") | ||
endif() | ||
|
||
file(GLOB ENTRIES "${NGFX_SDK}/*") | ||
|
||
set(NGFX_VERSIONS "") | ||
foreach(ENTRY ${ENTRIES}) | ||
if(IS_DIRECTORY ${ENTRY}) | ||
list(APPEND NGFX_VERSIONS ${ENTRY}) | ||
endif() | ||
endforeach() | ||
|
||
if(NOT NGFX_VERSIONS) | ||
message(FATAL_ERROR "Could not find any NGFX SDK Version!") | ||
endif() | ||
|
||
list(TRANSFORM NGFX_VERSIONS REPLACE "${NGFX_SDK}/" "") | ||
list(SORT NGFX_VERSIONS) | ||
list(GET NGFX_VERSIONS -1 LATEST_NGFX_VERSION) | ||
|
||
# on the cache variable init pick the latest version, then let user pick from list | ||
set(NGFX_SDK_VERSION "${LATEST_NGFX_VERSION}" CACHE STRING "NGFX SDK Version") | ||
set_property(CACHE NGFX_SDK_VERSION PROPERTY STRINGS ${NGFX_VERSIONS}) | ||
|
||
set(NGFX_SDK_VERSION "$CACHE{NGFX_SDK_VERSION}") | ||
set(NGFX_SDK_BASE "${NGFX_SDK}/${NGFX_SDK_VERSION}") | ||
|
||
# TODO: wanna support more *host* platforms? (*) | ||
# NOTE: also I'm hardcoding windows x64 library requests till I know the answer for (*) | ||
find_file(NBL_NGFX_INJECTION_HEADER NGFX_Injection.h PATHS ${NGFX_SDK_BASE}/include) | ||
find_file(NBL_NGFX_INJECTION_DLL NGFX_Injection.dll PATHS ${NGFX_SDK_BASE}/lib/x64) | ||
find_file(NBL_NGFX_INJECTION_IMPORT_LIBRARY NGFX_Injection.lib PATHS ${NGFX_SDK_BASE}/lib/x64) | ||
|
||
if(NBL_NGFX_INJECTION_HEADER AND NBL_NGFX_INJECTION_DLL AND NBL_NGFX_INJECTION_IMPORT_LIBRARY) | ||
message(STATUS "Enabled build with NVIDIA Nsight Graphics SDK ${NGFX_SDK_VERSION}\nlocated in: \"${NGFX_SDK_BASE}\"") | ||
else() | ||
message(STATUS "Could not enable build with NVIDIA Nsight Graphics SDK ${NGFX_SDK_VERSION} - invalid components!") | ||
message(STATUS "Located in: \"${NGFX_SDK_BASE}\"") | ||
message(STATUS "NBL_NGFX_INJECTION_HEADER=\"${NBL_NGFX_INJECTION_HEADER}\"") | ||
message(STATUS "NBL_NGFX_INJECTION_DLL=\"${NBL_NGFX_INJECTION_DLL}\"") | ||
message(STATUS "NBL_NGFX_INJECTION_IMPORT_LIBRARY=\"${NBL_NGFX_INJECTION_IMPORT_LIBRARY}\"") | ||
message(FATAL_ERROR "You installation may be corupted, please fix it and re-run CMake or disable NBL_BUILD_WITH_NGFX!") | ||
endif() | ||
|
||
add_library(ngfx INTERFACE) | ||
target_sources(ngfx INTERFACE "${NBL_NGFX_INJECTION_HEADER}") | ||
target_include_directories(ngfx INTERFACE "${NGFX_SDK_BASE}/include") | ||
target_link_libraries(ngfx INTERFACE "${NBL_NGFX_INJECTION_IMPORT_LIBRARY}") | ||
target_link_options(ngfx INTERFACE "/DELAYLOAD:NGFX_Injection.dll") | ||
target_compile_definitions(ngfx INTERFACE NGFX_INJECTION_DLL_DIR="${NGFX_SDK_BASE}/lib/x64") | ||
target_compile_definitions(ngfx INTERFACE NGFX_VERSION="${NGFX_SDK_VERSION}") | ||
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
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
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.
@AnastaZIuk why is there an ngfx target cmake knows about?
we dont do this for Vulkan, CUDA, Optix, we delay load from DLL and are not supposed to crash if not found
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.
because I introduce modern cmake there as I do on
newBuildSystem
branch, interface targets don't have build rules but you can interact with themit's actually not quite true, for example when you call
find_package(Vulkan)
you get imported targets if found which are similar to interface targets and because there is no existingngfx
module which I can request I wrote a simple one myself just to embed whole logic into single logical target (it could also be imported target just like vulkan targets - doesn't matter too much). The only difference is the module is integral part of Nabla's cmake build system and we don't explicitly call the find_package on it but have an option to make it available for configure scope if possible.If the target is available it means:
NBL_BUILD_WITH_NGFX
and what makes you think it will crash? We do delay load from DLL here as well & we have
m_loaded
member to make sure we can call the API only if we first loaded the DLLThere 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.
ok thanks for explaining it to me
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.
but whats the deal with
target_link_libraries
and the ngfx being an INTERFACE instead of PRIVATE?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.
the reason for
is that in static build mode each executable must link against the import library - using the
INTERFACE
qualifier ensures this linkage is propagated.On the other hand when building Nabla as a DLL the
PRIVATE
qualifier is used to prevent any inheritance of ngfx linkage for targets linking Nabla since the import library is already linked directly to the DLL.