From 82446d394e4870586b2ae60366095234397f7b10 Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Thu, 19 Dec 2024 20:45:08 -0600 Subject: [PATCH] WIP: Exploring the behavior of SetSpacing 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. --- .../include/itkSpatialObjectToImageFilter.hxx | 90 ++++----------- .../itkSpatialObjectToImageFilterTest.cxx | 105 +++++++++++++----- 2 files changed, 99 insertions(+), 96 deletions(-) diff --git a/Modules/Core/SpatialObjects/include/itkSpatialObjectToImageFilter.hxx b/Modules/Core/SpatialObjects/include/itkSpatialObjectToImageFilter.hxx index 41b1e948dd3..16377c83f04 100644 --- a/Modules/Core/SpatialObjects/include/itkSpatialObjectToImageFilter.hxx +++ b/Modules/Core/SpatialObjects/include/itkSpatialObjectToImageFilter.hxx @@ -84,9 +84,8 @@ template void SpatialObjectToImageFilter::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(spacing[i]), m_Spacing[i])) { @@ -97,7 +96,7 @@ SpatialObjectToImageFilter::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]; } @@ -111,24 +110,20 @@ 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) + bool modified = false; + for (unsigned int i = 0; i < OutputImageDimension; ++i) { - for (i = 0; i < OutputImageDimension; ++i) + if (Math::NotExactlyEquals(static_cast(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(); } } @@ -137,24 +132,20 @@ template void SpatialObjectToImageFilter::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(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(); } } @@ -182,21 +173,8 @@ template void SpatialObjectToImageFilter::SetOrigin(const PointType & origin) { - unsigned int i; - - for (i = 0; i < OutputImageDimension; ++i) + if (ContainerCopyWithCheck(m_Origin, origin, OutputImageDimension)) { - if (Math::NotExactlyEquals(static_cast(origin[i]), m_Origin[i])) - { - break; - } - } - if (i < OutputImageDimension) - { - for (i = 0; i < OutputImageDimension; ++i) - { - m_Origin[i] = origin[i]; - } this->Modified(); } } @@ -206,21 +184,8 @@ template void SpatialObjectToImageFilter::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(); } } @@ -229,21 +194,8 @@ template void SpatialObjectToImageFilter::SetOrigin(const float * origin) { - unsigned int i; - - for (i = 0; i < OutputImageDimension; ++i) + if (ContainerCopyWithCheck(m_Origin, origin, OutputImageDimension)) { - if (Math::NotExactlyEquals(static_cast(origin[i]), m_Origin[i])) - { - break; - } - } - if (i < 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 f7384ba27bb..6f314a2a0da 100644 --- a/Modules/Core/SpatialObjects/test/itkSpatialObjectToImageFilterTest.cxx +++ b/Modules/Core/SpatialObjects/test/itkSpatialObjectToImageFilterTest.cxx @@ -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; @@ -119,24 +157,32 @@ 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; + } } } @@ -144,7 +190,7 @@ itkSpatialObjectToImageFilterTest(int, char *[]) 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; @@ -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;