Compilation with Visual Studio

[BorisMansencal]

Compilation of DGtal head fails on Windows 10 (1809) with Visual Studio Community 17 (15.9.5).

PR #1380 makes the code compile with Visual Studio (15.9.5).
It makes also the following tests pass :
testLongVol
testPNMReader
testVolReader
testGenericReader
testPNMRawWriter
testGenericWriter

In Release mode, the following 4 tests still fail:
testCloneAndAliases
testClone2
testVoxelComplex
testNeighborhoodConfigurations

In Debug mode, the code does not compile.
I get an error on tutorial-examples/shortcuts-geometry.cpp
error C1128 "number of sections exceeded object file format limit: compile with /bigobj".
The file has to be split or the CMakeLists.txt has to be changed to compile with /bigobj on Windows with Visual.

If I comment out some code in shortcults-geometry.cpp to make it compile, everything else compiles correctly.
However, 11 tests still fail:
testCloneAndAliases
testClone2
testContourHelper
testCombinFSS
testFMM
testChamferDT
testChamferVoro
testCubicalComplex
testVoxelComplex
testNeighborhoodConfigurations
testParDirCollapse

Visual Studio (15.9.5) seems to check more things regarding the STL use.
The compilation outputs some frightening error messages:

• "can't dereference out of range vector iterator"
• "cannot dereference string iterator because it is out of range"
• "map/set iterators incompatible"
• "list iterators incompatible"
  ...

[JacquesOlivierLachaud]

Do you have more information on why tests are not passing ? I do not have VS to check these tests.
For testClone2, I had a version of gcc (5.4) where it fails while it works with clang (3.8), so I might have found the issue (upcoming FixClone), It is a related to a bad implicit conversion.
Can you provide error logs or try to tell us where you think is a the problem.

[BorisMansencal]

I will try to provide more details.

[BorisMansencal]

Actually, I think you can replicate at least some of the errors by compiling wih gcc with the flag -D_GLIBCXX_DEBUG
(I tested on Ubuntu 16.04 by adding this flag to CMAKE_CXX_FLAGS )

I will try to detail what I have found in the following messages.
(I am not an expert in DGtal code and concepts, so bear with me...)

[BorisMansencal]

testFMM fail is due to the use of an invalid iterator at src/DGtal/geometry/volumes/distance/FMM.ih:416
Basically, we have this kind of code:

while ( (it != itEnd) && (!flagStop) ) {
	      myCandidatePoints.erase(*it); 
	      if ( ...) {
	      	  //... 
	      	  flagStop = true; 
	      }  
	      else {
		  it = myCandidatePoints.begin(); 
	      }  
  }

If erase() is called, 'it' becomes invalid and then we compare an invalid iterator with 'itEnd'

I think that just switching the checks in the first 'if' solves the problem:

while ( (!flagStop) && (it != itEnd)) {

[BorisMansencal]

testChamferDT and testChamferVoro fails are due to several dereferences of out of range vector iterators.

One error common to the two tests is at src/DGtal/geometry/volumes/distance/ChamferNorm2D.ih:288
We have this kind of code:

  if ((aEnd - aBegin) > 1)
    {
    ConstIterator mid = aBegin + (aEnd - aBegin) / 2;
       
    midPoint = aP + (*mid);
    nextMidPoint = aP + *(mid+1);    /// <= error here, on *(mid+1)
    //...

If we have aEnd-aBegin==2, we enter the 'if', and mid==aBegin+1 and mid+1==aEnd

I don't understand the code enough to propose a patch. There is probably a check to add here...

It seems there are other errors in tests/geometry/volumes/distance/testChamferDT.cpp, in testChamferSimple(), lines 120, 148, 163, where we have code like:

  trace.info() << " -> cone "<< *aMask.getCone(d2)
  << "  -- " << *(aMask.getCone(d2)+1) <<std::endl;

Here if *aMask.getCone(d2) return an iterator on the last element of the vector, then *(aMask.getCone(d2)+1) dereferences an invalid iterator.

We could add checks before these traces. But maybe these errors are related to the first one...

[BorisMansencal]

testContourHelper fail is also due to an access to an out of range iterator.

The error come from tests/geometry/testContourHelper.cpp:90
that calls src/DGtal/geometry/helpers/ContourHelper.ih:84 where we have the following code:

  unsigned int size = std::distance(itb, ite);
  while(i<(int)size-1)
    {

      short code = FreemanChain<int>::freemanCode4C((*(it+1))[0]-(*it)[0],(*(it+1))[1]-(*it)[1]);
      short codeNext = FreemanChain<int>::freemanCode4C((*(it+2))[0]-(*(it+1))[0],(*(it+2))[1]-(*(it+1))[1]);  // <= error here
     //...
    it++;
    i++;

Here, if it is ite-1 we access *(it+2) that point past the end of the vector.

[dcoeurjo]

thx a lot @BorisMansencal for this very insightful details..

[dcoeurjo]

Ping @kerautret and @troussil on this one ;). I’ll hanlde the chamferDT

[kerautret]

@dcoeurjo yes I look it, and thanks @BorisMansencal for the details ;)

[BorisMansencal]

testCubicalComplex, testVoxelComplex and testParDirCollapse fails seem all related to the same problem : comparison of iterators from different sequence at src/DGtal/topology/CubicalComplexFunctions.ih:180 and 185.
We have the following code:

	      CellMapIterator best_free_face_it = K.end( 0 );
	      for ( CMIVectorConstIterator it = Q_low.begin(), itE = Q_low.end();
                    it != itE; ++it )
		{
                  CellMapIterator low_ic = *it;
                  //...
		  if ( cur_d_type == CC::Free )
		    { // found a free n-1-face ic
		      if ( ( best_free_face_it == K.end( 0 ) )     // <= error here
			   || ( ! priority( low_ic, best_free_face_it ) ) )
			best_free_face_it = low_ic;
		    }
		}
	      if ( best_free_face_it != K.end( 0 ) )  // <= error here
		{
                    //...

best_free_face_it and K.end( 0 ) are both CellMapIterators, but they can iterate on different sequences.

Here are the call stacks if it may help:

tests/topology/testCubicalComplex.cpp:238
  src/DGtal/topology/CubicalComplexFunctions.ih:180

tests/topology/testVoxelComplex.cpp:300
 src/DGtal/topology/VoxelComplex.ih:896
  src/DGtal/topology/VoxelComplex.ih:880
   src/DGtal/topology/CubicalComplexFunctions.ih:180

tests/topology/testParDirCollapse.cpp:86
 src/DGtal/topology/ParDirCollapse.ih:97
  src/DGtal/topology/CubicalComplexFunctions.ih:185

[dcoeurjo]

Thanks for the report. Ping @phcerdan

[phcerdan]

That's interesting, @JacquesOlivierLachaud might have more insight as the implementor of CubicalComplexFunctions.

I can have a look it too if @JacquesOlivierLachaud is out of time. Let me know!

[JacquesOlivierLachaud]

I just had a look. I see the potential error. I use d+1 different std::map<Cell,Value> to speed up cell look-up.
Therefore there are d+1 "begin( k )" and d+1 "end( k )", one per map. In the loop, iterators are compared with end( 0 ) instead of end( k ) where k is their dimension. I used this trick to avoid the computation of the cell dimension and because I just need to compare with an invalid cell.
So the code should work in all reasonnable implementation of std::map::end(), meaning end() does not point in some other map. However I understand why a strict compiler could complain.

[JacquesOlivierLachaud]

I am not sure if there is a fix that keeps the same efficiency. I have to think about it. Here maximal faces can have any dimension, so the only fix I can think about is to make d+1 loops (one per dimension).

[JacquesOlivierLachaud]

Wait. Perhaps I can use an unitialized iterator and a boolean value.

[BorisMansencal]

Currently, there are still 4 tests still failing (with Visual Studio Community 17, version 15.9.8, and Visual 19 preview, version 16.0.0 Preview 4.1 SVC1):
testCloneAndAliases
testClone2
testVoxelComplex
testNeighborhoodConfigurations

Something I forgot to mention previously about Visual compilation: when configuring DGtal with ITK, DGtal needs to be configured with the same build type as ITK, otherwise configuration fails.
In particular, the current git version of ITK (5.0) compiles in Release mode by default. During DGtal configuration, the ITKcpp11Bug cmake check fails (and thus the configuration of DGtal) because it is not also built in Release mode. Besides the message we get is very misleading as it says that ITK was not build with C++11 [ITK is now build in C++11 by default].
If I force the CMAKE_BUILD_TYPE to Release in cmake/src/ITKcpp11Bug/CMakeLists.txt, the check passes and the build is configured correctly.
But I also have to configure and build DGtal in Release mode in order to compile correctly.

But if ITK is built in Debug and DGtal in Release, I think we have the same problem.
ITK build type should be detected during DGtal configuration, but I don't know if it is feasible.

I build from the command line using the "Ninja" generator. For reference, here is the command line I use to be able to compile:

cmake .. -DWITH_ITK=TRUE -DZLIB_LIBRARY="<path_to_zlib.lib>" -DZLIB_INCLUDE_DIR="<path_to_zlib_include>"  -DITK_DIR="<path_to_ITK_build_dir>" -DBUILD_SHARED_LIBS=FALSE -DCMAKE_BUILD_TYPE=RELEASE -DBUILD_TESTING=TRUE -G"Ninja"

[BinWang0213]

I have similar issues with following module failed by running RUN_TESTS:

96% tests passed, 8 tests failed out of 205
23 - testCloneAndAliases (Failed)
1> 24 - testClone2 (Failed)
1> 127 - testObjectBoostGraphInterface (Not Run)
1> 134 - testVoxelComplex (Failed)
1> 144 - testNeighborhoodConfigurations (Failed)
1> 167 - testITKReader (Not Run)
1> 185 - testArrayImageAdapter (SEGFAULT)
1> 203 - testLinearStructure (Exit code 0xc0000409

Compiler: Visual Studio Community 2015 Release x64
CMake: 3.16.0
DGtal: 1.0
ITK: 5.0.1
EIGEN:3.0

BTW, in order to compile with ITK without cpp11 error, I need to comment out
Line 225 @ DGtal-1.0\cmake\CheckDGtalOptionalDependencies.cmake
# MESSAGE(FATAL_ERROR "ITK was found but it appears that the package was not built with std-cpp11 extension and DGtal will not compile.")