Two small bugs with corrections

[eduardoatr]

Greetings Dr. Suárez,

First, I want to thank you for sharing this amazing work, which I've been studying recently by creating simple inputs to better visualize the edge drawing algorithm. During this process, I have encountered two issues that I will describe in the following sections, along with some possible fixes.

Skip Positions

The first one is pretty straightforward:

elsed_main: /home/eduardo/Desktop/Projects/ELSED/src/FullSegmentInfo.cpp:37: void upm::FullSegmentInfo::skipPositions(): Assertion 'pixels->size() == lastPxIndex + 1 + UPM_SKIP_EDGE_PT' failed.
[1]    279495 abort (core dumped)  /home/eduardo/Desktop/Projects/ELSED/build/elsed_main

Here is a code snippet to reproduce the error:

#include <iostream>
#include <opencv2/opencv.hpp>
#include "ELSED.h"

inline void
drawSegments(cv::Mat img,
             upm::Segments segs,
             int thickness = 1,
             int lineType = cv::LINE_4,
             int shift = 0) {
  cv::RNG rng(123456);
  for (const upm::Segment &seg: segs){
    const cv::Scalar color(rng.uniform(75.0, 255.0), rng.uniform(75.0, 255.0), rng.uniform(75.0, 255.0));
    cv::line(img, cv::Point2f(seg[0], seg[1]), cv::Point2f(seg[2], seg[3]), color, thickness, lineType, shift);
  }
}

int main() {
  cv::Mat img = (
    cv::Mat_<uint8_t>(7, 21) <<
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0
  );

  upm::ELSEDParams params;
  params.minLineLen = 5;

  upm::ELSED elsed(params);
  upm::Segments segs = elsed.detect(img);
  std::cout << "ELSED detected: " << segs.size() << " segments" << std::endl;

  cv::Mat img_show;
  cv::cvtColor(img, img_show, cv::COLOR_GRAY2BGR);

  drawSegments(img_show, segs, 1);
  cv::namedWindow("Segments", cv::WINDOW_NORMAL);
  cv::resizeWindow("Segments", cv::Size2i(1000, 800));
  cv::imshow("Segments", img_show);
  cv::waitKey(0);

  return 0;
}

While investigating this issue, I found that once the segment is initialized by setting the localSegInitialized variable, its size will always be larger than minLineLength on line 209, so this check always passes. However, in cases when the skipPositions method is called, each loop adds 1 pixel and skips UPM_SKIP_EDGE_PT = 2 pixels, which eventually causes a segmentation fault when accessing the list of pixels using lastPxIndex within FullSegmentInfo.

The fix appears to be simple. We need to ensure that at least UPM_SKIP_EDGE_PT pixels are added to the list after the local segment is initialized. This can be done by comparing the size of the list of pixels against lastPxIndex. I am submitting a pull request with this fix. Here are the results after implementing the suggested fix:

Junctions

For this second issue, I believe the junctions are not working as expected. In the following example, I've set ksize = 1 to avoid blurring the image and to simplify the problem by generating straight gradient lines. I expected ELSED to detect 4 segments, but it found 6, indicating that it failed to correctly identify the junctions for the horizontal gradients. Here is the code snippet to reproduce the issue:

#include <iostream>
#include <opencv2/opencv.hpp>
#include "ELSED.h"

inline void
drawSegments(cv::Mat img,
             upm::Segments segs,
             int thickness = 1,
             int lineType = cv::LINE_4,
             int shift = 0) {
  cv::RNG rng(123456);
  for (const upm::Segment &seg: segs){
    const cv::Scalar color(rng.uniform(75.0, 255.0), rng.uniform(75.0, 255.0), rng.uniform(75.0, 255.0));
    cv::line(img, cv::Point2f(seg[0], seg[1]), cv::Point2f(seg[2], seg[3]), color, thickness, lineType, shift);
  }
}

int main() {
  cv::Mat img = (
    cv::Mat_<uint8_t>(23, 21) <<
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99, 99,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 99,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
      0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0
  );

  upm::ELSEDParams params;
  params.minLineLen = 3;
  params.ksize = 1;
  params.listJunctionSizes = {3};

  upm::ELSED elsed(params);
  upm::Segments segs = elsed.detect(img);
  std::cout << "ELSED detected: " << segs.size() << " segments" << std::endl;

  cv::Mat img_show;
  cv::cvtColor(img, img_show, cv::COLOR_GRAY2BGR);

  drawSegments(img_show, segs, 1);
  cv::namedWindow("Segments", cv::WINDOW_NORMAL);
  cv::resizeWindow("Segments", cv::Size2i(1000, 800));
  cv::imshow("Segments", img_show);
  cv::waitKey(0);

  return 0;
}

And here is the result:

By checking the label of each pixel, we can see that indeed the junctions were not found in this case, when extending the first end of the horizontal segments, and it starts to drawn the the first vertical segment from the top. However the junctions are found when extending the second end of the segments on the right side.

I also managed to identify the problem with this one. It lies on line 212, which breaks the loop for zero gradients, as described in the comments, without triggering the line extension block on line 379. I believe the best fix here is to store the return value of findNextPxWithGradient, and also add it as a condition for the extension block. These are the results after the modification:

Finally, this change slightly improves the results of the detector in some tests I have conducted, so I believe this
is a good change, but I'm sure you have more specific benchmarks to tell.