[Insight-developers] Code / Common / itkScaleSkewVersor3DTransform.txx

[Simon Warfield]

I don't think the changes to the itkScaleSkewVersor3DTransform.txx are 
correct.
Around line 228, m_Skew[3] has been initialized to 0.0, and it seems 
intended for the user to manipulate only
m_Skew[0], m_Skew[1] and m_Skew[2].  m_Skew[2] never appears in any 
calculation.
It appears to me that m_Skew[2] was intended where m_Skew[3] is written.

Furthermore, if this correction was made, I think the use of the three 
parameters to provide the skew is incorrect.
I don't think the skew as computed is what was intended.


I also think it is wrong to utilize only 3 skew parameters since this 
restricts the nature of transforms that can be constructed.
I think it should be possible to construct an arbitrary affine transform 
using this class.


Changing the number of parameters in this transform breaks existing 
code.  It is not backwards compatible.
It appears that there is no test case or cases exercising the correct 
operation of this transformation.


I think the class should be called
itkShearScaleVersor3DTransform
  and that six Shear parameters should be provided.


-- 
Simon K. Warfield, Ph.D.   simon.warfield at childrens.harvard.edu
Associate Professor of Radiology,     Harvard Medical School
Director, Computational Radiology Laboratory,
Wolbach 215, Department of Radiology, 
Children's Hospital Boston
300 Longwood Avenue Boston MA 02115
Phone: 617-355-4566                           FAX: 617-730-0635