keypoints indices feature

[nizar-sallem]

Add method to get keypoints indices fixes #303

[jspricke]

I still wonder why we need the PointIndices struct at all, as it's really just a std::vector. Adding more vector methods doesn't sound like the right way to go for me, as you could always access the vector directly.
Regarding the changes to Harris:

• Wouldn't it make more sense to push this into the Keypoint class, so all can benefit?
• I think a more streamlined API would be nicer, so that we don't keep the almost the same information two times. Something like only computing the keypoint indices and having an extra function to extract indices from the cloud (don't we have that already?). Only a suggestion.

[nizar-sallem]

Hi Jochen,

I thought of that and totally agree but here is the problem:
some detector use average weighted or other kinds of interpolation so the XYZ component can not be given from the indices so they are useless.
Others use 2D keypoints and store indices in the XY components so it doesn't make sense to do it there either.
Other gives the keypoint as an index so there is no need to add the indices.

Concerning the PointIndices structure it is there since you could pass keypoints indices to some filter or any other processing class that would have a setIndices method.

Now why am I adding the vectors method to it is simply to get a vector like behavior, we still can move indices to private section and do some sed on the whole PCL to fix that because I don't understand why would an outer need access to the inner vector but that is another story.

[jspricke]

Hi Nizar,

• nizar sallem notifications@github.com [2013-10-14 12:02]:

  some detector use average weighted or other kinds of interpolation so the XYZ component can not be given from the indices so they are useless.

Agreed.

Others use 2D keypoints and store indices in the XY components so it doesn't make sense to do it there either.

Which one is that?

Other gives the keypoint as an index so there is no need to add the indices.

I'm not sure I got you here, so they are haven your proposed patch
already?

Concerning the PointIndices structure it is there since you could pass keypoints indices to some filter or any other processing class that would have a setIndices method.

Now why am I adding the vectors method to it is simply to get a vector like behavior, we still can move indices to private section and do some sed on the whole PCL to fix that because I don't understand why would an outer need access to the inner vector but that is another story.

I don't see any argument for having such a structure. Syntactically it's
a vector and semantically we could get the same using a typedef. Also,
it's a remnant of the old ROS types (cf. the header we don't use at all,
as far as I've seen). I would propose to deprecate it, what do you
think?

Cheers Jochen

[nizar-sallem]

I will answer to the points

1. Cool
2. Agast and Brisk
3. NARF through setting PointOutT to int
4. Totally agree

Cheers,

Nizar

[nizar-sallem]

@jspricke,

I am going to remove the vector interface I added to PointIndices so that if we typedef it the work would be the same for all the classes in PCL.
I will add the keypoints_indices_ attribute and the getKeypointsIndices method to the Keypoint class. Indices would be populated at daughter class level if any else it will give an empty set.
What do you think ?

[jspricke]

@nizar-sallem +1 Thanks!

[nizar-sallem]

Great will do that now :)

[nizar-sallem]

@jspricke done

[nizar-sallem]

fixed date on the header

[nizar-sallem]

what dou mean you mean ?
if you mean the getKeypointsIndices than yes it is always there since it is inside the mother class difference is either indices are being populated or not so I protected it inside this condition if (!keypoints_indices->indices.empty ()).
That should do it.

[jspricke]

• nizar sallem notifications@github.com [2013-10-16 08:35]:

  what dou mean you mean ?

That's a comment to commit
18ffa0d keypoints/include/pcl/keypoints/impl/susan.hpp:465. You changed
output.is_dense = input_->is_dense;
to
output.is_dense = true;

[nizar-sallem]

Yeah totally :)
SUSAN can not use the infinite points while non maximal suppression is applied.