VulkanMemoryAllocator icon indicating copy to clipboard operation
VulkanMemoryAllocator copied to clipboard

Add Linux support for VulkanSample

Open IAmNotHanni opened this issue 8 months ago • 6 comments

Hello

as discussed in https://github.com/GPUOpen-LibrariesAndSDKs/VulkanMemoryAllocator/pull/446, I want to port VulkanSample app to Linux and I would like to discuss the strategy behind it here. The port seems necessary since you want to move to GitHub actions and you would like to have Linux builds in the CI as well. This is the next step after this, and I will discuss it in another issue here.

From what I can see for now, there is (almost) no Linux specific code required for this port. This means that the Linux version of VulkanSample app would compile and work on Windows just as well. There are some exceptions though: I don't know how easy it is to change console color on Linux. I would not like to introduce a dependency on a library just to do this. It would be better to not have such cosmetic features on Linux at all. The most important objective this is to get the app to work on Linux and to run the tests.

I think we should avoid having duplicate code for Windows and Linux like this:

#if WIN32_
   // The entire code for Windows
#else
   // The entire code, but adjusted for Linux
#else

Such code duplication is always a bad idea because it makes it hard to maintain. Instead, we can keep the code which is Windows specific so far, and make it work on Linux using the following approach:

Functions like for example ZeroMemory are Windows API functions which are not available on Linux. We could replace the ZeroMemory function entirely for Windows and Linux with std::memset, which does the same thing. However, to minimize code changes, I would suggest to simply offer a selfmade ZeroMemory function on Linux which uses std::memset under the hood, like this:

#ifndef WIN32_ // Only for Linux
#include <cstring>
#define ZeroMemory(Destination, Length) std::memset(Destination, 0, Length)
#endif

This way, we can keep all ZeroMemory in code. This is just one example how to make Windows specific code work by proving Linux versions for it.

For window management, I would like to use glfw on Linux. This means we keep the Windows API code for Windows, although we could also replace it all with glfw. However, I want to keep code changes for Windows as minimal as possible, so we can keep the Windows API code. For Linux, glfw is necessary because Window management is much more complex on Linux. There are many different UI systems, and glfw does a good job making our life easy. To introduce glfw, we can simply use FetchContent in CMake.

Again: In the end, there should be no problem with compiling the Linux specific code on Windows as well, because it does not use any Linux-specific code.

EDIT: I might add that using the Linux code for Windows leads to errors of symbols being defined twice, meaning that is not as simple as enabling this code block for Linux on Windows an to test it. If we strictly replace all the Windows specific code with the modern C++ code for Linux though (for example, remove ZeroMemory and use std::memset everywhere), it should compile easily.

best regards Johannes

IAmNotHanni avatar Mar 13 '25 16:03 IAmNotHanni

It really looks like most of the port could be achieved by adding macros like these for Linux code:

#ifndef _WIN32
#define ZeroMemory(dst_memory, length) std::memset(dst_memory, 0, length)
#define GetTickCount() static_cast<std::uint32_t>(std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::high_resolution_clock::now().time_since_epoch()).count())
#define _countof(array) (sizeof(array) / sizeof(array[0]))
#define _wcsicmp(wstring1, wstring2) std::wstring(wstring1).compare(std::wstring(wstring2))
#define _wtoi(wstring) std::stoi(wstring)
#define MAX_PATH 260
#endif

IAmNotHanni avatar Mar 13 '25 21:03 IAmNotHanni

Update: There seem to be two issues which are a little harder to port:

  1. Using _aligned_realloc seems to be very hard to port on Linux. This has also been discussed here: https://github.com/KhronosGroup/Vulkan-Docs/issues/103. I am not sure what to do about this yet.
  2. The test TestPool_Benchmark which uses parallelization based on Windows API events can be ported using modern C++ concurrency features like std::condition_variable. It should not be hard to do this, but it's an advanced topic of the port.

IAmNotHanni avatar Mar 17 '25 17:03 IAmNotHanni

Hello

Thank you for this effort and I'm sorry for my delayed response.

I like the idea of setting up automated CI builds for Windows and Linux. However, I would like to start from building the sample app only on Windows, and on Linux only compiling the library (with VMA_IMPLEMENTATION) and maybe some dummy empty main if necessary - just like it was done before. Porting the full sample app to Linux may be a larger endeavor that can come later, if ever.

A agree that changing the color on the console can stay a Windows-only feature.

I am fine with either replacing ZeroMemory with memset or defining a custom ZeroMemory for Linux as you proposed.

Adding dependency on a new external library like glfw is a bigger issue, because it will require approval from our legal department.

I have a concern about your implementation of GetTickCount. This function in WinAPI returns time in milliseconds, not nanoseconds.

adam-sawicki-a avatar Mar 26 '25 18:03 adam-sawicki-a

Hmm right, glfw uses zlib license. Could we make it so on Linux we don't even have a window, but only the console itself? We should not need a window or a surface in oder to run the tests I believe?

IAmNotHanni avatar Mar 27 '25 23:03 IAmNotHanni

OK, I like the idea of having just a console app that can run the tests.

adam-sawicki-a avatar Mar 28 '25 09:03 adam-sawicki-a

Ooops, sorry, I didn't mean to close this ticket.

adam-sawicki-a avatar Apr 03 '25 15:04 adam-sawicki-a