[BUG] vulkan.hpp accidentally included in sources/third_party/vulkan, very broken #1974
Comments
|
I'm not actually sure what that header is doing there. That directory I thought was just for the validation layers, which are no longer bundled with the NDK itself (#1374). As best as I can tell this was only ever included because the validation layers needed it to build? You probably don't want the vulkan headers from that directory. You want the one from the sysroot, which is automatically on the The one time I tried using vulkan.hpp with the NDK, I did that by checking the vulkan version in the NDK ( I suspect the actual fix you want here is "please support vulkan.hpp", which is that other bug I linked, and that in at least the short term we should be less confusing and delete the directory with the stale headers? |
|
I looks like there was a merge error at some point. @trevordblack, can you take a look? I can't find the bad merge, but you can see the end result by looking at the history here: |
|
Discussed more with cnorthrop over IM, and it sounds like my guess is probably correct: these were part of the validation layer source and were accidentally left behind when the rest of the validation layers were removed. We'll drop the headers from that location in r27 since those are obviously misleading. I'd very much prefer to do that alongside vulkan.hpp being added to the sysroot though. We may not have meant to ship that, but if people had found it and were using it, that'd still look like a feature regression :( Within r26, if @trevordblack can resolve the bad merge, we can probably cherry-pick the fix. If the main branch has moved forward since r26 that might be difficult. Alternatively we could just accept that the band-aid has been ripped off and leave it broken since we're going to remove it anyway. |
DanAlbert
changed the title
|
I wasn't sure about Vulkan-Hpp support, because on the one side there is a issue whether to support Vulkan-Hpp, on the other side you can find updated Vulkan-Hpp in NDK. So far, I've been using the headers from this folder because it was the easiest way for me to add Vulkan-Hpp to my project, but if you wants to remove it, I don't mind. If it's not supported ofically, I agree that it should be removed because, some people probably use these headers..... I managed to find a way to add Vulkan-Hpp to my Android project using vcpkg and it seems to work for me Thank you for clarifying the situation |
|
Ah, I didn't realize that was in vcpkg. Yeah, that's probably your best bet until we can support it officially. You may need to be cautious about matching the version to what's in the NDK sysroot as before though. vcpkg does support AAR generation, which might be easier to integrate than the normal vcpkg output depending on what your build looks like (hard if you're not using gradle, but definitely easier to keep builds reproducible across your team if you are): https://learn.microsoft.com/en-us/vcpkg/users/platforms/android#consume-libraries-using-vcpkg-and-android-prefab-archives-aar-files |
|
"Ugh. who was the idiot that caused the merge error". It looks like From my understanding there's a couple of actions
Honestly the "fix" here is the painful one (downstream from vulkan-headers to external/vulkan-headers), and I don't know when I'm going to find the time to do that... January? If there's a 5 minute band-aid NDK thing to do, I'm happy to do that soon.... Hmm. I guess, is the explicit ask for me (as person with OS changing power) to just remove the duplicates above in |
Been there :)
We ship directly from external/vulkan-headers. A band aid in the NDK repo would be a band aid in external/vulkan-headers. If we want to fix this in a point release of r26c, we could do that in the aosp/ndk-r26-release branch of that project, which wouldn't auto merge anywhere. That's an option, but since we didn't support this on purpose, and the OP sounds happy just using vcpkg until we do, we may not need to.
We're not shipping anything until January anyway.
The best thing would be to fix external/vulkan-headers when you're able, but also update https://cs.android.com/android/platform/superproject/main/+/main:external/vulkan-headers/Android.bp;l=51-56;drc=1783d5ae806ca53d8f180dc03f470712527072eb to include this header, which would let us close #1767. You may also want to add a test that does nothing but A fix for a future release of the NDK has even more time than until January, btw. I'd only need a fix by late Feb for that? |
|
OK I'll try to get to this late December or early January. I'd rather not push it to the end of the month. |
|
@trevordblack we're going to be cutting a release in the near future. I'm going to drop the headers that are being installed to |
|
https://android-review.googlesource.com/c/platform/ndk/+/2924824 removes the broken and misleading stuff. Unclear whether or not we're going to be able to fix #1767 in r27, but it doesn't sound too likely right now. |
|
We're working on fixing the broken build and should have that done within a week or two.
After internal discussion we're not in a position to support the .hpp files, and so aren't going to be explicitly adding them |
It'll go out in r27 beta 1 if that's the case 👍 |
|
For future readers of this thread. Maybe it's fixed in r27. But maybe you can't use that version for whatever reason. If you go to the official VulkanHeaders on github, go through the release tags to match the one google is using. I think it's 237 at the moment, not sure of new release. You can get get that repository-tag and then set up a directory with all of the ndk's vulkan/.h and the VulkanHeaders/vulkab/.hpp symbolically linked in, and it seems to work. |
|
The much better fix is to use vcpkg like sstarosz suggested above. The NDK's vulkan.hpp was only ever shipped by accident and thus never tested. |
|
Possibly so, I would rather not. Package managers are great until they aren't. |
|
In that case you're still better off getting it from somewhere else, like the official repo: https://github.com/KhronosGroup/Vulkan-Hpp. Just make sure you download the version that's matched to the vulkan.h in whatever NDK you're using. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Description
I updated the NDK version of my project from version 25.2.9519653 to 26.1.10909125, after which my Vulkan project stopped compiling due to errors in with repeated definitions in the vulkan.hpp header shipped with NDK
(26.1.10909125\sources\third_party\vulkan\src\include\vulkan\)Version 26.1.10909125 has a lot of repeated definitions in the vulkan.hpp and vulkan_enums.hpp headers. At least from these files come most of the compilation errors, I don't know if other files in the vulkan folder are not also affected.
Compilation output
Some errors look like typical merge mistakes, because the function code itself is the same, only the formatting is different:
Vulkan.hpp:
Vulkan_enums.hpp:
Affected versions
r26
Canary version
No response
Host OS
Windows
Host OS version
19045.3693
Affected ABIs
armeabi-v7a, arm64-v8a, x86, x86_64
Build system
CMake
Other build system
No response
minSdkVersion
24
Device API level
No response
The text was updated successfully, but these errors were encountered: