From 66a1cf1042e873643e345bf9ff4d4d3bc6b7d139 Mon Sep 17 00:00:00 2001 From: "Hans J. Johnson" Date: Wed, 5 Aug 2026 09:30:17 -0500 Subject: [PATCH] COMP: Make ITKVtkGlue wrapping abi3-compatible Exchange pointers with VTK's Python layer through the `__this__` and `Addr=0x...` encodings using only Limited API calls, instead of through vtkPythonUtil, whose header chain accesses PyTypeObject members that Py_LIMITED_API hides. Dropping VTK::WrappingPythonCore from the wrapping link interface is required for correctness, not tidiness: that library is built against one libpython, so an extension linking it cannot be version-agnostic. Neither encoding is documented VTK API, so PythonVtkGlueABI3EncodingTest asserts both still hold and PythonVtkGlueRoundTripTest exercises the typemaps end to end. Closes: #6711 --- Modules/Bridge/VtkGlue/CMakeLists.txt | 1 - Modules/Bridge/VtkGlue/itk-module-init.cmake | 1 - .../Bridge/VtkGlue/wrapping/CMakeLists.txt | 16 +- Modules/Bridge/VtkGlue/wrapping/VtkGlue.i | 137 +++++++++++++++--- .../VtkGlue/wrapping/test/CMakeLists.txt | 32 ++++ .../wrapping/test/VtkGlueABI3EncodingTest.py | 100 +++++++++++++ .../wrapping/test/VtkGlueRoundTripTest.py | 111 ++++++++++++++ 7 files changed, 362 insertions(+), 36 deletions(-) create mode 100644 Modules/Bridge/VtkGlue/wrapping/test/CMakeLists.txt create mode 100644 Modules/Bridge/VtkGlue/wrapping/test/VtkGlueABI3EncodingTest.py create mode 100644 Modules/Bridge/VtkGlue/wrapping/test/VtkGlueRoundTripTest.py diff --git a/Modules/Bridge/VtkGlue/CMakeLists.txt b/Modules/Bridge/VtkGlue/CMakeLists.txt index e4f395a2de0..74231aaff7e 100644 --- a/Modules/Bridge/VtkGlue/CMakeLists.txt +++ b/Modules/Bridge/VtkGlue/CMakeLists.txt @@ -99,7 +99,6 @@ set(_required_vtk_libraries ) if(ITK_WRAP_PYTHON) list(APPEND _required_vtk_libraries - VTK::WrappingPythonCore VTK::CommonCore VTK::CommonDataModel VTK::CommonExecutionModel) diff --git a/Modules/Bridge/VtkGlue/itk-module-init.cmake b/Modules/Bridge/VtkGlue/itk-module-init.cmake index fa7ad74c8c2..550f1dce272 100644 --- a/Modules/Bridge/VtkGlue/itk-module-init.cmake +++ b/Modules/Bridge/VtkGlue/itk-module-init.cmake @@ -37,7 +37,6 @@ if(ITK_WRAP_PYTHON) list( APPEND _required_vtk_libraries - VTK::WrappingPythonCore VTK::CommonCore VTK::CommonDataModel VTK::CommonExecutionModel diff --git a/Modules/Bridge/VtkGlue/wrapping/CMakeLists.txt b/Modules/Bridge/VtkGlue/wrapping/CMakeLists.txt index a0a587c0b76..5222e6ce330 100644 --- a/Modules/Bridge/VtkGlue/wrapping/CMakeLists.txt +++ b/Modules/Bridge/VtkGlue/wrapping/CMakeLists.txt @@ -1,15 +1,3 @@ itk_wrap_module(ITKVtkGlue) -if(ITK_USE_PYTHON_LIMITED_API) - message( - FATAL_ERROR - "The ITKVtkGlue module can only built without Python limited API due to VTK limitations." - "Please set `ITK_USE_PYTHON_LIMITED_API` to `FALSE`." - ) -else() - list( - APPEND - WRAPPER_SWIG_LIBRARY_FILES - "${CMAKE_CURRENT_SOURCE_DIR}/VtkGlue.i" - ) - itk_auto_load_and_end_wrap_submodules() -endif() +list(APPEND WRAPPER_SWIG_LIBRARY_FILES "${CMAKE_CURRENT_SOURCE_DIR}/VtkGlue.i") +itk_auto_load_and_end_wrap_submodules() diff --git a/Modules/Bridge/VtkGlue/wrapping/VtkGlue.i b/Modules/Bridge/VtkGlue/wrapping/VtkGlue.i index 54d41993bd9..4cd3a5cd324 100644 --- a/Modules/Bridge/VtkGlue/wrapping/VtkGlue.i +++ b/Modules/Bridge/VtkGlue/wrapping/VtkGlue.i @@ -51,44 +51,141 @@ %module(package="itk",threads="1") VtkGluePython %{ -#include "vtkPythonUtil.h" -#include "vtkVersion.h" -#if (VTK_MAJOR_VERSION > 5 ||((VTK_MAJOR_VERSION == 5)&&(VTK_MINOR_VERSION > 6))) -#define vtkPythonGetObjectFromPointer vtkPythonUtil::GetObjectFromPointer -#define vtkPythonGetPointerFromObject vtkPythonUtil::GetPointerFromObject -#endif +#include +#include +#include + +// Pointer exchange with VTK's Python layer using only the Limited API, so this +// module stays abi3 and needs no link against VTK::WrappingPythonCore. +namespace itkVtkGlueABI3 +{ + +inline PyObject * +ImportClass(const char * moduleName, const char * className) +{ + PyObject * mod = PyImport_ImportModule(moduleName); + if (!mod) + { + return nullptr; + } + PyObject * cls = PyObject_GetAttrString(mod, className); + Py_DECREF(mod); + return cls; +} + +// Parses the `__p_` encoding VTK publishes as `__this__`. The +// isinstance() gate is what makes trusting that string safe: without it any +// object exposing a forged `__this__` would be cast to a native pointer. +inline void * +GetPointerFromObject(PyObject * obj, const char * moduleName, const char * className) +{ + PyObject * cls = ImportClass(moduleName, className); + if (!cls) + { + return nullptr; + } + const int isInstance = PyObject_IsInstance(obj, cls); + Py_DECREF(cls); + if (isInstance < 0) + { + return nullptr; + } + if (isInstance == 0) + { + PyErr_Format(PyExc_TypeError, "expected a VTK %s instance", className); + return nullptr; + } + + PyObject * thisStr = PyObject_GetAttrString(obj, "__this__"); + if (!thisStr) + { + PyErr_Clear(); + PyErr_Format(PyExc_TypeError, "expected a VTK %s instance", className); + return nullptr; + } + + void * ptr = nullptr; + Py_ssize_t len = 0; + const char * s = PyUnicode_AsUTF8AndSize(thisStr, &len); + if (s && len > 4 && s[0] == '_' && std::strlen(s) == static_cast(len)) + { + const char * sep = std::strstr(s + 1, "_p_"); + if (sep && std::strcmp(sep + 3, className) == 0) + { + std::uintptr_t addr = 0; + // '_' is not a hex digit, so the conversion stops at the separator. + if (std::sscanf(s + 1, "%" SCNxPTR, &addr) == 1 && addr != 0) + { + ptr = reinterpret_cast(addr); + } + } + } + Py_DECREF(thisStr); + + if (!ptr) + { + PyErr_Format(PyExc_TypeError, "expected a VTK %s instance", className); + } + return ptr; +} + +// Reconstructs through VTK's own `Addr=0x...` path so the IsA() check, the +// object map, and reference counting all stay VTK's responsibility. +// `__new__` is called explicitly rather than `cls(addr)`: vtkmodules.util.data_model +// registers keyword-only `override` subclasses for the data-model classes, whose +// __init__ would reject the positional address string. +inline PyObject * +GetObjectFromPointer(void * ptr, const char * moduleName, const char * className) +{ + if (!ptr) + { + Py_RETURN_NONE; + } + + PyObject * cls = ImportClass(moduleName, className); + if (!cls) + { + return nullptr; + } + + char addr[64]; + std::snprintf(addr, sizeof(addr), "Addr=0x%" PRIxPTR, reinterpret_cast(ptr)); + PyObject * obj = PyObject_CallMethod(cls, "__new__", "Os", cls, addr); + Py_DECREF(cls); + return obj; +} + +} // namespace itkVtkGlueABI3 %} %typemap(out) vtkImageExport* { - PyImport_ImportModule("vtk"); - $result = vtkPythonGetObjectFromPointer ( (vtkImageExport*)$1 ); + $result = itkVtkGlueABI3::GetObjectFromPointer($1, "vtkmodules.vtkIOImage", "vtkImageExport"); + if (!$result) { SWIG_fail; } } %typemap(out) vtkImageImport* { - PyImport_ImportModule("vtk"); - $result = vtkPythonGetObjectFromPointer ( (vtkImageImport*)$1 ); + $result = itkVtkGlueABI3::GetObjectFromPointer($1, "vtkmodules.vtkIOImage", "vtkImageImport"); + if (!$result) { SWIG_fail; } } %typemap(out) vtkImageData* { - PyImport_ImportModule("vtk"); - $result = vtkPythonGetObjectFromPointer ( (vtkImageData*)$1 ); + $result = itkVtkGlueABI3::GetObjectFromPointer($1, "vtkmodules.vtkCommonDataModel", "vtkImageData"); + if (!$result) { SWIG_fail; } } %typemap(in) vtkImageData* { - $1 = NULL; - $1 = (vtkImageData*) vtkPythonGetPointerFromObject ( $input, "vtkImageData" ); - if ( $1 == NULL ) { SWIG_fail; } + $1 = static_cast(itkVtkGlueABI3::GetPointerFromObject($input, "vtkmodules.vtkCommonDataModel", "vtkImageData")); + if (!$1) { SWIG_fail; } } %typemap(out) vtkPolyData* { - PyImport_ImportModule("vtk"); - $result = vtkPythonGetObjectFromPointer ( (vtkPolyData*)$1 ); + $result = itkVtkGlueABI3::GetObjectFromPointer($1, "vtkmodules.vtkCommonDataModel", "vtkPolyData"); + if (!$result) { SWIG_fail; } } %typemap(in) vtkPolyData* { - $1 = NULL; - $1 = (vtkPolyData*) vtkPythonGetPointerFromObject ( $input, "vtkPolyData" ); - if ( $1 == NULL ) { SWIG_fail; } + $1 = static_cast(itkVtkGlueABI3::GetPointerFromObject($input, "vtkmodules.vtkCommonDataModel", "vtkPolyData")); + if (!$1) { SWIG_fail; } } #endif diff --git a/Modules/Bridge/VtkGlue/wrapping/test/CMakeLists.txt b/Modules/Bridge/VtkGlue/wrapping/test/CMakeLists.txt new file mode 100644 index 00000000000..5038faae83f --- /dev/null +++ b/Modules/Bridge/VtkGlue/wrapping/test/CMakeLists.txt @@ -0,0 +1,32 @@ +list(FIND ITK_WRAP_IMAGE_DIMS 2 wrap_2_index) +if( + ITK_WRAP_PYTHON + AND + VTK_WRAP_PYTHON + AND + ITK_WRAP_float + AND + wrap_2_index + GREATER + -1 +) + itk_python_add_test( + NAME PythonVtkGlueABI3EncodingTest + COMMAND + ${CMAKE_CURRENT_SOURCE_DIR}/VtkGlueABI3EncodingTest.py + ) + itk_python_add_test( + NAME PythonVtkGlueRoundTripTest + COMMAND + ${CMAKE_CURRENT_SOURCE_DIR}/VtkGlueRoundTripTest.py + ) + # itkTestDriver prepends ITK's own entries to whatever PYTHONPATH it inherits. + set_property( + TEST + PythonVtkGlueABI3EncodingTest + PythonVtkGlueRoundTripTest + PROPERTY + ENVIRONMENT + "PYTHONPATH=${VTK_PREFIX_PATH}/${VTK_PYTHONPATH}" + ) +endif() diff --git a/Modules/Bridge/VtkGlue/wrapping/test/VtkGlueABI3EncodingTest.py b/Modules/Bridge/VtkGlue/wrapping/test/VtkGlueABI3EncodingTest.py new file mode 100644 index 00000000000..1264b3434d1 --- /dev/null +++ b/Modules/Bridge/VtkGlue/wrapping/test/VtkGlueABI3EncodingTest.py @@ -0,0 +1,100 @@ +# ========================================================================== +# +# Copyright NumFOCUS +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0.txt +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# ========================================================================== + +"""Canary for the two VTK wrapper encodings the ITKVtkGlue abi3 typemaps rely on. + +VtkGlue.i exchanges pointers with VTK through `__this__` and the `Addr=0x...` +argument to `__new__` rather than through vtkPythonUtil, because vtkPythonUtil's +header chain is not usable under Py_LIMITED_API. Neither encoding is documented +VTK API, so this test fails loudly and specifically if VTK changes either one. + +`__new__` is used rather than plain construction because vtkmodules.util.data_model +registers keyword-only `override` subclasses for vtkImageData and vtkPolyData; +`cls(addr)` reaches those and raises TypeError. + +Both encodings and the `__new__` string branch are present in every VTK 9.x from +9.0.0 onward, so the module's VTK 9.1 floor is unchanged. The branch is gated on +the type not being a heap type, which is the thing to re-check if VTK ever makes +its wrapped types heap types. +""" + +import re +import sys + +from vtkmodules.vtkCommonDataModel import vtkImageData, vtkPolyData +from vtkmodules.vtkIOImage import vtkImageExport, vtkImageImport + +# `_<2*sizeof(void*) hex digits>_p_`, per vtkPythonUtil::ManglePointer. +THIS_RE = re.compile(r"^_([0-9a-fA-F]+)_p_(\w+)$") + +failures = [] + + +def check(condition, message): + if not condition: + failures.append(message) + + +for cls in (vtkImageData, vtkPolyData, vtkImageExport, vtkImageImport): + name = cls.__name__ + obj = cls() + + this = getattr(obj, "__this__", None) + check(this is not None, f"{name}: instance has no __this__ attribute") + if this is None: + continue + + match = THIS_RE.match(this) + check( + match is not None, + f"{name}: __this__ {this!r} does not match __p_", + ) + if match is None: + continue + + address = int(match.group(1), 16) + check(address != 0, f"{name}: __this__ encodes a null address") + check( + match.group(2) == name, + f"{name}: __this__ encodes class {match.group(2)!r}, expected {name!r}", + ) + + # The reconstruction path VtkGlue.i's `out` typemaps drive. + try: + rebuilt = cls.__new__(cls, f"Addr=0x{address:x}") + except Exception as exception: # noqa: BLE001 - report any refusal verbatim + failures.append(f"{name}: Addr=0x... reconstruction raised {exception!r}") + continue + + rebuilt_this = getattr(rebuilt, "__this__", None) + check( + rebuilt_this == this, + f"{name}: reconstruction yielded __this__ {rebuilt_this!r}, expected {this!r}", + ) + check( + isinstance(rebuilt, cls), + f"{name}: reconstruction yielded {type(rebuilt)!r}, not a {name} instance", + ) + +if failures: + print("VTK wrapper encodings assumed by VtkGlue.i have changed:", file=sys.stderr) + for failure in failures: + print(f" - {failure}", file=sys.stderr) + sys.exit(1) + +print("VTK __this__ and Addr=0x... encodings are intact.") diff --git a/Modules/Bridge/VtkGlue/wrapping/test/VtkGlueRoundTripTest.py b/Modules/Bridge/VtkGlue/wrapping/test/VtkGlueRoundTripTest.py new file mode 100644 index 00000000000..6c6eb808955 --- /dev/null +++ b/Modules/Bridge/VtkGlue/wrapping/test/VtkGlueRoundTripTest.py @@ -0,0 +1,111 @@ +# ========================================================================== +# +# Copyright NumFOCUS +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0.txt +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# ========================================================================== + +"""Round-trip itk.Image -> vtkImageData -> itk.Image through the VtkGlue filters. + +This exercises the SWIG typemaps in VtkGlue.i directly: ImageToVTKImageFilter +returns `vtkImageData *` (the `out` typemap) and VTKImageToImageFilter accepts +`vtkImageData *` (the `in` typemap). The pure-numpy helpers in itk.support.extras +bypass both, so they are deliberately not used here. +""" + +import sys + +import numpy as np +from vtkmodules.vtkCommonDataModel import vtkImageData + +import itk + +Dimension = 2 +PixelType = itk.F +ImageType = itk.Image[PixelType, Dimension] + +reference_array = np.arange(6 * 4, dtype=np.float32).reshape((4, 6)) +image = itk.image_from_array(reference_array) +image.SetSpacing([0.5, 2.0]) +image.SetOrigin([-3.0, 7.0]) + +to_vtk = itk.ImageToVTKImageFilter[ImageType].New() +to_vtk.SetInput(image) +to_vtk.Update() +vtk_image = to_vtk.GetOutput() + +if not isinstance(vtk_image, vtkImageData): + print( + f"out typemap returned {type(vtk_image)!r}, expected vtkImageData", + file=sys.stderr, + ) + sys.exit(1) + +if vtk_image.GetDimensions()[:2] != (6, 4): + print(f"unexpected VTK dimensions {vtk_image.GetDimensions()}", file=sys.stderr) + sys.exit(1) + +from_vtk = itk.VTKImageToImageFilter[ImageType].New() +from_vtk.SetInput(vtk_image) +from_vtk.Update() +result = from_vtk.GetOutput() + +failures = [] + +result_array = itk.array_from_image(result) +if not np.array_equal(result_array, reference_array): + failures.append( + f"pixel buffer differs:\n{result_array}\nexpected:\n{reference_array}" + ) + +if not np.allclose(list(result.GetSpacing()), [0.5, 2.0]): + failures.append(f"spacing {list(result.GetSpacing())}, expected [0.5, 2.0]") + +if not np.allclose(list(result.GetOrigin()), [-3.0, 7.0]): + failures.append(f"origin {list(result.GetOrigin())}, expected [-3.0, 7.0]") + +direction = itk.array_from_matrix(result.GetDirection()) +if not np.allclose(direction, np.identity(Dimension)): + failures.append(f"direction {direction}, expected identity") + + +# The `in` typemap must reject bad input rather than dereference it. The forged +# case matters most: without an isinstance() gate a crafted `__this__` would be +# cast to a native pointer and segfault instead of raising. +class ForgedVtkImageData: + __this__ = "_00000000deadbeef_p_vtkImageData" + + +for bad, description in ( + ("not a vtkImageData", "a str"), + (ForgedVtkImageData(), "an object with a forged __this__"), + (vtkImageData, "the class rather than an instance"), +): + try: + itk.VTKImageToImageFilter[ImageType].New().SetInput(bad) + except TypeError: + pass + except Exception as exception: # noqa: BLE001 - anything but TypeError is a defect + failures.append( + f"in typemap raised {exception!r} for {description}, expected TypeError" + ) + else: + failures.append(f"in typemap accepted {description}; expected TypeError") + +if failures: + for failure in failures: + print(f" - {failure}", file=sys.stderr) + sys.exit(1) + +print("itk.Image <-> vtkImageData round trip preserved geometry and pixels.")