search

PickNikRobotics / rviz_visual_tools

C++ API wrapper for displaying shapes and meshes in Rviz
771 stars 243 forks source link

Publish plane from Ax+By+Cz+D=0 equation #119

Closed AndyZe closed 5 years ago

AndyZe commented 5 years ago

Issue #119

AndyZe commented 5 years ago

I think a reviewer will ask, "Why are the arguments only A,B,C if the plane equation is Ax+By+Cz+D=0?"

D isn't needed because the user specifies a center point for the graphic. So the burden of calculating a point on the plane is shifted to the user.

I thought about doing that calculation here, but it can be tricky. For example, how do you calculate z if C==0?

henningkayser commented 5 years ago

I think a reviewer will ask, "Why are the arguments only A,B,C if the plane equation is Ax+By+Cz+D=0?"

D isn't needed because the user specifies a center point for the graphic. So the burden of calculating a point on the plane is shifted to the user.

As mentioned above this actually isn't ABCD then.

I thought about doing that calculation here, but it can be tricky. For example, how do you calculate z if C==0?

I don't see the issue here. If C==0 then the plane is parallel to the x/y coordinate plane and it's clearly defined that only one of A,B,C must be non-zero.

ABCD defines a plane with normal vector (A,B,C) and distance |D|/|(A,B,C) from the origin. It's really straightforward to implement this and to compute the center point, i.e. by computing the normalized product: D * (A, B, C) / |(A,B,C)|

AndyZe commented 5 years ago

You are right, it was not very hard to add D as an argument.

Wrong however about having more than one of A/B/C == 0. It is possible and the first demo datapoint happens to be that case. :P And C == 0 doesn't mean xy plane... it's actually the opposite. Large C ==> parallel to xy plane.

Anyway, I believe all of your changes have been addressed.

henningkayser commented 5 years ago

You are right, it was not very hard to add D as an argument.

Wrong however about having more than one of A/B/C == 0. It is possible and the first demo datapoint happens to be that case. :P

I mentioned that at least one must be non-zero and the first datapoint is an example for that.

And C == 0 doesn't mean xy plane... it's actually the opposite. Large C ==> parallel to xy plane.

That's true, the plane is parallel to xy if A==B==0 and C!=0.