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

Fixed rounding error angle calculation of parallel and anti-parallel vec... #1035

Merged
merged 1 commit into from
Dec 8, 2014

Conversation

mschoeler
Copy link
Contributor

This fixes angle convexity calculation for parallel and anti-parallel normals, where a rounding error occasionally caused NaN angles.
This solves issue #1033.

@mschoeler
Copy link
Contributor Author

There actually exists a function in common doing something similar:
getAngle3D (const Eigen::Vector4f &v1, const Eigen::Vector4f &v2)
but it takes Vector4f and returns an angle in rad.
Since I'm using cross products (not defined on Vector4f) and angles in degrees using this function would make the code look ugly. This is why I implemented it as a static member function.

@taketwo
Copy link
Member

taketwo commented Dec 8, 2014

What about adding a new overload in common?

getAngle3D (const Eigen::Vector3f &v1, const Eigen::Vector3f &v2)

As for the radians to degree conversion, there is rad2deg() in "common/angles.h". So inside LCCP this would look something like:

float intersection_angle =  rad2deg (getAngle3D (ncross, vec_t_to_s));

(Does not look too ugly to me, what do you think? :))

@mschoeler
Copy link
Contributor Author

I thought about the the same. I guess if it is ok to add a new overloaded function to common, I would just change it to that.

@mschoeler
Copy link
Contributor Author

what about a boolean flag in the function
getAngle3D (const Eigen::Vector3f &v1, const Eigen::Vector3f &v2, const bool in_degree = false)

@VictorLamoine
Copy link
Contributor

I'm using cross products (not defined on Vector4f)

Cross Dot product is defined between two Eigen::Vector4f: (why would it not?)

  Eigen::Vector4f v1, v2;
  v1 << 1, 0, 0, 0;
  v2 << 1, 1, 1, 1;
  double test = v1.dot(v2);

Just make sure the last coordinates are zero.

@mschoeler
Copy link
Contributor Author

dot product yes, but not cross product? At least I don't know a definition ouside of three dimensions.

@VictorLamoine
Copy link
Contributor

Oh sorry I misread; you should be able to do these computations without a cross product though.
http://stackoverflow.com/questions/14066933/direct-way-of-computing-clockwise-angle-between-2-vectors

@mschoeler
Copy link
Contributor Author

Implementing cross functions was actually what I had in the code in the first place, but it was confusing and redundent. In my oppinion an overloaded function in common makes most sense.
I also added an optional flag to the function, which determines if an angle in radians or degrees is prefered. This should not affect any existing code.

@mschoeler
Copy link
Contributor Author

i also made the code for getAngle3D a little easier (Removed / sqrt (sqNorm...) )

@mschoeler
Copy link
Contributor Author

oh, forgot to change the if to else if.
Will do that, too.

@@ -428,11 +429,11 @@ pcl::LCCPSegmentation<PointT>::connIsConvex (uint32_t source_label_arg,
typename pcl::Supervoxel<PointT>::Ptr& sv_source = sv_label_to_supervoxel_map_[source_label_arg];
typename pcl::Supervoxel<PointT>::Ptr& sv_target = sv_label_to_supervoxel_map_[target_label_arg];

const pcl::PointXYZRGBA& source_centroid = sv_source->centroid_;
const pcl::PointXYZRGBA& target_centroid = sv_target->centroid_;
const Eigen::Vector3f source_centroid = sv_source->centroid_.getVector3fMap ();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make this a reference to avoid copying.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point

taketwo added a commit that referenced this pull request Dec 8, 2014
Fixed rounding error angle calculation of parallel and anti-parallel vec...
@taketwo taketwo merged commit 9fd7067 into PointCloudLibrary:master Dec 8, 2014
@mschoeler mschoeler deleted the LCCP_bugfix051214 branch December 8, 2014 16:06
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

Successfully merging this pull request may close these issues.

3 participants