Skip to content

Commit

Permalink
STYLE: Change aggregate classes to mirror std::array
Browse files Browse the repository at this point in the history
In reviewing the the 3 aggregate special classes itkIndex.h, itkSize.h,
and itkOffset.h, these classes are intended to have construction and
performance characteristics that are similar to the std::array class.
During review of the std::array implementation details as aggregate POD
type, some modifications were identified to ensure that these itk
classes are intended to meet the requirements of POD data types.

A primary benefit of being conformant with std::array is that
understanding these classes is equivalent to understanding the
well documented subtlties of the std::array class.

Each class has an additional set of overlaoded "math" operations
to allow specialized behaviors necessary for image Index/Size/Offset
relationships.

In addition to providing the array API that allows
the Index class to be used from methods in the <algorithms>,
the interal data storage is aligned following the pattern
of the std::array storage in order to allow for potential
optimizations.

** Since this is an aggregate base type, prevent
prevent deriving from this class by marking it
as ``final'' as is highly recommended for the std::array class.

** Ensure that these classes are POD conformant

** Added  itkIndexTest.cpp that test the class (Previously missing).  The
itkSizeTest.cxx and the current itkIndexTest.cxx exercises the code, but
does not actually fail if one of the computations is incorrect. Test are
written with GoogleTest

** The IndexLexicographicCompare and OffsetLexicographicCompare
were used in very few places only one place
to allow storing Index values in a std::map.
This behavior is moved to a separate file in
itkIndexLexicographicCompare.h

Replace forward declared embeded functional for compare typedefs with
independant common functional template in an independant header file.

IndexLexicographicCompare & SizeLexicographicCompare types from the
Functional:: namespace are now replaced by a single wrapper
of the std::lexicographical() functional in the itkLexicographic.h
file.

** Make seldom used direct access to public member function names
accross the 3 classes match.  This greatly facilitated identifying
unnecessary differences in the implementations of these 3 classes.

   m_Index -> m_InteralArray
   m_Size -> m_InternalArray
   m_Offset -> m_InternalArray

A review of Slicer, BRAINSTools, all ITK Remote Modules, and ANTs
indicated that these public member variables were never accessed in
those packages.

Provide comprehensive GTest for all aggregate classes (Index, Size, Offset)

Change-Id: I02fe8fa32e1fb4587b329bdf4471c72028167f77
  • Loading branch information
hjmjohnson committed Feb 16, 2018
1 parent 1fb0c22 commit 549a19c
Show file tree
Hide file tree
Showing 24 changed files with 1,521 additions and 690 deletions.
2 changes: 1 addition & 1 deletion Examples/Iterators/NeighborhoodIterators6.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ int main( int argc, char ** argv )
itk::RandomImageSource<ImageType>::Pointer noise
= itk::RandomImageSource<ImageType>::New();

noise->SetSize(size.m_Size);
noise->SetSize(size.m_InternalArray);
noise->SetMin(-.7);
noise->SetMax(.8);
adder->SetInput1(noise->GetOutput());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,11 @@ FloodFilledFunctionConditionalConstIterator< TImage, TFunction >
{
if ( i != k )
{
tempIndex.m_Index[k] = topIndex[k];
tempIndex.m_InternalArray[k] = topIndex[k];
}
else
{
tempIndex.m_Index[k] = topIndex[k] + j;
tempIndex.m_InternalArray[k] = topIndex[k] + j;
}
} // end build the index of a neighbor

Expand Down
8 changes: 4 additions & 4 deletions Modules/Core/Common/include/itkImageRegionSplitterBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ class ITKCommon_EXPORT ImageRegionSplitterBase
unsigned int requestedNumber) const
{
return this->GetNumberOfSplitsInternal( VImageDimension,
region.GetIndex().m_Index,
region.GetSize().m_Size,
region.GetIndex().m_InternalArray,
region.GetSize().m_InternalArray,
requestedNumber);
}
inline unsigned int GetNumberOfSplits(const ImageIORegion &region,
Expand Down Expand Up @@ -109,8 +109,8 @@ class ITKCommon_EXPORT ImageRegionSplitterBase
return this->GetSplitInternal( VImageDimension,
i,
numberOfPieces,
region.GetModifiableIndex().m_Index,
region.GetModifiableSize().m_Size );
region.GetModifiableIndex().m_InternalArray,
region.GetModifiableSize().m_InternalArray );
}
unsigned int GetSplit( unsigned int i,
unsigned int numberOfPieces,
Expand Down
Loading

0 comments on commit 549a19c

Please sign in to comment.