Difference between revisions of "List of patches in Code folder with comments and recommandation"
Julien.Malik (Talk | contribs) (→itkConnectedComponentFunctorImageFilter.h) |
Julien.Malik (Talk | contribs) (→itkResampleImageFilter.h) |
||
Line 249: | Line 249: | ||
Comment: No reason to set Interpolator precision to double arbitrarily. | Comment: No reason to set Interpolator precision to double arbitrarily. | ||
− | Recommandation: Ask Emmanuel why he did this and fork or discard. | + | Recommandation: Ask Emmanuel why he did this and fork or discard (support for complex ?). |
==== itkResampleImageFilter.txx ==== | ==== itkResampleImageFilter.txx ==== |
Revision as of 13:24, 18 March 2013
Contents
- 1 itkMIRegistrationFunction.txx
- 2 itkBinaryDilateImageFilter.txx
- 3 itkBinaryErodeImageFilter.txx
- 4 itkBSplineDecompositionImageFilter.h
- 5 itkConnectedComponentFunctorImageFilter.h
- 6 itkMaskImageFilter.h
- 7 itkResampleImageFilter.h
- 8 itkResampleImageFilter.txx
- 9 itkTernaryFunctorImageFilter.txx
- 10 itkUnaryFunctorImageFilter.txx
- 11 itkWarpImageFilter.h
- 12 itkWarpImageFilter.txx
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.
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.
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