inexorgame / vulkan-renderer

A new 3D game engine for Linux and Windows using C++20 and Vulkan API 1.3, in very early but ongoing development
https://inexor.org
MIT License
756 stars 33 forks source link

[cmake] Remove conan package manager and use only CMake instead #528

Closed IAmNotHanni closed 1 year ago

IAmNotHanni commented 1 year ago

Please test this!

Closes #522 Closes #512 Closes #501

History

Conan package manager is no longer required, as all dependencies can be downloaded simply through CMake by now. While in my personal opinion CMake has improved a lot over time, conan has not. It has been a constant source of frustration and broken build setups, most importantly our broken CI. Furthermore, I get the impression that almost nobody who comes into our project has any experience with conan, while most people have heard or use CMake. Using CMake+conan is just some thing in our project that has grown over time, but I think it's justified to say goodbye to it.

Note: Because we will no longer use conan to install Vulkan tools, this pull request will reverse https://github.com/inexorgame/vulkan-renderer/pull/491

Outlook

IAmNotHanni commented 1 year ago

The problem here is that vulkan-1.dll needs to be copied into the correct folder. We had this in issue https://github.com/inexorgame/vulkan-renderer/issues/499 which was fixed in pr https://github.com/inexorgame/vulkan-renderer/pull/502

This issue is because of us statically linking to vulkan runtime. This basically means the application just crashes without our control if the dll is not available on Windows. This gives us no control to display a nice error message. We should be using a metaloder such as volk, as proposed in https://github.com/inexorgame/vulkan-renderer/issues/501.

I'm testing this with CMake UI 3.24.2 and Visual Studio 17.5.4 2022 on my Windows 10. I get the following error when I launch inexor-vulkan-renderer-example.

image

IAmNotHanni commented 1 year ago
IAmNotHanni commented 1 year ago

Oh I just see I removed all of python from the docs installation guide, but we need it for the docs itself

IAmNotHanni commented 1 year ago
IAmNotHanni commented 1 year ago
IAmNotHanni commented 1 year ago

Looks like we need to install Vulkan SDK in our workflow: https://github.com/humbletim/setup-vulkan-sdk/blob/main/action.yml

IAmNotHanni commented 1 year ago

On Windows, VMA must be configured with VMA_DYNAMIC_VULKAN_FUNCTIONS 1 and VMA_STATIC_VULKAN_FUNCTIONS 0

EDIT: This is only the case if your (my) outdated graphics card does not support Vulkan 1.3 hahah. I guess it's also time for my new machine to arrive :)

Read more here: https://github.com/GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator/issues/280

Oh man it's really about time we use volk metaloader!

IAmNotHanni commented 1 year ago

What's wrong with the static analysis suddenly?

IAmNotHanni commented 1 year ago

If we download Vulkan through CMake, do we even need to specify the manual download of the full Vulkan SDK in the CI? EDIT: Yes, we need to keep it because of glslangValidator!

IceflowRE commented 1 year ago

What's wrong with the static analysis suddenly?

Fails on third party libraries.

If we download Vulkan through CMake, do we even need to specify the manual download of the full Vulkan SDK in the CI?

Not sure what else we need apart from the headers.

IAmNotHanni commented 1 year ago

@IceflowRE We do use fmt in the instance wrapper directly though:

// This code will throw an exception if the required version of Vulkan API is not available on the system
if (VK_API_VERSION_MAJOR(REQUIRED_VK_API_VERSION) > VK_API_VERSION_MAJOR(available_api_version) ||
    (VK_API_VERSION_MAJOR(REQUIRED_VK_API_VERSION) == VK_API_VERSION_MAJOR(available_api_version) &&
     VK_API_VERSION_MINOR(REQUIRED_VK_API_VERSION) > VK_API_VERSION_MINOR(available_api_version))) {
    std::string exception_message = fmt::format(
        "Your system does not support the required version of Vulkan API. Required version: {}.{}.{}. Available "
        "Vulkan API version on this machine: {}.{}.{}. Please update your graphics drivers!",
        std::to_string(VK_API_VERSION_MAJOR(REQUIRED_VK_API_VERSION)),
        std::to_string(VK_API_VERSION_MINOR(REQUIRED_VK_API_VERSION)),
        std::to_string(VK_API_VERSION_PATCH(REQUIRED_VK_API_VERSION)),
        std::to_string(VK_API_VERSION_MAJOR(available_api_version)),
        std::to_string(VK_API_VERSION_MINOR(available_api_version)),
        std::to_string(VK_API_VERSION_PATCH(available_api_version)));
    throw std::runtime_error(exception_message);
}

So we need to keep fmt as a direct dependency in our CMake setup

IAmNotHanni commented 1 year ago

What's wrong with the static analysis suddenly?

Fails on third party libraries.

If we download Vulkan through CMake, do we even need to specify the manual download of the full Vulkan SDK in the CI?

Not sure what else we need apart from the headers.

We can't leave out the Download of Vulkan SDK because we need glslangValidator. I think keeping it in the CI is the easiest solution, although there might be another way to get.

IAmNotHanni commented 1 year ago

I just enabled building tests and benchmarks for all compilers by default. They must be stable as well when reviewing a pull request. Now that means the tests do not run in CI yet. Our tests don't require a gpu to run. Writing tests which require gpus is way too much work. Benchmarks don't run in CI either, although they would also not require a gpu.

IceflowRE commented 1 year ago

Can i get a small presentation what volk does? What is the difference to what we did before.

IAmNotHanni commented 1 year ago

Can i get a small presentation what volk does? What is the difference to what we did before.

Let me give you an answer that we could copy into our docs right away:

volk Is a meta-loader for Vulkan API written by Arseny Kapoulkine. It allows you to dynamically load the function entrypoints to use Vulkan without linking to any Vulkan dynamic link library or statically linking to the Vulkan loader. This is important, because on Windows for example, without volk the application would crash before the main function is even reached in case Vulkan would not be installed on the system. Without volk, this prevents us completely from displaying a nice error message if the right version of Vulkan is not available.

This is an example of an error message generated by Windows because of missing entrypoints (Note that this error message would show up automatically and we wouldn't be able to run any code or display a nicer error message.): grafik

Volk has another benefit: It can optimize Vulkan device calls by dynamically loading the device-related Vulkan function entrypoints directly from the driver using vkGetDeviceProcAddr. This bypasses the overhead of Vulkan loader's dispatch code, which can come with measurable overhead. Using volk is easy, because once you include it instead of vulkan.h, all Vulkan functions are provided by volk as function pointers. This means you don't need to change Vulkan function calls at all when using volk.

In short: We didn't change our Vulkan function calls, but error handling for missing Vulkan is easier and we might expect a little performance improvement.

Maybe @yeetari can correct me if sth is wrong.

IAmNotHanni commented 1 year ago