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

[Luis Ibanez]

Hi Alex,


Yes, we maintain ITK 3.20.

That's still the official release of ITK.

Please create a bug in  MANTIS describing the issue.

Assign it to me,
and if possible indicate what code patch
from ITKv4 must be applied to ITK 3.20.


     Thanks


           Luis


-----------------------------------------------------------------
On Tue, Sep 14, 2010 at 11:01 AM, Alexandre GOUAILLARD
<agouaillard at gmail.com> wrote:
> hi kent.
>
> thanks for your feedback.
>
> QEMesh is still alive.
>
> The API is supposed to be stable, even though I plan to modify
> slightly the internals to improve speed (copy) and to help addressing
> non-manifoldness.
>
> I am in the process of moving it out of review, but it impacts a lot
> of things. I plan to have a branch ready and approved by a few senior
> developpers by november when we have our next meeting.
>
> I will proceed and correct the bug your student found right away in
> v4. Luis, can you advice how to fix it (if we should) for 3.20. Do we
> plan a maintenance release for 3.20. What is the backporting policy?
>
> thanks in advance.
>
> alex.
>
>
> On Tue, Sep 14, 2010 at 3:26 PM, kent williams
> <norman-k-williams at uiowa.edu> 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
>>
> _______________________________________________
> 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
>