wjakob / nanogui

Minimalistic GUI library for OpenGL
Other
4.64k stars 603 forks source link

Make nanogui more self-contained #226

Open AndySomogyi opened 7 years ago

AndySomogyi commented 7 years ago

Nanogui uses the vector class out of Eigen, and doesn't really use a lot of anything else from Eigen.

Eigen is a pretty monstrous library, and because different libraries use Eigen, this causes issues when one library is coded against one version, and other coded against a different Eigen version.

It would make it a LOT simpler to integrate/use Nanogui if it did not use Eigen.

It seems like pretty much everything that the Eigen vector is used for, you could just use either a std::vector or std::array.

svenevs commented 7 years ago

Hi @AndySomogyi,

I will not speak to whether it should or will change, only provide the mechanism by which to bypass. Generally speaking, NanoGUI is designed to be included as a submodule for a project that depends on it -- rather than expecting a user to already have it installed. So in my projects, I employ the following tactic:

# Grab or find the Eigen3 include directory.
if(NOT DSCAN_EIGEN3_INCLUDE_DIR)
  find_package(Eigen3 QUIET)
  if(EIGEN3_INCLUDE_DIR)
    set(DSCAN_EIGEN3_INCLUDE_DIR ${EIGEN3_INCLUDE_DIR})
  else()
    # Note: this CMakeLists.txt resides in the same directory as nanogui
    set(DSCAN_EIGEN3_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/nanogui/ext/eigen)
  endif()
endif()
message(STATUS "Using Eigen3 from directory: ${DSCAN_EIGEN3_INCLUDE_DIR}")
set(NANOGUI_EIGEN_INCLUDE_DIR    ${EIGEN3_INCLUDE_DIR} CACHE BOOL " " FORCE)
list(APPEND DSCAN_EXTRA_INC_DIRS ${DSCAN_EIGEN3_INCLUDE_DIR})
# ... set some other variables, then add_subdirectory as shown in the linked docs

In the parent CMakeLists.txt (after making sure that DSCAN_EXTRA_INC_DIRS is set to PARENT_SCOPE at the bottom of this file), I now include_directories(${DSCAN_EXTRA_INC_DIRS}). It seems this feature was never included in the compilation docs, but you can view the as-yet-to-be-documented features of the NanoGUI build process via ccmake or the cmake-gui.

  1. I provide the same bypass mechanism as NanoGUI: allow parent projects to specify the eigen include dir for the same reason.
  2. This ensures that my library and NanoGUI use the same version of Eigen.
  3. Eigen vector types are a core part of NanoGUI. I doubt this will change because the underlying serialization logic depends on what Eigen guarantees about its internal storage.
  4. If the user does not have Eigen installed, the reverse is possible: your library that depends on NanoGUI now gets a copy of Eigen.

Hopefully this helps you get past conflicting Eigen dependencies. I suspect the owner of the project will encourage you to create a fork if you would rather not have Eigen in NanoGUI at all.

AndySomogyi commented 7 years ago

I'm trying to figure out a decent way of managing external dependencies, as my project includes a few external deps, all placed in a 'extern' folder, and each one of these is a git submodule.

The problem here is nanogui in turn has an extern folder that includes glfw and eigen, and another dependency also does the same thing: I have two extern submodules that have submodules of their own. So I end up with two copies of eigen, and three copies of glfw.

An extremely horribly bad solution, especially with something as unstable as eigen is to have each submodule try to reference a system eigen. This I think would be the worst possible choice. The only way template libraries like eigen work is if they're packaged as source in a build.

What I'd like to figure out how to do is use the modern CMake transitive dependencies, so that in glfw say, it would have a

target_include_directories(gfw PUBLIC ...)

Then every other thing that gets built simple adds a (target_link_libraries MyProgram glfw), and all of glfw's include dirs and libraries automatically become dependencies of MyProgram. This approach works really well when you have full control over your project.

The nightmare becomes getting other projects to accept changes. For example, I tried to submit patches to imgui where it would have a CMakeLists that would create imgui as a subproject and set up the correct transitive dependencies, but the maintainer wouldn't accept any pull requests. Hence, I abandoned imgui, and I'm simply not going to try to maintain a fork and go trough the nightmare of merging in upstream changes.

Here also, I fixed a few bugs in the CMakeLists to work around a known issue with CMake and XCode, and the pull request, https://github.com/wjakob/nanogui/pull/220 is ignored, so if I want to continue using nanogui, it looks like I'm going to have to maintain a fork with these bug fixes.

So, I just don't really know what to do at this point of trying to sanely manage dependencies.

AndySomogyi commented 7 years ago

Hi @svenevs , thanks for the info, I'm going to try an approach similar to yours.

One thing that seems to be solving a number of issues is building on the suggested compilation approach at https://nanogui.readthedocs.io/en/latest/compilation.html. What I'm doing is this:

# glfw does not set it's target include directories, but it turns out we
# can set them after the glfw subproject is processed.
# This approach enables us to simply target_link_libraries(MyProgram glfw), and
# all the glfw include directories automatically get added to the MyProgram.
target_include_directories(glfw PUBLIC
  $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/nanogui/ext/glfw/include>
  )

target_include_directories(glfw PUBLIC
  $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/nanogui/ext/glfw/deps>
  )

But another issue with the way external dependencies are handled with subprojects is updating the dependencies of the dependencies. For example, GLFW 3.3 fixes a bug on OSX, however, nanogui packages GLFW 3.2. I need to do some investigation to see if I can come up with a workaround that DOES NOT involve modifying nanogui.

svenevs commented 7 years ago

Without forking projects you probably won't be able to get this done, but I think what you are having trouble with (?) is getting a sub-project that also uses glfw to link against the "right" one? Suppose my project has two dependencies: nanogui and hackedProject. In hackedProject, they just have a raw

if(BUILD_DEMOS)
  FIND_PACKAGE(GLFW3)
  FIND_PACKAGE(OpenGL)
  IF(GLFW3_FOUND AND OPENGL_FOUND)
  ...

So their interface doesn't let me specify where glfw is coming from, which is your current problem. I changed it to be

if(BUILD_DEMOS)
  IF(NOT GLFW3_FOUND) # allows parent to set this, assumes incl / libs set CORRECTLY
    FIND_PACKAGE(GLFW3)
  ENDIF()
  ...

So of course I haven't done this publicly yet but when I do I'll have to maintain a fork if the owners of the main repo don't want that in their system. You can argue both cases, it makes the build more flexible but it also makes the build more vulnerable to operator error. However, now I can do the following in my CMakeLists.txt

# AFTER i have added nanogui
set(GLFW3_FOUND ON) # set found and ALSO include / libs
set(GLFW3_INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/nanogui/ext/glfw/include)
set(GLFW3_LIBRARIES $<TARGET_FILE:glfw>) # target glfw from nanogui

# adding my hacked project that was looking for `glfw`
add_subdirectory(hackedProject)
# it's a dependency of my project, as is nanogui / its glfw etc
set_property(TARGET hackedProject PROPERTY FOLDER "dependencies")
# this is the key: force hackedProject to build after nanogui (and therefore glfw)
add_dependencies(hackedProject nanogui glfw glfw_objects) # NanoGUI builds these

Not sure if that's what you're looking for but that's how you can get a different project to build using the NanoGUI glfw. FWIW glfw is a really small library, unless it's creating actual conflicts (e.g. you end up linking against two different glfw versions), just leave it :wink: If you are having link problems, then choose which ever one is the "right" glfw and hack the remaining to use the same one. If you really want to avoid forks, you can be extra-hacky and throw in some patching on-the fly to modify sub-project cmakelists.txt. But that's extremely error prone...

cdcseacave commented 7 years ago

It would indeed be very helpful if NANOGUI_EIGEN_INCLUDE_DIR was exposed in cmake-gui. A newcomer has no idea of its existence otherwise.