Description
There is a bug in RLEImage<TPixel, VImageDimension, CounterType>::CleanUp():
|
void RLEImage<TPixel, VImageDimension, CounterType>::CleanUp() const |
The member myBuffer is defined as:
typedef typename itk::Image<RLLine, VImageDimension - 1> BufferType;
BufferType myBuffer;
However, the implementation of CleanUp() treats myBuffer as if it were a 2D std::vector:
assert(!myBuffer.empty());
if (this->GetLargestPossibleRegion().GetSize(0) == 0)
return;
#pragma omp parallel for
for (CounterType z = 0; z < myBuffer.size(); z++)
for (CounterType y = 0; y < myBuffer[0].size(); y++)
CleanUpLine(myBuffer[z][y]);
itk::Image does not provide:
empty()
size()
operator[]
This code cannot compile and appears inconsistent with BufferType.
It looks like an earlier version used std::vector<std::vector<RLLine>>, but later it was replaced with itk::Image<RLLine,...> while leaving the old loops in place.
Could you confirm the intended structure of myBuffer and whether this function needs refactoring?
Proposed fix
Below is a corrected version of CleanUp() using proper ITK image access and OpenMP-compatible iteration:
template< typename TPixel, unsigned int VImageDimension, typename CounterType >
void RLEImage<TPixel, VImageDimension, CounterType>::CleanUp() const
{
if (!myBuffer)
return;
if (this->GetLargestPossibleRegion().GetSize(0) == 0)
return;
typedef typename BufferType::RegionType BufferRegionType;
BufferRegionType region = myBuffer->GetBufferedRegion();
itk::ImageRegionIterator<BufferType> it(myBuffer, region);
for (it.GoToBegin(); !it.IsAtEnd(); ++it)
{
RLLine& line = it.Value();
CleanUpLine(line);
}
}
Notes
-
This fix preserves the existing const qualifier on CleanUp() thanks to mutable myBuffer.
-
The patch assumes that CleanUpLine processes each RLLine independently.
-
The logic now correctly supports arbitrary (VImageDimension - 1) dimensions.