OpenKinect / libfreenect2

Open source drivers for the Kinect for Windows v2 device
2.08k stars 751 forks source link

cmake flexibility requests (solutions available pending discussion) #879

Closed svenevs closed 7 years ago

svenevs commented 7 years ago
$ git log -1 --oneline
6236635 docs: Fix a typo

The goal: enable some flexibility when building libfreenect2 by a parent project.

There are two requested features I have implemented, and 3 is along the same line of thought

  1. Only set the CMAKE_*_OUTPUT_DIRECTORY variables if they are not set yet.

    • Not a common practice, but it would be nice to have Protonect show up next to the parent executables so I can coach users to try running Protonect to determine if it is my application or something went wrong with libfreenect2
      • AKA they didn't copy the file to /etc/udev/rules.d and disconnect / reconnect
  2. Enable the parent project to specify the location of glfw3.

    • In my use-case, the gui library I am building compiles its own version of glfw3, so I can either a) disable OpenGL for libfreenect2 b) fail at link time because I end up with links to two separate glfw3
  3. Since LibUSB gets kind of muddy in terms of the "right" solution (almost every project has a similar but slightly different solution), allow the parent project to specify the libusb include dirs and libraries.

    • I got around issues with libfreenect2 by doing things dubiously unworthy of describing
    • And then eventually just stole your LibUSB cmake module (with citing of course!)

For 1 and 2, this is the diff:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 057bef2..7d018d8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -82,11 +82,17 @@ INCLUDE(SetupLibfreenect2Threading)
 INCLUDE(GenerateResources)

 #set the default path for built executables to the "bin" directory
-SET(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/bin)
+if(NOT CMAKE_RUNTIME_OUTPUT_DIRECTORY)
+  SET(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/bin)
+endif()

 #set the default path for built libraries to the "lib" directory
-SET(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib)
-SET(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib)
+if(NOT CMAKE_LIBRARY_OUTPUT_DIRECTORY)
+  SET(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib)
+endif()
+if(NOT CMAKE_ARCHIVE_OUTPUT_DIRECTORY)
+  SET(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/lib)
+endif()

 # dependencies
 FIND_PACKAGE(PkgConfig)    # try find PKGConfig as it will be used if found
@@ -262,7 +268,9 @@ ENDIF()

 SET(HAVE_OpenGL disabled)
 IF(ENABLE_OPENGL)
-  FIND_PACKAGE(GLFW3)
+  IF(NOT GLFW3_FOUND)
+    FIND_PACKAGE(GLFW3)
+  ENDIF()
   FIND_PACKAGE(OpenGL)
   SET(HAVE_OpenGL no)
   IF(GLFW3_FOUND AND OPENGL_FOUND)
diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt
index 8741f67..18c7cfb 100644
--- a/examples/CMakeLists.txt
+++ b/examples/CMakeLists.txt
@@ -52,7 +52,9 @@ SET(Protonect_DLLS
 )

 IF(ENABLE_OPENGL)
-  FIND_PACKAGE(GLFW3)
+  IF(NOT GLFW3_FOUND)
+    FIND_PACKAGE(GLFW3)
+  ENDIF()
   FIND_PACKAGE(OpenGL)
   IF(GLFW3_FOUND AND OPENGL_FOUND)
     INCLUDE_DIRECTORIES(

I'm not entirely sure if this is the best approach, but it lets me in my parent project do

set(DEPENDS_DIR OFF CACHE BOOL " " FORCE)
set(GLFW3_FOUND ON CACHE BOOL " " FORCE)
set(GLFW3_INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/nanogui/ext/glfw/include CACHE STRING " " FORCE)
set(GLFW3_LIBRARIES $<TARGET_FILE:glfw> CACHE STRING " " FORCE)

add_subdirectory(libfreenect2)
set_property(TARGET freenect2 PROPERTY FOLDER "dependencies")
add_dependencies(freenect2 nanogui glfw glfw_objects) # NanoGUI builds these

The assumption being that if somebody came through and hard-marked GLFW_FOUND, they also set GLFW3_INCLUDE_DIRS and GLFW3_LIBRARIES correctly.

Particularly where glfw3 is concerned, even if the parent project does not build its own glfw3, but instead uses find_package you can still potentially get conflicting versions since libfreenect2 is building gettings its deps with pkg-config. But this seems too unlikely to worry about -- anybody who runs into this scenario knows enough about configurations that they could solve it with environment variables...

What are your thoughts? Would you like a PR with solutions to 1-3? A subset? If you are willing to incorporate these things in the build system but want a different mechanism, what would you like?

Thanks!

xlz commented 7 years ago

I think using it as a third-party project instead of a child project would be easier.

You can refer to https://github.com/OpenKinect/libfreenect2/wiki/Test-Plan#build-tests for the intended usage of this as a third-party library.

xlz commented 7 years ago

The report is very good. But the issue will be closed at this time.

svenevs commented 7 years ago

Hey @xlz sorry for dropping off (again). I've been involved in a lot of unexpected int{ra,er}national travel. I'm just going to maintain a fork for my purposes, but the core issues here are

  1. libfreenect2's use of glfw demands that anybody using a project that also links against glfw use the exact same version.

    • If they are different, there will be linking errors.
    • My specialized case is the GUI library I use embeds glfw internally, but this will affect anybody who ends up with different glfw versions in the mix.
  2. libfreenect2 is not ABI compatible (there are some stl containers in method signatures e.g. std::string).

    • If I compile libfreenect2 with my host compiler (say gcc 5.3.1), any project that uses this libfreenect2 must be compiled with the same compiler (or at least a gcc that uses the same stl library)
    • I cannot compile a project with say clang and libc++abi without compiling a separate libfreenect2.

These were the primary incentives for enabling compilation as a sub-project. The above diff will fix them, I added the CMAKE_*_OUTPUT_DIRECTORY stuff for selfish obsessive reasons of liking to be able to control where build artifacts end up.

I had briefly talked about some libusb stuff, I don't have the time or understanding to investigate the appropriate changes. The libusb stuff for libfreenect2 is generally more complicated than most other projects (totally justified btw!), so users who want to use libfreenect2 as a sub-project can simply utilize the libfreenect2 setup.

In the end, you can make the reasonable assumption that anybody trying to build libfreenect2 as a sub-project knows what they are doing. I am of course happy to add some documentation explaining exactly what to do if this stuff is merged upstream, but even in those docs I'd include an admonition explaining that it is not suggested. Most people don't really worry about ABI compatibility (myself included), because in general a given user has a preferred compiler they use. In my project I have explicit benchmarking needs for various compilers, and I got sick of changing my ~/.bash_profile :wink: