zeux / volk

Meta loader for Vulkan API
MIT License
1.31k stars 114 forks source link

CMake: Should we install volk.c by default? #169

Open zeux opened 7 months ago

zeux commented 7 months ago

When building with VOLK_INSTALL (and no other options), the behavior is a little odd:

In addition to include/volk.h and lib/libvolk.a (that make sense), we also install include/volk.c.

On one hand, this is unusual - this is not how most libraries work, and if you link to libvolk.a you never need volk.c. On the other hand, I think this was added to support header-only mode so that instead of linking to libvolk.a you could instead define VOLK_IMPLEMENTATION and use volk like a stb-style header library. Additionally, I believe Vulkan SDK currently packages volk sources but not volk build artifacts due to some complexity in the SDK build process.

I'm considering changing this so that volk.c is only installed when VOLK_HEADERS_ONLY is defined, as I believe these came in the same PR (https://github.com/zeux/volk/pull/34). I want to double check that this would be consistent with how people expect to use volk, and that it won't conflict with the SDK build process. Additionally I would probably want to change VOLK_HEADERS_ONLY so that it doesn't force static library build when VOLK_INSTALL is defined. So after this you could:

cc @jeweg (as the author of the original PR) and @johnzupin to make sure this will not interfere with Vulkan SDK build/packaging.

zeux commented 7 months ago

Alternatively we could also remove volk.c from the install target list and add a separate option, like VOLK_INSTALL_SOURCE, that adds it back. I'm fine with either option - I'd probably prefer repurposing VOLK_HEADERS_ONLY but I'm open for either.

Demonese commented 7 months ago

~Is it possible to provide .h file only?~

~The .c file ~(~can be created by developers themselves~)~ only contains minimal codes, for example:~

#define VOLK_IMPLEMENTATION
#include "volk.h"

~reference:~ ~https://github.com/nothings/stb~ (~see stb_image.h~)

Sorry, I misunderstood the question.

I think install .c file and static library didn't conflict, because developers can choose whether to define VOLK_ IMPLEMENTATION.