Regression fix for commit #646

[VictorLamoine]

Fix matrix size error in planeWithPlaneIntersection.
This error leads to undefined behaviour when using << operator

http://stackoverflow.com/questions/23169570/why-does-this-code-compile

[VictorLamoine]

I don't think it is possible to write a regression test for this.

[taketwo]

I don't think it is possible to write a regression test for this.

Why not? I'm not concerned with initializing 5x3 matrix with 25 values, that's okay. What bothers me is that instead of 5x5 system of equations we constructed 5x3 system, solved, and the result was correct. There should be a case where the result is wrong, otherwise why do we need that extra 2 columns in the system at all?

[VictorLamoine]

We would need to enable Eigen assertions in the test unit, it would throw an assert error like this (without changing the code):
test_eigen: /usr/include/eigen3/Eigen/src/Core/CommaInitializer.h:55: Eigen::CommaInitializer<MatrixType>& Eigen::CommaInitializer<MatrixType>::operator,(const Scalar&) [with XprType = Eigen::Matrix<double, 5, 3>; Eigen::CommaInitializer<MatrixType>::Scalar = double]: Assertion m_row<m_xpr.rows() && "Too many rows passed to comma initializer (operator<<)"' failed.
Aborted (core dumped)`

I guess that without assertions;

• Sometimes the data structure is filled with the right values and the data can be accessed
• Sometimes; it's not and it crashes

[taketwo]

Sometimes the data structure is filled with the right values and the data can be accessed

It does not really matter with which values the structure is filled. You declared it as 5x3 matrix, therefore in line 118 a 5x3 system is solved.

[VictorLamoine]

That puzzled me; In fact in the test unit of planeWithPlaneIntersection the result is never tested !
I didn't see that until now. I'll rewrite the test unit.

[taketwo]

Indeed, only the returned bool is tested! Would be very nice if you could add a check for the plane coefficients as well. If it was there before we would discover the "5x3" bug earlier.

[VictorLamoine]

Here are the changes :

• The angular test was working only with normalized plane normals (Hessian form); this is why the test unit was strange. Now it's working even if plane normals are not normalized.
• The test unit has been rewritten and values are tested. The test won't pass without the fix.

Test unit works:
https://travis-ci.org/PointCloudLibrary/pcl/jobs/23421748#L2267

[taketwo]

Hi Victor, sorry for the delay. Thanks for your work, merging it!