Skip to content

Commit

Permalink
WIP: Exploring the behavior of SetSpacing
Browse files Browse the repository at this point in the history
The SpatialObjectToImageFilter::SetSpacing has a specialized behavior when zeros are passed
for the spacing size.

It is unclear what the intended behavior is.

Expanded the testsuite to demonstrate that sending
all zeros as the spacing values to update results in no changes to the state of itkSpatialObjectToImageFilter, but does trigger a Modified status on the Object.
  • Loading branch information
hjmjohnson committed Dec 20, 2024
1 parent e917d9f commit 82446d3
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,8 @@ template <typename TInputSpatialObject, typename TOutputImage>
void
SpatialObjectToImageFilter<TInputSpatialObject, TOutputImage>::SetSpacing(const SpacingType & spacing)
{
unsigned int i;

for (i = 0; i < TOutputImage::ImageDimension; ++i)
unsigned int i = 0;
for (; i < TOutputImage::ImageDimension; ++i)
{
if (Math::NotExactlyEquals(static_cast<double>(spacing[i]), m_Spacing[i]))
{
Expand All @@ -97,7 +96,7 @@ SpatialObjectToImageFilter<TInputSpatialObject, TOutputImage>::SetSpacing(const
{
for (i = 0; i < TOutputImage::ImageDimension; ++i)
{
if (spacing[i] != 0)
if (spacing[i] != 0) // Is this correct?
{
m_Spacing[i] = spacing[i];
}
Expand All @@ -111,24 +110,20 @@ template <typename TInputSpatialObject, typename TOutputImage>
void
SpatialObjectToImageFilter<TInputSpatialObject, TOutputImage>::SetSpacing(const double * spacing)
{
unsigned int i;

for (i = 0; i < OutputImageDimension; ++i)
{
if (Math::NotExactlyEquals(spacing[i], m_Spacing[i]))
{
break;
}
}
if (i < OutputImageDimension)
bool modified = false;
for (unsigned int i = 0; i < OutputImageDimension; ++i)
{
for (i = 0; i < OutputImageDimension; ++i)
if (Math::NotExactlyEquals(static_cast<double>(spacing[i]), m_Spacing[i]))
{
if (spacing[i] != 0)
if (spacing[i] != 0) // Is this correct? Should m_Spacing retain old value if spacing is zero?
{
m_Spacing[i] = spacing[i];
}
modified = true;
}
}
if (modified)
{
this->Modified();
}
}
Expand All @@ -137,24 +132,20 @@ template <typename TInputSpatialObject, typename TOutputImage>
void
SpatialObjectToImageFilter<TInputSpatialObject, TOutputImage>::SetSpacing(const float * spacing)
{
unsigned int i;

for (i = 0; i < OutputImageDimension; ++i)
bool modified = false;
for (unsigned int i = 0; i < OutputImageDimension; ++i)
{
if (Math::NotExactlyEquals(static_cast<double>(spacing[i]), m_Spacing[i]))
{
break;
}
}
if (i < OutputImageDimension)
{
for (i = 0; i < OutputImageDimension; ++i)
{
if (spacing[i] != 0)
if (spacing[i] != 0) // Is this correct? Should m_Spacing retain old value if spacing is zero?
{
m_Spacing[i] = spacing[i];
}
modified = true;
}
}
if (modified)
{
this->Modified();
}
}
Expand Down Expand Up @@ -182,21 +173,8 @@ template <typename TInputSpatialObject, typename TOutputImage>
void
SpatialObjectToImageFilter<TInputSpatialObject, TOutputImage>::SetOrigin(const PointType & origin)
{
unsigned int i;

for (i = 0; i < OutputImageDimension; ++i)
if (ContainerCopyWithCheck(m_Origin, origin, OutputImageDimension))
{
if (Math::NotExactlyEquals(static_cast<double>(origin[i]), m_Origin[i]))
{
break;
}
}
if (i < OutputImageDimension)
{
for (i = 0; i < OutputImageDimension; ++i)
{
m_Origin[i] = origin[i];
}
this->Modified();
}
}
Expand All @@ -206,21 +184,8 @@ template <typename TInputSpatialObject, typename TOutputImage>
void
SpatialObjectToImageFilter<TInputSpatialObject, TOutputImage>::SetOrigin(const double * origin)
{
unsigned int i;

for (i = 0; i < OutputImageDimension; ++i)
if (ContainerCopyWithCheck(m_Origin, origin, OutputImageDimension))
{
if (Math::NotExactlyEquals(origin[i], m_Origin[i]))
{
break;
}
}
if (i < OutputImageDimension)
{
for (i = 0; i < OutputImageDimension; ++i)
{
m_Origin[i] = origin[i];
}
this->Modified();
}
}
Expand All @@ -229,21 +194,8 @@ template <typename TInputSpatialObject, typename TOutputImage>
void
SpatialObjectToImageFilter<TInputSpatialObject, TOutputImage>::SetOrigin(const float * origin)
{
unsigned int i;

for (i = 0; i < OutputImageDimension; ++i)
if (ContainerCopyWithCheck(m_Origin, origin, OutputImageDimension))
{
if (Math::NotExactlyEquals(static_cast<double>(origin[i]), m_Origin[i]))
{
break;
}
}
if (i < OutputImageDimension)
{
for (i = 0; i < OutputImageDimension; ++i)
{
m_Origin[i] = origin[i];
}
this->Modified();
}
}
Expand Down
105 changes: 78 additions & 27 deletions Modules/Core/SpatialObjects/test/itkSpatialObjectToImageFilterTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -83,31 +83,69 @@ itkSpatialObjectToImageFilterTest(int, char *[])
// Testing spacing
std::cout << "Testing Spacing: ";

float spacingFloat[2];
double spacingDouble[2];

for (unsigned int i = 0; i < 2; ++i)
constexpr float floatCheckValue = 1.5;
constexpr double doubleCheckValue = 1.25;
constexpr float vspacingFloat[2] = { floatCheckValue, floatCheckValue };
constexpr double vspacingDouble[2] = { doubleCheckValue, doubleCheckValue };
{
spacingFloat[i] = 1.0;
spacingDouble[i] = 1.0;
imageFilter->SetSpacing(vspacingFloat);
const double * spacing_result = imageFilter->GetSpacing();
for (unsigned int i = 0; i < 2; ++i)
{
if (spacing_result[i] != floatCheckValue)
{
std::cout << "[FAILURE]" << std::endl;
return EXIT_FAILURE;
}
}
}
{
imageFilter->SetSpacing(vspacingDouble);
const double * spacing_result = imageFilter->GetSpacing();
for (unsigned int i = 0; i < 2; ++i)
{
if (spacing_result[i] != doubleCheckValue)
{
std::cout << "[FAILURE]" << std::endl;
return EXIT_FAILURE;
}
}
}
imageFilter->SetSpacing(spacingFloat);
imageFilter->SetSpacing(spacingDouble);
const double * spacing_result = imageFilter->GetSpacing();

for (unsigned int i = 0; i < 2; ++i)
{
if (spacing_result[i] != 1.0)
// NOTE: Passing all zeros does not change the spacing, but the timestamp is modified.
constexpr double allzeros{};
imageFilter->Update();
auto preTimestamp = imageFilter->GetTimeStamp();
imageFilter->SetSpacing(allzeros);
auto postTimestamp = imageFilter->GetTimeStamp();
if (preTimestamp != postTimestamp)
{
std::cout << "[FAILURE]" << std::endl;
std::cout << "Time Stamp modified." << std::endl;
}
else
{
std::cout << "Time Stamp not modified with passing all zero values to SetSpacing." << std::endl;
return EXIT_FAILURE;
}

const double * spacing_result = imageFilter->GetSpacing();
for (unsigned int i = 0; i < 2; ++i)
{
if (spacing_result[i] != doubleCheckValue)
{
std::cout << "[FAILURE]" << std::endl;
return EXIT_FAILURE;
}
}
std::cout << "Spacing values not changed when all zeros requested in change" << std::endl;
}


const auto spacing_vector_result = imageFilter->GetSpacingVector();
for (unsigned int i = 0; i < 2; ++i)
{
if (spacing_vector_result[i] != 1.0)
if (spacing_vector_result[i] != doubleCheckValue)
{
std::cout << "[FAILURE]" << std::endl;
return EXIT_FAILURE;
Expand All @@ -119,32 +157,40 @@ itkSpatialObjectToImageFilterTest(int, char *[])
// Testing Origin
std::cout << "Testing Origin: ";

float originFloat[2];
double originDouble[2];
constexpr float voriginFloat[2] = { floatCheckValue, floatCheckValue };
constexpr double voriginDouble[2] = { doubleCheckValue, doubleCheckValue };


for (unsigned int i = 0; i < 2; ++i)
{
originFloat[i] = 0.0;
originDouble[i] = 0.0;
imageFilter->SetOrigin(voriginFloat);
const double * origin_result = imageFilter->GetOrigin();
for (unsigned int i = 0; i < 2; ++i)
{
if (origin_result[i] != floatCheckValue)
{
std::cout << "[FAILURE]" << std::endl;
return EXIT_FAILURE;
}
}
}
imageFilter->SetOrigin(originFloat);
imageFilter->SetOrigin(originDouble);
const double * origin_result = imageFilter->GetOrigin();

for (unsigned int i = 0; i < 2; ++i)
{
if (origin_result[i] != 0.0)
imageFilter->SetOrigin(voriginDouble);
const double * origin_result = imageFilter->GetOrigin();
for (unsigned int i = 0; i < 2; ++i)
{
std::cout << "[FAILURE]" << std::endl;
return EXIT_FAILURE;
if (origin_result[i] != doubleCheckValue)
{
std::cout << "[FAILURE]" << std::endl;
return EXIT_FAILURE;
}
}
}

const auto origin_point_result = imageFilter->GetOriginPoint();

for (unsigned int i = 0; i < 2; ++i)
{
if (origin_point_result[i] != 0.0)
if (origin_point_result[i] != doubleCheckValue)
{
std::cout << "[FAILURE]" << std::endl;
return EXIT_FAILURE;
Expand All @@ -154,6 +200,11 @@ itkSpatialObjectToImageFilterTest(int, char *[])

std::cout << "[PASSED]" << std::endl;

// Now test with common values
constexpr double spacingDouble[2] = { 1.0, 1.0 };
constexpr double originDouble[2] = { 0.0, 0.0 };
imageFilter->SetSpacing(spacingDouble);
imageFilter->SetOrigin(originDouble);
// Testing PrintSelf
std::cout << imageFilter << std::endl;

Expand Down

0 comments on commit 82446d3

Please sign in to comment.