Skip to content

Commit

Permalink
BUG: Similarity3DTransform::SetScale should recompute m_Offset
Browse files Browse the repository at this point in the history
Addressed issue #2629
"Similarity3DTransform::SetScale should recompute m_Offset", which said:

> When `Similarity3DTransform::SetScale` is called, it does not compute
> `m_Offset`, which appears to cause an incorrect result from
> `similarity3DTransform.TransformPoint` when `TransformPoint` is called
> directly after `SetScale`.

Included a unit test (GoogleTest) to ensure that the issue is fixed.
  • Loading branch information
N-Dekker authored and thewtex committed Aug 11, 2021
1 parent 68944d5 commit dd893fa
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Similarity3DTransform<TParametersValueType>::SetScale(ScaleType scale)
{
m_Scale = scale;
this->ComputeMatrix();
this->ComputeOffset();
}

// Directly set the matrix
Expand Down
1 change: 1 addition & 0 deletions Modules/Core/Transform/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -183,5 +183,6 @@ set(ITKTransformGTests
itkBSplineTransformGTest.cxx
itkEuler3DTransformGTest.cxx
itkMatrixOffsetTransformBaseGTest.cxx
itkSimilarityTransformGTest.cxx
)
CreateGoogleTestDriver(ITKTransform "${ITKTransform-Test_LIBRARIES}" "${ITKTransformGTests}")
66 changes: 66 additions & 0 deletions Modules/Core/Transform/test/itkSimilarityTransformGTest.cxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*=========================================================================
*
* Copyright NumFOCUS
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0.txt
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*=========================================================================*/

// First include the header file to be tested:
#include "itkSimilarity2DTransform.h"
#include "itkSimilarity3DTransform.h"

#include <gtest/gtest.h>


namespace
{

template <typename TTransform>
void
Test_SetCenterAndScale()
{
using PointType = typename TTransform::InputPointType;

const auto transform = TTransform::New();

for (double center{ -1.0 }; center <= 1.0; ++center)
{
transform->SetCenter(PointType(center));

for (double scale{ 0.5 }; scale <= 2.0; scale *= 2.0)
{
transform->SetScale(scale);

for (double input{ -1.0 }; input <= 1.0; ++input)
{
EXPECT_EQ(transform->TransformPoint(PointType(input)), PointType(center + (scale * (input - center))));
}
}
}
}

} // namespace


// Tests that `transform->TransformPoint` yields the expected result, when
// setting both the center and the scale (in that specific order), of the
// similarity transform. Note that in previous versions of ITK (including
// ITK 5.2.0), Similarity3DTransform did sometimes produce an incorrect result,
// as reported at https://github.com/InsightSoftwareConsortium/ITK/issues/2629
// "Similarity3DTransform::SetScale should recompute m_Offset"
TEST(SimilarityTransform, SetCenterAndScale)
{
Test_SetCenterAndScale<itk::Similarity2DTransform<>>();
Test_SetCenterAndScale<itk::Similarity3DTransform<>>();
}

0 comments on commit dd893fa

Please sign in to comment.