BUG: Adapt VtkGlue extent callback to VTK future-const signature#6472
Conversation
vtkImageImport and vtkImageExport apply VTK_FUTURE_CONST to the PropagateUpdateExtent callback, so its extent parameter is const int* when VTK is built with VTK_USE_FUTURE_CONST=ON. ITK's VTKImageExport and VTKImageImport keep an int* signature, so the function-pointer types are incompatible and the VtkGlue assignment fails to compile. Bridge each assignment with a captureless lambda whose signature matches the target setter exactly, forwarding through the existing callback getter. Closes InsightSoftwareConsortium#6462
| // Adapt the exporter callback to vtkImageImport's VTK_FUTURE_CONST extent | ||
| // signature; the const_cast is safe because the callback only reads the extent. |
There was a problem hiding this comment.
The in-source comment spans two lines, which exceeds ITK's prose-budget hard cap of one short line (≤ 100 chars). The same pattern appears in
itkVTKImageToImageFilter.hxx. Both comments carry genuinely load-bearing content (the non-obvious const_cast safety invariant), so the information should be preserved — just condensed.
| // Adapt the exporter callback to vtkImageImport's VTK_FUTURE_CONST extent | |
| // signature; the const_cast is safe because the callback only reads the extent. | |
| // VTK_FUTURE_CONST extent param; const_cast safe because callback only reads the extent. |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // Adapt vtkImageExport's VTK_FUTURE_CONST extent callback to the importer's | ||
| // int* signature; the int* argument binds to the VTK_FUTURE_CONST parameter. |
There was a problem hiding this comment.
Same prose-budget violation as in
itkImageToVTKImageFilter.hxx: the two-line comment exceeds the one-line ≤ 100-char cap.
| // Adapt vtkImageExport's VTK_FUTURE_CONST extent callback to the importer's | |
| // int* signature; the int* argument binds to the VTK_FUTURE_CONST parameter. | |
| // VTK_FUTURE_CONST extent; int* binds to const int* implicitly, no cast needed. |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Fixes a compile regression in the ITK↔VTK glue: when VTK is built with
VTK_USE_FUTURE_CONST=ON,vtkImageImport/vtkImageExportdeclare thePropagateUpdateExtentcallback's extent parameter asconst int *, which no longer matches ITK'sint *callback type. Each assignment is bridged with a captureless-lambda adapter that matches the target setter's exact signature.Closes #6462
Root cause
VTK commit
99e95adeae("Added VTK_FUTURE_CONST to extents related API") wrapped the extent parameter ofvtkImageImport::SetPropagateUpdateExtentCallbackandvtkImageExport::GetPropagateUpdateExtentCallbackinVTK_FUTURE_CONST. WithVTK_USE_FUTURE_CONST=ONthat macro expands toconst, so the callback type becomesvoid (*)(void *, const int *).ITK's
itk::VTKImageExportBase/itk::VTKImageImporthardcodevoid (*)(void *, int *). Function-pointer types do not implicitly convert across thatconstdifference, so both glue assignments fail to compile:itkImageToVTKImageFilter.hxx— ITK exporter callback →vtkImageImportitkVTKImageToImageFilter.hxx—vtkImageExportcallback → ITK importerWhy the fix lives in VtkGlue (and not by changing ITK's callback signature)
The const-incorrect typedef lives in
Modules/Bridge/VTK(ITKVTK), which is intentionally buildable without VTK: it has no VTK headers (grep '#include [<"]vtk'is empty), declaresDEPENDS ITKCommononly, and ships compiled aslibITKVTKwithint *baked into its ABI. The classes reproduce VTK's callback ABI by hand precisely so applications can wire ITK↔VTK without ITK itself depending on VTK.Consequences for the fix:
VTK_FUTURE_CONST(by including a VTK header, or via a CMake-injected compile definition) would either end ITKVTK's ability to build without VTK, or turn a public ITK type into a build-flag-dependent ABI —int *in one build,const int *in another, for the same ITK version. That is a silent-ODR hazard for downstream consumers unless every one of them inherits the identical definition. Note also that VTK does not exportVTK_USE_FUTURE_CONSTas a CMake variable, so detection would require a configure-time compile probe.const int *does not fix the build — it only flips the breakage to the default (non-future-const) VTK configuration, which is the common case, and is a public API/ABI change toVTKImageExportBaseand subclasses.Keeping the adaptation in
Modules/Bridge/VtkGlue— the module that already depends on VTK — leaves ITKVTK's ABI unconditionallyint *(one stable type, no detection, no propagation) and confines allVTK_FUTURE_CONSTawareness to the lambda parameter lists. Theconst_cast<int *>in the import direction is safe because every implementation of the callback only reads the extent.Test plan
masterwithVTK_USE_FUTURE_CONST=ON(reproduces the regression before the fix at both sites).Module_ITKVtkGlue=ONagainst that VTK.ITKVtkGlueTestDriverbuilds clean (-Wall -Wextra, no warnings) with the fix.itkImageToVTKImageFilterTest,itkImageToVTKImageFilterRGBTest,itkVTKImageToImageFilterTestall pass.