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

[kent williams]

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.