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

PERF: Make ImageRegion trivially copyable, remove inheritance (ITK_FUTURE_LEGACY_REMOVE) #4344

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 33 additions & 7 deletions Modules/Core/Common/include/itkImageRegion.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@
#include "itkContinuousIndex.h"
#include "itkMath.h"

// Macro added to each `ImageRegion` member function that overrides a virtual member function of `Region`. In the
// future, `ImageRegion` will no longer inherit from `Region`, so then those `ImageRegion` member functions will no
// longer override.
#ifdef ITK_FUTURE_LEGACY_REMOVE
# define itkRegionOverrideMacro // nothing (in the future)
#else
# define itkRegionOverrideMacro override
#endif

namespace itk
{
// Forward declaration of ImageBase so it can be declared a friend
Expand Down Expand Up @@ -66,15 +75,26 @@ class ITK_TEMPLATE_EXPORT ImageBase;
* \endsphinx
*/
template <unsigned int VImageDimension>
class ITK_TEMPLATE_EXPORT ImageRegion final : public Region
class ITK_TEMPLATE_EXPORT ImageRegion final
#ifndef ITK_FUTURE_LEGACY_REMOVE
// This inheritance is to be removed in the future.
: public Region
#endif
{
public:
/** Standard class type aliases. */
using Self = ImageRegion;

#ifndef ITK_FUTURE_LEGACY_REMOVE
using Superclass = Region;
#endif

/** Standard part of all itk objects. */
itkTypeMacro(ImageRegion, Region);
const char *
GetNameOfClass() const itkRegionOverrideMacro
{
return "ImageRegion";
}

/** Dimension of the image available at compile time. */
static constexpr unsigned int ImageDimension = VImageDimension;
Expand Down Expand Up @@ -106,20 +126,24 @@ class ITK_TEMPLATE_EXPORT ImageRegion final : public Region
using SliceRegion = ImageRegion<Self::SliceDimension>;

/** Return the region type. Images are described with structured regions. */
Superclass::RegionEnum
GetRegionType() const override
Region::RegionEnum
GetRegionType() const itkRegionOverrideMacro
{
return Superclass::RegionEnum::ITK_STRUCTURED_REGION;
return Region::RegionEnum::ITK_STRUCTURED_REGION;
}

/** Print the region. */
void
Print(std::ostream & os, Indent indent = 0) const itkRegionOverrideMacro;

/** Constructor. ImageRegion is a lightweight object that is not reference
* counted, so the constructor is public. Its two data members are filled
* with zeros (using C++11 default member initializers). */
ImageRegion() noexcept = default;

/** Destructor. ImageRegion is a lightweight object that is not reference
* counted, so the destructor is public. */
~ImageRegion() override = default;
~ImageRegion() itkRegionOverrideMacro = default;

/** Copy constructor. ImageRegion is a lightweight object that is not
* reference counted, so the copy constructor is public. */
Expand Down Expand Up @@ -339,7 +363,7 @@ class ITK_TEMPLATE_EXPORT ImageRegion final : public Region
* instead) but used in the hierarchical print process to combine the
* output of several classes. */
void
PrintSelf(std::ostream & os, Indent indent) const override;
PrintSelf(std::ostream & os, Indent indent) const itkRegionOverrideMacro;

private:
IndexType m_Index = { { 0 } };
Expand All @@ -354,6 +378,8 @@ std::ostream &
operator<<(std::ostream & os, const ImageRegion<VImageDimension> & region);
} // end namespace itk

#undef itkRegionOverrideMacro

#ifndef ITK_MANUAL_INSTANTIATION
# include "itkImageRegion.hxx"
#endif
Expand Down
10 changes: 8 additions & 2 deletions Modules/Core/Common/include/itkImageRegion.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,16 @@ ImageRegion<VImageDimension>::GetNumberOfPixels() const -> SizeValueType

template <unsigned int VImageDimension>
void
ImageRegion<VImageDimension>::PrintSelf(std::ostream & os, Indent indent) const
ImageRegion<VImageDimension>::Print(std::ostream & os, Indent indent) const
{
Superclass::PrintSelf(os, indent);
os << indent << this->GetNameOfClass() << " (" << this << ")\n";
this->PrintSelf(os, indent.GetNextIndent());
}

template <unsigned int VImageDimension>
void
ImageRegion<VImageDimension>::PrintSelf(std::ostream & os, Indent indent) const
{
os << indent << "Dimension: " << this->GetImageDimension() << std::endl;
os << indent << "Index: " << m_Index << std::endl;
os << indent << "Size: " << m_Size << std::endl;
Expand Down
22 changes: 22 additions & 0 deletions Modules/Core/Common/test/itkImageRegionGTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,28 @@
#include <type_traits> // For remove_const_t and remove_reference_t.


namespace
{
template <unsigned int VDimension>
constexpr bool
CheckTrivialCopyabilityOfImageRegion()
{
constexpr bool isImageRegionTriviallyCopyable{ std::is_trivially_copyable_v<itk::ImageRegion<VDimension>> };

#ifdef ITK_FUTURE_LEGACY_REMOVE
static_assert(isImageRegionTriviallyCopyable, "In the future, ImageRegion<VDimension> should be trivially copyable.");
return isImageRegionTriviallyCopyable;
#else
static_assert(!isImageRegionTriviallyCopyable, "ImageRegion<VDimension> should *not* be trivially copyable.");
return !isImageRegionTriviallyCopyable;
#endif
}
} // namespace

static_assert(CheckTrivialCopyabilityOfImageRegion<2>() && CheckTrivialCopyabilityOfImageRegion<3>(),
"ImageRegion<VDimension> should be trivially copyable when legacy support is removed.");


// Tests that a zero-sized region is not considered to be inside of another region.
TEST(ImageRegion, ZeroSizedRegionIsNotInside)
{
Expand Down