Gmsh integration #968
Gmsh integration #968
Conversation
|
Oh, and we should keep in mind that we still have to adapt the CMake integration of gmsh. Using gmsh-integration should be optional and a user should be able to turn it on and off using a CMAKE flag. Finding and/or installing of CMake during the configuration step could probably be improved as well. Just writing this here as a reminder. Nothing has to be done as part of this PR. |
There was a problem hiding this comment.
I think, the integration of GMESH needs to be thought through. Since
- It increases build requirements / dependencies
- It is tightly integrated into the tigl library
I think a better strategy is to design a library on top of tigl and gmsh, that add mesh generation via gmsh. (just me two cents ;) )
| if ( GMSH_FOUND ) | ||
| MESSAGE( STATUS "GMSH found: header(${GMSH_INCLUDE_DIR}) lib(${GMSH_LIBRARY}) executable(${GMSH_EXECUTABLE})" ) | ||
| MESSAGE( STATUS "GL2PS found: lib(${GL2PS_LIBRARY})" ) | ||
| MESSAGE( STATUS "GL found: lib(${GL_LIBRARY})" ) |
There was a problem hiding this comment.
You should create an imported library GMSH target here to avoid using the old school variable approach. (I.e. the requirement to use ${GMSH_LIBRARY} and ${GMSH_INCLUDE_DIR}). See for example: https://github.com/DLR-SC/tixi/blob/master/cmake/FindLibXml2.cmake#L99
| @@ -87,6 +87,7 @@ target_include_directories(tigl3_objects | |||
| PRIVATE ${TIGL_INCLUDES} | |||
| PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/api | |||
| PRIVATE ${CMAKE_CURRENT_BINARY_DIR} | |||
| PRIVATE ${GMSH_INCLUDE_DIR} | |||
There was a problem hiding this comment.
Avoid using variables from findXXX.cmake scripts (i.e. use an imported target instead)
| target_link_libraries (tigl3 PRIVATE ${OpenCASCADE_LIBRARIES} ${VLD_LIBRARIES} | ||
| PRIVATE Boost::filesystem Boost::thread Boost::system Boost::atomic Boost::chrono Boost::date_time | ||
| PRIVATE ${CMAKE_THREAD_LIBS_INIT} | ||
| PRIVATE ${GMSH_LIBRARY} |
Good point. But that rules out ever having a Mesh-Exporter in TiGLViewer. Either way, I suggest to keep developing the Mesher as part of TiGL as part of this seperate branch gmsh-integration for now. I think porting the Mesher to a new library that links to tigl and gmsh should not be too hard if we decide to go that way at a later point. |
|
@ole-alb There are still some CMake issues. We should keep @rainman110's comments in mind and define an imported library target. Also, in the current design, gmsh is required and not optional. I will go ahead and merge this into |
Description
How Has This Been Tested?
Screenshots, that help to understand the changes(if applicable):
Checklist: