Fix get fitness score ignore invalid points

[QiuYilin]

In https://github.com/PointCloudLibrary/pcl/pull/6054 ，the ICP calculation process has ignored invalid points, but the getFitnessScore method will still report an error when encountering invalid points.

I think it would be safer to add invalid point input tests for each module in gtest later.

[mvieth]

Adding the check to getFitnessScore makes sense, however I am not sure how useful the new example program is: there is a tutorial on ICP (https://pcl.readthedocs.io/projects/tutorials/en/master/iterative_closest_point.html) as well as two "tools" using ICP (https://github.com/PointCloudLibrary/pcl/blob/master/tools/icp.cpp and https://github.com/PointCloudLibrary/pcl/blob/master/tools/iterative_closest_point.cpp). Does the new example demonstrate anything that these three don't demonstrate already?

[QiuYilin]

@mvieth This new example has no other meaning. I added it when debugging. I did notice that some demo codes are scattered in tutorial, example and tool. There is no clear standard for dividing these three. I personally think that the way of using tutorial to refer to example code will be more unified and easier to maintain. As for tool, there is no need for demo code in it.

In short, if you think it is unnecessary to add a new example, you can delete it, because there is no plan to transfer all tutorials to examples at present (or you can set standards for examples and gradually transfer the tutorial code here). I think the really valuable work is to add the ordered point cloud with invalid values ​​to the gtest step (I am not familiar with this, so I choose to debug directly in example).

[mvieth]

@mvieth This new example has no other meaning. I added it when debugging. I did notice that some demo codes are scattered in tutorial, example and tool. There is no clear standard for dividing these three. I personally think that the way of using tutorial to refer to example code will be more unified and easier to maintain. As for tool, there is no need for demo code in it.

You are right, the purposes of those modules are somewhat fuzzy. For apps as well. I would say the main purpose of the tutorials is to show how to use the PCL classes and functions, and the "tools" are command line tools that are useful in everyday practice. By the way, the tutorials source code is here.

In short, if you think it is unnecessary to add a new example, you can delete it, because there is no plan to transfer all tutorials to examples at present (or you can set standards for examples and gradually transfer the tutorial code here). I think the really valuable work is to add the ordered point cloud with invalid values ​​to the gtest step (I am not familiar with this, so I choose to debug directly in example).

If the new example was just for debugging, I think it should be deleted. There is also a formatting mistake in registration.hpp (an extra whitespace at the end of line 148) that must be corrected before merging

[QiuYilin]

Updated. Does pcl have .clang-format file ?

[mvieth]

Updated. Does pcl have .clang-format file ?

Yes, however it is not yet enforced for all files (see here: https://github.com/PointCloudLibrary/pcl/blob/master/.dev/format.sh#L11 ). For the registration module, it is enforced.

Please also undo the remaining changes in examples/CMakeLists.txt

[QiuYilin]

updated

[QiuYilin]

@mvieth According to the definition in clang-format in the root directory, the curly braces after the class definition do not need to be wrapped.

BraceWrapping:
  AfterFunction: true
  AfterClass: false
  AfterStruct: false
  BeforeElse: true
  SplitEmptyFunction: false
  SplitEmptyRecord: false
  SplitEmptyNamespace: false

But the current code is basically wrapped. Is this .clang-format file still valid?

[mvieth]

@mvieth According to the definition in clang-format in the root directory, the curly braces after the class definition do not need to be wrapped.

BraceWrapping:
  AfterFunction: true
  AfterClass: false
  AfterStruct: false
  BeforeElse: true
  SplitEmptyFunction: false
  SplitEmptyRecord: false
  SplitEmptyNamespace: false

But the current code is basically wrapped. Is this .clang-format file still valid?

You mean here ? Seems correct, according to https://clang.llvm.org/docs/ClangFormatStyleOptions.html#bracewrapping :

AfterClass=true:
class foo
{};

AfterClass=false:
class foo {};

[QiuYilin]

@mvieth I mean AfterClass is true in the current code : https://github.com/PointCloudLibrary/pcl/blob/master/filters/include/pcl/filters/filter_indices.h#L75

[mvieth]

As I mentioned above, the clang-format rules are not yet enforced for all files/modules. See here for which files they are enforced: https://github.com/PointCloudLibrary/pcl/blob/master/.dev/format.sh#L11 They are not yet enforced for the filters module, so filter_indices.h does not yet follow the clang-format rules. We are formatting more files by those rules step-by-step, but the problem is: if a file is modified by an open pull request, we cannot format it, otherwise the pull request will have too many merge conflicts.

[QiuYilin]

@mvieth Sorry, I misunderstood the whitelist as the unsupported list.