geometry-central icon indicating copy to clipboard operation
geometry-central copied to clipboard

Update CMakeLists.txt

Open rishabhbhutani013 opened this issue 1 year ago • 4 comments

rishabhbhutani013 avatar Sep 16 '24 01:09 rishabhbhutani013

Hi, I think a bit of context would help the devs who maintain this library :)

I assume this is meant to fix the same bug that I encountered: on Windows the symlink creation fails with an error like so:

file Failed to create link
'C:/.../build/deps/include/nanoflann/nanoflann.hpp'
because existing path cannot be removed: No error

Your solution to make a copy instead could work, but I think you forgot the DESTINATION word, should be something like that:

file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/nanoflann/include/nanoflann.hpp
     DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/include/nanoflann/nanoflann.hpp)

DavidJourdan avatar Nov 16 '24 01:11 DavidJourdan

Hi all, I'm clearing out old PRs, pardon for not addressing this sooner.

I don't quite understand what's going on here. I guess this code was added in case users depend on nanoflann.hpp living at that location, so after moving it we added a copy step at CMake-time, but that copy is failing on some Windows builds? Can you confirm? Maybe it would be better to just delete this and bump the version?

CC @qnzhou who it seems wrote this line originally, in case you have any opinions on this.

nmwsharp avatar Sep 15 '25 00:09 nmwsharp

This is related to https://github.com/nmwsharp/geometry-central/pull/157

The issue is about the nanoflann include path. In geometry central, it is sometimes included as the following:

https://github.com/nmwsharp/geometry-central/blob/56152a8afc42405ad071aaaf5a2876429ed1f7bd/src/surface/detect_symmetry.cpp#L4

which assumes nanoflann.hpp is in a nanoflann folder. This is not the case in the vanilla nanoflann repo: https://github.com/jlblancoc/nanoflann/tree/master/include

If an external project (e.g. Lagrange) uses both nanoflann and geometry-central, it will lead to a header file not found error.

After thinking it a bit more, I feel it is better to consistently use the following include path in geometry central: https://github.com/nmwsharp/geometry-central/blob/56152a8afc42405ad071aaaf5a2876429ed1f7bd/src/utilities/knn.cpp#L3

This way, we do not need to do any cmake magic to have compatible include path.

qnzhou avatar Sep 16 '25 19:09 qnzhou

Just to add, https://github.com/nmwsharp/geometry-central/pull/208 is in the same boat, just for nanort instead.

qnzhou avatar Sep 16 '25 19:09 qnzhou