When COLMAP is used as a third-party library with add_subdirectory or with FetchContent (like GLOMAP does) all the macro definitions are not properly propagated to the top-level CMakeLists.txt project. I noticed this after getting a very long compilation error related to Glog:
FAILED: CMakeFiles/utils.dir/src/utils.cpp.o
/dir/include/colmap/util/logging.h:163:38: error: 'CheckOpString' in namespace 'google' does not name a type
163 | const google::CheckOpString& result)
| ^~~~~~~~~~~~~
This was because GLOG_VERSION_MAJOR and GLOG_VERSION_MINOR macros were not visible in the top-level project. Building GLOMAP with FETCH_COLMAP also causes the same compilation error.
The problem with add_compile_definitions is that the macros have a scope limited to the current directory and all the subdirectories, but they don't propagate upwards to parent directories. I was able to quickly ignore this error with a find_package(colmap REQUIRED) call which makes all the macros visible in the main CMakeLists.txt file, even if I don't use the target there.
I'm proposing a solution by replacing all the add_compile_definitions with target_compile_definitions which allows to better control the scope and propagate macros upwards with PUBLIC. These are the macro definitions I changed:
- From
cmake/FindDependencies.cmake:GLOG_VERSION_MAJOR,GLOG_VERSION_MINOR,COLMAP_CGAL_ENABLED,COLMAP_DOWNLOAD_ENABLED,COLMAP_CUDA_ENABLED,COLMAP_GUI_ENABLED,COLMAP_GPU_ENABLED. - From the main
CMakeLists.txtfile:COLMAP_LSD_ENABLED.
COLMAP_OPENGL_ENABLED is defined in cmake/FindDependencies.cmake but it's never used in the code. That's why I didn't add it anywhere. I also removed it from the FindDependencies.cmake file. However, I'd like to know if this is correct.
I used PUBLIC for the cases where the macros are used in header files, and PRIVATE otherwise. This solved the compilation error for me.
I kept the add_compile_definitions call for the macros WIN32_LEAN_AND_MEAN, GLOG_USE_GLOG_EXPORT, GLOG_NO_ABBREVIATED_SEVERITIES, GL_GLEXT_PROTOTYPES, NOMINMAX and EIGEN_INITIALIZE_MATRICES_BY_NAN. I think these compile definitions are more for internal use and don't need to be propagated to parent directories. However, I would like to know if I'm wrong.
I also decided to explicitly add the compile definitions to all the static libraries even if they can be inherited from other libraries. For example, COLMAP_CGAL_ENABLED needs to be public for colmap_mvs target and colmap_controllers library links against colmap_mvs, so adding target_compile_definitions(colmap_controllers PRIVATE COLMAP_CGAL_ENABLED) is unnecessary but I decided to do it. This can be improved if you consider so.
An alternative solution to this problem could be to keep the add_compile_definitions calls and keep track of all the macros that needs to be visible in parent directories and add them to the COLMAP interface library that holds all the colmap_<name> targets.
Also as a side note I wanted to ask, the colmap_exe static lib is created in src/colmap/exe/CMakeLists.txt but I see that it's not used at all. It's not even linked against colmap_main. Does it serve any purpose?