Incorrect Vulkan SDK `Include` directory case on Linux

Triang3l commented 2 years ago

cmake/find/FindVulkanSDK.cmake contains the following code for obtaining the path to the Vulkan SDK headers:

if(EXISTS "${VULKANSDK_ROOT_DIR}/Include/vulkan/vulkan.h")
  SET(VULKANSDK_INCLUDE_DIR "${VULKANSDK_ROOT_DIR}/Include" CACHE PATH "path")
else()
  SET(VULKANSDK_INCLUDE_DIR "${VULKANSDK_ROOT_DIR}" CACHE PATH "path")
endif()

Due to case sensitivity of paths in Linux file systems, it's unable to find vulkan.h in the include directory, as it attempts to locate it in Include starting with the capital I (which is the directory name in the Windows release of the Vulkan SDK).

Adding an elseif trying the include directory resolves this issue and makes CMake succeed for me, but I'm not sure what fix would look the nicest here. I haven't checked the paths in the macOS version of the SDK, but for Windows, just looking for include would be enough as the file systems usually used on Windows are case-insensitive, so Include will be located too. Or possibly, a WIN32/UNIX conditional would be the best here, though the simplest solution will probably be just to check the include directory.

The way I'm invoking CMake is VULKAN_SDK=/path/to/VulkanSDK/1.3.224.1/x86_64/ cmake . from within the vk_order_independent_transparency sample directory, due to issues with locating libshaderc_combined.a unless the explicit Vulkan SDK path is specified, at least on the distribution that I'm using (ALT Linux Workstation 10).

NBickford-NV commented 1 year ago

Hi Triang3l - thanks for the find! I've just added a commit that implements the elseif solution:

if(EXISTS "${VULKANSDK_ROOT_DIR}/Include/vulkan/vulkan.h")
  SET(VULKANSDK_INCLUDE_DIR "${VULKANSDK_ROOT_DIR}/Include" CACHE PATH "Path to the directory including vulkan/vulkan.h")
elseif(EXISTS "${VULKANSDK_ROOT_DIR}/include/vulkan/vulkan.h")
  set(VULKANSDK_INCLUDE_DIR "${VULKANSDK_ROOT_DIR}/include" CACHE PATH "Path to the directory including vulkan/vulkan.h")
else()
  SET(VULKANSDK_INCLUDE_DIR "${VULKANSDK_ROOT_DIR}"         CACHE PATH "Path to the directory including vulkan/vulkan.h")
endif()

Let me know if this fixes the build on Linux for you!

Thanks again,

--Neil

tcoppex commented 1 year ago

Hey,

I just had the issue on GNU / Linux with cmake 3.22.1 and it seems the module FindVulkanSDK.cmake was renamed to FindVulkan.cmake (apparently since CMake 3.7), so changing this line may work everywhere : https://github.com/nvpro-samples/nvpro_core/blob/cb69a9da7946acb1e52e6af0c713f90522e61217/cmake/setup.cmake#L411

Some refs :

Cheers !

NBickford-NV commented 1 year ago

Hi tcoppex! This is pretty neat, I didn't know CMake had built-in support for finding Vulkan until now! We've been using our own nvpro_core/cmake/find/FindVulkanSDK.cmake, since when this part of the framework was created in 2016, CMake 3.4 had just been released.

We might look into replacing our FindVulkanSDK.cmake with CMake's FindVulkan.cmake in the future; in the meantime, did updating to cb69a9d fix this issue for you?

Thanks!

NBickford-NV commented 1 year ago

Hi all, one more quick update on this: we've now replaced our FindVulkanSDK.cmake script with CMake's built-in find_package(Vulkan) module in https://github.com/nvpro-samples/nvpro_core/commit/9cf7b4814bd2046d0048c5ed56176ae51c3f5cb5 . Closing this issue because the script that had this bug no longer exists; please let me know if you run into any issues with the new Vulkan inclusion method!

nvpro-samples / nvpro_core

Incorrect Vulkan SDK `Include` directory case on Linux #42