libigl / libigl.github.io

Repository for the libigl website, online tutorial and documentation
http://libigl.github.io
4 stars 30 forks source link

Update docs/example-project.md #20

Closed BruegelN closed 4 years ago

BruegelN commented 4 years ago

Please let me know, what you think. Should I include a link to static-library.md with a hint about compile times?


By the way I've noticed that libigl-example-project recommends to clone libigl recursive, which is not required anymore.

jdumas commented 4 years ago

Thanks! I've updated the text a little bit. But you're not really supposed to pass LIBIGL_INCLUDE_DIR to CMake to specify the libigl root directory. Right now the only way to set the libigl root directory is by setting an environment variable, but this can be annoying on Windows. We should modify the FindLIBIGL.cmake in the example project to also look up the CMake variable LIBIGL_DIR so it can be passed on directly via the command-line.

BruegelN commented 4 years ago

Thank you for the improvement!

Regarding LIBIGL_INCLUDE_DIR: Currently find_path() is used inside FindLIBIGL.cmake While I personaly think an older version of the find_path documentation more clear on the topic: https://cmake.org/cmake/help/v3.0/command/find_path.html also the latest version of the find_path states that one can pass -DVAR=value via the command line (https://cmake.org/cmake/help/latest/command/find_path.html see 2.) Maybe I didn't get your point but calling

cmake ../ -DLIBIGL_INCLUDE_DIR=~/foo/libigl/include/

worked for me so far. Also set or changing LIBIGL_INCLUDE_DIR via cmake-gui worked for me.

But to be honest one thing that's quite confusing: if using an ENV you have to specify the root of libigl (\~/foo/libigl/) where as LIBIGL_INCLUDE_DIR will point to the include dir (\~/foo/libigl/include/)

jdumas commented 4 years ago

The fact that this work is not because find_path() uses the -DVAR=value option in 2. of the documentation (this doesn't refer to the same VAR used to store the result of the find_path() command). This works because find_path() stores the result in a cache variable. But anyway the fact that it works this way is not intentional, and as you say having to specify the root of the lbigl installation with the ENV variable LIBIGL_DIR, but having to specify the include directory if you want to pass -DLIBIGL_INCLUDE=... to the CMake command line is quite confusing.

What I am suggesting is to simply modify the FindLIBIGL.cmake as follows:

# - Try to find the LIBIGL library
# Once done this will define
#
#  LIBIGL_FOUND - system has LIBIGL
#  LIBIGL_INCLUDE_DIR - **the** LIBIGL include directory
if(LIBIGL_FOUND)
    return()
endif()

find_path(LIBIGL_INCLUDE_DIR igl/readOBJ.h
    HINTS
        ENV LIBIGL_DIR
        LIBIGL_DIR
    PATHS
        ${CMAKE_SOURCE_DIR}/../..
        ${CMAKE_SOURCE_DIR}/..
        ${CMAKE_SOURCE_DIR}
        ${CMAKE_SOURCE_DIR}/libigl
        ${CMAKE_SOURCE_DIR}/../libigl
        ${CMAKE_SOURCE_DIR}/../../libigl
        /usr
        /usr/local
        /usr/local/igl/libigl
    PATH_SUFFIXES include
)

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(LIBIGL
    "\nlibigl not found --- You can download it using:\n\tgit clone --recursive https://github.com/libigl/libigl.git ${CMAKE_SOURCE_DIR}/../libigl"
    LIBIGL_INCLUDE_DIR)
mark_as_advanced(LIBIGL_INCLUDE_DIR)

list(APPEND CMAKE_MODULE_PATH "${LIBIGL_INCLUDE_DIR}/../cmake")
include(libigl)

Then you will be able to set the root directory LIBIGL_DIR by either passing it as a command-line argument, or as an environment variable.

jdumas commented 4 years ago

@alecjacobson @danielepanozzo can I get write capabilities on the example-project repository so I can implement this suggested change to our FindLIBIGL.cmake?

BruegelN commented 4 years ago

Hi @jdumas thank you for your suggestion! I forgot to reply to you, sorry for that :/ I've started to implement changes to FindLIBIGL.cmake but hadn't have time to fully test it (https://github.com/BruegelN/libigl-example-project/tree/rework-FindLIBIGL.cmake) do you want me to submit a PR on the libigl-example-project repo? Later on we can update the documentation accordingly.

jdumas commented 4 years ago

I don't have merge capabilities on the libigl-example-project for now, but in the meantime feel free to submit a PR with those changes yes.