cgal icon indicating copy to clipboard operation
cgal copied to clipboard

Provide graph traits and properties for pmp::SurfaceMesh

Open afabri opened this issue 5 years ago • 5 comments

Summary of Changes

This PR will make pmp::SurfaceMesh a model of a MutableFaceGraph. It further needs some additional typedefs and postfix increment of pmp iterators which I discuss with the authors.

See http://www.pmp-library.org/ and PR29 and PR30

Release Management

afabri avatar Jan 10 '20 11:01 afabri

TODO: add CMake support for pmp (http://www.pmp-library.org/).

lrineau avatar Jan 10 '20 11:01 lrineau

Note that the build system is cmake so it works directly in config mode. It might just be a matter of documentation on our side. Side note, Eigen seems to be required so it should come with a find Eigen.

sloriot avatar Jan 14 '20 15:01 sloriot

@afabri what is the status of this PR?

MaelRL avatar Sep 22 '20 10:09 MaelRL

There are still issues in pmp-library in the generatedpmpTargets.cmake: The following is generated:

set_target_properties(pmp PROPERTIES
  INTERFACE_COMPILE_OPTIONS "-std=c++11"
  INTERFACE_INCLUDE_DIRECTORIES "/home/sloriot/projects/pmp-library.git/src/pmp/../"
  INTERFACE_LINK_LIBRARIES "OpenMP::OpenMP_CXX"
)

while something like that should be generated:

find_package(OpenMP)
if(OpenMP_CXX_FOUND)
set_target_properties(pmp PROPERTIES
  INTERFACE_LINK_LIBRARIES "OpenMP::OpenMP_CXX"
)
endif()

set_target_properties(pmp PROPERTIES
  CXX_STANDARD 11
  INTERFACE_INCLUDE_DIRECTORIES "/home/sloriot/projects/pmp-library.git/src/pmp/../"
)

I don't know how to do it property in the cmake machinery without using a template file.

@mbotsch @dsieger do you see a solution? Note that it is not specific to CGAL but to any project using cmake that do not use OpenMP and/or that use a CXX standard greater than c++11

sloriot avatar Aug 13 '21 10:08 sloriot

Hi Sebastien,

I'm sorry, but to me it's also not obvious what the solution would be.

Regarding the C++ standard I would even say that prescribing the exact version is the right thing to do. Simply using a later standard might lead to incompatibilities / deprecation issues.

Regarding OpenMP I would assume that it is not added to the interface libraries if OpenMP is not found. After all, we only add it conditionally:

find_package(OpenMP)
if(OpenMP_CXX_FOUND)
  target_link_libraries(pmp PUBLIC OpenMP::OpenMP_CXX)
endif()

But then, I'm not too familiar with that part of CMake...

dsieger avatar Aug 17 '21 21:08 dsieger