Skip to content

Commit 01e56fe

Browse files
committed
BUG: Address issues related to ITK update to modern interfaces
Recent changes to ITK have been made to use CMake interfaces over raw libraries and files, along with adding the ITK namespace to targets: See InsightSoftwareConsortium/ITK#5721 Refactored installation of proxTV so that it is no longer done by the fetched project but controlled through the ITK module macros as a target. Note there is still some "odd" behavior with the Eigen3 dependency when provided by ITK it is not a target provided by idea, but needs a find_package done to locate. This change consistently uses the proxTV namespace for compatibility between uses of the proxTV.
1 parent 97587e4 commit 01e56fe

File tree

2 files changed

+20
-70
lines changed

2 files changed

+20
-70
lines changed

CMakeLists.txt

Lines changed: 20 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,9 @@
1-
cmake_minimum_required(VERSION 3.16.2)
1+
cmake_minimum_required(VERSION 3.22.0)
22
project(TotalVariation)
33

4-
if(ITK_USE_SYSTEM_proxTV)
5-
set(TotalVariation_LIBRARIES proxTV::proxTV)
6-
else()
7-
set(TotalVariation_LIBRARIES proxTV)
8-
endif()
94

10-
set(${PROJECT_NAME}_THIRD_PARTY 1)
5+
set(TotalVariation_LIBRARIES proxTV::proxTV)
6+
117

128
include(itk-module-init.cmake)
139

@@ -16,19 +12,12 @@ if(NOT ITK_SOURCE_DIR)
1612
list(APPEND CMAKE_MODULE_PATH ${ITK_CMAKE_DIR})
1713
endif()
1814

19-
include(CMakeParseArguments)
2015
include(FetchContent)
2116

22-
# Compile definition needed for proxTV headers
23-
# This will propagate to wrapping.
24-
# For wrapping to work, requires ITK with patch:
25-
# https://github.com/InsightSoftwareConsortium/ITK/pull/707
26-
add_compile_definitions(NOMATLAB)
27-
2817
message(STATUS "TotalVariation_proxTV_USE_EIGEN: ${TotalVariation_proxTV_USE_EIGEN}")
2918
if(TotalVariation_proxTV_USE_EIGEN)
3019
if(NOT ITK_USE_SYSTEM_EIGEN)
31-
# Set Eigen3_DIR to the internal Eigen3 used in ITK
20+
# Set Eigen3_DIR to the internal Eigen3 used in ITK, per documentation in ITK's Eigen3
3221
set(Eigen3_DIR "${ITKInternalEigen3_DIR}")
3322
message(STATUS "ITKTotalVariation: Using internal ITK Eigen Config found in: ${Eigen3_DIR}")
3423
endif()
@@ -53,14 +42,12 @@ else()
5342
add_compile_definitions(PROXTV_USE_LAPACK)
5443
endif()
5544

56-
# _proxTV_lib will be `proxTV::proxTV` when find_package, and `proxTV` when add_subdirectory(proxTV_folder)
57-
set(_proxTV_lib "")
5845
# if proxTV is built elsewhere
5946
if(ITK_USE_SYSTEM_proxTV)
6047
find_package(proxTV REQUIRED CONFIG)
61-
set(_proxTV_lib proxTV::proxTV)
6248
set(proxTV_DIR_INSTALL ${proxTV_DIR})
6349
set(proxTV_DIR_BUILD ${proxTV_DIR})
50+
6451
# It is only needed to EXPORT_CODE_BUILD when using external proxTV
6552
set(${PROJECT_NAME}_EXPORT_CODE_BUILD
6653
"${${PROJECT_NAME}_EXPORT_CODE_BUILD}
@@ -70,6 +57,14 @@ if(NOT ITK_BINARY_DIR)
7057
find_package(proxTV REQUIRED CONFIG)
7158
endif()
7259
")
60+
# When this module is loaded by an app, it is needed to find proxTV and OpenMP
61+
set(${PROJECT_NAME}_EXPORT_CODE_INSTALL
62+
"${${PROJECT_NAME}_EXPORT_CODE_INSTALL}
63+
find_package(OpenMP)
64+
set(proxTV_DIR \"${proxTV_DIR_INSTALL}\")
65+
find_package(proxTV REQUIRED CONFIG)
66+
")
67+
7368
else() # build proxTV here with the selected Eigen3
7469
# Build proxTV with C++11
7570
if(NOT CMAKE_CXX_STANDARD)
@@ -81,64 +76,24 @@ else() # build proxTV here with the selected Eigen3
8176
if(NOT CMAKE_CXX_EXTENSIONS)
8277
set(CMAKE_CXX_EXTENSIONS OFF)
8378
endif()
84-
79+
message(STATUS "proxTV_Eigen_LIBRARIES: ${proxTV_Eigen_LIBRARIES}")
8580
set(proxTV_GIT_REPOSITORY "https://github.com/phcerdan/proxTV.git")
86-
set(proxTV_GIT_TAG "use_eigen")
81+
set(proxTV_GIT_TAG "itk_installation")
8782
FetchContent_Declare(
8883
proxtv_fetch
8984
GIT_REPOSITORY ${proxTV_GIT_REPOSITORY}
90-
GIT_TAG ${proxTV_GIT_TAG})
85+
GIT_TAG ${proxTV_GIT_TAG}
86+
FIND_PACKAGE_ARGS NAMES proxTV REQUIRED CONFIG
87+
)
9188
FetchContent_GetProperties(proxTV_fetch)
9289
# proxTV options:
93-
set(Eigen3_DIR "${Eigen3_DIR}") # This is not needed but explicit helps reader.
9490
set(proxTV_USE_LAPACK 0)
95-
if(NOT DEFINED proxTV_INSTALL_INCLUDE_DIR)
96-
if(NOT ITK_SOURCE_DIR)
97-
set(ITK_INSTALL_INCLUDE_DIR include/ITK-${ITK_VERSION_MAJOR}.${ITK_VERSION_MINOR})
98-
endif()
99-
set(proxTV_INSTALL_INCLUDE_DIR ${ITK_INSTALL_INCLUDE_DIR}/proxTV)
100-
endif()
101-
if(NOT DEFINED proxTV_INSTALL_LIB_DIR)
102-
if(NOT ITK_SOURCE_DIR)
103-
set(ITK_INSTALL_LIBRARY_DIR lib)
104-
endif()
105-
set(proxTV_INSTALL_LIB_DIR ${ITK_INSTALL_LIBRARY_DIR})
106-
endif()
107-
if(NOT DEFINED proxTV_INSTALL_CMAKE_DIR)
108-
if(NOT ITK_SOURCE_DIR)
109-
set(ITK_INSTALL_PACKAGE_DIR "lib/cmake/ITK-${ITK_VERSION_MAJOR}.${ITK_VERSION_MINOR}")
110-
endif()
111-
set(proxTV_INSTALL_CMAKE_DIR ${ITK_INSTALL_PACKAGE_DIR}/Modules)
112-
endif()
91+
11392
# end proxTV options
11493

11594
FetchContent_MakeAvailable(proxTV_fetch)
116-
# proxTV will generate a target proxTV::proxTV when using find_package,
117-
# or a library proxTV when using add_subdirectory
118-
set(_proxTV_lib proxTV) # proxTV generated in subdirectory
119-
set(proxTV_DIR_INSTALL "${CMAKE_INSTALL_PREFIX}/${proxTV_INSTALL_CMAKE_DIR}")
120-
endif()
121-
122-
# When this module is loaded by an app, load proxTV too.
123-
set(${PROJECT_NAME}_EXPORT_CODE_INSTALL
124-
"${${PROJECT_NAME}_EXPORT_CODE_INSTALL}
125-
find_package(OpenMP)
126-
set(proxTV_DIR \"${proxTV_DIR_INSTALL}\")
127-
find_package(proxTV REQUIRED CONFIG)
128-
")
12995

130-
set(_populate_include_dirs_for_swig TRUE)
131-
if(${_populate_include_dirs_for_swig})
132-
# SWIG (wrapping) requires INCLUDE_DIRS
133-
get_target_property(proxTV_INCLUDE_DIRS ${_proxTV_lib} INTERFACE_INCLUDE_DIRECTORIES)
134-
string(REGEX REPLACE
135-
".*BUILD_INTERFACE:(.*/proxtv_fetch-build/src/include).*"
136-
"\\1"
137-
proxTV_INCLUDE_DIRS_STRIP
138-
${proxTV_INCLUDE_DIRS})
139-
# message(STATUS "proxTV_INCLUDE_DIRS: ${proxTV_INCLUDE_DIRS}")
140-
# message(STATUS "proxTV_INCLUDE_DIRS_STRIP: ${proxTV_INCLUDE_DIRS_STRIP}")
141-
set(TotalVariation_INCLUDE_DIRS ${proxTV_INCLUDE_DIRS_STRIP})
96+
itk_module_target(proxTV NAMESPACE proxTV::)
14297
endif()
14398

14499

@@ -151,5 +106,3 @@ else()
151106
endif()
152107

153108

154-
# Add the proxTV library to Modules/Targets/TotalVariationTargets.cmake
155-
itk_module_target(${_proxTV_lib} NO_INSTALL)

itk-module-init.cmake

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
option(ITK_USE_SYSTEM_proxTV "Use external proxTV" OFF)
22
mark_as_advanced(ITK_USE_SYSTEM_proxTV)
33

4-
option(ITK_USE_SYSTEM_EIGEN "Use External Eigen3" OFF)
5-
mark_as_advanced(ITK_USE_SYSTEM_EIGEN)
6-
74
# In case in the future we switch to use lapack instead of eigen for proxTV.
85
option(TotalVariation_proxTV_USE_EIGEN "proxTV in TotalVariation uses EIGEN" ON)
96
mark_as_advanced(TotalVariation_proxTV_USE_EIGEN)

0 commit comments

Comments
 (0)