Add check for invalid plane coefficients in MovingLeastSquares
#2805
Conversation
|
@Levi-Armstrong: Would you please review, since you authored the original code. I made this change already six months ago in our local repository and have been using it since. |
| @@ -738,9 +738,20 @@ pcl::MLSResult::computeMLSSurface (const pcl::PointCloud<PointT> &cloud, | |||
| model_coefficients.head<3> ().matrix () = eigen_vector; | |||
| model_coefficients[3] = -1 * model_coefficients.dot (xyz_centroid); | |||
|
|
|||
| query_point = cloud.points[index].getVector3fMap ().template cast<double> (); | |||
|
|
|||
| if (!pcl_isfinite (eigen_vector[0]) || !pcl_isfinite (eigen_vector[1]) || !pcl_isfinite (eigen_vector[2])) | |||
There was a problem hiding this comment.
We have a pending PR that removes pcl_isfinite macro (#2798). Please use eigen_vector.isFinite() instead.
|
Sorry if I was not clear enough. I only meant that you should not add new code using |
ThorstenHarter
force-pushed
the
master
branch
from
4fa0d0a to
0805899
Compare
|
I have corrected my last commit, so that the |
|
@SergioRAgostinho should we squash on merge? |
|
In the optimal case, I would squash the 1st, 3rd and 4th commits while keeping the 2nd (code inspection one) separated. |
|
@ThorstenHarter would you mind doing this? |
If the input cloud is non-dense, the algorithm creates invalid output points. Keep the input point in this case instead.
- Add const wherever possible - Join local variable declaration and assignment - Initialize structs
ThorstenHarter
force-pushed
the
master
branch
from
38980d1 to
8d1dd8b
Compare
|
@taketwo Done. |
|
Thanks! |
taketwo
changed the title
MovingLeastSquares
Observation:
If the input cloud is non-dense or if the points in the neighborhood are linearly dependent, the algorithm creates invalid output points.
Proposal:
Check the values of the computed eigen vector for NaN (line 740 ff.) and keep the input point in this case instead (53af42f).