From 9d0cfc86b7591035aa2e030f3cea7af2a4b80755 Mon Sep 17 00:00:00 2001
From: Robin <102535177+rxba@users.noreply.github.com>
Date: Wed, 13 Nov 2024 16:41:54 +0100
Subject: [PATCH] fix infinite loop in segment_plane if num_points < ransac_n
 (#7032)

* fix infinite loop in segment_plane if num_points < ransac_n

* update CHANGELOG.md

* remove superfluous locking in RandomSampler

---------

Co-authored-by: Benjamin Ummenhofer <benjamin.ummenhofer@intel.com>
---
 CHANGELOG.md                                  |  1 +
 .../geometry/PointCloudSegmentation.cpp       | 28 ++++++++-----------
 2 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7fb1cf1e1aa..ba4497f10bc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -51,6 +51,7 @@
 -   Fix tensor EstimatePointWiseNormalsWithFastEigen3x3 (PR #6980)
 -   Fix alpha shape reconstruction if alpha too small for point scale (PR #6998)
 -   Fix render to depth image on Apple Retina displays (PR #7001)
+-   Fix infinite loop in segment_plane if num_points < ransac_n (PR #7032)
 
 ## 0.13
 
diff --git a/cpp/open3d/geometry/PointCloudSegmentation.cpp b/cpp/open3d/geometry/PointCloudSegmentation.cpp
index 514739f026b..8eb0a4972da 100644
--- a/cpp/open3d/geometry/PointCloudSegmentation.cpp
+++ b/cpp/open3d/geometry/PointCloudSegmentation.cpp
@@ -29,7 +29,6 @@ class RandomSampler {
     explicit RandomSampler(const size_t total_size) : total_size_(total_size) {}
 
     std::vector<T> operator()(size_t sample_size) {
-        std::lock_guard<std::mutex> lock(mutex_);
         std::vector<T> samples;
         samples.reserve(sample_size);
 
@@ -43,13 +42,11 @@ class RandomSampler {
                 valid_sample++;
             }
         }
-
         return samples;
     }
 
 private:
     size_t total_size_;
-    std::mutex mutex_;
 };
 
 /// \class RANSACResult
@@ -156,21 +153,7 @@ std::tuple<Eigen::Vector4d, std::vector<size_t>> PointCloud::SegmentPlane(
         utility::LogError("Probability must be > 0 and <= 1.0");
     }
 
-    RANSACResult result;
-
-    // Initialize the best plane model.
-    Eigen::Vector4d best_plane_model = Eigen::Vector4d(0, 0, 0, 0);
-
     size_t num_points = points_.size();
-    RandomSampler<size_t> sampler(num_points);
-    // Pre-generate all random samples before entering the parallel region
-    std::vector<std::vector<size_t>> all_sampled_indices;
-    all_sampled_indices.reserve(num_iterations);
-    for (int i = 0; i < num_iterations; i++) {
-        all_sampled_indices.push_back(sampler(ransac_n));
-    }
-
-    // Return if ransac_n is less than the required plane model parameters.
     if (ransac_n < 3) {
         utility::LogError(
                 "ransac_n should be set to higher than or equal to 3.");
@@ -183,6 +166,17 @@ std::tuple<Eigen::Vector4d, std::vector<size_t>> PointCloud::SegmentPlane(
                                std::vector<size_t>{});
     }
 
+    RANSACResult result;
+    Eigen::Vector4d best_plane_model = Eigen::Vector4d(0, 0, 0, 0);
+
+    RandomSampler<size_t> sampler(num_points);
+    // Pre-generate all random samples before entering the parallel region
+    std::vector<std::vector<size_t>> all_sampled_indices;
+    all_sampled_indices.reserve(num_iterations);
+    for (int i = 0; i < num_iterations; i++) {
+        all_sampled_indices.push_back(sampler(ransac_n));
+    }
+
     // Use size_t here to avoid large integer which acceed max of int.
     size_t break_iteration = std::numeric_limits<size_t>::max();
     int iteration_count = 0;