Add threePlanesIntersection #571
Conversation
| EXPECT_EQ(1.0f, plane_a.x()); | ||
| EXPECT_EQ(0.0f, plane_a.y()); | ||
| EXPECT_EQ(0.0f, plane_a.z()); | ||
| EXPECT_EQ(-0.5f, plane_a.w()); |
There was a problem hiding this comment.
What is the purpose of these checks? Do you suspect that assignment won't work for Eigen::Vector4f? :)
There was a problem hiding this comment.
Sounded weird to me; in the original files it is done several times to check the assignement. Lines 207->210 etc.. Should I remove it ?
There was a problem hiding this comment.
Well... sometimes existing code-base is not the best example. These checks are totally useless (apart from the fact that EXPECT_FLOAT_EQ should be used to check floating point numbers).
There was a problem hiding this comment.
I removed the useless assignement check and replaced checks to EXPECT_FLOAT_EQ
|
I am not sure if the function name is good. It sounds like it will compute an intersection of three planes and a point (which of course does not make sense). Hmm,
Any other ideas? |
|
I like |
|
Nice :) Okay, then let's go with |
|
Everything looks fine now: Should I change anything else @taketwo ? |
| plane_a.x() = x; | ||
| plane_a.y() = y; | ||
| plane_a.z() = z; | ||
| plane_a.w() = w; |
There was a problem hiding this comment.
I understand that you copied over from the other test, but this really looks bizarre. These 9 lines could be replaced with just
Eigen::Vector4f plane_a (1.0f, 0.0f, 0.0f, -0.5f);
|
There are a few missing spaces between function names and braces that should be added to conform to PCL style guide. Apart from that I think we are pretty close to merging. |
|
I corrected the spaces. |
| } | ||
|
|
||
| double determinant = normals_in_lines.determinant (); | ||
| if (fabs(determinant) < determinant_tolerance) |
|
Done, the test function is now much shorter and readable! |
|
Oh yeah, looks great now. So let's give Travis a chance to verify everything is all right. Once it's done I'll merge. |
|
Thanks! |
This new function allow the user to get the intersection point between three planes :