Bring back itkVtkGlue to Python wrapping #5502

AlexyPellegrini · 2025-09-01T15:14:00Z

Brief

Python wrapping of itkVtkGlue has been removed by #4392. This PR fixes the build issue and enables it again.

Issue

The XML generations failed because the response file referencing the include directories for castxml was not generated correctly. All modules used the same .castxml.inc instead of their own <modulename>.castxml.inc. Once this is fixed, the module wraps correctly.

Why is it needed

Even though numpy enables to share an itk::Image and a vtkImageData, it does not support pipelining. Pipelining is important because some VTK and ITK filters support requesting only a subextent of a resource, enabling better performances especially for readers of formats meant to be streamed, such as OME-Zarr.

image = itk.imread(sys.argv[1], imageio=itk.OMEZarrNGFFImageIO.New())
itktovtk = itk.ImageToVTKImageFilter[type(image)].New()
itktovtk.SetInput(image)
reslice = vtkImageReslice() # vtkImageReslice may query a subextent
reslice.SetInputConnection(itktovtk.GetImporter().GetOutputPort())
#...

PR Checklist

No API changes were made (or the changes have been approved)
No major design changes were made (or the changes have been approved)
Added test (or behavior not changed)
Updated API documentation (or API not changed)
Added license to new files (if any)
Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
Added ITK examples for all new major features (if any)

FYI @finetjul

github-actions

Thank you for contributing a pull request! 🙏

Welcome to the ITK community! 🤗👋☀️

We are glad you are here and appreciate your contribution. Please keep in mind our community participation guidelines. 📜
More support and guidance on the contribution process can be found in our contributing guide. 📖

This is an automatic message. Allow for time for the ITK community to be able to read the pull request and comment
on it.

dzenanz · 2025-09-01T18:50:28Z

If this is patch set is meant to be included in the release branch, make release branch the target. Otherwise please base in on a newer commit to avoid merge conflicts.

dzenanz · 2025-09-01T18:51:14Z

If that is a lot of work, maybe wait for a review by Matt.

thewtex · 2025-09-01T21:41:08Z

Wrapping/macro_files/itk_auto_load_submodules.cmake


  #Write compile definitions and include paths to file.  @CONFIG_CASTXML_INC_CONTENTS@ expanded in configure_file
-  set(castxml_inc_file "${WRAPPER_LIBRARY_OUTPUT_DIR}/castxml_inputs/${library_name}.castxml.inc")
+  set(castxml_inc_file "${WRAPPER_LIBRARY_OUTPUT_DIR}/castxml_inputs/${WRAPPER_LIBRARY_NAME}.castxml.inc")


Can a further explanation be provided as to why this is changed and why the change does not impact other libraries and modules?

ofc,

Why this is changed?
BUG: Restructured python type hint generation procedure #3200 reworked the Python wrapping code, and is the reason why itkVtkGlue couldn't be wrapped anymore.
If you take a look at this line you will see that the original code was in the itk_wrap_module macro directly, so it had access to library_name parameter. When the code has been moved to a different macro library_name became empty (because macro arguments are not real variables, so they do not outlive the macro body).
Using WRAPPER_LIBRARY_NAME actually reverts back to the original behavior, where each module has its own include directories list, named <module-name>.castxml.inc.

Why the change does not impact other libraries and modules?
Each module will have its own file, otherwise no behavior changed!

Note that this also fixes a subtle issue about incremental builds, because enabling or disabling a module with python wrapping caused all module wrapping to be rebuilt, because the .castxml.inc was modified, so all xml regenerated, and so on. With this change, the old behavior is restored, so already build modules shouldn't modify their own castxml.inc, so they won't be rebuilt for nothing!

Let me know if something is still unclear!

The information here, especially about incremental builds, deserves to be in the commit message.

I updated the commit description!

AlexyPellegrini · 2025-09-02T12:57:10Z

If this is patch set is meant to be included in the release branch, make release branch the target. Otherwise please base in on a newer commit to avoid merge conflicts.

Indeed, I rebased on main!

All module generated the same ".castxml.inc" file. This caused several issues: - Each CMake configure step repeatedly overwrote .castxml.inc, causing a full rebuild of all wrapping related files because the XML descriptions were considered out-of-date. - Wrapping of module itkVtkGlue failed, because it requires additional include directories that were removed by other modules. This change makes each module now generates its own "<module>.castxml.inc" file.

dzenanz

LGTM

dzenanz · 2025-09-04T12:56:21Z

Thank you Alexy!

dzenanz · 2025-09-05T14:09:22Z

This PR broke some of the builds on the nightly dashboard. Error message (from configure step):

-- Setting build type to 'Release' as none was specified.
-- The CXX compiler identification is MSVC 19.44.35215.0
-- The C compiler identification is MSVC 19.44.35215.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.44.35207/bin/Hostx64/x64/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.44.35207/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Using MSVC's dynamic CRT (/MD and /MDd)
-- Found Git: C:/Program Files/Git/mingw64/bin/git.exe (found version "2.51.0.windows.1")
-- Performing Test C_HAS_WARNING-Wno-uninitialized

...

-- Looking for include file sys/select.h - not found
CMake Error at C:/Libs/VTK-8.2.0/CMake/vtkModuleAPI.cmake:140 (message):
  Requested modules not available:

    vtkvtksys
Call Stack (most recent call first):
  Modules/Bridge/VtkGlue/itk-module-init.cmake:93 (vtk_module_config)
  CMake/ITKModuleEnablement.cmake:436 (include)
  CMakeLists.txt:754 (include)


-- Configuring incomplete, errors occurred!

More info here: https://open.cdash.org/builds/10653427

dzenanz · 2025-09-05T14:22:23Z

I am working on a fix locally.

dzenanz · 2025-09-05T14:26:06Z

Fix in #5512. Passes configure step locally with VTK 8.2 and 9.4.

dzenanz · 2025-09-05T15:12:32Z

There are also some build errors on Mac: /Users/dzenan.zukic/Documents/Dashboards/vtk/Wrapping/PythonCore/PyVTKObject.h:64:37: error: unknown type name 'PyBufferProcs'.

AlexyPellegrini · 2025-09-05T16:16:50Z

This one might be harder to tackle, ITK enables the Python limited API by default when Python version is superior to 3.11 here and PyBufferProcs structure is defined in cpython/object.h that is only included when using the "unlimited" API here.

Maybe we could enable the unlimited API just for VtkGlue module?

dzenanz · 2025-09-05T18:27:03Z

The point of the limited API is compatibility of later versions with 3.11. We could make options ITK_USE_PYTHON_LIMITED_API and Module_ITKVtkGlue mutually exclusive. Can you make that PR?

hjmjohnson · 2025-09-05T19:24:06Z

Could we remove the dependence on PyBufferProcs?

dzenanz · 2025-09-09T17:49:33Z

There are more errors, such as this: LINK : fatal error LNK1181: cannot open input file 'vtkkwiml.lib' [T:\Dashboard\ITK-build\Modules\Bridge\VtkGlue\ITKVtkGlueHeaderTest1.vcxproj]. It also affects VTK glue test driver:

Build started at 13:46...
1>------ Build started: Project: ITKVtkGlueTestDriver, Configuration: Debug x64 ------
1>LINK : fatal error LNK1104: cannot open file 'vtkkwiml.lib'
1>Done building project "ITKVtkGlueTestDriver.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 62 up-to-date, 0 skipped ==========
========== Build completed at 13:46 and took 01.438 seconds ==========

but not ITKVtkGlue target. Indeed, there is no vtkkwiml.lib anywhere on my computer.

AlexyPellegrini · 2025-09-11T09:08:08Z

vtkkwiml is header-only so there won't be any archive or shared library for sure. This means that the target "vtkkwiml" is not found by CMake, so it is replaced with a random "-lvtkkwiml" (or windows equivalent). I don't see any reason why this target would be undefined though. Maybe a specific VTK version in which the module wasn't renamed yet?

dzenanz · 2025-09-11T14:51:53Z

My local VTK was at 5e878068c23f6b87a398dbe8000424971dcb7afa. When I updated it to 9.5.1, vtkkwiml.lib error is gone.

dzenanz · 2025-09-11T15:06:29Z

The nightly builds (e.g. Windows11-VS2022-Static-DebugTBB) use VTK 8.2.0 built with VS 2015. They are still afflicted by vtkkwiml.lib error.

dzenanz · 2025-09-11T17:04:17Z

Modules/Bridge/VtkGlue/itk-module-init.cmake

  _required_vtk_libraries
  ${_target_prefix}IOImage
  ${_target_prefix}ImagingSources
+  ${_target_prefix}kwiml


We might need special handling for this, like we do for vtksys. But critical version might be different from 8.90.0. @AlexyPellegrini

The critical version is probably much newer, as 5e87806, which is affected, is only 6 months old.

dzenanz · 2025-09-12T15:14:32Z

I created PR #5522, please review.

github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Python wrapping Python bindings for a class area:Bridge Issues affecting the Bridge module labels Sep 1, 2025

github-actions bot reviewed Sep 1, 2025

View reviewed changes

dzenanz requested a review from thewtex September 1, 2025 18:48

thewtex reviewed Sep 1, 2025

View reviewed changes

AlexyPellegrini force-pushed the vtk-glue-python branch from 66bf521 to a3f15ed Compare September 2, 2025 12:38

thewtex requested a review from hjmjohnson September 2, 2025 15:13

AlexyPellegrini added 2 commits September 3, 2025 11:14

COMP: Enable back ITKVtkGlue module in Python wrapping

bcb6876

AlexyPellegrini force-pushed the vtk-glue-python branch from a3f15ed to bcb6876 Compare September 3, 2025 09:27

dzenanz approved these changes Sep 3, 2025

View reviewed changes

hjmjohnson approved these changes Sep 3, 2025

View reviewed changes

hjmjohnson merged commit e91d475 into InsightSoftwareConsortium:main Sep 3, 2025
16 checks passed

AlexyPellegrini mentioned this pull request Sep 4, 2025

VtkGlue fails to build because it cannot find the headers of VTK #4375

Open

AlexyPellegrini mentioned this pull request Sep 8, 2025

COMP: Disable limited Python API for itkVtkGlue module wrapping #5517

Merged

2 tasks

dzenanz reviewed Sep 11, 2025

View reviewed changes

dzenanz mentioned this pull request Sep 12, 2025

COMP: Fix cannot open 'vtkkwiml.lib' occurring with older VTK #5522

Merged

2 tasks

Uh oh!

Bring back itkVtkGlue to Python wrapping #5502

Bring back itkVtkGlue to Python wrapping #5502

Uh oh!

Conversation

AlexyPellegrini commented Sep 1, 2025

Brief

Issue

Why is it needed

PR Checklist

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

dzenanz commented Sep 1, 2025

Uh oh!

dzenanz commented Sep 1, 2025

Uh oh!

thewtex Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

AlexyPellegrini Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

dzenanz Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

AlexyPellegrini Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

AlexyPellegrini commented Sep 2, 2025

Uh oh!

dzenanz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dzenanz commented Sep 4, 2025

Uh oh!

dzenanz commented Sep 5, 2025

Uh oh!

dzenanz commented Sep 5, 2025

Uh oh!

dzenanz commented Sep 5, 2025

Uh oh!

dzenanz commented Sep 5, 2025

Uh oh!

AlexyPellegrini commented Sep 5, 2025

Uh oh!

dzenanz commented Sep 5, 2025

Uh oh!

hjmjohnson commented Sep 5, 2025

Uh oh!

dzenanz commented Sep 9, 2025

Uh oh!

AlexyPellegrini commented Sep 11, 2025

Uh oh!

dzenanz commented Sep 11, 2025

Uh oh!

dzenanz commented Sep 11, 2025

Uh oh!

dzenanz Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

dzenanz Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

dzenanz commented Sep 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants