ENH: Ingest ITKTotalVariation into Modules/Filtering (with ITKproxTV third-party module)

[hjmjohnson]

Ingests the TotalVariation remote module into Modules/Filtering/TotalVariation, and adds a new Modules/ThirdParty/proxTV third-party module (DCMTK-style: ExternalProject-built at build time from the ISC proxTV fork, EXCLUDE_FROM_DEFAULT, no vendored source). Builds and tests under C++17 against current main. Part of #6160; consumes blowekamp's ITKTotalVariation#57 (ISC-forked proxTV).

Local validation: Module_TotalVariation=ON configures, proxTV builds via ExternalProject under C++17 (Eigen backend), and 3/3 tests pass including itkProxTVImageFilterTest. pre-commit run --all-files is clean.

proxTV integration model ("like DCMTK")

• Modules/ThirdParty/proxTV/ holds only CMake glue — proxTV source is not vendored into ITK git. It is git-cloned + built at build time via ExternalProject from InsightSoftwareConsortium/proxTV (tag pinned in proxTVGitTag.cmake).
• EXCLUDE_FROM_DEFAULT: only configured/built when a dependent module (TotalVariation) is enabled. ITK_USE_SYSTEM_proxTV selects an external build.
• proxTV is built with the Eigen backend (proxTV_USE_LAPACK=OFF) against ITK's internal Eigen3, under ITK's C++17 standard (-DCMAKE_CXX_STANDARD=17). The proxTV fork needed no change for C++17.
• The imported target carries IMPORTED_LOCATION, the source src/ include dir, NOMATLAB (guards proxTV's mex.h), and Threads::Threads.
• Library + public headers are installed into the ITK install tree, with build- and install-tree EXPORT_CODE so external consumers of an installed ITK resolve proxTV and TVopt.h.

Commits

1. ENH: Ingest merge — merge-preserving filter-repo (122→56 commits, upstream merge topology + authorship preserved).
2. COMP: Add ITKproxTV third-party module; wire TotalVariation in-tree.
3. COMP: Remove TotalVariation.remote.cmake.
4. ENH: Enable Module_TotalVariation in configure-ci.
5. DOC: Module README.
6. STYLE: gersemi-format ingested test CMakeLists.
7. BUG: Correct ProxTVImageFilter OutputImageType (TOutputImage, not TInputImage); explicit size→int cast; header-guard fix.

Reviewer notes

• Install/packaging: install-tree consumption (an external project compiling against an installed ITK with TotalVariation) is wired but only validated via the generated cmake_install.cmake; a real make install + downstream-consume pass in CI would be a good confirmation.
• Python wrapping was not exercised locally (non-Python build tree); CI covers it. The .wrap covers 2D WRAP_ITK_REAL.
• The ingested filter test runs the filter but does not regression-check output values (upstream --compare baseline was commented out); not added here.

Per ingest workflow, upstream ITKTotalVariation archival is a separate post-merge follow-up.

[dzenanz]

Looks good after a relatively quick look.

[blowekamp]

@hans May I take over this work?

The modern CMake usage now available in ITK is to use the interface modules as opposed to the legacy cmake variables. Changes were made in the proxTV library fork to support this. It would be good to use the modern best practices with new code integration opposed to the older approaches (that the agents may be picking up on).

Please let me know if I can continue this effort with the modern approach.

[hjmjohnson]

May I take over this work?

@blowekamp YES! We're very close to closing issue #6160 for remote module ingestions. Once the open PR's are included, we will be able to close that issue!

My only request is that (a) the improvements are made in the next few days, or (b) we include the minimally working ingestion as is, and follow up with improvements.

Hans

[blowekamp]

I will be taking over this PR. Mostly to work on the proxTV integration as a third party.

[hjmjohnson]

I will be taking over this PR. Mostly to work on the proxTV integration as a third party.

Awesome! Thank you.

[dzenanz]

A significant simplification.

[dzenanz]

Ubuntu 24.04 comes with CMake 3.28.3, and Ubuntu 26.04 was released recently. Should we bump general CMake version requirement, instead of just here? Or wait for ITK 6.1 to do that?

[dzenanz]

Version details here: https://linuxcapable.com/how-to-install-cmake-on-ubuntu-linux/

[blowekamp]

@hjmjohnson What do you think of updating ITK's CMake version? May make some of the fetch content work easier if we are going down that route further.

[hjmjohnson]

I think that getting modern CMake versions is so easy for all developers that we should be more aggressive in updating the versions. New versions of cmake can be easily downloaded via pixi, uv, simple shell commands,.

CMake version 3.28.3 seems like a great floor.

My one concern is the Slicer support. That codebase is already littered with many conditionals, so they may be receptive to bumping the cmake minimum version as well:

┌─[johnsonhj@hj] - [~/src/Slicer] - [2026-06-05 07:34:18]
└─[0] rg CMAKE_VERSION                                                                                                          (main|✔)
Extensions/CMake/SlicerBlockBuildPackageAndUploadExtensions.cmake
230:  if(CMAKE_VERSION GREATER_EQUAL "3.28")

CMakeLists.txt
6:# OLDEST_VALIDATED_POLICIES_VERSION and CMAKE_VERSION (used for this build)
8:# between the OLDEST_VALIDATED_POLICIES_VERSION and CMAKE_VERSION.
17:if("${CMAKE_VERSION}" VERSION_EQUAL "3.21.0")
18:  message(FATAL_ERROR "CMake version is ${CMAKE_VERSION} and using CMake==3.21.0 is not supported.\nSee https://gitlab.kitware.com/cmake/cmake/-/issues/22476")
21:if("${CMAKE_VERSION}" VERSION_GREATER_EQUAL "3.25.0" AND "${CMAKE_VERSION}" VERSION_LESS_EQUAL "3.25.2")
22:  message(FATAL_ERROR "CMake version is ${CMAKE_VERSION} and using CMake >=3.25.0,<=3.25.2 is not supported.\nSee https://gitlab.kitware.com/cmake/cmake/-/issues/24567")
912:if("${CMAKE_VERSION}" VERSION_GREATER_EQUAL "3.29.0")

CMake/SlicerMacroBuildScriptedCLI.cmake
47:    if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.20")

CMake/ExternalProjectDependency.cmake
469:  if(NOT CMAKE_VERSION VERSION_LESS "3.16")
487:  if(NOT CMAKE_VERSION VERSION_LESS "3.16")
516:  if(CMAKE_VERSION VERSION_GREATER "3.0")
520:  if(CMAKE_VERSION VERSION_EQUAL "3.4" OR CMAKE_VERSION VERSION_GREATER "3.4")
528:  if(CMAKE_VERSION VERSION_EQUAL "3.24" OR CMAKE_VERSION VERSION_GREATER "3.24")
532:  if(CMAKE_VERSION VERSION_EQUAL "3.28" OR CMAKE_VERSION VERSION_GREATER "3.28")

SuperBuild/External_python.cmake
60:  if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.24")

[hjmjohnson]

@blowekamp At a worst case, if building ITKTtotalVariaion, REQUIRE cmake 3.28 or greater as a stopgap until the rest of the community hits the same floor.

[blowekamp]

@hjmjohnson I think I am done with the PR. It looks like it needs to be rebased due to a conflict, but with the merge and the orphan branch a simple rebase will now work.

[hjmjohnson]

@blowekamp I will rebase this morning.

[hjmjohnson]

@blowekamp Rebased onto latest upstream/main (now at 3274f3029b). As you noted, the un-rooted ingest merge + orphan module branch survived a merge-preserving rebase intact — the ingest merge 5541af6c keeps its original second parent bf442db6a3 (the proxTV/TotalVariation module tip), only the first parent moved to current main, and the four follow-on fixups replayed on top. The only conflict was the one-line pyproject.toml CI-module list (your TwoProjectionRegistration landed in the same slot); both are now enabled alphabetically. pre-commit run --all-files is clean. CI re-running on f185688419.

Verification

merges since upstream/main: 11 (1 ingest + 10 preserved upstream module merges)
ingest merge parents: main(3274f3029b) + module-tip(bf442db6a3, unchanged)
fixups (first-parent): proxTV third-party / remove remote decl / enable in CI / harden ProxTVImageFilter
net diff vs main: TotalVariation + proxTV module trees, pyproject.toml, removed Modules/Remote/TotalVariation.remote.cmake

[hjmjohnson]

@blowekamp Fixed the proxTV/Eigen3 in-tree wiring (25745550e) along the modern interface-module lines you set up in the fork — verified with a full local build of the TotalVariation module and all 3 of its tests passing (incl. itkProxTVImageFilterTest).

Root cause + fix

Your fork only calls find_package(Eigen3) as a fallback (elseif(NOT proxTV_Eigen_LIBRARIES)). The in-tree module was setting proxTV_Eigen_LIBRARIES ${ITKEigen3_LIBRARIES}, which is empty for the header-only internal Eigen3, so the fallback ran and died during the ITK build. Fix:

• set(proxTV_Eigen_LIBRARIES ITK::ITKEigen3Module) — the interface target, so the fork skips find_package entirely.
• target_include_directories(proxTV SYSTEM PRIVATE "${ITKEigen3_SOURCE_DIR}/src/itkeigen") — proxTV sources use #include <Eigen/...>, but ITK vendors Eigen under the itkeigen/ prefix, so the directory holding Eigen/ directly is added (PRIVATE: only lapackFunctionsWrap.cpp includes Eigen; no public header does).

Local validation (Ninja, internal Eigen3): configure clean → proxTV builds → TotalVariationTestDriver builds → ctest -R TotalVariation 3/3 passed.

[hjmjohnson]

@blowekamp Some additional changes needed to get the builds to pass.

[blowekamp]

Yea, this is fine it works.

I should have uninstalled the system Eigen locally to see this error mode.

The added ITK::ITKEigen3Module rep doesn't really add libraries or include or anything. And adding that manual path should really happen. I guess ideally there should be an "ITK::Eigen3"interface library that is the raw unguarded headers... or provTV should be updated to use the ITKEIGEN(x) macro.

But this is fine for now.