Various fixes for ViSP Canny implementation

[rolalaro]

• Fix throwing an exception when the Canny's ratio for automatic computation of the thresholds are wrong
• Fix of the list of edge points
• Cleaning Canny tutorial + added a test there for the edge list

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 60 lines in your changes missing coverage. Please review.

Project coverage is 53.84%. Comparing base (5c2e8aa) to head (9f10679).
Report is 46 commits behind head on master.

Files with missing lines	Patch %	Lines
modules/core/include/visp3/core/vpImageFilter.h	0.00%	21 Missing ⚠️
modules/core/src/image/vpImageFilter_canny.cpp	0.00%	21 Missing ⚠️
modules/core/src/image/vpCannyEdgeDetection.cpp	0.00%	18 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1490      +/-   ##
==========================================
- Coverage   53.88%   53.84%   -0.04%     
==========================================
  Files         442      442              
  Lines       53778    53848      +70     
==========================================
+ Hits        28978    28996      +18     
- Misses      24800    24852      +52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[s-trinh]

For some reasons, changing the ulimit (any values) beforehand seems to lead to some error on my computer:

ulimit -s 8192

./tutorial-canny -i /tmp/img2.png --gradient 3 0.5
Canny Configuration:
	Filtering + gradient operators = gaussianblur+sobel-filtering
	Gaussian filter kernel size = 3
	Gaussian filter standard deviation = 0.500000
	Gradient filter kernel size = 3
	Canny edge filter thresholds = [-1.000000 ; -1.000000]
	Canny edge filter thresholds ratio (for auto-thresholding) = [0.600000 ; 0.800000]

setrlimit returned result = -1 ; errno=Invalid argument

---

My issue occurs on some really over-exposed images and should not be the typical use-case. To avoid putting too much efforts on some edge cases, I would propose:

• to count the number of recursion calls (as a fallback if setrlimit fails or for other platforms) and throw an exception?
• let the user configures the value of kStackSize? Or maybe by default --> do not change RLIMIT_STACK (this should satisfy most of the use cases) and otherwise let the user update this value using a specific function?

diff --git a/modules/core/include/visp3/core/vpCannyEdgeDetection.h b/modules/core/include/visp3/core/vpCannyEdgeDetection.h
index e70360f40..0e9026d32 100644
--- a/modules/core/include/visp3/core/vpCannyEdgeDetection.h
+++ b/modules/core/include/visp3/core/vpCannyEdgeDetection.h
@@ -385,7 +385,7 @@ private:
    * \return true We found a strong edge point in its 8-connected neighborhood.
    * \return false We did not found a strong edge point in its 8-connected neighborhood.
    */
-  bool recursiveSearchForStrongEdge(const std::pair<unsigned int, unsigned int> &coordinates);
+  bool recursiveSearchForStrongEdge(const std::pair<unsigned int, unsigned int> &coordinates, int &depth);
 
   /**
    * \brief Perform edge tracking.
diff --git a/modules/core/src/image/vpCannyEdgeDetection.cpp b/modules/core/src/image/vpCannyEdgeDetection.cpp
index e8a5b5f01..e2aa11033 100644
--- a/modules/core/src/image/vpCannyEdgeDetection.cpp
+++ b/modules/core/src/image/vpCannyEdgeDetection.cpp
@@ -258,8 +258,7 @@ vpCannyEdgeDetection::detect(const vpImage<unsigned char> &I)
       rl.rlim_cur = kStackSize;
       result = setrlimit(RLIMIT_STACK, &rl);
       if (result != 0) {
-        throw(vpException(vpException::fatalError, "setrlimit returned result = %d\n", result));
-
+        throw(vpException(vpException::fatalError, "setrlimit returned result = %d ; errno=%s\n", result, strerror(errno)));
       }
     }
   }
@@ -530,22 +529,34 @@ vpCannyEdgeDetection::performEdgeTracking()
 {
   std::map<std::pair<unsigned int, unsigned int>, EdgeType>::iterator it;
   std::map<std::pair<unsigned int, unsigned int>, EdgeType>::iterator m_edgePointsCandidates_end = m_edgePointsCandidates.end();
+  unsigned int nbStrongEdges = 0, nbWeakEdges = 0;
   for (it = m_edgePointsCandidates.begin(); it != m_edgePointsCandidates_end; ++it) {
     if (it->second == STRONG_EDGE) {
       m_edgeMap[it->first.first][it->first.second] = 255;
       if (m_storeListEdgePoints) {
         m_edgePointsList.push_back(vpImagePoint(it->first.first, it->first.second));
       }
+      ++nbStrongEdges;
     }
     else if (it->second == WEAK_EDGE) {
-      recursiveSearchForStrongEdge(it->first);
+      int depth = 0;
+      recursiveSearchForStrongEdge(it->first, depth);
+      std::cout << "[vpCannyEdgeDetection::performEdgeTracking]depth = " << depth << std::endl;
+      ++nbWeakEdges;
     }
   }
+  std::cout << "[vpCannyEdgeDetection::performEdgeTracking]#strong edges = " << nbStrongEdges << std::endl;
+  std::cout << "[vpCannyEdgeDetection::performEdgeTracking]#weak edges = " << nbWeakEdges << std::endl;
 }
 
 bool
-vpCannyEdgeDetection::recursiveSearchForStrongEdge(const std::pair<unsigned int, unsigned int> &coordinates)
+vpCannyEdgeDetection::recursiveSearchForStrongEdge(const std::pair<unsigned int, unsigned int> &coordinates, int &depth)
 {
+  depth++;
+  if (depth >= 65390) {
+    throw vpException(vpException::fatalError, "Reach the max number of allowed function calls recursion!");
+  }
+
   bool hasFoundStrongEdge = false;
   int nbRows = m_dIx.getRows();
   int nbCols = m_dIx.getCols();
@@ -575,10 +586,9 @@ vpCannyEdgeDetection::recursiveSearchForStrongEdge(const std::pair<unsigned int,
         // the continue is replaced by the test
       }
       if (edge_in_image_limit == false) {
-
-        try {
-          std::pair<unsigned int, unsigned int> key_candidate(idRow, idCol);
-          // Checking if the 8-neighbor point is in the list of edge candidates
+        std::pair<unsigned int, unsigned int> key_candidate(idRow, idCol);
+        // Checking if the 8-neighbor point is in the list of edge candidates
+        if (m_edgePointsCandidates.find(key_candidate) != m_edgePointsCandidates.end()) {
           EdgeType type_candidate = m_edgePointsCandidates.at(key_candidate);
           if (type_candidate == STRONG_EDGE) {
             // The 8-neighbor point is a strong edge => the weak edge becomes a strong edge
@@ -586,12 +596,9 @@ vpCannyEdgeDetection::recursiveSearchForStrongEdge(const std::pair<unsigned int,
           }
           else if (type_candidate == WEAK_EDGE) {
             // Checking if the WEAK_EDGE neighbor has a STRONG_EDGE neighbor
-            hasFoundStrongEdge = recursiveSearchForStrongEdge(key_candidate);
+            hasFoundStrongEdge = recursiveSearchForStrongEdge(key_candidate, depth);
           }
         }
-        catch (...) {
-          // continue - nothing to do
-        }
       }
       ++dc;
     }

[s-trinh]

Or maybe add two functions bool/int setStackSize(int) / bool/int resetStackSize() (int for errno?).
This would leave the responsibility to the user to change the stack size appropriately and most of the time the behaviour/stack size would not be changed.

Add another function void setMaxRecursionDepth(int) (by default the value would be something like INT_MAX?) for non Unix platform to avoid having a potential segfault, be able to catch the exception?

[rolalaro]

Or maybe add two functions bool/int setStackSize(int) / bool/int resetStackSize() (int for errno?). This would leave the responsibility to the user to change the stack size appropriately and most of the time the behaviour/stack size would not be changed.

Add another function void setMaxRecursionDepth(int) (by default the value would be something like INT_MAX?) for non Unix platform to avoid having a potential segfault, be able to catch the exception?

From what I read, only POSIX systems can set dynamically the stack size. On windows systems, the stack size of a program can only be changed during compilation. So the setStackSize function should either be available only for POSIX systems or throw an exception for Windows systems.
The depth parameter could be an alternative for Windows systems, but there is no way to guess a "good" depth limit. From what I read, the stack size on Windows is typically 1Mb but there is no way to know how much of the size is already used by the program before calling the recursive function. Additionally, the depth value should not limit the program if the setStackSize method has been called previously.
@fspindle do you have any preferences on how things should be handled ?