Skip to content

Minor modifications for better usuability #184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 5, 2024
Merged

Minor modifications for better usuability #184

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6fdd962
Set const in the inputs of the matcher.h and matcher.cc
LimHyungTae May 2, 2024
3702c72
Add Quatro mode in a Python wrapper
LimHyungTae May 2, 2024
de7a6ae
Make normal estimation based on OMP as well.
LimHyungTae May 2, 2024
e4375d9
Add API with normal vectors for easy integration with the `xyznormal`…
LimHyungTae May 2, 2024
6f98c3b
Clean unused variables
LimHyungTae May 2, 2024
e71de4d
Fix error in the example (change == to =)
LimHyungTae May 2, 2024
a1a0ec9
Remove weird semicolon to cover #99
LimHyungTae May 2, 2024
7275ee8
Remove [[nodiscard]] to cover #140
LimHyungTae May 2, 2024
e33d700
Add 'InsertNewlineAtEOF' in the .clang-format
LimHyungTae May 5, 2024
0383de1
Add newline at the end of file using .clang-format
LimHyungTae May 5, 2024
6b60fdb
(Minor) Clean codes, e.g., managing double space, redundant line brea…
LimHyungTae May 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions examples/teaser_cpp_fpfh/quatro_cpp_fpfh.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ inline void getParams(const double noise_bound, const std::string reg_type,
if (reg_type == "Quatro") {
params.rotation_estimation_algorithm =
teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM::QUATRO;
params.inlier_selection_mode == teaser::RobustRegistrationSolver::INLIER_SELECTION_MODE::PMC_HEU;
params.inlier_selection_mode = teaser::RobustRegistrationSolver::INLIER_SELECTION_MODE::PMC_HEU;
} else if (reg_type == "TEASER") {
params.rotation_estimation_algorithm =
teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM::GNC_TLS;
params.inlier_selection_mode == teaser::RobustRegistrationSolver::INLIER_SELECTION_MODE::PMC_EXACT;
params.inlier_selection_mode = teaser::RobustRegistrationSolver::INLIER_SELECTION_MODE::PMC_EXACT;
} else {
throw std::invalid_argument("Not implemented!");
}
Expand Down
2 changes: 1 addition & 1 deletion examples/teaser_python_3dsmooth/teaser_python_3dsmooth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def visualize_correspondences(
frag2_temp.translate(translate)

# estimate normals
vis_list = [target, source, frag1_temp, frag2_temp];
vis_list = [target, source, frag1_temp, frag2_temp]
for ii in vis_list:
ii.estimate_normals()
vis_list.extend([*inlier_line_mesh_geoms, *outlier_line_mesh_geoms])
Expand Down
21 changes: 15 additions & 6 deletions python/teaserpp_python/teaserpp_python.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ PYBIND11_MODULE(teaserpp_python, m) {
py::enum_<teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM>(
solver, "ROTATION_ESTIMATION_ALGORITHM")
.value("GNC_TLS", teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM::GNC_TLS)
.value("FGR", teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM::FGR);
.value("FGR", teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM::FGR)
.value("QUATRO", teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM::QUATRO);

// Python bound for teaser::RobustRegistrationSolver::INLIER_GRAPH_FORMULATION
py::enum_<teaser::RobustRegistrationSolver::INLIER_GRAPH_FORMULATION>(solver,
Expand Down Expand Up @@ -125,11 +126,19 @@ PYBIND11_MODULE(teaserpp_python, m) {
.def("__repr__", [](const teaser::RobustRegistrationSolver::Params& a) {
std::ostringstream print_string;

std::string rot_alg =
a.rotation_estimation_algorithm ==
teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM::FGR
? "FGR"
: "GNC-TLS";
std::string rot_alg;
if (a.rotation_estimation_algorithm ==
teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM::FGR) {
rot_alg = "FGR";
}
if (a.rotation_estimation_algorithm ==
teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM::GNC_TLS) {
rot_alg = "GNC_TLS";
}
if (a.rotation_estimation_algorithm ==
teaser::RobustRegistrationSolver::ROTATION_ESTIMATION_ALGORITHM::QUATRO) {
rot_alg = "QUATRO";
}

std::string inlier_selection_alg;
if (a.inlier_selection_mode ==
Expand Down
18 changes: 14 additions & 4 deletions teaser/include/teaser/fpfh.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <boost/smart_ptr/shared_ptr.hpp>
#include <pcl/features/fpfh.h>
#include <pcl/features/fpfh_omp.h>
#include <pcl/features/normal_3d_omp.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this header always there? Will there be any PCL installation that does not have this header?

Copy link
Member Author

@LimHyungTae LimHyungTae May 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I double-checked it, and I found that it is stabilized since PCL 1.8 version as follow:
https://github.com/PointCloudLibrary/pcl/releases/tag/pcl-1.8.1
Thus, I think it's fine because in our CMakelLists.txt, we mention "1.8" version when finding the package.

In addition, until the latest version (1.14), the PCL library still supports normal_3d_omp as follows:
https://pcl-docs.readthedocs.io/en/latest/api/program_listing_file_pcl_features_include_pcl_features_normal_3d_omp.h.html

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


#include "teaser/geometry.h"

Expand All @@ -21,10 +22,11 @@ using FPFHCloudPtr = pcl::PointCloud<pcl::FPFHSignature33>::Ptr;

class FPFHEstimation {
public:
FPFHEstimation()
: fpfh_estimation_(
new pcl::FPFHEstimationOMP<pcl::PointXYZ, pcl::Normal, pcl::FPFHSignature33>){};

FPFHEstimation() {
fpfh_estimation_.reset(
new pcl::FPFHEstimationOMP<pcl::PointXYZ, pcl::Normal, pcl::FPFHSignature33>());
normals_.reset(new pcl::PointCloud<pcl::Normal>());
}
/**
* Compute FPFH features.
*
Expand All @@ -50,6 +52,8 @@ class FPFHEstimation {
// pcl::FPFHEstimation<pcl::PointXYZ, pcl::Normal, pcl::FPFHSignature33>::Ptr fpfh_estimation_;
pcl::FPFHEstimationOMP<pcl::PointXYZ, pcl::Normal, pcl::FPFHSignature33>::Ptr fpfh_estimation_;

pcl::PointCloud<pcl::Normal>::Ptr normals_;

/**
* Wrapper function for the corresponding PCL function.
* @param output_cloud
Expand Down Expand Up @@ -78,6 +82,12 @@ class FPFHEstimation {
* Wrapper function for the corresponding PCL function.
*/
void setRadiusSearch(double);

/**
* Return the normal vectors of the input cloud that are used in the calculation of FPFH
* @return
*/
pcl::PointCloud<pcl::Normal> getNormals();
};

} // namespace teaser
12 changes: 6 additions & 6 deletions teaser/include/teaser/graph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,34 +128,34 @@ class Graph {
* Get the number of vertices
* @return total number of vertices
*/
[[nodiscard]] int numVertices() const { return adj_list_.size(); }
int numVertices() const { return adj_list_.size(); }

/**
* Get the number of edges
* @return total number of edges
*/
[[nodiscard]] int numEdges() const { return num_edges_; }
int numEdges() const { return num_edges_; }

/**
* Get edges originated from a specific vertex
* @param [in] id
* @return an unordered set of edges
*/
[[nodiscard]] const std::vector<int>& getEdges(int id) const { return adj_list_[id]; }
const std::vector<int>& getEdges(int id) const { return adj_list_[id]; }

/**
* Get all vertices
* @return a vector of all vertices
*/
[[nodiscard]] std::vector<int> getVertices() const {
std::vector<int> getVertices() const {
std::vector<int> v;
for (int i = 0; i < adj_list_.size(); ++i) {
v.push_back(i);
}
return v;
}

[[nodiscard]] Eigen::MatrixXi getAdjMatrix() const {
Eigen::MatrixXi getAdjMatrix() const {
const int num_v = numVertices();
Eigen::MatrixXi adj_matrix(num_v, num_v);
for (size_t i = 0; i < num_v; ++i) {
Expand All @@ -171,7 +171,7 @@ class Graph {
return adj_matrix;
}

[[nodiscard]] std::vector<std::vector<int>> getAdjList() const { return adj_list_; }
std::vector<std::vector<int>> getAdjList() const { return adj_list_; }

/**
* Preallocate spaces for vertices
Expand Down
4 changes: 2 additions & 2 deletions teaser/include/teaser/matcher.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ class Matcher {
* @return
*/
std::vector<std::pair<int, int>>
calculateCorrespondences(teaser::PointCloud& source_points, teaser::PointCloud& target_points,
teaser::FPFHCloud& source_features, teaser::FPFHCloud& target_features,
calculateCorrespondences(const teaser::PointCloud& source_points, const teaser::PointCloud& target_points,
const teaser::FPFHCloud& source_features, const teaser::FPFHCloud& target_features,
bool use_absolute_scale = true, bool use_crosscheck = true,
bool use_tuple_test = true, float tuple_scale = 0);

Expand Down
10 changes: 6 additions & 4 deletions teaser/src/fpfh.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ teaser::FPFHCloudPtr teaser::FPFHEstimation::computeFPFHFeatures(
const teaser::PointCloud& input_cloud, double normal_search_radius, double fpfh_search_radius) {

// Intermediate variables
pcl::PointCloud<pcl::Normal>::Ptr normals(new pcl::PointCloud<pcl::Normal>);
teaser::FPFHCloudPtr descriptors(new pcl::PointCloud<pcl::FPFHSignature33>());
pcl::PointCloud<pcl::PointXYZ>::Ptr pcl_input_cloud(new pcl::PointCloud<pcl::PointXYZ>);
for (auto& i : input_cloud) {
Expand All @@ -25,16 +24,17 @@ teaser::FPFHCloudPtr teaser::FPFHEstimation::computeFPFHFeatures(
}

// Estimate normals
pcl::NormalEstimation<pcl::PointXYZ, pcl::Normal> normalEstimation;
normals_->clear();
pcl::NormalEstimationOMP<pcl::PointXYZ, pcl::Normal> normalEstimation;
normalEstimation.setInputCloud(pcl_input_cloud);
normalEstimation.setRadiusSearch(normal_search_radius);
pcl::search::KdTree<pcl::PointXYZ>::Ptr kdtree(new pcl::search::KdTree<pcl::PointXYZ>);
normalEstimation.setSearchMethod(kdtree);
normalEstimation.compute(*normals);
normalEstimation.compute(*normals_);

// Estimate FPFH
setInputCloud(pcl_input_cloud);
setInputNormals(normals);
setInputNormals(normals_);
setSearchMethod(kdtree);
setRadiusSearch(fpfh_search_radius);
compute(*descriptors);
Expand All @@ -59,3 +59,5 @@ void teaser::FPFHEstimation::compute(pcl::PointCloud<pcl::FPFHSignature33>& outp
fpfh_estimation_->compute(output_cloud);
}
void teaser::FPFHEstimation::setRadiusSearch(double r) { fpfh_estimation_->setRadiusSearch(r); }

pcl::PointCloud<pcl::Normal> teaser::FPFHEstimation::getNormals() { return *normals_; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to add a new line at the end of the file (I think clang format can do this automatically?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there's

InsertFinalNewline: 'yes'

option. May I add that option to .clang_format and add an additional line for all files?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I've noticed that other files also have no blank line; could you tell me why the new line at the end is needed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes about clang format. If the other files do not have newline character at the end, then we should also add them. See https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline

9 changes: 4 additions & 5 deletions teaser/src/matcher.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
namespace teaser {

std::vector<std::pair<int, int>> Matcher::calculateCorrespondences(
teaser::PointCloud& source_points, teaser::PointCloud& target_points,
teaser::FPFHCloud& source_features, teaser::FPFHCloud& target_features, bool use_absolute_scale,
bool use_crosscheck, bool use_tuple_test, float tuple_scale) {
const teaser::PointCloud& source_points, const teaser::PointCloud& target_points,
const teaser::FPFHCloud& source_features, const teaser::FPFHCloud& target_features,
bool use_absolute_scale, bool use_crosscheck, bool use_tuple_test, float tuple_scale) {

Feature cloud_features;
pointcloud_.push_back(source_points);
Expand Down Expand Up @@ -138,9 +138,8 @@ void Matcher::advancedMatching(bool use_crosscheck, bool use_tuple_test, float t
KDTree feature_tree_j(flann::KDTreeSingleIndexParams(15));
buildKDTree(features_[fj], &feature_tree_j);

std::vector<int> corres_K, corres_K2;
std::vector<int> corres_K;
std::vector<float> dis;
std::vector<int> ind;

std::vector<std::pair<int, int>> corres;
std::vector<std::pair<int, int>> corres_cross;
Expand Down
Loading