-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Adding bearing angle image to pcl #198
Conversation
Could you please reuse pull requests, instead of opening up a new one every time? Just force push into the same branch. |
|
||
/** \Based on the theta, calculate the gray value of every pixel point */ | ||
unsigned char | ||
pcl::BearingAngle::getGray (double theta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this as a separate method, it's just one line.
|
Please help me check them. Cheers |
Any questions? |
/** \brief Members: float x, y, z, unsigned char gray | ||
* \ingroup common | ||
*/ | ||
struct PointWithGray |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this to point_types.h or better use one of the structs defined there?
I have moved "struct PointWithGray" to point_types.h, please help me check them. Cheers |
@@ -162,6 +162,11 @@ | |||
*/ | |||
struct PointXYZINormal; | |||
|
|||
/** \brief Members: float x, y, z, unsigned char gray |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add more explanation what gray should be? unit, use case..
Please help me check them. Thanks. Cheers |
Any questions? |
|
||
/** \transform 3D point cloud into a 2D Bearing Angle(BA) image */ | ||
void | ||
BearingAngleImage::generateBAImage (std::vector< std::vector<pcl::PointXYZ> > &point_cloud, int cloud_width, int cloud_height) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it make sense to create these image for other point clouds than PointXYZ? I would propose to use a template here. Also, the typical type in PCL is a PointCloud, not a std::vector ;). You could check if the point cloud is dense, so you don't have to pass in the width and height.
Cheers |
HI Yan, your patch failed on Travis, could you please fix this? Thanks! |
According to your guide, I have fixed the patch. Thanks! |
Timeouts are ok, that's a limitation of the setup. But as you can see here in the log [1], your unit test didn't worked. [1] https://s3.amazonaws.com/archive.travis-ci.org/jobs/13194638/log.txt |
The clang was finished successfully, but the gcc interrupted again because of timeout. |
Any questions? |
points.clear (); | ||
} | ||
|
||
/** \calculate the angle between the laser beam and the segment joining two consecutive measurement points */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to repeat documentation comments inside source files. doxygen
will pick them up from header files.
Cheers |
Any questions ? |
Hi,
Please don't bump issues on the tracker. It's in the list and we will Cheers Jochen |
I'm very sorry for my incomprehension to it. Cheers |
I'm not entirely sure we need a new global point type, if this is only used by this algorithm. Perhaps we can definite it only in the class where it gets used? |
Yes, I agree with it and had defined it in my class before jspricke commented (Please refer to the above part.) I will fix it. Thank you. |
I would rather propose to export it to PointXYZRGBA in that way we would be compatible with the other tools in PCL. |
Do you mean I should use PointXYZRGBA in point_types.h instead of PointWithGray I have defined ? |
Yes, as you are defining a color anyhow, you can just set all channels to the same value. |
Hi wnl, can you please attribute the commit with your real name? Also, can you remove the commented lines? Otherwise I'm fine with this patch. |
How to remove all commented lines? |
a = p1.squaredNorm (); | ||
b = (p1 - p2).squaredNorm (); | ||
c = p2.squaredNorm (); | ||
// a = sqrt (point1.x * point1.x + point1.y * point1.y + point1.z * point1.z); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was referring to these lines.
Any questions? |
Adding bearing angle image to pcl
I guess we were to fast with merging this, there are complaints in pcl-users already: http://www.pcl-users.org/Problem-building-from-trunk-pcl-commons-td4031835.html. |
Oh, I'm so sorry for my carelessness. Thanks for your selfless help and work. Cheers |
To represent a 3D laser point cloud in a more simplified way, a Bearing Angle(BA) model was employed to transform a 3D point cloud into a 2D image in pcl/bearing_angle, so that image processing methods can be used in many aspects.
Bearing Angle is defined as the angle between the laser beam and the segment connecting two consecutive measurement points (see Fig. 1). Grey values for a Bearing Angle image are proportional to the reflected laser light energy, which depends not only on how far away the measured object is from the scanner but also on the surface characteristics of the object.
Fig. 1 The Bearing Angle is calculated along the diagonal direction