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

[Bill Lorensen]

We have a policy for fixing bugs in releases. It should probably be updated:
http://www.vtk.org/Wiki/ITK_Rules_for_CVS_Contributors#Release_Branches

Bill

On Wed, Sep 15, 2010 at 6:49 PM, Alexandre GOUAILLARD
<agouaillard at gmail.com> wrote:
> i luis,
>
> arnaud went ahead and fixed the bug directly in v4, so I ll let him
> provide the patch to you.
>
> in the meantime, I just wanted to clarify the process (those questions
> should be answered by the ISC board I guess):
>
> - are we going to have any other release of itk 3.x ?
> - What items should be backported (I suppose bug fixes are, not enhancement)
> - what is the process for backporting items to itk 3.x
>   - untill itk v4 is out?
>   - later on ?
> - how long do we plan to support 3.x for backports ?
>
> I guess we will have to face the sacro-saint question ...
> do we commit in branches ... if yes, who does, when and how?
>
> regards.
>
> alex.
>
>
> On Wed, Sep 15, 2010 at 11:08 PM, Luis Ibanez <luis.ibanez at kitware.com> wrote:
>> 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
>>>
>>
> _______________________________________________
> 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
>