Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

planeWithPlaneIntersection, error in parallel plane test #1695

Closed
sand-box opened this issue Aug 25, 2016 · 2 comments
Closed

planeWithPlaneIntersection, error in parallel plane test #1695

sand-box opened this issue Aug 25, 2016 · 2 comments

Comments

@sand-box
Copy link
Contributor

The function template <typename Scalar> bool pcl::planeWithPlaneIntersection (const Eigen::Matrix<Scalar, 4, 1> &plane_a, const Eigen::Matrix<Scalar, 4, 1> &plane_b, Eigen::Matrix<Scalar, Eigen::Dynamic, 1> &line, double angular_tolerance) in file intersections.hpp checks for parallel planes like this:

  double test_cos = plane_a_norm.dot (plane_b_norm);
  double upper_limit = 1 + angular_tolerance;
  double lower_limit = 1 - angular_tolerance;
  if ((test_cos > lower_limit) && (test_cos < upper_limit))
  {
      PCL_DEBUG ("Plane A and Plane B are parallel.\n");
      return (false);
  }

I think there a three issues with this code

  • check for anti-parallel plane is missing (cos ~= -1)
  • no need for second condition (test_cos < upper_limit)
  • documentation says: \param[in] angular_tolerance tolerance in radians. In code the parameter is not used in radian domain.

Your Environment

  • Operating System and version: ubuntu 14.04
  • Compiler: gcc 4.8.5
  • PCL Version: 1.7.2

Expected Behavior

Checking for anti-parallel planes, correct documentation.

@SergioRAgostinho
Copy link
Member

SergioRAgostinho commented Aug 25, 2016

Agreed! Something like this should do the trick

abs (plane_a_norm.dot (plane_b_norm)) > (1 - sin ( abs (angular_tolerance)))

Can you file a pull request with the changes?

Edit: mistake on the expression to be checked.

@SergioRAgostinho SergioRAgostinho modified the milestone: pcl-1.8.1 Aug 25, 2016
sand-box added a commit to sand-box/pcl that referenced this issue Aug 26, 2016
sand-box added a commit to sand-box/pcl that referenced this issue Aug 26, 2016
SergioRAgostinho added a commit that referenced this issue Aug 26, 2016
Fix issue #1695. Test for parallel planes in planePlaneIntersection.
@SergioRAgostinho
Copy link
Member

Closed by #1698.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants