BUG: Interpretation of the Euler angles ZYX or ZXY was not exported.

The flag indicating whether the Euler angles should be composed in ZYX
or ZXY order was not part of the fixed parameters as it should have
been. This patch adds it to the fixed parameters. This was an issue
with serialization de-serialization of the transform. The choice of
composition method was not serialized and writing followed by reading
changed the transform.

Change-Id: I0a2230022c35b8a7ef2c5c4d91e7aabfa50b9d7c

Modules/Core/Transform/include/itkEuler3DTransform.h

@@ -95,6 +95,9 @@ class Euler3DTransform :
 
   const ParametersType & GetParameters(void) const ITK_OVERRIDE;
 
+  const FixedParametersType & GetFixedParameters() const ITK_OVERRIDE;
+  virtual void SetFixedParameters(const FixedParametersType & parameters) ITK_OVERRIDE;
+
   /** Set the rotational part of the transform. */
   void SetRotation(ScalarType angleX, ScalarType angleY, ScalarType angleZ);
 
@@ -108,8 +111,13 @@ class Euler3DTransform :
    * transform is invertible at this point. */
   virtual void ComputeJacobianWithRespectToParameters( const InputPointType  & p, JacobianType & jacobian) const ITK_OVERRIDE;
 
-  /** Set/Get the order of the computation. Default ZXY */
-  itkSetMacro(ComputeZYX, bool);
+  /** The Euler angle representation of a rotation is not unique and
+   * depends on the order of rotations. In general there are 12
+   * options. This class supports two of them, ZXY and ZYX. The
+   * default is ZXY. These functions set and get the value which
+   * indicates whether the rotation is ZYX or ZXY.
+   */
+  virtual void SetComputeZYX (const bool flag);
   itkGetConstMacro(ComputeZYX, bool);
 
   virtual void SetIdentity(void) ITK_OVERRIDE;

Modules/Core/Transform/include/itkEuler3DTransform.hxx

@@ -30,6 +30,8 @@ Euler3DTransform<TParametersValueType>
 {
   m_ComputeZYX = false;
   m_AngleX = m_AngleY = m_AngleZ = NumericTraits<ScalarType>::ZeroValue();
+  this->m_FixedParameters.SetSize(SpaceDimension+1);
+  this->m_FixedParameters.Fill(0.0);
 }
 
 // Constructor with default arguments
@@ -45,6 +47,8 @@ Euler3DTransform<TParametersValueType>
   off[1] = offset[1];
   off[2] = offset[2];
   this->SetOffset(off);
+  this->m_FixedParameters.SetSize(SpaceDimension+1);
+  this->m_FixedParameters.Fill(0.0);
 }
 
 // Constructor with arguments
@@ -55,6 +59,8 @@ Euler3DTransform<TParametersValueType>
 {
   m_ComputeZYX = false;
   m_AngleX = m_AngleY = m_AngleZ = NumericTraits<ScalarType>::ZeroValue();
+  this->m_FixedParameters.SetSize(SpaceDimension+1);
+  this->m_FixedParameters.Fill(0.0);
 }
 
 // Set Angles
@@ -119,6 +125,40 @@ const typename Euler3DTransform<TParametersValueType>::ParametersType
   return this->m_Parameters;
   }
 
+template<typename TParametersValueType>
+const typename Euler3DTransform<TParametersValueType>::FixedParametersType &
+Euler3DTransform<TParametersValueType>
+::GetFixedParameters() const
+{
+  // Call the superclass GetFixedParameters so that it fills the
+  // array, we ignore the returned data and add the additional
+  // information to the updated array.
+  Superclass::GetFixedParameters();
+  this->m_FixedParameters[3] = this-> m_ComputeZYX;
+  return this->m_FixedParameters;
+}
+
+template<typename TParametersValueType>
+void
+Euler3DTransform<TParametersValueType>
+::SetFixedParameters(const FixedParametersType & parameters)
+{
+  InputPointType c;
+  for( unsigned int i = 0; i < InputSpaceDimension; i++ )
+    {
+    c[i] = this->m_FixedParameters[i] = parameters[i];
+    }
+  this->SetCenter(c);
+  //conditional is here for backwards compatibility: the
+  //m_ComputeZYX flag was not serialized so it may or may
+  //not be included as part of the fixed parameters
+  if( parameters.Size() == 4 )
+    {
+    this->m_FixedParameters[3] = parameters[3];
+    this->SetComputeZYX(this->m_FixedParameters[3]);
+    }
+}
+
 // Set Rotational Part
 template<typename TParametersValueType>
 void
@@ -299,6 +339,21 @@ Euler3DTransform<TParametersValueType>
     }
 }
 
+template<typename TParametersValueType>
+void
+Euler3DTransform<TParametersValueType>
+::SetComputeZYX (const bool flag)
+{
+  if( this->m_ComputeZYX != flag )
+    {
+    this->m_ComputeZYX = flag;
+    this->ComputeMatrix();
+    // The meaning of the parameters has changed so the transform
+    // has been modified even if the parameter values have not.
+    this->Modified();
+    }
+}
+
 // Print self
 template<typename TParametersValueType>
 void

Modules/Core/Transform/test/itkEuler3DTransformTest.cxx

@@ -111,6 +111,40 @@ int itkEuler3DTransformTest(int, char *[] )
     std::cout << " [ PASSED ] " << std::endl;
     }
 
+  std::cout << "Testing Rotation Change from ZXY to ZYX consistency:";
+
+  EulerTransformType::Pointer eulerTransform2 = EulerTransformType::New();
+  EulerTransformType::OutputPointType r1, r2;
+
+  //rotation angles already set above
+  eulerTransform->SetComputeZYX(true);
+
+  eulerTransform2->SetComputeZYX(true);
+  eulerTransform2->SetRotation(angleX, angleY, angleZ);
+
+  r1 = eulerTransform->TransformPoint( p );
+  r2 = eulerTransform2->TransformPoint( p );
+  for( unsigned int i = 0; i < N; i++ )
+    {
+    if( std::fabs( r1[i] - r2[i] ) > epsilon )
+      {
+      Ok = false;
+      break;
+      }
+    }
+  if( !Ok )
+    {
+    std::cout << "[ FAILED ]" << std::endl;
+    std::cerr << "Setting rotation parameters followed by change in "
+              << "Euler angle representation is not consistent with "
+              << "operations performed in reverse order." << std::endl;
+    return EXIT_FAILURE;
+    }
+  else
+    {
+    std::cout << " [ PASSED ] " << std::endl;
+    }
+
   std::cout << "Testing Translation:";
 
   eulerTransform->SetRotation(0, 0, 0);
@@ -168,6 +202,37 @@ int itkEuler3DTransformTest(int, char *[] )
     }
   std::cout << " [ PASSED ] " << std::endl;
 
+  //Testing fixed parameters
+  std::cout << "Testing Set/Get Fixed Parameters: ";
+  EulerTransformType::FixedParametersType oldVersion(3), newVersion(4), res(4);
+  oldVersion.Fill(0);
+  newVersion.Fill(0);
+  eulerTransform->SetFixedParameters( oldVersion );
+  eulerTransform->SetComputeZYX( true );
+  res = eulerTransform->GetFixedParameters();
+  if( res[0] != 0 ||
+      res[1] != 0 ||
+      res[2] != 0 ||
+      res[3] != 1 )
+    {
+    std::cout<<"Setting/Getting fixed parameters failed."<< std::endl;
+    std::cout << " [ FAILED ] " << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  eulerTransform->SetFixedParameters( newVersion );
+  res = eulerTransform->GetFixedParameters();
+  if( res[0] != 0 ||
+      res[1] != 0 ||
+      res[2] != 0 ||
+      res[3] != 0 )
+    {
+    std::cout<<"Setting/Getting fixed parameters failed."<< std::endl;
+    std::cout << " [ FAILED ] " << std::endl;
+    return EXIT_FAILURE;
+    }
+  std::cout << " [ PASSED ] " << std::endl;
+
   // Testing Jacobian
   std::cout << "Testing Jacobian: ";
   for( unsigned int i = 0; i < 3; i++ )

Modules/IO/TransformInsightLegacy/test/CMakeLists.txt

@@ -1,10 +1,15 @@
 itk_module_test()
 set(ITKIOTransformInsightLegacyTests
 itkIOTransformTxtTest.cxx
+itkIOEuler3DTransformTxtTest.cxx
 )
 
 CreateTestDriver(ITKIOTransformInsightLegacy "${ITKIOTransformInsightLegacy-Test_LIBRARIES}" "${ITKIOTransformInsightLegacyTests}")
 
 itk_add_test(NAME itkIOTransformTxtTest
       COMMAND ITKIOTransformInsightLegacyTestDriver itkIOTransformTxtTest
         ${ITK_TEST_OUTPUT_DIR} )
+
+itk_add_test(NAME itkIOTransformEuler3DTxtTest
+      COMMAND ITKIOTransformInsightLegacyTestDriver itkIOEuler3DTransformTxtTest
+        DATA{Input/euler3DOldFormat.tfm} ${ITK_TEST_OUTPUT_DIR}/euler3DNewFormat.tfm)

Modules/IO/TransformInsightLegacy/test/Input/euler3DOldFormat.tfm.md5

@@ -0,0 +1 @@
+29469c3fbf3328d149fee08294b32df1

Modules/IO/TransformInsightLegacy/test/itkIOEuler3DTransformTxtTest.cxx

@@ -0,0 +1,83 @@
+/*=========================================================================
+ *
+ *  Copyright Insight Software Consortium
+ *
+ *  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.
+ *
+ *=========================================================================*/
+
+#include "itkTransformFileWriter.h"
+#include "itkTransformFileReader.h"
+#include "itkTransformFactory.h"
+#include "itkTxtTransformIOFactory.h"
+#include "itkEuler3DTransform.h"
+#include "itkMath.h"
+
+int itkIOEuler3DTransformTxtTest(int argc, char *argv[])
+{
+  if( argc != 3 )
+    {
+    std::cerr<< "Usage: "
+             << argv[0]
+             <<" inputFileName outputFileName"
+             << std::endl;
+    return EXIT_FAILURE;
+    }
+
+  itk::ObjectFactoryBase::RegisterFactory(itk::TxtTransformIOFactory::New() );
+
+  typedef itk::Euler3DTransform<double> TransformType;
+  TransformType::Pointer oldStyleInput, newStyleInput;
+
+  typedef itk::TransformFileReaderTemplate<double> ReaderType;
+  ReaderType::Pointer reader = ReaderType::New();
+
+  typedef itk::TransformFileWriterTemplate<double> WriterType;
+  WriterType::Pointer writer = WriterType::New();
+
+  //read old style format in
+  reader->SetFileName( argv[1] );
+  reader->Update();
+  oldStyleInput = static_cast< TransformType*  >(( reader->GetTransformList()->begin() )->GetPointer());
+
+  //modify the interpretation of the Euler angles
+  oldStyleInput->SetComputeZYX( true );
+
+  //write in new style format
+  writer->SetFileName( argv[2] );
+  writer->SetInput( oldStyleInput );
+  writer->Update();
+
+  //read new style format back in
+  reader->SetFileName( argv[2] );
+  reader->Update();
+  newStyleInput = static_cast< TransformType*  >(( reader->GetTransformList()->begin() )->GetPointer());
+
+  const TransformType::MatrixType &oldStyleMat = oldStyleInput->GetMatrix();
+  const TransformType::MatrixType &newStyleMat = newStyleInput->GetMatrix();
+
+  if(!(itk::Math::FloatAlmostEqual(oldStyleMat(0,0), newStyleMat(0,0)) &&
+       itk::Math::FloatAlmostEqual(oldStyleMat(0,1), newStyleMat(0,1)) &&
+       itk::Math::FloatAlmostEqual(oldStyleMat(0,2), newStyleMat(0,2)) &&
+       itk::Math::FloatAlmostEqual(oldStyleMat(1,0), newStyleMat(1,0)) &&
+       itk::Math::FloatAlmostEqual(oldStyleMat(1,1), newStyleMat(1,1)) &&
+       itk::Math::FloatAlmostEqual(oldStyleMat(1,2), newStyleMat(1,2)) &&
+       itk::Math::FloatAlmostEqual(oldStyleMat(2,0), newStyleMat(2,0)) &&
+       itk::Math::FloatAlmostEqual(oldStyleMat(2,1), newStyleMat(2,1)) &&
+       itk::Math::FloatAlmostEqual(oldStyleMat(2,2), newStyleMat(2,2))))
+    {
+    std::cerr<< "Error reading new style format, different from data in memory." << std::endl;
+    return EXIT_FAILURE;
+    }
+  return EXIT_SUCCESS;
+}