Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Python utility functions: ITK image <-> volume node #7094

Merged
merged 18 commits into from Aug 16, 2023

Conversation

@dzenanz dzenanz requested review from jcfr and lassoan July 14, 2023 18:27
@dzenanz dzenanz added this to the Slicer 5.3 milestone Jul 14, 2023
@lassoan
Copy link
Contributor

lassoan commented Jul 15, 2023

Thank you, this s very useful. Is it only for ITKPython images or SimpleITK as well? Can you make it work for multiple components (such as color images) as well?

@dzenanz
Copy link
Member Author

dzenanz commented Jul 15, 2023

It is only for ITK images, but SimpleITK has methods for converting from and to ITK images.

I didn't need multi-component images so far, so I forgot about them. I just started my 2 week vacation, so it will be a while before I can get to it.

Base/Python/slicer/util.py Outdated Show resolved Hide resolved
Base/Python/slicer/util.py Outdated Show resolved Hide resolved
Base/Python/slicer/util.py Outdated Show resolved Hide resolved
Base/Python/slicer/util.py Outdated Show resolved Hide resolved
Base/Python/slicer/util.py Outdated Show resolved Hide resolved
@dzenanz dzenanz force-pushed the itkVolumeUtil branch 5 times, most recently from d8049c3 to 2532363 Compare August 7, 2023 17:23
@dzenanz
Copy link
Member Author

dzenanz commented Aug 7, 2023

The limitation regarding multi-component images comes from https://github.com/InsightSoftwareConsortium/ITK/blob/v5.4rc01/Wrapping/Generators/Python/itk/support/extras.py#L740
I am looking into it.

dzenanz and others added 4 commits August 8, 2023 09:42
For consistency with the existing API:

* Remove the "Node" suffix from "VolumeNode"
  Consistent with arrayFromVolume, arrayFromModel, ...

* Append "Array" to "RASAffine".
  Consistent with vtkMatrixFromArray, vtkMatrixFromArray, ...)

Rename the following functions:
* volumeNodeFromITKImage -     -> addVolumeFromITKImage
* itkImageFromVolumeNode       -> itkImageFromVolume
* _getRASAffineFromITK         -> _getRASAffineArrayFromITKImage
- _getITKMetadataFromRASAffine -> _getITKMetadataFromRASAffineArray
* updateVolumeNodeFromITKImage -> updateVolumeFromITKImage

Also fix spacing and add additional empty lines to improve readability.
Base/Python/slicer/util.py Outdated Show resolved Hide resolved
Base/Python/slicer/util.py Outdated Show resolved Hide resolved
…rLayers

Update of the Slice viewer is expected to be done at the application level.
Rely on the fact ITK functions vtk_image_from_image and image_from_vtk_image
already account for VTL image direction introduced in VTK 9.

Adapted from https://discourse.slicer.org/t/installing-itk-python-package-in-slicer-5-1-and-above/25636
Base/Python/slicer/util.py Outdated Show resolved Hide resolved
@dzenanz
Copy link
Member Author

dzenanz commented Aug 10, 2023

This is quite a big and useful simplification!

jcfr and others added 2 commits August 10, 2023 10:56
@jcfr
Copy link
Member

jcfr commented Aug 10, 2023

This has been tested and is functionally complete.

Copy link
Member Author

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@jcfr
Copy link
Member

jcfr commented Aug 10, 2023

Remaining:

  • document how the buffer is shared between ITK and Volume node.
  • add ITK version check to warn if the ITK version installed does not support a type of MRML Volume node to ITK conversion.

Summary: Buffer is shared between MRML Volume Node and ITK image data using either slicer.util.itkImageFromVolume or slicer.util.updateVolumeFromITKImage


MRML Volume Node -> ITK Image: slicer.util.itkImageFromVolume

slicer.util.itkImageFromVolume relies on itk.image_from_vtk_image which internally

(1) uses vtk.util.numpy_support.vtk_to_numpy1:

point_data = vtk_image.GetPointData()
array = vtk_to_numpy(point_data.GetScalars())

which internally uses numpy.frombuffer 2

This function creates a view into the original object. This should be safe in general, but it may make sense to copy the result when the original object is mutable or untrusted.

(2) also uses itk.image_view_from_array

l_image = itk.image_view_from_array(array, is_vector)

Conclusion: This means that the buffer is shared between MRML Volume Node and ITK image data


ITK Image -> MRML Volume Node: slicer.util.updateVolumeFromITKImage

slicer.util.updateVolumeFromITKImage relies on itk.vtk_image_from_image which internally

(1) uses itk.array_view_from_image to get a numpy array view

from vtk.util.numpy_support import numpy_to_vtk

array = itk.array_view_from_image(l_image)

(2) and also uses vtk.util.numpy_support.numpy_to_vtk3 to

vtk_image = vtk.vtkImageData()
data_array = numpy_to_vtk(array.reshape(-1))

with the deep parameter of numpy_to_vtk set to 0 (this is the default),

Conclusion: This means that the buffer is shared between ITK image data
and MRML Volume Node

Relevant files

Footnotes

  1. https://github.com/Slicer/VTK/blob/slicer-v9.2.20230607-1ff325c54/Wrapping/Python/vtkmodules/util/numpy_support.py#L200-L248

  2. https://numpy.org/doc/stable/reference/generated/numpy.frombuffer.html

  3. https://github.com/Slicer/VTK/blob/slicer-v9.2.20230607-1ff325c54/Wrapping/Python/vtkmodules/util/numpy_support.py#L104-L127

@jcfr
Copy link
Member

jcfr commented Aug 10, 2023

Proposed commit message when squashing:

ENH: Add Python utility functions: ITK image <-> volume node (#7094)

Update `slicer.util` adding these functions:
* itkImageFromVolume
* addVolumeFromITKImage
* updateVolumeFromITKImage
* itkImageFromVolumeModified

Adapted from https://discourse.slicer.org/t/installing-itk-python-package-in-slicer-5-1-and-above/25636

Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>

@jcfr
Copy link
Member

jcfr commented Aug 10, 2023

@lassoan When you have a chance, could you review the docstings related to memory ownership.

Copy link
Member Author

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks JC.

Base/Python/slicer/util.py Outdated Show resolved Hide resolved
Base/Python/slicer/util.py Outdated Show resolved Hide resolved
Base/Python/slicer/util.py Outdated Show resolved Hide resolved
Base/Python/slicer/util.py Show resolved Hide resolved
Base/Python/slicer/util.py Show resolved Hide resolved
jcfr and others added 2 commits August 14, 2023 22:30
Co-authored-by: Andras Lasso <lasso@queensu.ca>
Co-authored-by: Andras Lasso <lasso@queensu.ca>
Comment on lines +2639 to +2641
vtkImageCopy = vtk.vtkImageData()
vtkImageCopy.DeepCopy(vtkImage)
volumeNode.SetAndObserveImageData(vtkImageCopy)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lassoan deepCopy is now the default

@dzenanz I tested the changes in the context of SlicerITKUltrasound and it didn't see any issues (with/without speechifying deepCopy to False)

@jcfr
Copy link
Member

jcfr commented Aug 16, 2023

I plan on integrating (by squashing) this afternoon 🚀

and I will be using the following message:

ENH: Add Python utility functions: ITK image <-> volume node (#7094)

Update `slicer.util` adding these functions:
* itkImageFromVolume
* addVolumeFromITKImage
* updateVolumeFromITKImage
* itkImageFromVolumeModified

Adapted from https://discourse.slicer.org/t/installing-itk-python-package-in-slicer-5-1-and-above/25636

Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
Co-authored-by: Andras Lasso <lasso@queensu.ca>

@dzenanz
Copy link
Member Author

dzenanz commented Aug 16, 2023

Thanks for taking care of this JC!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants