Fix segfault (infinite recursion) of PointCloud::DetectPlanarPatches …

[nicolaloi]

…if multiple points have same coordinates

Type

• Bug fix (non-breaking change which fixes an issue): Fixes PointCloud.detect_planar_patches() Crushes When the PointCloud has 10 or more Points with the Same Coordinates #6727
• New feature (non-breaking change which adds functionality). Resolves #
• Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

The PointCloud::DetectPlanarPatches function will raise a Segmentation Fault due to infinite recursion when the point cloud has at least N points with the same coordinates, where N is the value of the function's argument min_num_points.

Checklist:

• I have run python util/check_style.py --apply to apply Open3D code style
  to my code.
• This PR changes Open3D behavior or adds new functionality.
  • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
    updated accordingly.
  • I have added or updated C++ and / or Python unit tests OR included test
    results (e.g. screenshots or numbers) here.
• I will follow up and update the code if CI fails.
• For fork PRs, I have selected Allow edits from maintainers.

Description

The PointCloud::DetectPlanarPatches function calls SplitAndDetectPlanesRecursive, a recursive function that recursively splits the point cloud into 8 smaller point clouds (children nodes). This function continues to call itself until no BoundaryVolumeHierarchy node can be partitioned.

However, since the stop criteria to not partition a node into new children nodes is (indices_.size() <= min_points_ || child_size < min_size_ || indices_.size() < 2) in PointCloudPlanarPatchDetection.cpp#L90 (where indices_.size() is the n. of points in the node, min_points_ is hardcoded 1, and min_size_ is hardcoded 0) and if (node->indices_.size() < min_num_points) return false; in PointCloudPlanarPatchDetection.cpp#L737 (where min_num_points is the argument of DetectPlanarPatches), with the current parameters if the point cloud contains >= min_num_points points with same coordinates, no stop criteria will ever be satisfied.

Hence, this PR simply changes the value of min_size_ from 0 to 1e-10. This value is small enough (1 tenth of a nanometer) to not interfere with microscopic point clouds, but big enough to avoid a too-deep recursion.

With this change, the segfault occurrence due to infinite recursion is fixed, while before it was jointly dependent on min_num_points and the number of points with the same coordinates

Test

Testing code (modify N_EQUAL_POINTS and PLANAR_DETECTION_MIN_POINTS to test the segfault edge case)

import open3d as o3d
import numpy as np

# PLAY WITH THESE VALUES TO REPRODUCE SEGFAULT:
# if N_EQUAL_POINTS is >= than PLANAR_DETECTION_MIN_POINTS, detect_planar_patches will segfault
N_EQUAL_POINTS = 10
PLANAR_DETECTION_MIN_POINTS = 10

pcd = o3d.geometry.PointCloud()
# define two planes
plane_a = np.random.rand(1000, 3)
plane_a[:, 2] *= 0.01
plane_b = np.random.rand(1000, 3)
plane_b[:, 1] *= 0.01
plane_b[:, 1] += 0.5

# prepare N points with the same coordinates
plane_b[:N_EQUAL_POINTS, :] = [0, 1, 2]
pcd.points = o3d.utility.Vector3dVector(np.vstack((plane_a, plane_b)))

pcd.estimate_normals(search_param=o3d.geometry.KDTreeSearchParamKNN(knn=30))

# if N_EQUAL_POINTS is >= than PLANAR_DETECTION_MIN_POINTS, the following line will segfault
planar_boxes = pcd.detect_planar_patches(min_num_points=PLANAR_DETECTION_MIN_POINTS)

print(planar_boxes)

geometries = [pcd]
for i, plane in enumerate(planar_boxes):
    mesh = o3d.geometry.TriangleMesh.create_from_oriented_bounding_box(plane, scale=[1,1,0.0001])
    mesh.paint_uniform_color(plane.color)
    geometries.append(mesh)
    geometries.append(plane)

o3d.visualization.draw_geometries(geometries)

Output before the change

...
[Open3D WARNING] node level: 43602,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43603,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43604,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43605,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43606,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43607,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43608,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43609,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43610,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43611,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43612,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43613,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43614,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43615,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43616,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43617,  node size: 0, node n. points: 10
[Open3D WARNING] node level: 43618,  node size: 0, node n. points: 10
Segmentation fault (core dumped)

Output with new change

...
[Open3D WARNING] node level: 29,  node size: 3.725280891510182e-09, node n. points: 10
[Open3D WARNING] node level: 30,  node size: 1.862640445755091e-09, node n. points: 10
[Open3D WARNING] node level: 31,  node size: 9.313202228775455e-10, node n. points: 10
[Open3D WARNING] node level: 32,  node size: 4.656601114387727e-10, node n. points: 10
[Open3D WARNING] node level: 33,  node size: 2.3283005571938636e-10, node n. points: 10
[Open3D WARNING] node level: 34,  node size: 1.1641502785969318e-10, node n. points: 10
...
[Open3D WARNING] node level: 3,  node size: 0.24999936871015555, node n. points: 48
[Open3D WARNING] node level: 4,  node size: 0.12499968435507777, node n. points: 12
[Open3D WARNING] node level: 4,  node size: 0.12499968435507777, node n. points: 12
[Open3D WARNING] node level: 4,  node size: 0.12499968435507777, node n. points: 15
[OrientedBoundingBox: center: (0.495142, 0.505172, 0.521091), extent: 0.968814, 1.01205, 0.00942836),
OrientedBoundingBox: center: (0.500209, 0.500061, 0.00513315), extent: 1.00919, 1.01789, 0.00849829)] 

Warning logs were obtained by adding

utility::LogWarning("node level: {},  node size: {}, node n. points: {}", node->level_, node->size_, node->indices_.size());

at line 737 of PointCloudPlanarPatchDetection.cpp (after node->Partition();)

---

Note:

• the node level is not the actual recursion level, it's the node partition level.
• if argument min_num_points is not specified, it will be set to 0 by the pybinding, and then in the DetectPlanaPatches function: if (min_num_points == 0) { min_num_points = std::max(static_cast<size_t>(10), static_cast<size_t>(points_.size() * 0.001)); }
• it may seem you could just use the DetectPlanarPatches' argument min_num_points to set the min_size_ value for the stop criterion of the node partitioning (instead of the hardcoded 1), but in reality, this will only change the segfault condition from >= min_num_points with same coordinates to >min_num_points with same coordinates