[Insight-developers] question about preprocessor condition in itk::QuadEdgeMesh<>::AddFace method

[Alexandre GOUAILLARD]

Dear arnaud,

that happen to all of us at least once :)

I kind of remember that ITK was defining that flag. I guess we can
safely assumed that the flag is always present when building in
release mode, and then my code should do what I expect it to do,
across platform.

alex.

alex.


On Tue, Apr 17, 2012 at 5:09 PM, Arnaud Gelas <arnaudgelas at gmail.com> wrote:
> Alex,
>
> Thanks for the quick reply! You made me realized that I was looking at the
> code in Release with deb info :)
>
> Here are the default value for CMAKE (at least on my machine)
> CMAKE_CXX_FLAGS_DEBUG            -g
> CMAKE_CXX_FLAGS_MINSIZEREL       -Os -DNDEBUG
> CMAKE_CXX_FLAGS_RELEASE          -O3 -DNDEBUG
> CMAKE_CXX_FLAGS_RELWITHDEBINFO   -O2 -g
>
> CMAKE_C_FLAGS_DEBUG              -g
> CMAKE_C_FLAGS_MINSIZEREL         -Os -DNDEBUG
> CMAKE_C_FLAGS_RELEASE            -O3 -DNDEBUG
> CMAKE_C_FLAGS_RELWITHDEBINFO     -O2 -g
>
> As you can see NDEBUG is defined in Release and I guess it should be the
> same on all platform (?)
>
> Thanks,
> Arnaud
>
>
> On 04/17/2012 10:51 AM, Alexandre GOUAILLARD wrote:
>>
>> Dear arnaud,
>>
>> The QuadEdgeMesh classes are checking for half the world out there
>> (and their moms) which is great in term of robustness, but bad in term
>> of speed. Especially, it was great when writing those classes, and
>> when creating a QEMesh, but it is disastrous in term of performance in
>> a pipeline, when the input is already a QEMesh.
>>
>> Using NDEBUG was an attempt during one of the SLC project week at
>> speed up QE filters by:
>> 1 - checking things only in debug mode.
>> 2 - Know when the mesh is already "secure" and bypass the tests during
>> copy for example
>> Note that 1 is a very crude attempt, as NDEBUG is not standard, and I
>> m open to suggestions on how to make it better / more standard / cross
>> platform.
>>
>> For this specific code, the idea would be to go ahead with the test
>> only in debug mode, i.e. only when NDEBUG is *NOT* defined (MSVC
>> defines NDEBUG for you in release mode, as well as _DEBUG. NDEBUG is
>> specifically for asserts, _DEBUG is supposed to be used for CRT.)
>>
>> HTH.
>>
>> regards,
>>
>> alex.
>> PS: enjoy european food for us ;-)
>>
>> On Tue, Apr 17, 2012 at 3:59 PM, Arnaud Gelas<arnaudgelas at gmail.com>
>>  wrote:
>>>
>>> Hi Alex,
>>>
>>> In the AddFace method of itk::QuadEdgeMesh, there is a preprocessor
>>> condition (#ifndef NDEBUG, see code below).
>>> Is it correct to make these tests only when NDEBUG is undefined? Is it
>>> the
>>> other way around? or should it be tested all the time?
>>>
>>> Thanks,
>>> Arnaud
>>>
>>> ---
>>>
>>> template<  typename TPixel, unsigned int VDimension, typename TTraits>
>>> typename QuadEdgeMesh<  TPixel, VDimension, TTraits>::QEPrimal *
>>> QuadEdgeMesh<  TPixel, VDimension, TTraits>
>>> ::AddFace(const PointIdList&  points)
>>>
>>> {
>>> #ifndef NDEBUG
>>>  // Check that there are no duplicate points
>>>  for ( size_t i = 0; i<  points.size(); i++ )
>>>    {
>>>    typename PointIdList::const_iterator itr = points.begin();
>>>    typename PointIdList::const_iterator end = points.end();
>>>    PointIdentifier count = NumericTraits<  PointIdentifier>::Zero;
>>>    const PointIdentifier pointId = points[i];
>>>    while ( itr != end )
>>>      {
>>>      if ( *itr == pointId )
>>>        {
>>>        ++count;
>>>        }
>>>      ++itr;
>>>      }
>>>    if ( count != 1 )
>>>      {
>>>      itkDebugMacro("Point "<<  i<<  " is duplicated");
>>>      return ( (QEPrimal *)NULL );
>>>      }
>>>    }
>>>
>>>  // Check that all points exist
>>>  for ( size_t i = 0; i<  points.size(); i++ )
>>>    {
>>>    if ( !this->GetPoints()->IndexExists(points[i]) )
>>>      {
>>>      itkDebugMacro("Point "<<  i<<  " is missing in the mesh");
>>>      return (QEPrimal *)NULL;
>>>      }
>>>    }
>>> #endif
>>>
>>>  // Check if existing edges have no face on the left.
>>>  for ( size_t i = 0; i<  points.size(); i++ )
>>>    {
>>>    PointIdentifier pid0 = points[i];
>>>    PointIdentifier pid1 = points[( i + 1 ) % points.size()];
>>>
>>>    QEPrimal *edge = this->FindEdge(pid0, pid1);
>>>
>>>    if ( edge )
>>>      {
>>>      if ( edge->IsLeftSet() )
>>>        {
>>>        itkDebugMacro("Edge ["<<  i<<  " "<<  ( ( i + 1 ) % points.size()
>>> )
>>> <<  " has a left face.");
>>>        return (QEPrimal *)NULL;
>>>        }
>>>      }
>>>    }
>>>
>>>  return AddFaceWithSecurePointList(points);
>>> }
>
>