Skip to content
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

Use vulkan-hpp will definitely kill almost all performace of C++ compiling. #13

Closed
HuazyYang opened this issue Apr 29, 2022 · 5 comments

Comments

@HuazyYang
Copy link

Vulkan-hpp is definitely a bad practise to manipulate vulkan API which abuse template usage so as to kill C++ compiling performace.

When just including <vulkan.hpp> in any of emty source, compiling duration can take quater of a hour. This dependency will definitely
make your work useless.

@apanteleev
Copy link
Contributor

The entire library builds in way less than a minute on any reasonably modern machine.
Maybe you could get some help with troubleshooting build times on your system, but not with this attitude calling nvrhi useless. Closing.

@BlurryLight
Copy link
Contributor

<vulkan.hpp> is referenced in nvrhi/include/nvrhi/vulkan.h
, which could easily propogate to application-side framework code and be unexpectedly included in alot of source files.
It does have some negatvie impact on compliation speed, and also on IDE indexing/intelligence, especially when nvrhi/vulkan.h is included in alot of translate units.
Since there are only some strcture/function definitions in nvrhi/vulkan.h, I replaced all vulkan.hpp related structs with corresponding vanilla C structures, and replaced vulkan.hpp with vulkan.h and it worked well.

@apanteleev
Copy link
Contributor

Ok, that is a good observation @BlurryLight

I just went through the same exercise replacing vulkan.hpp with vulkan.h in the public header, and it's not that many changes. What worries me a bit here is that it breaks compatibility with existing code using NVRHI: such code will need to include vulkan.hpp on their own (if needed), and add type casts when using the NVRHI functions that changed, like createHandleForNativeFramebuffer because the vk:: and Vk types are not trivially compatible.

@apanteleev apanteleev reopened this Aug 16, 2023
@BlurryLight
Copy link
Contributor

Agree with it. It will be a break change since it need application side code to add <vulkan.hpp> in their code, and adjust some API signature.

@apanteleev
Copy link
Contributor

The change removing vulkan.hpp is done in 2f6a8c3
Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants