InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.37k stars 660 forks source link

WIP: Add static member, `itk::Image::CreateInitialized(const RegionType &)` #4599

Open N-Dekker opened 2 months ago

N-Dekker commented 2 months ago

itk::Image::CreateInitialized aims to simplify creating an image with an allocated buffer of zero-initialized pixels.

Replaced more than sixty cases of code like:

    auto image = ImageType::New();
    image->SetRegions(region);
    image->AllocateInitialized();

with the following single statement:

    auto image = ImageType::CreateInitialized(region);

in ITK test files ("itk*Test*.cxx").

blowekamp commented 2 months ago

Are there any similarly named methods to "Obj::CreateInitialized"? What about something like MakeImage? We already have similarly name methods for other classes?

Note: I'm not suggesting preference, but starting a conversion about what may be the best and most consistent interface.

N-Dekker commented 2 months ago

Are there any similarly named methods to "Obj::CreateInitialized"? What about something like MakeImage?

Thanks for asking @blowekamp I agree that it's good to discuss the name.

What do you think?

blowekamp commented 2 months ago

Good points. We also need to consider other image type such as VectorImage, and LabelMap and even module image types like RLE image. Perhaps itk::MakeImage<TImage> could trivially be made to work with all of them?

N-Dekker commented 2 months ago

Good points. We also need to consider other image type such as VectorImage, and LabelMap and even module image types like RLE image. Perhaps itk::MakeImage<TImage> could trivially be made to work with all of them?

VectorImage would need an extra parameter to specify the vector length. So I think it would need to have its own static member function VectorImage::CreateInitialized(region, vectorLength) (or its own free function, itk::MakeVectorImage<TVectorImage>(region, vectorLength)).

I'm not sure about supporting LabelMap with this pull request . LabelMap seems so completely different from a regular itk::Image. For example, LabelMap::Allocate(bool initialize = false) does not really allocate anything. It just does ClearLabels(). And it does not seem to differentiate initialize = false versus initialize = true. Is it necessary to do labelMap->SetRegions(region), before using a labelMap, like it is for itk::Image? I don't know 🤷

Just like LabelMap, RLEImage also ignores the initialize parameter of its Allocate(bool initialize = false) member function: https://github.com/KitwareMedical/ITKRLEImage/blob/85559c99ece93d695889dbc908c52efbc2bf50a9/include/itkRLEImage.hxx#L44

N-Dekker commented 2 months ago

@blowekamp I guess a static Image::CreateInitialized(const RegionType &) member function is automatically wrapped by SWIG, for each supported pixel type and image dimension. On the other hand I guess a free function template like itk::MakeImage<TImage> would need to be "SWIG-ed" separately for each supported image type. Right?

Just something to be taken into account. I'm not against a free function template like itk::MakeImage<TImage>, but I still prefer the Image::CreateInitialized(const RegionType &) member function that I'm currently proposing 😃