From 97587e4ab5a1d60ccd83f97d34d53da05eaf2709 Mon Sep 17 00:00:00 2001 From: Bradley Lowekamp Date: Thu, 12 Feb 2026 21:52:34 +0000 Subject: [PATCH 1/2] BUG: Use correct variable for linking libraries for tests --- test/CMakeLists.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 43d480f..6cd4334 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -4,10 +4,7 @@ set(TotalVariationTests itkProxTVImageFilterTest.cxx ) -set(Libraries ${TotalVariation-Test_LIBRARIES}) -list(APPEND Libraries ${_proxTV_lib}) - -CreateTestDriver(TotalVariation "${Libraries}" "${TotalVariationTests}") +CreateTestDriver(TotalVariation "${TotalVariation-Test_LIBRARIES}" "${TotalVariationTests}") itk_add_test(NAME itkProxTVImageFilterTest COMMAND TotalVariationTestDriver From 01e56feecd72759f28e870b8376bd85e000800ec Mon Sep 17 00:00:00 2001 From: Bradley Lowekamp Date: Thu, 12 Feb 2026 21:53:42 +0000 Subject: [PATCH 2/2] 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. --- CMakeLists.txt | 87 ++++++++++--------------------------------- itk-module-init.cmake | 3 -- 2 files changed, 20 insertions(+), 70 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 99ab4e7..eff0cb4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,13 +1,9 @@ -cmake_minimum_required(VERSION 3.16.2) +cmake_minimum_required(VERSION 3.22.0) project(TotalVariation) -if(ITK_USE_SYSTEM_proxTV) - set(TotalVariation_LIBRARIES proxTV::proxTV) -else() - set(TotalVariation_LIBRARIES proxTV) -endif() -set(${PROJECT_NAME}_THIRD_PARTY 1) +set(TotalVariation_LIBRARIES proxTV::proxTV) + include(itk-module-init.cmake) @@ -16,19 +12,12 @@ if(NOT ITK_SOURCE_DIR) list(APPEND CMAKE_MODULE_PATH ${ITK_CMAKE_DIR}) endif() -include(CMakeParseArguments) include(FetchContent) -# Compile definition needed for proxTV headers -# This will propagate to wrapping. -# For wrapping to work, requires ITK with patch: -# https://github.com/InsightSoftwareConsortium/ITK/pull/707 -add_compile_definitions(NOMATLAB) - message(STATUS "TotalVariation_proxTV_USE_EIGEN: ${TotalVariation_proxTV_USE_EIGEN}") if(TotalVariation_proxTV_USE_EIGEN) if(NOT ITK_USE_SYSTEM_EIGEN) - # Set Eigen3_DIR to the internal Eigen3 used in ITK + # Set Eigen3_DIR to the internal Eigen3 used in ITK, per documentation in ITK's Eigen3 set(Eigen3_DIR "${ITKInternalEigen3_DIR}") message(STATUS "ITKTotalVariation: Using internal ITK Eigen Config found in: ${Eigen3_DIR}") endif() @@ -53,14 +42,12 @@ else() add_compile_definitions(PROXTV_USE_LAPACK) endif() -# _proxTV_lib will be `proxTV::proxTV` when find_package, and `proxTV` when add_subdirectory(proxTV_folder) -set(_proxTV_lib "") # if proxTV is built elsewhere if(ITK_USE_SYSTEM_proxTV) find_package(proxTV REQUIRED CONFIG) - set(_proxTV_lib proxTV::proxTV) set(proxTV_DIR_INSTALL ${proxTV_DIR}) set(proxTV_DIR_BUILD ${proxTV_DIR}) + # It is only needed to EXPORT_CODE_BUILD when using external proxTV set(${PROJECT_NAME}_EXPORT_CODE_BUILD "${${PROJECT_NAME}_EXPORT_CODE_BUILD} @@ -70,6 +57,14 @@ if(NOT ITK_BINARY_DIR) find_package(proxTV REQUIRED CONFIG) endif() ") + # When this module is loaded by an app, it is needed to find proxTV and OpenMP + set(${PROJECT_NAME}_EXPORT_CODE_INSTALL + "${${PROJECT_NAME}_EXPORT_CODE_INSTALL} + find_package(OpenMP) + set(proxTV_DIR \"${proxTV_DIR_INSTALL}\") + find_package(proxTV REQUIRED CONFIG) + ") + else() # build proxTV here with the selected Eigen3 # Build proxTV with C++11 if(NOT CMAKE_CXX_STANDARD) @@ -81,64 +76,24 @@ else() # build proxTV here with the selected Eigen3 if(NOT CMAKE_CXX_EXTENSIONS) set(CMAKE_CXX_EXTENSIONS OFF) endif() - + message(STATUS "proxTV_Eigen_LIBRARIES: ${proxTV_Eigen_LIBRARIES}") set(proxTV_GIT_REPOSITORY "https://github.com/phcerdan/proxTV.git") - set(proxTV_GIT_TAG "use_eigen") + set(proxTV_GIT_TAG "itk_installation") FetchContent_Declare( proxtv_fetch GIT_REPOSITORY ${proxTV_GIT_REPOSITORY} - GIT_TAG ${proxTV_GIT_TAG}) + GIT_TAG ${proxTV_GIT_TAG} + FIND_PACKAGE_ARGS NAMES proxTV REQUIRED CONFIG + ) FetchContent_GetProperties(proxTV_fetch) # proxTV options: - set(Eigen3_DIR "${Eigen3_DIR}") # This is not needed but explicit helps reader. set(proxTV_USE_LAPACK 0) - if(NOT DEFINED proxTV_INSTALL_INCLUDE_DIR) - if(NOT ITK_SOURCE_DIR) - set(ITK_INSTALL_INCLUDE_DIR include/ITK-${ITK_VERSION_MAJOR}.${ITK_VERSION_MINOR}) - endif() - set(proxTV_INSTALL_INCLUDE_DIR ${ITK_INSTALL_INCLUDE_DIR}/proxTV) - endif() - if(NOT DEFINED proxTV_INSTALL_LIB_DIR) - if(NOT ITK_SOURCE_DIR) - set(ITK_INSTALL_LIBRARY_DIR lib) - endif() - set(proxTV_INSTALL_LIB_DIR ${ITK_INSTALL_LIBRARY_DIR}) - endif() - if(NOT DEFINED proxTV_INSTALL_CMAKE_DIR) - if(NOT ITK_SOURCE_DIR) - set(ITK_INSTALL_PACKAGE_DIR "lib/cmake/ITK-${ITK_VERSION_MAJOR}.${ITK_VERSION_MINOR}") - endif() - set(proxTV_INSTALL_CMAKE_DIR ${ITK_INSTALL_PACKAGE_DIR}/Modules) - endif() + # end proxTV options FetchContent_MakeAvailable(proxTV_fetch) - # proxTV will generate a target proxTV::proxTV when using find_package, - # or a library proxTV when using add_subdirectory - set(_proxTV_lib proxTV) # proxTV generated in subdirectory - set(proxTV_DIR_INSTALL "${CMAKE_INSTALL_PREFIX}/${proxTV_INSTALL_CMAKE_DIR}") -endif() - -# When this module is loaded by an app, load proxTV too. -set(${PROJECT_NAME}_EXPORT_CODE_INSTALL -"${${PROJECT_NAME}_EXPORT_CODE_INSTALL} -find_package(OpenMP) -set(proxTV_DIR \"${proxTV_DIR_INSTALL}\") -find_package(proxTV REQUIRED CONFIG) -") -set(_populate_include_dirs_for_swig TRUE) -if(${_populate_include_dirs_for_swig}) - # SWIG (wrapping) requires INCLUDE_DIRS - get_target_property(proxTV_INCLUDE_DIRS ${_proxTV_lib} INTERFACE_INCLUDE_DIRECTORIES) - string(REGEX REPLACE - ".*BUILD_INTERFACE:(.*/proxtv_fetch-build/src/include).*" - "\\1" - proxTV_INCLUDE_DIRS_STRIP - ${proxTV_INCLUDE_DIRS}) - # message(STATUS "proxTV_INCLUDE_DIRS: ${proxTV_INCLUDE_DIRS}") - # message(STATUS "proxTV_INCLUDE_DIRS_STRIP: ${proxTV_INCLUDE_DIRS_STRIP}") - set(TotalVariation_INCLUDE_DIRS ${proxTV_INCLUDE_DIRS_STRIP}) + itk_module_target(proxTV NAMESPACE proxTV::) endif() @@ -151,5 +106,3 @@ else() endif() -# Add the proxTV library to Modules/Targets/TotalVariationTargets.cmake -itk_module_target(${_proxTV_lib} NO_INSTALL) diff --git a/itk-module-init.cmake b/itk-module-init.cmake index fe45460..473b9bf 100644 --- a/itk-module-init.cmake +++ b/itk-module-init.cmake @@ -1,9 +1,6 @@ option(ITK_USE_SYSTEM_proxTV "Use external proxTV" OFF) mark_as_advanced(ITK_USE_SYSTEM_proxTV) -option(ITK_USE_SYSTEM_EIGEN "Use External Eigen3" OFF) -mark_as_advanced(ITK_USE_SYSTEM_EIGEN) - # In case in the future we switch to use lapack instead of eigen for proxTV. option(TotalVariation_proxTV_USE_EIGEN "proxTV in TotalVariation uses EIGEN" ON) mark_as_advanced(TotalVariation_proxTV_USE_EIGEN)