Use Vulkan-Headers to find current Vulkan Version #1272
Conversation
|
CI gfxreconstruct build queued with queue ID 48289. |
|
|
|
CI gfxreconstruct build # 3249 running. |
|
CI gfxreconstruct build # 3249 failed. |
| add_library(vulkan_registry INTERFACE) | ||
| target_include_directories(vulkan_registry INTERFACE ${GFXRECON_SOURCE_DIR}/external/Vulkan-Headers/include) | ||
| target_compile_definitions(vulkan_registry INTERFACE VK_NO_PROTOTYPES VK_ENABLE_BETA_EXTENSIONS) | ||
| add_subdirectory(${GFXRECON_SOURCE_DIR}/external/Vulkan-Headers) |
There was a problem hiding this comment.
Execution failed for task ':layer:generateJsonModelRelease'.
> /home/runner/work/gfxreconstruct/gfxreconstruct/android/layer/CMakeLists.txt : C/C++ release|armeabi-v7a : CMake Error at /home/runner/work/gfxreconstruct/gfxreconstruct/android/framework/cmake-config/PlatformConfig.cmake:54 (add_subdirectory):
add_subdirectory not given a binary directory but the given source
directory
"/home/runner/work/gfxreconstruct/gfxreconstruct/external/Vulkan-Headers"
is not a subdirectory of
"/home/runner/work/gfxreconstruct/gfxreconstruct/android/layer". When
specifying an out-of-tree source a binary directory must be explicitly
specified.
Call Stack (most recent call first):
CMakeLists.txt:5 (include)
There was a problem hiding this comment.
The android/layer/CMakeLists.txt includes several instances of add_subdirectory with explicit binary folders specified. I tried to model that in an effort to make things work. Not sure about it because the cmake code is run through include(cmake-config/PlatformConfig.cmake), and I'm unsure how relative paths work in that context.
There was a problem hiding this comment.
Execution failed for task ':layer:generateJsonModelRelease'.
> /home/runner/work/gfxreconstruct/gfxreconstruct/android/layer/CMakeLists.txt : C/C++ release|armeabi-v7a : CMake Error at /home/runner/work/gfxreconstruct/gfxreconstruct/external/Vulkan-Headers/CMakeLists.txt:8 (cmake_minimum_required):
CMake 3.15...3.25 or higher is required. You are running version 3.10.2
Did the actions runner change??? Seems I have an invalid cmake version now for who knows what reason.
There was a problem hiding this comment.
OH I am being silly.
The Vulkan-Headers/CMakelists.txt requires CMake 3.15, but the android code is running using 3.10.2. Which means that either the CMake version in android is updated or the android code reverts to the old way (manually creating a library). The latter may be difficult if there is shared CMake code between android and the rest of the project.
There was a problem hiding this comment.
Looking into it further, there are two issues. One is that the CMake minimum for android is 3.4 and the other is that the runner loads I misunderstood gfxreconstruct-dev-android which contains CMake 3.10.2.gfxreconstruct-dev-android, that is an output of the build not an input. The CMake version is likely coming from the android SDK being used, which makes sense (as ubuntu 22 ships cmake 3.16 iirc).
There was a problem hiding this comment.
The linux build in .github/workflows/ci_build.xml invokes - uses: lukka/get-cmake@latest under steps:. I wonder if we need that for the android build as well? @charles-lunarg you could try adding it as part of your PR, see what GH does.
There was a problem hiding this comment.
I believe that would work, but that would have to happen in conjunction with raising the minimum (that way we don't advertise a minimum lower than what we actually require).
I would like to raise the minimum to 3.16.3, which is what top level CMakeLists.txt require.
There was a problem hiding this comment.
Seems that the - uses: lukka/get-cmake@latest is not able to change the CMake version used. According to the android docs I found, 3.10.2 is the newest version provided in the NDK. They do mention being able to use newer CMake versions, you just have to provide the binary yourself.
There was a problem hiding this comment.
Does that mean we need to specify the cmake binary to use in an environment variable or some such? @juan-lunarg any thoughts or should we just wait until we're finished transitioning our Android build process?
There was a problem hiding this comment.
I have reverted the android cmake changes - seems that the CMake version provided by the NDK is too old for the Vulkan-headers cmake code. And because it doesn't make use of the version information, it is fine to keep as is.
|
This looks pretty good to me, but I'd like to see the Android Release/Debug GH actions build succeeding. |
|
CI gfxreconstruct build queued with queue ID 49475. |
|
CI gfxreconstruct build # 3251 running. |
|
CI gfxreconstruct build # 3251 failed. |
ad45bcf to
1521b44
Compare
|
CI gfxreconstruct build queued with queue ID 49595. |
|
CI gfxreconstruct build # 3252 running. |
|
CI gfxreconstruct build queued with queue ID 49650. |
|
CI gfxreconstruct build # 3253 running. |
The Vulkan-Headers CMakelists.txt has the necessary logic to decode the Vulkan API version from itself. This allows removal of FindVulkanVersion.cmake which duplicated that logic. The Android build is left unchanged for the time being due to the NDK provided CMake being older than the version required by Vulkan-Headers.
3c6c0a7 to
3484721
Compare
|
CI gfxreconstruct build queued with queue ID 49706. |
|
CI gfxreconstruct build # 3255 running. |
|
CI gfxreconstruct build # 3255 passed. |
This reverts commit f3de76c.
The Vulkan-Headers CMakelists.txt has the necessary logic to decode the Vulkan API version from itself. This allows removal of FindVulkanVersion.cmake which duplicated that logic.
This is my first proper PR to gfxreconstruct, so do let me know if there are any procedures or things I should be aware of!