List of patches in Code folder with comments and recommandation

From OTBWiki
Jump to: navigation, search

itkMIRegistrationFunction.txx

< OTB/Utilities/ITK/Code/Algorithms/itkMIRegistrationFunction.txx
> ITK-3.20.0/Code/Algorithms/itkMIRegistrationFunction.txx 


4c4
< Module:    $RCSfile: itkMIRegistrationFunction.txx,v $
---
> Module:    itkMIRegistrationFunction.txx
6,7c6,7
< Date:      $Date: 2009-01-26 21:45:50 $
< Version:   $Revision: 1.22 $
---
> Date:      $Date$
> Version:   $Revision$
287,288c287
< //     unsigned int numberOfSamples=20;
<     unsigned int numberOfSamples=10;
---
>     unsigned int numberOfSamples=20;
294d292
<     randasamit.ReinitializeSeed(0);

Comment: No reason why nbsamples is set to 20.

Related changesets are :

changeset: 5704:41494a924e26 parent: 5700:e4f79b2780ab user: Guillaume Borrut <guillaume.borrut@c-s.fr> date: Thu Jun 11 18:46:36 2009 +0200 summary: BUG: no more segfault otbMIRegistrationFilter

Recommandation: discard

itkBinaryDilateImageFilter.txx

< OTB/Utilities/ITK/Code/BasicFilters/itkBinaryDilateImageFilter.txx
> ITK-3.20.0/Code/BasicFilters/itkBinaryDilateImageFilter.txx 

4c4
<   Module:    $RCSfile: itkBinaryDilateImageFilter.txx,v $
---
>   Module:    itkBinaryDilateImageFilter.txx
6,7c6,7
<   Date:      $Date: 2009-04-17 18:58:09 $
<   Version:   $Revision: 1.22 $
---
>   Date:      $Date$
>   Version:   $Revision$
136,139c136,139
<   const unsigned char backgroundTag  = 0;
<   const unsigned char onTag          = 1;
<   const unsigned char borderTag      = 2;
<   const unsigned char innerTag       = 3;
---
>   static const unsigned char backgroundTag  = 0;
>   static const unsigned char onTag          = 1;
>   static const unsigned char borderTag      = 2;
>   static const unsigned char innerTag       = 3;

Comment: Needed to avoid LD error in applications

Recommandation: Submit to ITK and fork class while in review

itkBinaryErodeImageFilter.txx

< OTB/Utilities/ITK/Code/BasicFilters/itkBinaryErodeImageFilter.txx
> ITK-3.20.0/Code/BasicFilters/itkBinaryErodeImageFilter.txx 

4c4
<   Module:    $RCSfile: itkBinaryErodeImageFilter.txx,v $
---
>   Module:    itkBinaryErodeImageFilter.txx
6,7c6,7
<   Date:      $Date: 2008-08-13 12:37:50 $
<   Version:   $Revision: 1.26 $
---
>   Date:      $Date$
>   Version:   $Revision$
130,133c130,133
<   const unsigned char backgroundTag  = 0;
<   const unsigned char onTag          = 1;
<   const unsigned char borderTag      = 2;
<   const unsigned char innerTag       = 3;
---
>   static const unsigned char backgroundTag  = 0;
>   static const unsigned char onTag          = 1;
>   static const unsigned char borderTag      = 2;
>   static const unsigned char innerTag       = 3;

Comment: Needed to avoid LD error in applications

Recommandation: Submit to ITK and fork class while in review

itkBSplineDecompositionImageFilter.h

< OTB/Utilities/ITK/Code/BasicFilters/itkBSplineDecompositionImageFilter.h
> ITK-3.20.0/Code/BasicFilters/itkBSplineDecompositionImageFilter.h 

4c4
<   Module:    $RCSfile: itkBSplineDecompositionImageFilter.h,v $
---
>   Module:    itkBSplineDecompositionImageFilter.h
6,7c6,7
<   Date:      $Date: 2010-03-19 07:06:01 $
<   Version:   $Revision: 1.12 $
---
>   Date:      $Date$
>   Version:   $Revision$
107,111c107,111
< //  itkConceptMacro(InputConvertibleToOutputCheck,
< //     (Concept::Convertible<typename TInputImage::PixelType,
< //                           typename TOutputImage::PixelType>));
< //  itkConceptMacro(DoubleConvertibleToOutputCheck,
< //    (Concept::Convertible<double, typename TOutputImage::PixelType>));
---
>   itkConceptMacro(InputConvertibleToOutputCheck,
>      (Concept::Convertible<typename TInputImage::PixelType,
>                            typename TOutputImage::PixelType>));
>   itkConceptMacro(DoubleConvertibleToOutputCheck,
>     (Concept::Convertible<double, typename TOutputImage::PixelType>));

Comment: No reason why concept checking should be commented. VectorImage support ?

Recommandation:

  • if no test exits in OTB with VectorImage, write one
  • does it compile ?
  • if no, discard this patch
  • if yes, does it compiles with ITKv4 without the patch ?
    • if yes, discard the patch
    • if no, fork + report a feature request in ITK

itkConnectedComponentFunctorImageFilter.h

< OTB/Utilities/ITK/Code/BasicFilters/itkConnectedComponentFunctorImageFilter.h
> ITK-3.20.0/Code/BasicFilters/itkConnectedComponentFunctorImageFilter.h 

4c4
<   Module:    $RCSfile: itkConnectedComponentFunctorImageFilter.h,v $
---
>   Module:    itkConnectedComponentFunctorImageFilter.h
6,7c6,7
<   Date:      $Date: 2008-10-13 18:54:27 $
<   Version:   $Revision: 1.5 $
---
>   Date:      $Date$
>   Version:   $Revision$
55c55
<     public ImageToImageFilter< TInputImage, TOutputImage > 
---
>     public ConnectedComponentImageFilter< TInputImage, TOutputImage, TMaskImage > 
62c62
<   typedef ImageToImageFilter< TInputImage, TOutputImage > Superclass;
---
>   typedef ConnectedComponentImageFilter< TInputImage, TOutputImage, TMaskImage > Superclass;
153,173d152
<   
<   
<   /**
<    * Set/Get whether the connected components are defined strictly by
<    * face connectivity or by face+edge+vertex connectivity.  Default is
<    * FullyConnectedOff.  For objects that are 1 pixel wide, use
<    * FullyConnectedOn.
<    */
<   itkSetMacro(FullyConnected, bool);
<   itkGetConstReferenceMacro(FullyConnected, bool);
<   itkBooleanMacro(FullyConnected);
< 
<   void SetMaskImage(TMaskImage* mask)
<   {
<     this->SetNthInput(1, const_cast<TMaskImage *>( mask ));
<   }
< 
<   const TMaskImage* GetMaskImage() const
<   {
<     return (static_cast<const TMaskImage*>(this->ProcessObject::GetInput(1)));
<   }
176,179c155
<   ConnectedComponentFunctorImageFilter() 
<     {
<     m_FullyConnected = false;
<     }
---
>   ConnectedComponentFunctorImageFilter() {}
189,191d164
< 
<   bool m_FullyConnected;
< 

Comment: Wrong inheritance. Other patches seem to define new functions

Recommandation:

  • Fork
  • Study if this can be proposed upstream
  • What is this "FullyConnected" parameter which does not even seem to be used ???

itkMaskImageFilter.h

< OTB/Utilities/ITK/Code/BasicFilters/itkMaskImageFilter.h
> ITK-3.20.0/Code/BasicFilters/itkMaskImageFilter.h 

4c4
<   Module:    $RCSfile: itkMaskImageFilter.h,v $
---
>   Module:    itkMaskImageFilter.h
6,7c6,7
<   Date:      $Date: 2009-04-01 14:36:27 $
<   Version:   $Revision: 1.18 $
---
>   Date:      $Date$
>   Version:   $Revision$
61c61
<   MaskInput() {};
---
>   MaskInput(): m_OutsideValue(NumericTraits< TOutput >::Zero) {};

Comment: The itk version seems better, but we need to check that ITK4.0 use the vector zero() macro

Recommandation: Discard. ITK 4.x does this properly for both images and vector images.

itkResampleImageFilter.h

< OTB/Utilities/ITK/Code/BasicFilters/itkResampleImageFilter.h
> ITK-3.20.0/Code/BasicFilters/itkResampleImageFilter.h 

4c4
<   Module:    $RCSfile: itkResampleImageFilter.h,v $
---
>   Module:    itkResampleImageFilter.h
6,7c6,7
<   Date:      $Date: 2009-04-25 12:28:07 $
<   Version:   $Revision: 1.62 $
---
>   Date:      $Date$
>   Version:   $Revision$
110,111c110
<   typedef double CoordRepType;
<   typedef Transform<CoordRepType,
---
>   typedef Transform<TInterpolatorPrecisionType, 
117c116
<   typedef InterpolateImageFunction<InputImageType,CoordRepType > InterpolatorType;
---
>   typedef InterpolateImageFunction<InputImageType, TInterpolatorPrecisionType> InterpolatorType;

Comment: No reason to set Interpolator precision to double arbitrarily.

Recommandation: Ask Emmanuel why he did this and fork or discard (support for complex ?).

itkResampleImageFilter.txx

< OTB/Utilities/ITK/Code/BasicFilters/itkResampleImageFilter.txx
> ITK-3.20.0/Code/BasicFilters/itkResampleImageFilter.txx 

4c4
<   Module:    $RCSfile: itkResampleImageFilter.txx,v $
---
>   Module:    itkResampleImageFilter.txx
6,7c6,7
<   Date:      $Date: 2009-03-25 21:10:39 $
<   Version:   $Revision: 1.66 $
---
>   Date:      $Date$
>   Version:   $Revision$
58,60c58,59
<   typedef double CoordRepType;
<   m_Transform = IdentityTransform<CoordRepType, ImageDimension>::New();
<   m_Interpolator = LinearInterpolateImageFunction<InputImageType, CoordRepType>::New();
---
>   m_Transform = IdentityTransform<TInterpolatorPrecisionType, ImageDimension>::New();
>   m_Interpolator = LinearInterpolateImageFunction<InputImageType, TInterpolatorPrecisionType>::New();
227,228c226
<   typedef double CoordRepType;
<   typedef ContinuousIndex<CoordRepType, ImageDimension> ContinuousIndexType;
---
>   typedef ContinuousIndex<TInterpolatorPrecisionType, ImageDimension> ContinuousIndexType;
286,288c284,295
<       pixval = static_cast<PixelType>(
<                NumericTraits<OutputType>::Clamp(value,minOutputValue,maxOutputValue)
<                                      );
---
>       if( value < minOutputValue )
>         {
>         pixval = minValue;
>         }
>       else if( value > maxOutputValue )
>         {
>         pixval = maxValue;
>         }
>       else 
>         {
>         pixval = static_cast<PixelType>( value );
>         }
329,330c336
<   typedef double CoordRepType;
<   typedef ContinuousIndex<CoordRepType, ImageDimension> ContinuousIndexType;
---
>   typedef ContinuousIndex<TInterpolatorPrecisionType, ImageDimension> ContinuousIndexType;
457,460c463,474
< 
<         pixval = static_cast<PixelType>(
<                  NumericTraits<OutputType>::Clamp(value,minOutputValue,maxOutputValue)
<                                         );
---
>         if( value <  minOutputValue )
>           {
>           pixval = minValue;
>           }
>         else if( value > maxOutputValue )
>           {
>           pixval = maxValue;
>           }
>         else 
>           {
>           pixval = static_cast<PixelType>( value );
>           }

Comment: No reason to set Interpolator precision to double arbitrarily.

Recommandation: Ask Emmanuel why he did this and fork or discard (support for complex ?).

itkTernaryFunctorImageFilter.txx

< OTB/Utilities/ITK/Code/BasicFilters/itkTernaryFunctorImageFilter.txx
> ITK-3.20.0/Code/BasicFilters/itkTernaryFunctorImageFilter.txx 

4c4
<   Module:    $RCSfile: itkTernaryFunctorImageFilter.txx,v $
---
>   Module:    itkTernaryFunctorImageFilter.txx
6,7c6,7
<   Date:      $Date: 2008-10-18 16:11:14 $
<   Version:   $Revision: 1.39 $
---
>   Date:      $Date$
>   Version:   $Revision$
49c49
<   this->SetNthInput( 0, const_cast<TInputImage1 *>( image1 ) );
---
>   SetNthInput( 0, const_cast<TInputImage1 *>( image1 ) );
62c62
<   this->SetNthInput( 1, const_cast<TInputImage2 *>( image2 ) );
---
>   SetNthInput( 1, const_cast<TInputImage2 *>( image2 ) );
75c75
<   this->SetNthInput( 2, const_cast<TInputImage3 *>( image3 ) );
---
>   SetNthInput( 2, const_cast<TInputImage3 *>( image3 ) );

Comment: this-> added for wrapping. Will change in ITK 4.0

Recommandation: Discard

itkUnaryFunctorImageFilter.txx

< OTB/Utilities/ITK/Code/BasicFilters/itkUnaryFunctorImageFilter.txx
> ITK-3.20.0/Code/BasicFilters/itkUnaryFunctorImageFilter.txx 

4c4
<   Module:    $RCSfile: itkUnaryFunctorImageFilter.txx,v $
---
>   Module:    itkUnaryFunctorImageFilter.txx
6,7c6,7
<   Date:      $Date: 2009-10-28 03:37:14 $
<   Version:   $Revision: 1.34 $
---
>   Date:      $Date$
>   Version:   $Revision$
64,67d63
< 
<   // Copy information from input to output. If input and output are of
<   // different dimensions, Region information will not be copied.
<   outputPtr->CopyInformation(inputPtr);

Comment: Copy information mandatory for most functor filter to preserve medata

Recommandation: Can not be discarded. Fork ?

itkWarpImageFilter.h

< OTB/Utilities/ITK/Code/BasicFilters/itkWarpImageFilter.h
> ITK-3.20.0/Code/BasicFilters/itkWarpImageFilter.h 

4c4
<   Module:    $RCSfile: itkWarpImageFilter.h,v $
---
>   Module:    itkWarpImageFilter.h
6,7c6,7
<   Date:      $Date: 2009-10-29 11:19:00 $
<   Version:   $Revision: 1.31 $
---
>   Date:      $Date$
>   Version:   $Revision$
226,227c226,227
<   /** itkConceptMacro(InputHasNumericTraitsCheck,
<     (Concept::HasNumericTraits<typename TInputImage::PixelType>));*/
---
>   itkConceptMacro(InputHasNumericTraitsCheck,
>     (Concept::HasNumericTraits<typename TInputImage::PixelType>));
235c235
<   virtual ~WarpImageFilter() {};
---
>   ~WarpImageFilter() {};

Comment: Concept checking should not be commented. Destructor should be virtual

Recommandation: Check the forked class and discard

itkWarpImageFilter.txx

< OTB/Utilities/ITK/Code/BasicFilters/itkWarpImageFilter.txx
> ITK-3.20.0/Code/BasicFilters/itkWarpImageFilter.txx 

4c4
<   Module:    $RCSfile: itkWarpImageFilter.txx,v $
---
>   Module:    itkWarpImageFilter.txx
6,7c6,7
<   Date:      $Date: 2009-10-29 11:19:10 $
<   Version:   $Revision: 1.34 $
---
>   Date:      $Date$
>   Version:   $Revision$
27,30d26
< 
< #include "itkVariableLengthVector.h"
< #include "itkPixelBuilder.h"
< 
49c45
<   PixelBuilder<PixelType>::Zero(m_EdgePaddingValue,1);
---
>   m_EdgePaddingValue = NumericTraits<PixelType>::Zero;
286c282
<       for(unsigned int k = 0; k < PixelSizeFinder(input); k++ )
---
>       for(unsigned int k = 0; k < DisplacementType::Dimension; k++ )
301,309d296
< }
< 
< template <class PixelType> unsigned int PixelSizeFinder(itk::VariableLengthVector<PixelType> pix)
< {
<   return pix.Size();
< }
< template <class PixelType> unsigned int PixelSizeFinder(PixelType pix)
< {
<   return PixelType::Dimension;

Comment: This is adressed by forked class

Recommandation: Discard