yuphin / Lumen

A Vulkan Raytracing framework for various bidirectional path tracing techniques
MIT License
449 stars 29 forks source link

OS inpependent CMake Port #8

Closed Lachei closed 1 year ago

Lachei commented 1 year ago

This PR addresses Issue #7 .

The .vcxproj files have been deleted and fully replaced by CMake.

The CMake port itself only relies on Vulkan and the Threads library to be found by CMake, all other repositories are downloaded as submodules and compiled with the project. This is necessary as unfortunately the windows and linux Vulkan SDK do not provide the same extensions. While on Linux glslang comes preinstalled with most distros, on Windows there is no glslang but instead shaderc and SPIRV-Cross. One idea for the future to strip off a lot of boilerplate libraries is to write everything only using glslang and making it the only submodule required. This however needs some code refactoring which i did not want to tackle right now.

I further added an example_config folder which describes a OS independent (at least for windows and linux) config for VSCode. This is especially useful to quickly get the project not just compiling, but also up for debugging (Especially getting the relative paths to the assets right can get tricky for beginners). VSCode was chosen as i personally find it the best OS indepent developing environment right now.

The project should however still be buildable with Visual Studio and its CMake support.

Compilation and execution was checked with Windows11[inteli7,nvidiaRTX3070m], Manjaro22.1.0(Linux6.1.25-1)[nvidiaRTX3070], Ubuntu22.04(nvidiaRTX2080ti). Unfortunately i do not have an AMD graphics card available to also test it on such a system.

Cheers, Josef

yuphin commented 1 year ago

Just tried it out now and it mostly works out of the box. Great work! 😃

For the topic regarding submodules: The idea is that we'd use glslangValidator as a fallback (if USE_SHADERC macro is off) during shader compilation. So for that macro path we only need the glslangValidator binary (not the glslang lib) and the binary itself should be included in both Windows and Linux SDKs so there is no need to include glslang in submodules.

Regarding SPIRV-Cross, as you said its headers are already included in both SDKs; so I don't think we need it. Actually in ser branch I've implemented a very basic Shader Execution Reordering sample and there, we modify SPIRV-Cross so it doesn't crash while parsing SER related instructions: so we definitely don't want SPIRV-Cross as a submodule going forward.

In addition, the choice regarding linking the shaderc library is important, as we want to compile shaders with release binaries in both Release and Debug configuration. You'll see that in libs/libshaderc_util I've only added the relevant parts to resolve #includes in shaders. But I think we can also use it as a submodule only to include the contents in that folder. (I.e we could use this submodule only for the headers and not for linking). The linking should be done to shaderc_shared.lib on Windows or libshaderc_shared.so on Linux, both of which are included with the Vulkan SDK.

We should be able to also remove SPIRV-Headers and SPIRV-Headers as they should be redundant for our use cases.

Regarding the extensions, we can just remove the extensions_vk.cpp file. This was already removed in the current master branch and we actually use volk instead. Maybe I misunderstood your point regarding extensions not mismatching between Linux and Windows so please correct me if this raises any issues with my above suggestions regarding submodules.

So in summary, I suggest removing SPIRV-Cross, SPIRV-Headers, SPIRV-Tools and glslang submodules along with the extensions_vk.cpp file and use shaderc_shared.lib/so (i.e the Release binary ) within the Vulkan SDK to link with shaderc.

Other than that, thanks for the contribution! Great job! 👍

Lachei commented 1 year ago

Glad everything works out of the box :).

Regarding glslang: Unfortunately shaderc depends on multiple libraries to be installed/available at compilation time. Specifically glslang as well as SPIRV-Tools (see the README for shaderc). SPIRV-Tools then depends on SPIRV-Headers. So the main reason for these libraries is that these were needed for shaderc. In my experience on Linux the shaderc_shared.so are often not available, making it actually a bad option for cross platform compilation. However using glslang which is quite widely supported with the linux vulkan-sdk is often not available on windows. Looking at other larger repositories it seems that they also are using either a submodule or a git download via cmake of glslang to properly support cross platform runtime compilation.

This would also be my suggestion to refactor the code to only use glslang, it is after all the basis of shaderc and is mainly a smaller library.

Regarding SPIRV-Cross, as you said its headers are already included in both SDKs; so I don't think we need it. Actually in ser branch I've implemented a very basic Shader Execution Reordering sample and there, we modify SPIRV-Cross so it doesn't crash while parsing SER related instructions: so we definitely don't want SPIRV-Cross as a submodule going forward.

I do not remember everything exact, but i think I had the SPIRV-Cross missing on Ubuntu.

One thing I also tried was installing shaderc and other packages with the respective package managers and resolve the cmake and build errors. I simply gave up after 6 hours of debugging build errors. The errors range from different library link variables on different linux versions over mismatching function definitions to missing vulkan components on windows. So i simply went for the submodule approach which only needs standard vulkan to be installed which proved to be the easiest to compile on all systems (And especially being also the most reliable).

Regarding linking to shaderc in release, it should be possible to set the mode to release in the libs/cmake only for shaderc

So what would you say? Stay with shaderc or switching to glslang?

Cheers, Josef

PS: Another alternative would be to add the compiled binaries by downloading and unpacking them with cmake (200 - 500 MB size depending on OS).

yuphin commented 1 year ago

The main reason we use shaderc instead of glslang is that is provides a cleaner interface and most importantly an ability to use #includes in shaders. Couple of months ago I had an episode of going over the alternatives (i.e the glslang way) and that was the approach I've ended up with.

Can you elaborate on why shaderc_shared.so not being available on Linux? When I download the SDK for Linux and unzip, I can see the shaderc_shared.so file in libs/ so shouldn't they always be included in the SDK? (Same with Spirv-cross)

If shaderc pulls in other dependencies, can we just have the libshaderc_util folder in libs (like we have in the main repo) instead of a submodule then? Then we'd still be linking the shaderc_shared that's in the SDK.

Basically my main concern regarding the dependencies is that I want to keep them as minimal as possible. So if the SDK already provides most of the headers and libraries, I'd suggest trying to use them as much as we can. Going in with glslang means we'd have to implement our own file includer. It's not particularly hard to implement it ourselves, but since shaderc already exposes a very basic implementation of it (see libshaderc_util/ *.cc files, I've also noticed these need to be included in the build if we go with this route), why not use it?

PS: Just installed vulkan-sdk via apt on Ubuntu and shaderc_shared.so also comes with it. (In my case it's in /usr/lib/x86_64_linux-gnu/). Can you also double check it again?

Lachei commented 1 year ago

Actually in my previous projects i never looked for the shaderc_shared.so file. The main problem was that find_package(Vulkan) did not find the Vulkan::shaderc_combined linking component on Linux, while on Windows this worked fine. However I will check if using find_package(shaderc) or something like that is able to find shaderc on my platforms.

The reason why shaderc is beneficial is something i am familiar with, as i have encountered the problems in previous projects numerous times. If glslang simply would support defines and includes...

I am going to try out the other find_package later to day and will keep you up to data.

Cheers, Josef

Lachei commented 1 year ago

So in summary, I suggest removing SPIRV-Cross, SPIRV-Headers, SPIRV-Tools and glslang submodules along with the extensions_vk.cpp file and use shaderc_shared.lib/so (i.e the Release binary ) within the Vulkan SDK to link with shaderc.

All done. On my systems everything now seems to compile fine, windows needed a bit of effort to compile to the correct spirv libraries but i think the solution now is fine.

Cheers, Josef

yuphin commented 1 year ago

Thanks :) Works quite well now. I'll add some VS related flags after the merge. (Namely setting the VS working directory and adding the multi processor compilation flag)