cmake: fix vulkan headers detection on Windows #77
Conversation
There are multiple paths in the build scripts for either finding shaderc
on the filesystem or fetching the source and building it, before linking
it into the sample applications. These paths use separate CMake
variable/target names, which makes it confusing to understand
dependencies and makes refactoring / bug-fixing difficult.
This change attempts to slightly improve this situation by consistently
using the SHADERC_SHARED_LIBRARY variable/target wherever linkage to
shaderc is necessary. This means removing the use of explicit linker
flags for linking on Linux and letting CMake infer the required linker
flags/options from the specified variable/target.
SHADERC_SHARED_LIBRARY has been chosen instead of SHADERC_LIB_SHARED
because the latter is a hard-coded name ("shaderc_shared") in the build
script and does not correspond to the actual library name in all cases
(for example, the library installed by using the package manager on
Ubuntu 24.04 is named libshaderc.so.1).
The SHADERC_LIB_SHARED_DIR variable was only used along with explicit
linker flags, to specify the directory to search for shaderc. With the
removal of explicit linker flags, this variable is no longer used.
Signed-off-by: Srinath Kumarapuram <skr@nvidia.com>
These went out of use after the previous commit. Signed-off-by: Srinath Kumarapuram <skr@nvidia.com>
This limits the "shaderc found" message to the case where shaderc was actually found, and also slightly reduces the verbosity of the output. Signed-off-by: Srinath Kumarapuram <skr@nvidia.com>
dabrain34
force-pushed
the
dab_fix_shaderc_cmake
branch
from
5827744 to
e9b6202
Compare
|
@srinathkr-nv can you have a look to this change ? I had issue on Windows when I was building tip of the main branch of VVS on a system where Vulkan SDK was installed but not having the right version to support VP9 decode extension. |
dabrain34
force-pushed
the
dab_fix_shaderc_cmake
branch
from
e9b6202 to
6b1f73d
Compare
|
I went through the changes in d391432 but I will need to review them a bit more carefully; I will do that later today. The shaderc include directory changes in f37a251 seem fine to me. I will also test out this change. The update to BUILD.md in 6b1f73d looks good to me. I'm not sure that I understand the use of the word "proposition" in the commit message; it's probably easier and simpler to write either "fix the CMAKE_INSTALL_PREFIX setting for Windows" or "fix the CMAKE_INSTALL_PREFIX assignment for Windows". |
dabrain34
force-pushed
the
dab_fix_shaderc_cmake
branch
from
6b1f73d to
4e63a68
Compare
I updated the commit message |
|
Thank you @dabrain34, all the changes here LGTM. |
|
@srinathkr-nv thank you for your review. Double checking the code I added a fixup! VulkanSDK: reorganize version detection commit to the PR to cleanup FindVulkanSDK.cmake. Can you have a look and if its okay I will squash it. |
|
Yes, the fixup commit also looks good to me and I agree with folding it into the "VulkanSDK: reorganize version detection" commit. |
dabrain34
force-pushed
the
dab_fix_shaderc_cmake
branch
from
e0a80a3 to
e12fc62
Compare
First it detects if the system version is sufficient to support a given version such as VP9 decode extension 1.3.317 then if not it will use the main branch of Vulkan-Headers.
As the system can have a vulkan sdk but not the sufficient version such as 1.3.317 for VP9 decode extension,VULKAN_HEADERS_INCLUDE_DIR should go first and then SHADERC_INCLUDE_DIR in the list of include dirs because SHADERC_INCLUDE_DIR might be the Vulkan headers include dir. So include_directories has been removed from FindShaderc and SHADERC_INCLUDE_DIR has been added in root CMakeLists.txt
libmirclient-dev does not exist anymore. Fix the CMAKE_INSTALL_PREFIX setting for Windows.
dabrain34
force-pushed
the
dab_fix_shaderc_cmake
branch
from
e12fc62 to
ca4cebe
Compare
dabrain34
changed the title
So include_directories has been removed from FindShaderc and SHADERC_INCLUDE_DIR has been added in CMakeLists.txt
Regarding the Vulkan Headers, first it detects if the system version is sufficient to support the VP9 decode extension 1.3.317 then if not it will download the Vulkan-Headers and use the main branch to provide the vulkan headers.
Backport patches from Shaderc detection fixes nvpro-samples/vk_video_samples#143 to improve shaderc detection