[Insight-developers] Problem with ITK 3.20 itkQuadEdgeMeshDiscretePrincipalCurvaturesEstimator.h

[Arnaud GELAS]

  Hi Kent,

When this filter was coded, it was indeed only tested and validated for 
float and double pixel type. I definitively agree with you for the 
corrections!!!

However, I foresee some problems when using integer based values for 
curvature, you may end up with null curvature quite often; and I am not 
sure that in practice you will be really happy to find null principal 
curvatures (i.e. a plane) for cortical surface.

Arnaud

On 09/14/2010 10:26 AM, kent williams wrote:
> This has implications for ITK4, if the review QuadEdgeMesh stuff is being
> moved out of review. One of our students got the following compiler error:
>
> InsightToolkit/Review/itkQuadEd
> geMeshDiscretePrincipalCurvaturesEstimator.h:141:
> error: ISO C++ says that these are ambiguous, even though the worst
> conversion for the first is better than the worst conversion for the second:
>
> InsightToolkit/Utilities/vxl/core/vnl/vnl_math.h:536: note: candidate 1:
> double vnl_math_max(double, double)
> InsightToolkit/Utilities/vxl/core/vnl/vnl_math.h:535: note: candidate 2:
> float vnl_math_max(float, float)
>
> The problem is this method of
> itk::QuadEdgeMeshDiscretePrincipalCurvatureEstimator:
>
>    virtual OutputCurvatureType ComputeDelta( )
>      {
>      return  vnl_math_max( 0., m_Mean * m_Mean - m_Gaussian );
>      }
>
> The type of m_Mean and m_Gaussian is the pixel type of the output mesh
> PixelType.  I presume that whomever wrote this code only tried it with one
> pixel type (float or double) and if the other type is used, it's too
> ambiguous for the compiler.
>
> 3.20 is closed, and I don't know the status of the review QuadEdgeMesh code.
> But it's pretty clear to me that this function could be re-written to be
> un-ambiguous, eg:
>
> virtual OutputCurvatureType ComputeDelta()
> {
> return vnl_math_max(static_cast<OutputCurvatureType>(0.0),
>                                   m_Mean * m_Mean - m_Gaussian);
> }
>
> And I'd feel happier with
>
> (m_Mean * m_Mean) - m_Gaussian
>
> As well, because while the compiler always remembers operator precedence, I
> have to think 'My Dear Aunt Sally' to understand what that expression means.
>
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.html
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers