Skip to content

Commit 6ba19c8

Browse files
committed
Apply PR6199 review fixes without build artifacts
1 parent b45c4ea commit 6ba19c8

File tree

7 files changed

+56
-159
lines changed

7 files changed

+56
-159
lines changed

registration/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ set(srcs
162162
src/transformation_estimation_point_to_plane_weighted.cpp
163163
src/transformation_estimation_point_to_plane_lls.cpp
164164
src/transformation_estimation_point_to_plane_lls_weighted.cpp
165-
src/transformation_estimation_point_to_point_robust.cpp
166165
src/transformation_validation_euclidean.cpp
167166
src/sample_consensus_prerejective.cpp
168167
)

registration/include/pcl/registration/anderson_acceleration.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,11 @@ class AndersonAcceleration {
139139
}
140140
else {
141141
// update Gram matrix row/column corresponding to the newest column
142-
const Eigen::VectorXd new_inner_prod = prev_dF_.col(column_index_).transpose() *
143-
prev_dF_.block(0, 0, dimension_, m_k);
144-
normal_eq_matrix_.block(column_index_, 0, 1, m_k) = new_inner_prod.transpose();
145-
normal_eq_matrix_.block(0, column_index_, m_k, 1) = new_inner_prod;
142+
const Eigen::RowVectorXd new_inner_prod =
143+
prev_dF_.col(column_index_).transpose() *
144+
prev_dF_.block(0, 0, dimension_, m_k);
145+
normal_eq_matrix_.block(column_index_, 0, 1, m_k) = new_inner_prod;
146+
normal_eq_matrix_.block(0, column_index_, m_k, 1) = new_inner_prod.transpose();
146147

147148
cod_.compute(normal_eq_matrix_.block(0, 0, m_k, m_k));
148149
theta_.head(m_k) =

registration/include/pcl/registration/fricp.h

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,10 @@
11
/*
2-
* Software License Agreement (BSD License)
2+
* SPDX-License-Identifier: BSD-3-Clause
33
*
44
* Point Cloud Library (PCL) - www.pointclouds.org
55
* Copyright (c) 2010-, Open Perception, Inc.
66
*
77
* All rights reserved.
8-
*
9-
* Redistribution and use in source and binary forms, with or without
10-
* modification, are permitted provided that the following conditions
11-
* are met:
12-
*
13-
* * Redistributions of source code must retain the above copyright
14-
* notice, this list of conditions and the following disclaimer.
15-
* * Redistributions in binary form must reproduce the above
16-
* copyright notice, this list of conditions and the following
17-
* disclaimer in the documentation and/or other materials provided
18-
* with the distribution.
19-
* * Neither the name of the copyright holder(s) nor the names of its
20-
* contributors may be used to endorse or promote products derived
21-
* from this software without specific prior written permission.
22-
*
23-
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
24-
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
25-
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
26-
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
27-
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
28-
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
29-
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
30-
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
31-
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
32-
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
33-
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
34-
* POSSIBILITY OF SUCH DAMAGE.
35-
*
368
*/
379

3810
#pragma once
@@ -46,12 +18,13 @@
4618

4719
#include <Eigen/Core>
4820

21+
#include <cmath>
4922
#include <vector>
5023

5124
namespace pcl {
5225
/**
5326
* \brief FastRobustIterativeClosestPoint implements the FRICP variant described in
54-
* "Fast and Robust Iterative Closest Point", Zhang et al., 2022.
27+
* "Fast and Robust Iterative Closest Point", Zhang et al., 2021.
5528
*
5629
* The solver relies on Welsch reweighting for robustness and optional Anderson
5730
* acceleration for faster convergence.
@@ -174,7 +147,7 @@ class FastRobustIterativeClosestPoint
174147

175148
double
176149
findKNearestMedian(const pcl::PointCloud<pcl::PointXYZ>& cloud,
177-
pcl::search::KdTree<pcl::PointXYZ>& tree,
150+
pcl::search::Search<pcl::PointXYZ>& tree,
178151
int neighbors) const;
179152

180153
Matrix4d

registration/include/pcl/registration/impl/fricp.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ template <typename PointSource, typename PointTarget, typename Scalar>
449449
double
450450
FastRobustIterativeClosestPoint<PointSource, PointTarget, Scalar>::findKNearestMedian(
451451
const pcl::PointCloud<pcl::PointXYZ>& cloud,
452-
pcl::search::KdTree<pcl::PointXYZ>& tree,
452+
pcl::search::Search<pcl::PointXYZ>& tree,
453453
int neighbors) const
454454
{
455455
if (cloud.empty() || neighbors < 2)

registration/include/pcl/registration/impl/transformation_estimation_point_to_point_robust.hpp

Lines changed: 42 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,20 @@
11
/*
2-
* Software License Agreement (BSD License)
2+
* SPDX-License-Identifier: BSD-3-Clause
33
*
44
* Point Cloud Library (PCL) - www.pointclouds.org
55
* Copyright (c) 2010, Willow Garage, Inc.
66
* Copyright (c) 2012-, Open Perception, Inc.
77
*
88
* All rights reserved.
9-
*
10-
* Redistribution and use in source and binary forms, with or without
11-
* modification, are permitted provided that the following conditions
12-
* are met:
13-
*
14-
* * Redistributions of source code must retain the above copyright
15-
* notice, this list of conditions and the following disclaimer.
16-
* * Redistributions in binary form must reproduce the above
17-
* copyright notice, this list of conditions and the following
18-
* disclaimer in the documentation and/or other materials provided
19-
* with the distribution.
20-
* * Neither the name of the copyright holder(s) nor the names of its
21-
* contributors may be used to endorse or promote products derived
22-
* from this software without specific prior written permission.
23-
*
24-
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
25-
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
26-
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
27-
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
28-
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
29-
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
30-
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
31-
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
32-
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
33-
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
34-
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
35-
* POSSIBILITY OF SUCH DAMAGE.
36-
*
37-
* $Id$
38-
*
399
*/
4010

4111
#ifndef PCL_REGISTRATION_TRANSFORMATION_ESTIMATION_POINT_TO_POINT_ROBUST_HPP_
4212
#define PCL_REGISTRATION_TRANSFORMATION_ESTIMATION_POINT_TO_POINT_ROBUST_HPP_
4313

4414
#include <pcl/common/eigen.h>
4515

16+
#include <limits>
17+
4618
namespace pcl {
4719

4820
namespace registration {
@@ -58,7 +30,7 @@ TransformationEstimationPointToPointRobust<PointSource, PointTarget, Scalar>::
5830
if (cloud_tgt.size() != nr_points) {
5931
PCL_ERROR("[pcl::TransformationEstimationPointToPointRobust::"
6032
"estimateRigidTransformation] Number "
61-
"or points in source (%zu) differs than target (%zu)!\n",
33+
"of points in source (%zu) differs than target (%zu)!\n",
6234
static_cast<std::size_t>(nr_points),
6335
static_cast<std::size_t>(cloud_tgt.size()));
6436
return;
@@ -78,7 +50,8 @@ TransformationEstimationPointToPointRobust<PointSource, PointTarget, Scalar>::
7850
Matrix4& transformation_matrix) const
7951
{
8052
if (indices_src.size() != cloud_tgt.size()) {
81-
PCL_ERROR("[pcl::TransformationSVD::estimateRigidTransformation] Number or points "
53+
PCL_ERROR("[pcl::TransformationEstimationPointToPointRobust::"
54+
"estimateRigidTransformation] Number of points "
8255
"in source (%zu) differs than target (%zu)!\n",
8356
indices_src.size(),
8457
static_cast<std::size_t>(cloud_tgt.size()));
@@ -100,8 +73,9 @@ TransformationEstimationPointToPointRobust<PointSource, PointTarget, Scalar>::
10073
Matrix4& transformation_matrix) const
10174
{
10275
if (indices_src.size() != indices_tgt.size()) {
103-
PCL_ERROR("[pcl::TransformationEstimationSVD::estimateRigidTransformation] Number "
104-
"or points in source (%zu) differs than target (%zu)!\n",
76+
PCL_ERROR("[pcl::TransformationEstimationPointToPointRobust::"
77+
"estimateRigidTransformation] Number of points "
78+
"in source (%zu) differs than target (%zu)!\n",
10579
indices_src.size(),
10680
indices_tgt.size());
10781
return;
@@ -134,6 +108,11 @@ TransformationEstimationPointToPointRobust<PointSource, PointTarget, Scalar>::
134108
{
135109
// Convert to Eigen format
136110
const int npts = static_cast<int>(source_it.size());
111+
if (npts == 0) {
112+
PCL_ERROR("[pcl::TransformationEstimationPointToPointRobust::"
113+
"estimateRigidTransformation] Empty correspondence set.\n");
114+
return;
115+
}
137116
source_it.reset();
138117
target_it.reset();
139118
Eigen::Matrix<Scalar, Eigen::Dynamic, 1> weights(npts);
@@ -149,16 +128,21 @@ TransformationEstimationPointToPointRobust<PointSource, PointTarget, Scalar>::
149128
target_it++;
150129
}
151130

131+
const Scalar epsilon = std::numeric_limits<Scalar>::epsilon();
152132
Scalar sigma2;
153133
if (sigma_ < 0)
154-
sigma2 = square_distances.maxCoeff() / 9.0;
134+
sigma2 = std::max(square_distances.maxCoeff() / Scalar(9.0), epsilon);
155135
else
156-
sigma2 = sigma_ * sigma_;
136+
sigma2 = std::max(sigma_ * sigma_, epsilon);
157137

158138
for (int i = 0; i < npts; i++) {
159-
weights[i] = std::exp(-square_distances[i] / (2.0 * sigma2));
139+
weights[i] = std::exp(-square_distances[i] / (Scalar(2.0) * sigma2));
160140
}
161-
weights = weights / weights.sum();
141+
const Scalar weights_sum = weights.sum();
142+
if (weights_sum > epsilon)
143+
weights /= weights_sum;
144+
else
145+
weights.setConstant(Scalar(1.0) / static_cast<Scalar>(npts));
162146

163147
source_it.reset();
164148
target_it.reset();
@@ -225,10 +209,15 @@ TransformationEstimationPointToPointRobust<PointSource, PointTarget, Scalar>::
225209
centroid_tgt.template head<3>() - Rc;
226210

227211
if (pcl::console::isVerbosityLevelEnabled(pcl::console::L_DEBUG)) {
228-
size_t N = cloud_src_demean.cols();
229-
PCL_DEBUG("[pcl::registration::TransformationEstimationPointToPointRobust::"
230-
"getTransformationFromCorrelation] Loss: %.10e\n",
231-
(cloud_tgt_demean - R * cloud_src_demean).squaredNorm() / N);
212+
const auto N = cloud_src_demean.cols();
213+
if (N > 0) {
214+
PCL_DEBUG("[pcl::registration::TransformationEstimationPointToPointRobust::"
215+
"getTransformationFromCorrelation] Loss: %.10e\n",
216+
(cloud_tgt_demean.template topRows<3>() -
217+
R * cloud_src_demean.template topRows<3>())
218+
.squaredNorm() /
219+
static_cast<double>(N));
220+
}
232221
}
233222
}
234223

@@ -243,22 +232,27 @@ TransformationEstimationPointToPointRobust<PointSource, PointTarget, Scalar>::
243232
Eigen::Matrix<Scalar, 4, 1> accumulator{0, 0, 0, 0};
244233

245234
unsigned int cp = 0;
235+
Scalar sum_weights = Scalar(0);
236+
Eigen::Index weight_idx = 0;
246237

247238
// For each point in the cloud
248239
// If the data is dense, we don't need to check for NaN
249240
while (cloud_iterator.isValid()) {
250241
// Check if the point is invalid
251242
if (pcl::isFinite(*cloud_iterator)) {
252-
accumulator[0] += weights[cp] * cloud_iterator->x;
253-
accumulator[1] += weights[cp] * cloud_iterator->y;
254-
accumulator[2] += weights[cp] * cloud_iterator->z;
243+
const Scalar w = weights[weight_idx];
244+
accumulator[0] += w * cloud_iterator->x;
245+
accumulator[1] += w * cloud_iterator->y;
246+
accumulator[2] += w * cloud_iterator->z;
247+
sum_weights += w;
255248
++cp;
256249
}
250+
++weight_idx;
257251
++cloud_iterator;
258252
}
259253

260-
if (cp > 0) {
261-
centroid = accumulator;
254+
if (cp > 0 && sum_weights > std::numeric_limits<Scalar>::epsilon()) {
255+
centroid = accumulator / sum_weights;
262256
centroid[3] = 1;
263257
}
264258
return (cp);

registration/include/pcl/registration/transformation_estimation_point_to_point_robust.h

Lines changed: 4 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,11 @@
11
/*
2-
* Software License Agreement (BSD License)
2+
* SPDX-License-Identifier: BSD-3-Clause
33
*
44
* Point Cloud Library (PCL) - www.pointclouds.org
55
* Copyright (c) 2010-2011, Willow Garage, Inc.
66
* Copyright (c) 2012-, Open Perception, Inc.
77
*
88
* All rights reserved.
9-
*
10-
* Redistribution and use in source and binary forms, with or without
11-
* modification, are permitted provided that the following conditions
12-
* are met:
13-
*
14-
* * Redistributions of source code must retain the above copyright
15-
* notice, this list of conditions and the following disclaimer.
16-
* * Redistributions in binary form must reproduce the above
17-
* copyright notice, this list of conditions and the following
18-
* disclaimer in the documentation and/or other materials provided
19-
* with the distribution.
20-
* * Neither the name of the copyright holder(s) nor the names of its
21-
* contributors may be used to endorse or promote products derived
22-
* from this software without specific prior written permission.
23-
*
24-
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
25-
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
26-
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
27-
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
28-
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
29-
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
30-
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
31-
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
32-
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
33-
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
34-
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
35-
* POSSIBILITY OF SUCH DAMAGE.
36-
*
37-
* $Id$
38-
*
399
*/
4010

4111
#pragma once
@@ -49,7 +19,7 @@ namespace registration {
4919
/** @b TransformationEstimationPointToPointRobust implements SVD-based estimation of
5020
* the transformation aligning the given correspondences for minimizing the Welsch
5121
* function instead of L2-norm, For additional details, see "Fast and Robust Iterative
52-
* Closest Point", Juyong Zhang, Yuxin Yao, Bailin Deng, 2022.
22+
* Closest Point", Juyong Zhang, Yuxin Yao, Bailin Deng, 2021.
5323
* \note The class is templated on the source and target point types as well as on the
5424
* output scalar of the transformation matrix (i.e., float or double). Default: float.
5525
* \author Yuxin Yao
@@ -69,8 +39,7 @@ class TransformationEstimationPointToPointRobust
6939
using Matrix4 =
7040
typename TransformationEstimation<PointSource, PointTarget, Scalar>::Matrix4;
7141

72-
/** \brief Constructor
73-
* \param[in] use_umeyama Toggles whether or not to use 3rd party software*/
42+
/** \brief Constructor */
7443
TransformationEstimationPointToPointRobust() = default;
7544

7645
~TransformationEstimationPointToPointRobust() override = default;
@@ -134,7 +103,7 @@ class TransformationEstimationPointToPointRobust
134103
* estimator closer to standard least-squares behavior.
135104
*/
136105
void
137-
set_sigma(Scalar sigma)
106+
setSigma(Scalar sigma)
138107
{
139108
sigma_ = sigma;
140109
};

registration/src/transformation_estimation_point_to_point_robust.cpp

Lines changed: 0 additions & 39 deletions
This file was deleted.

0 commit comments

Comments
 (0)