Added 3d edge detection …

[ipa-rmb]

… functionalities that run directly on the point cloud (without normals)

This is part 1 of #1672 .

[taketwo]

Hi, thanks for the effort. I had a quick look and I saw you are adding a number of functions for 2D kernel filtering, as well as integral image calculation. Would it make sense to use the convolutions for the 2d module and IntegralImage2D from the features module? In case they don't entirely fit your needs, maybe it's better to improve them?

(I don't like the situation when we introduce functions that largely overlap with existing ones and hide them deep in the implementation of some particular class.)

[SergioRAgostinho]

Similar comment on that fast approximation on the atan2 calculation.

[taketwo]

Yeah, it can go to "common/angles.h" maybe?

[SergioRAgostinho]

Yeah, it can go to "common/angles.h" maybe?

Seems to be the best place 👍

[ipa-rmb]

Thanks for your comments. I will have to check this again, but as far as I remember I checked whether the existing filters could be useful but they were too slow. So maybe I can move the separable filter to the filters class as another filter? I did not do so initially because this is a very specific case having a separable 3x3 filter. However, if you encourage to do so, I will move it there. Please confirm that I should move it.

I did not use the IntegralImage2D because my code just uses a 1D integral image (e.g. integral along the rows). I could implement the function with the existing 2D integral image, but it would slow down the algorithm significantly (the algorithm was designed for speed). I could create a new 1D integral image class with this limited functionality if preferred, but it will not offer more functionality than computing the 1D integral along the rows of an image provided as pcl::PointCloudpcl::Intensity. Again, this is quite specific, but making the implementation more general would decrease the runtime performance of the algorithm. So what would you prefer here?

Thanks for providing a file where to move the atan approximation.

[SergioRAgostinho]

Thanks for your comments. I will have to check this again, but as far as I remember I checked whether the existing filters could be useful but they were too slow. So maybe I can move the separable filter to the filters class as another filter? I did not do so initially because this is a very specific case having a separable 3x3 filter. However, if you encourage to do so, I will move it there. Please confirm that I should move it.

So the separability of the kernel is not being properly exploited. There are ways to test if the kernel supplied is separable or not, and I'm not currently sure that check is being done but I assume it isn't. Hence probably one of the reasons behind the performance penalty. I would prefer to address these thing properly first, modifying the already provided classes the library has.

I would preferably open a PR for each of these topics, to exploit ways to make your filter and integral image 1D as part of the general filters interface etc, keeping in mind the goal of retaining the performance you need.

Let's hear more opinions on this.

[taketwo]

I'd definitely like to see the filtering added to the 2d module instead of staying inside your class. Regarding the integral image I'm not so sure how useful it will be outside your class and also where we would put it.

Actually, I was also wondering why you are representing images with pcl::PointCloud<pcl::Intensity> instead of raw arrays or vectors?

[stale]

This pull request has been automatically marked as stale because it hasn't had
any activity in the past 60 days. Commenting or adding a new commit to the
pull request will revert this.

Come back whenever you have time. We look forward to your contribution.

[stale]

This pull request has been automatically marked as stale because it hasn't had
any activity in the past 60 days. Commenting or adding a new commit to the
pull request will revert this.

Come back whenever you have time. We look forward to your contribution.

[stale]

This pull request has been automatically marked as stale because it hasn't had
any activity in the past 60 days. Commenting or adding a new commit to the
pull request will revert this.

Come back whenever you have time. We look forward to your contribution.