From 94b5bd10d12f003079269bca94ec8d8846c79f68 Mon Sep 17 00:00:00 2001 From: Atsushi Watanabe Date: Thu, 25 Dec 2025 10:45:39 +0900 Subject: [PATCH 1/5] Add test to reproduce ConvexHull infinite loop --- test/surface/test_convex_hull.cpp | 75 +++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/test/surface/test_convex_hull.cpp b/test/surface/test_convex_hull.cpp index f814cb89ae8..0c9970bb19f 100644 --- a/test/surface/test_convex_hull.cpp +++ b/test/surface/test_convex_hull.cpp @@ -39,7 +39,11 @@ #include +#include +#include +#include #include +#include #include #include @@ -484,6 +488,77 @@ TEST (PCL, ConvexHull_4points) EXPECT_NEAR (convex_hull.getTotalArea (), 1.0f, 1e-6); } +TEST (PCL, ConvexHull_insufficient_points) +{ + // ConvexHull must not enter an infinite busy loop + // even if insufficient points are passed + + // PCL 1.15.0 and 1.15.1 enter infinite busy loop if insufficient points are passed. + // Use EXPECT_EXIT to run the convex hull calculation in a sub-process to + // safely check the timeout. + const auto check_no_timeout = [](pcl::PointCloud::Ptr& cloud) { + std::atomic_bool finished = false; + std::thread thread([&]() { + using namespace std::chrono_literals; + std::this_thread::sleep_for(1s); + if (!finished) { + abort(); // Abort if deadline exceeded + } + }); + + ConvexHull convex_hull; + convex_hull.setComputeAreaVolume(true); + convex_hull.setInputCloud(cloud); + + PolygonMesh mesh; + convex_hull.reconstruct(mesh); + finished = true; + EXPECT_EQ(mesh.polygons.size(), 0); + thread.join(); + exit(0); // Exit if process returned + }; + + pcl::PointCloud::Ptr input_cloud(new pcl::PointCloud()); + pcl::PointXYZ p; + p.x = p.y = p.z = 0.f; + input_cloud->push_back(p); + + EXPECT_EXIT( + { + SCOPED_TRACE("1 point"); + check_no_timeout(input_cloud); + }, + ::testing::ExitedWithCode(0), + ".*") + << "Did not return against 1 point input"; + + p.x = 0.1f; + p.y = p.z = 0.f; + input_cloud->push_back(p); + + EXPECT_EXIT( + { + SCOPED_TRACE("2 points"); + check_no_timeout(input_cloud); + }, + ::testing::ExitedWithCode(0), + ".*") + << "Did not return against 2 point input"; + + p.x = 0.2f; + p.y = p.z = 0.f; + input_cloud->push_back(p); + + EXPECT_EXIT( + { + SCOPED_TRACE("3 points on a line"); + check_no_timeout(input_cloud); + }, + ::testing::ExitedWithCode(0), + ".*") + << "Did not return against 3 point input on a line"; +} + /* ---[ */ int main (int argc, char** argv) From 0bcb1a448122987f60dd6e68cfb58e92c61c52e4 Mon Sep 17 00:00:00 2001 From: Atsushi Watanabe Date: Thu, 25 Dec 2025 11:33:47 +0900 Subject: [PATCH 2/5] Limit number of initial convex selection retry in ConvexHull --- surface/include/pcl/surface/convex_hull.h | 20 +++++++++++++++++++ .../include/pcl/surface/impl/convex_hull.hpp | 4 ++++ 2 files changed, 24 insertions(+) diff --git a/surface/include/pcl/surface/convex_hull.h b/surface/include/pcl/surface/convex_hull.h index 330a4b80e51..4590230bc47 100644 --- a/surface/include/pcl/surface/convex_hull.h +++ b/surface/include/pcl/surface/convex_hull.h @@ -162,6 +162,23 @@ namespace pcl return (dimension_); } + /** \brief Sets the number of the initial convex selection retry. + * \param[in] num_initial_convex_selection_retry The number of the initial convex + * selection retry, defalut is 10000 + */ + void + setNumInitialConvexSelectionRetry(int num_initial_convex_selection_retry) + { + num_initial_convex_selection_retry_ = num_initial_convex_selection_retry; + } + + /** \brief Returns the number of the initial convex selection retry. */ + inline int + getNumInitialConvexSelectionRetry() const + { + return (num_initial_convex_selection_retry_); + } + /** \brief Retrieve the indices of the input point cloud that for the convex hull. * * \note Should only be called after reconstruction was performed. @@ -244,6 +261,9 @@ namespace pcl /** \brief The dimensionality of the concave hull (2D or 3D). */ int dimension_{0}; + /** \brief The number of the initial convex selection retry. */ + int num_initial_convex_selection_retry_{10000}; + /** \brief How close can a 2D plane's normal be to an axis to make projection problematic. */ double projection_angle_thresh_{std::cos (0.174532925)}; diff --git a/surface/include/pcl/surface/impl/convex_hull.hpp b/surface/include/pcl/surface/impl/convex_hull.hpp index d17f47b9d11..0e9bda43014 100644 --- a/surface/include/pcl/surface/impl/convex_hull.hpp +++ b/surface/include/pcl/surface/impl/convex_hull.hpp @@ -85,9 +85,13 @@ pcl::ConvexHull::performReconstruction2D (PointCloud &hull, std::vecto PointInT p0 = (*input_)[(*indices_)[0]]; PointInT p1 = (*input_)[(*indices_)[indices_->size () - 1]]; PointInT p2 = (*input_)[(*indices_)[indices_->size () / 2]]; + int retries = num_initial_convex_selection_retry_; while (!pcl::isXYZFinite(p0) || !pcl::isXYZFinite(p1) || !pcl::isXYZFinite(p2) || (p1.getVector3fMap() - p0.getVector3fMap()).cross(p2.getVector3fMap() - p0.getVector3fMap()).stableNorm() < Eigen::NumTraits::dummy_precision ()) { + if (retries-- <= 0) { + break; + } p0 = (*input_)[(*indices_)[rand () % indices_->size ()]]; p1 = (*input_)[(*indices_)[rand () % indices_->size ()]]; p2 = (*input_)[(*indices_)[rand () % indices_->size ()]]; From 70ae28e69b197e3d849670e8da542183b1e51889 Mon Sep 17 00:00:00 2001 From: Atsushi Watanabe Date: Thu, 25 Dec 2025 12:29:54 +0900 Subject: [PATCH 3/5] Fix build error on some compilers --- test/surface/test_convex_hull.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/surface/test_convex_hull.cpp b/test/surface/test_convex_hull.cpp index 0c9970bb19f..c429255beef 100644 --- a/test/surface/test_convex_hull.cpp +++ b/test/surface/test_convex_hull.cpp @@ -497,7 +497,7 @@ TEST (PCL, ConvexHull_insufficient_points) // Use EXPECT_EXIT to run the convex hull calculation in a sub-process to // safely check the timeout. const auto check_no_timeout = [](pcl::PointCloud::Ptr& cloud) { - std::atomic_bool finished = false; + std::atomic_bool finished(false); std::thread thread([&]() { using namespace std::chrono_literals; std::this_thread::sleep_for(1s); From 58f245d6f435c9e50fd8ecf8ab2d983021194cc3 Mon Sep 17 00:00:00 2001 From: Atsushi Watanabe Date: Fri, 26 Dec 2025 10:32:41 +0900 Subject: [PATCH 4/5] Fallback to use all points to calculate covariance instead of retry --- surface/include/pcl/surface/convex_hull.h | 20 --------- .../include/pcl/surface/impl/convex_hull.hpp | 44 ++++++++++--------- 2 files changed, 23 insertions(+), 41 deletions(-) diff --git a/surface/include/pcl/surface/convex_hull.h b/surface/include/pcl/surface/convex_hull.h index 4590230bc47..330a4b80e51 100644 --- a/surface/include/pcl/surface/convex_hull.h +++ b/surface/include/pcl/surface/convex_hull.h @@ -162,23 +162,6 @@ namespace pcl return (dimension_); } - /** \brief Sets the number of the initial convex selection retry. - * \param[in] num_initial_convex_selection_retry The number of the initial convex - * selection retry, defalut is 10000 - */ - void - setNumInitialConvexSelectionRetry(int num_initial_convex_selection_retry) - { - num_initial_convex_selection_retry_ = num_initial_convex_selection_retry; - } - - /** \brief Returns the number of the initial convex selection retry. */ - inline int - getNumInitialConvexSelectionRetry() const - { - return (num_initial_convex_selection_retry_); - } - /** \brief Retrieve the indices of the input point cloud that for the convex hull. * * \note Should only be called after reconstruction was performed. @@ -261,9 +244,6 @@ namespace pcl /** \brief The dimensionality of the concave hull (2D or 3D). */ int dimension_{0}; - /** \brief The number of the initial convex selection retry. */ - int num_initial_convex_selection_retry_{10000}; - /** \brief How close can a 2D plane's normal be to an axis to make projection problematic. */ double projection_angle_thresh_{std::cos (0.174532925)}; diff --git a/surface/include/pcl/surface/impl/convex_hull.hpp b/surface/include/pcl/surface/impl/convex_hull.hpp index 0e9bda43014..989b09d4a3d 100644 --- a/surface/include/pcl/surface/impl/convex_hull.hpp +++ b/surface/include/pcl/surface/impl/convex_hull.hpp @@ -81,32 +81,34 @@ pcl::ConvexHull::performReconstruction2D (PointCloud &hull, std::vecto bool yz_proj_safe = true; bool xz_proj_safe = true; + Eigen::Vector4d normal_calc_centroid; + Eigen::Matrix3d normal_calc_covariance; + // Check the input's normal to see which projection to use PointInT p0 = (*input_)[(*indices_)[0]]; PointInT p1 = (*input_)[(*indices_)[indices_->size () - 1]]; PointInT p2 = (*input_)[(*indices_)[indices_->size () / 2]]; - int retries = num_initial_convex_selection_retry_; - while (!pcl::isXYZFinite(p0) || !pcl::isXYZFinite(p1) || !pcl::isXYZFinite(p2) || - (p1.getVector3fMap() - p0.getVector3fMap()).cross(p2.getVector3fMap() - p0.getVector3fMap()).stableNorm() < Eigen::NumTraits::dummy_precision ()) - { - if (retries-- <= 0) { - break; - } - p0 = (*input_)[(*indices_)[rand () % indices_->size ()]]; - p1 = (*input_)[(*indices_)[rand () % indices_->size ()]]; - p2 = (*input_)[(*indices_)[rand () % indices_->size ()]]; + if (pcl::isXYZFinite(p0) && pcl::isXYZFinite(p1) && pcl::isXYZFinite(p2) && + (p1.getVector3fMap() - p0.getVector3fMap()) + .cross(p2.getVector3fMap() - p0.getVector3fMap()) + .stableNorm() > Eigen::NumTraits::dummy_precision()) { + pcl::PointCloud normal_calc_cloud; + normal_calc_cloud.resize(3); + normal_calc_cloud[0] = p0; + normal_calc_cloud[1] = p1; + normal_calc_cloud[2] = p2; + + pcl::compute3DCentroid(normal_calc_cloud, normal_calc_centroid); + pcl::computeCovarianceMatrixNormalized( + normal_calc_cloud, normal_calc_centroid, normal_calc_covariance); + } + else { + // Three points do not form a valid triangle, fallback to use all points to + // calculate the covariance matrix + pcl::compute3DCentroid(*input_, normal_calc_centroid); + pcl::computeCovarianceMatrixNormalized( + *input_, normal_calc_centroid, normal_calc_covariance); } - - pcl::PointCloud normal_calc_cloud; - normal_calc_cloud.resize (3); - normal_calc_cloud[0] = p0; - normal_calc_cloud[1] = p1; - normal_calc_cloud[2] = p2; - - Eigen::Vector4d normal_calc_centroid; - Eigen::Matrix3d normal_calc_covariance; - pcl::compute3DCentroid (normal_calc_cloud, normal_calc_centroid); - pcl::computeCovarianceMatrixNormalized (normal_calc_cloud, normal_calc_centroid, normal_calc_covariance); // Need to set -1 here. See eigen33 for explanations. Eigen::Vector3d::Scalar eigen_value; From 76f37a788bb153048a0a82b89bc3ed92d5a24f0a Mon Sep 17 00:00:00 2001 From: Atsushi Watanabe Date: Sat, 27 Dec 2025 10:43:30 +0900 Subject: [PATCH 5/5] Use indices to calculate normal Co-authored-by: Markus Vieth <39675748+mvieth@users.noreply.github.com> --- surface/include/pcl/surface/impl/convex_hull.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/surface/include/pcl/surface/impl/convex_hull.hpp b/surface/include/pcl/surface/impl/convex_hull.hpp index 989b09d4a3d..6fff1f5c1bd 100644 --- a/surface/include/pcl/surface/impl/convex_hull.hpp +++ b/surface/include/pcl/surface/impl/convex_hull.hpp @@ -105,9 +105,9 @@ pcl::ConvexHull::performReconstruction2D (PointCloud &hull, std::vecto else { // Three points do not form a valid triangle, fallback to use all points to // calculate the covariance matrix - pcl::compute3DCentroid(*input_, normal_calc_centroid); + pcl::compute3DCentroid(*input_, *indices_, normal_calc_centroid); pcl::computeCovarianceMatrixNormalized( - *input_, normal_calc_centroid, normal_calc_covariance); + *input_, *indices_, normal_calc_centroid, normal_calc_covariance); } // Need to set -1 here. See eigen33 for explanations.