From e1a44fd108d313cf44a7a96343481f3867a143c4 Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Thu, 19 Dec 2024 22:00:40 -0600 Subject: [PATCH] BUG: Do not allow non-positive spacing values Non-positive spacing values are not allowed. Make the SpatialObjectToImage filter behave the same way as itkImageBase when attempting to set a non-positive spacing value. --- .../include/itkSpatialObjectToImageFilter.hxx | 108 ++++-------------- .../itkSpatialObjectToImageFilterTest.cxx | 51 +++++---- 2 files changed, 51 insertions(+), 108 deletions(-) diff --git a/Modules/Core/SpatialObjects/include/itkSpatialObjectToImageFilter.hxx b/Modules/Core/SpatialObjects/include/itkSpatialObjectToImageFilter.hxx index d56e2d83555..438383e6710 100644 --- a/Modules/Core/SpatialObjects/include/itkSpatialObjectToImageFilter.hxx +++ b/Modules/Core/SpatialObjects/include/itkSpatialObjectToImageFilter.hxx @@ -84,25 +84,18 @@ template void SpatialObjectToImageFilter::SetSpacing(const SpacingType & spacing) { - unsigned int i; - - for (i = 0; i < TOutputImage::ImageDimension; ++i) - { - if (Math::NotExactlyEquals(static_cast(spacing[i]), m_Spacing[i])) - { - break; - } - } - if (i < TOutputImage::ImageDimension) + if (ContainerCopyWithCheck(m_Spacing, spacing, TOutputImage::ImageDimension)) { - for (i = 0; i < TOutputImage::ImageDimension; ++i) + this->Modified(); + for (unsigned int i = 0; i < TOutputImage::ImageDimension; ++i) { - if (spacing[i] != 0) + if (spacing[i] <= 0) { - m_Spacing[i] = spacing[i]; + itkExceptionMacro("Zero-valued spacing and negative spacings are not supported and may result in undefined " + "behavior.\nRefusing to change spacing from " + << this->m_Spacing << " to " << spacing); } } - this->Modified(); } } @@ -111,25 +104,18 @@ template void SpatialObjectToImageFilter::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) + if (ContainerCopyWithCheck(m_Spacing, spacing, TOutputImage::ImageDimension)) { - for (i = 0; i < OutputImageDimension; ++i) + this->Modified(); + for (unsigned int i = 0; i < TOutputImage::ImageDimension; ++i) { - if (spacing[i] != 0) + if (spacing[i] <= 0) { - m_Spacing[i] = spacing[i]; + itkExceptionMacro("Zero-valued spacing and negative spacings are not supported and may result in undefined " + "behavior.\nRefusing to change spacing from " + << this->m_Spacing << " to " << spacing); } } - this->Modified(); } } @@ -137,25 +123,18 @@ template void SpatialObjectToImageFilter::SetSpacing(const float * spacing) { - unsigned int i; - - for (i = 0; i < OutputImageDimension; ++i) - { - if (Math::NotExactlyEquals(static_cast(spacing[i]), m_Spacing[i])) - { - break; - } - } - if (i < OutputImageDimension) + if (ContainerCopyWithCheck(m_Spacing, spacing, TOutputImage::ImageDimension)) { - for (i = 0; i < OutputImageDimension; ++i) + this->Modified(); + for (unsigned int i = 0; i < TOutputImage::ImageDimension; ++i) { - if (spacing[i] != 0) + if (spacing[i] <= 0) { - m_Spacing[i] = spacing[i]; + itkExceptionMacro("Zero-valued spacing and negative spacings are not supported and may result in undefined " + "behavior.\nRefusing to change spacing from " + << this->m_Spacing << " to " << spacing); } } - this->Modified(); } } @@ -182,21 +161,8 @@ template void SpatialObjectToImageFilter::SetOrigin(const PointType & origin) { - unsigned int i; - - for (i = 0; i < OutputImageDimension; ++i) - { - if (Math::NotExactlyEquals(static_cast(origin[i]), m_Origin[i])) - { - break; - } - } - if (i < OutputImageDimension) + if (ContainerCopyWithCheck(m_Origin, origin, OutputImageDimension)) { - for (i = 0; i < OutputImageDimension; ++i) - { - m_Origin[i] = origin[i]; - } this->Modified(); } } @@ -206,21 +172,8 @@ template void SpatialObjectToImageFilter::SetOrigin(const double * origin) { - unsigned int i; - - for (i = 0; i < OutputImageDimension; ++i) - { - if (Math::NotExactlyEquals(origin[i], m_Origin[i])) - { - break; - } - } - if (i < OutputImageDimension) + if (ContainerCopyWithCheck(m_Origin, origin, OutputImageDimension)) { - for (i = 0; i < OutputImageDimension; ++i) - { - m_Origin[i] = origin[i]; - } this->Modified(); } } @@ -229,21 +182,8 @@ template void SpatialObjectToImageFilter::SetOrigin(const float * origin) { - unsigned int i; - - for (i = 0; i < OutputImageDimension; ++i) - { - if (Math::NotExactlyEquals(static_cast(origin[i]), m_Origin[i])) - { - break; - } - } - if (i < OutputImageDimension) + if (ContainerCopyWithCheck(m_Origin, origin, OutputImageDimension)) { - for (i = 0; i < OutputImageDimension; ++i) - { - m_Origin[i] = origin[i]; - } this->Modified(); } } diff --git a/Modules/Core/SpatialObjects/test/itkSpatialObjectToImageFilterTest.cxx b/Modules/Core/SpatialObjects/test/itkSpatialObjectToImageFilterTest.cxx index ba9f7d33e72..2519e55a12a 100644 --- a/Modules/Core/SpatialObjects/test/itkSpatialObjectToImageFilterTest.cxx +++ b/Modules/Core/SpatialObjects/test/itkSpatialObjectToImageFilterTest.cxx @@ -94,7 +94,7 @@ itkSpatialObjectToImageFilterTest(int, char *[]) { if (spacing_result[i] != floatCheckValue) { - std::cout << "[FAILURE]" << std::endl; + std::cout << "[FAILURE] floatCheckValue" << std::endl; return EXIT_FAILURE; } } @@ -106,48 +106,51 @@ itkSpatialObjectToImageFilterTest(int, char *[]) { if (spacing_result[i] != doubleCheckValue) { - std::cout << "[FAILURE]" << std::endl; + std::cout << "[FAILURE] doubleCheckValue" << std::endl; return EXIT_FAILURE; } } } + const auto spacing_vector_result = imageFilter->GetSpacingVector(); + for (unsigned int i = 0; i < 2; ++i) + { + if (spacing_vector_result[i] != doubleCheckValue) + { + std::cout << "[FAILURE] spacing_vector_result" << std::endl; + return EXIT_FAILURE; + } + } + { // NOTE: Passing all zeros does not change the spacing, but the timestamp is modified. constexpr double allzeros[2]{ 0.0, 0.0 }; imageFilter->Update(); auto preTimestamp = imageFilter->GetTimeStamp(); - imageFilter->SetSpacing(allzeros); - auto postTimestamp = imageFilter->GetTimeStamp(); - if (preTimestamp != postTimestamp) + bool exceptionThrown = false; + try { - std::cout << "Time Stamp modified." << std::endl; + imageFilter->SetSpacing(allzeros); } - else + catch (const itk::ExceptionObject &) { - std::cout << "Time Stamp not modified with passing all zero values to SetSpacing." << std::endl; - return EXIT_FAILURE; + exceptionThrown = true; } - const double * spacing_result = imageFilter->GetSpacing(); - for (unsigned int i = 0; i < 2; ++i) + if (!exceptionThrown) { - if (spacing_result[i] != doubleCheckValue) - { - std::cout << "[FAILURE]" << std::endl; - return EXIT_FAILURE; - } + std::cout << "[FAILURE] : Attempting to set spacing values to zero should have thrown an exception." << 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] != doubleCheckValue) + 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; } }