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 <[email protected]>
isl-org · Nov 13, 2024 · 9d0cfc8 · 9d0cfc8
1 parent fcc396e
commit 9d0cfc8
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 17 deletions.
diff --git 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
@@ -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;