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

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

Open
zeux opened this issue Jan 25, 2024 · 2 comments
Open

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

zeux opened this issue Jan 25, 2024 · 2 comments

Comments

@zeux
Copy link
Owner

zeux commented Jan 25, 2024

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 (#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:

  • Build with VOLK_INSTALL=ON, and get volk.h, libvolk.a as your install targets (as well as some CMake modules)
  • Build with VOLK_INSTALL=ON VOLK_HEADERS_ONLY=ON, and get volk.h, volk.c as your install targets (as well as some CMake modules)

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

@zeux
Copy link
Owner Author

zeux commented Jan 25, 2024

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
Copy link

Demonese commented Jan 29, 2024

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.

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

2 participants