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

[Arnaud Gelas]

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);
>> }