search

andy-thomason / Vookoo

A set of utilities for taking the pain out of Vulkan in header only modern C++
MIT License
522 stars 52 forks source link

Put glfw under subrepo #21

Closed FunMiles closed 4 years ago

FunMiles commented 4 years ago

I am suggesting to put glfw under subrepo. This follows a short discussion at the end of the MacOS issue (and thanks, @andy-thomason for taking that PR in).

Following the discussion of subrepo in https://github.com/andy-thomason/Vookoo/issues/19 :

I have a slight disagreement with @andy-thomason and @lhog view/comments on subrepo. One of the significant benefits of subrepo in this case is that users of Vookoo and in most cases developers as well, can ignore the existence of the subrepo. Let's say we put glfw3 under subrepo, appart from having the source code clone, all it does is add a file '.gitrepo' in the external/GLFW directory. Cloning the complete vookoo with the glfw3 will download the complete vookoo code, including GLFW transparently. No need to install gitrepo for that. Unless you need to update the version of glfw3 or want to push a change to glfw3, you don't even have to know about subrepo. You don't have to have the subrepo software installed. I think that is one of the greatest strenghts of subrepo vs submodule. So, @lhog, I think end-user experience is unaffected.

I will create a version/PR for it. PS: An auxiliary result of cloning glfw3 into external is that a CMakeLists in the main directory, (which can then install the header-only library vookoo library) is needed because one cannot use add_subdirectory to a sibling if it is not binary.

FunMiles commented 4 years ago

I have created a first basic version with glfw clone into the external directory. Can you check it out on windows, @lhog ? I have checked it also works on my Linux box. https://github.com/FunMiles/Vookoo. Branch glfw-subrepo

The changes also address #16

andy-thomason commented 4 years ago

Bear in mind that GLFW only exists to run the examples and is not a dependency of Vookoo itself (vku.hpp) which is dependant only on the Vulkan headers. However, we need an example framework to get us started - if only to create windows on three disparate platforms. I am not averse to using subrepos however, but we must document them carefully so that a new user who wants to run the examples will know how.

FunMiles commented 4 years ago

@andy-thomason So far, my CMakeLists.txt in the main directory only supports the building of the examples. I intend to add an install capability, but perhaps as a separate pull request, as to keep issues well separated. In the install capability, it will create cmake files. The cmake file would allow to use find_package(vookoo) and they could have two components, the base one for vku.hpp and the framework one for vku_framework.hpp. The first one would add no dependency, but the second could carry with it the glfw dependency. Maybe we can have a discussion about this in a separate issue about installation. I can also amend the README.md to cover the subject of the examples. It already does say clearly that it is an include-only library.

In the mean time, do you think the building of the examples should be under a CMake option? Should it be on or off by default?

lhog commented 4 years ago

Didn't work for me out of the box: image

FunMiles commented 4 years ago

@lhog, I rebooted my linux machine under windows and it works without trouble. Did you, by any chance, keep your build to point to the example as the CMake root directory? You need to select the main directory as the location of the CMakeLists.

lhog commented 4 years ago

@FunMiles yep, apologies, I missed the fact you added another CMakeLists. Works for me now too.

FunMiles commented 4 years ago

Great. I'm creating the pull request.

FunMiles commented 4 years ago

The PR has been merged. I'm closing this issue.