search

LukasBanana / LLGL

Low Level Graphics Library (LLGL) is a thin abstraction layer for the modern graphics APIs OpenGL, Direct3D, Vulkan, and Metal
BSD 3-Clause "New" or "Revised" License
2.05k stars 139 forks source link

Use native FindVulkan.cmake #51

Closed julianxhokaxhiu closed 4 years ago

julianxhokaxhiu commented 4 years ago

Backstory: I was trying to build this library under Visual Studio 2019 for Win32 Arch and I found out that Vulkan was not able to be found, with the latest SDK version ( 1.1.130 ). Looking at how you did include Vulkan I found out you're using the old suggested method. Nowadays it's better to use the native FindVulkan.cmake which is provided by any Cmake version that is >= 3.7.

I doubt someone will build this code with an older CMake version.

Thank you for the awesome work you're doing. Looking forward on this project.

julianxhokaxhiu commented 4 years ago

Sorry for the subsequent commits but it was unplanned as I thought it was all about fixing the CMakeLists.txt file, while it ended being a more sophisticated patch.

Overall I managed to use the Vulkan SDK on LInux Travis CI jobs, instead of Mesa Vulkan library ( allowing this library to be truly Vulkan cross-platform ). Although for some reasons the Linux job still fails in building the Vulkan code, while on Windows I do not have any kind of issues building the same code under VS2019 using CMake.

Proof: https://ci.appveyor.com/project/julianxhokaxhiu/llglci In order to build the Release configuration I had also to apply this patch: https://github.com/julianxhokaxhiu/LLGL/commit/db55884326192bf392651331bf731422b915a243

Feel free to squash this PR when merging.

julianxhokaxhiu commented 4 years ago

@LukasBanana Done! Let me know if you want also the Release patch into this PR or if you're doing to fix it yourself.

LukasBanana commented 4 years ago

Thank you for your contribution.

Regarding your Release patch: why do you need <stdexcept> in VKDeviceBuffer.h?

julianxhokaxhiu commented 4 years ago

@LukasBanana this is why: https://ci.appveyor.com/project/julianxhokaxhiu/llglci/build/job/fku2on40dcxatftt#L300

LukasBanana commented 4 years ago

Well then feel free to make another PR for that fix. Thank you