Update for ITK Interface Modules and use ITK targets and installation.#57
Open
blowekamp wants to merge 3 commits intoInsightSoftwareConsortium:mainfrom
Open
Update for ITK Interface Modules and use ITK targets and installation.#57blowekamp wants to merge 3 commits intoInsightSoftwareConsortium:mainfrom
blowekamp wants to merge 3 commits intoInsightSoftwareConsortium:mainfrom
Conversation
Member
Author
|
@phcerdan Are you still maintaining this module along with the forked proxTV code? |
Collaborator
|
@blowekamp well, It has been a while, but I can help. The fork proxTV is indeed under my username. Do we need to update the proxTV fork as well? |
Member
Author
Yes. As mentioned in the PR message: I had some local changes that seem to work with a this remote build in the ITK repo, but there are a few other usages that need to be checked. I was not able to follow all the installation and exporting of targets and files going on. Some of it looks off to me. |
Collaborator
|
Got it. How can I help? |
Member
Author
What is the current status of your proxTV fork with upstream? Look's like upstream is in active. Maybe we can be aggressive with deleting unneeded install code, as we don't expect upstream changes? Also it looks like the current build are not working, maybe not up-to date? I am not too familiar with all the remote python infrastructure. Would you be available to look at that? P.S. I am also testing this with wrapping in SimpleITK. |
blowekamp
force-pushed
the
cmake_interface_update
branch
from
17ba86f to
01e56fe
Compare
blowekamp
force-pushed
the
cmake_interface_update
branch
from
01e56fe to
978fe28
Compare
Member
Author
|
Here is the link to the changes for the proxTV fork: Some change highlights:
|
7 tasks
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. This change consistently uses the proxTV namespace for compatibility between uses of the proxTV.
blowekamp
force-pushed
the
cmake_interface_update
branch
from
978fe28 to
6e50186
Compare
blowekamp
changed the title
7 tasks
This was referenced May 6, 2026
hjmjohnson
added a commit
to InsightSoftwareConsortium/ITK
that referenced
this pull request
May 6, 2026
…onsumers
ITK ships its vendored Eigen3 headers at
Modules/ThirdParty/Eigen3/src/itkeigen/Eigen/<header>
ITK code accesses these via the ITK_EIGEN(<header>) macro
defined in itk_eigen.h, which expands to
<itkeigen/Eigen/<header>>
When ITK_USE_SYSTEM_EIGEN=OFF, the prior ITKEigen3_INCLUDE_DIRS
contained only ${ITKEigen3_SOURCE_DIR}/src — sufficient for the
macro form (the compiler then appends /itkeigen/Eigen/<header>) but
not for the upstream Eigen convention <Eigen/<header>>, which needs
${ITKEigen3_SOURCE_DIR}/src/itkeigen on the include path.
External consumers that legitimately use the upstream pattern hit
'fatal error: Eigen/Dense: No such file or directory' as soon as
they link against ITK::ITKEigen3Module or ITK::eigen_internal.
This blocks the modern-target-interfaces refactor in
InsightSoftwareConsortium/ITKTotalVariation#57: proxTV's
lapackFunctionsWrap.cpp uses #include <Eigen/Dense> and so cannot
build against the vendored Eigen via either of ITK's exported
imported targets.
Add the itkeigen subdirectory to ITKEigen3_INCLUDE_DIRS so both
include conventions resolve. ITK's own consumers continue to find
<itkeigen/Eigen/<header>> via the first entry; external consumers
now find <Eigen/<header>> via the second. Both entries flow into
INTERFACE_INCLUDE_DIRECTORIES of the IMPORTED ITK::ITKEigen3Module
target, so the fix applies in-tree and to find_package(ITK)
consumers alike.
The Eigen3::Eigen IMPORTED target produced by Eigen's own export
already exposes the correct path, but it is in-source-scope only
(declared as add_library(Eigen3::Eigen ALIAS eigen)) and therefore
invisible to FetchContent sub-builds; relying on it is not an
option for downstream remote modules that pull proxTV via
FetchContent.
This change is independent of the Eigen 5 vendored update in
PR #6176; the architectural mismatch predates that PR and applies
to upstream/main as-is.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BUG: Use correct variable for linking libraries for tests
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
This change consistently uses the proxTV namespace for compatibility between uses of the proxTV.
There are additional problems detected in the proxTV library including duplicate exporting of targets which cause configuration issues. Additionally, installation paths for all libraries and exports needs to be reviewed and verified for the correct importing of proxTV::proxTV.