Skip to content

Conversation

keineahnung2345
Copy link
Contributor

Attemp to implement the feature mentioned in #5422

@keineahnung2345
Copy link
Contributor Author

Have added a test that extract all.pcd, the effect is like:
image

Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

Thank you for implementing this!

@keineahnung2345
Copy link
Contributor Author

@mvieth I've noticed that formatting check would not be done on filters module, should it be enabled?

@mvieth
Copy link
Member

mvieth commented Sep 21, 2022

@mvieth I've noticed that formatting check would not be done on filters module, should it be enabled?

Unfortunately, clang-format wasn't used in the beginning of PCL (perhaps because it didn't exist back then 😄 ). Now we slowly enforce clang-format module by module, but we can only format a module if there are no open pull requests that change files in that module. Otherwise there would be so many merge conflicts for that pull request that we can't fix them and would have to close the pull request. Currently, there are several pull requests that change files in the filters module, so it is not a good idea to format the whole filters module now.

@keineahnung2345
Copy link
Contributor Author

I see, that's not an easy work.

@mvieth mvieth added changelog: new feature Meta-information for changelog generation module: filters labels Sep 21, 2022
float fp_h_d = float(2 * std::tan(vfov_rad / 2) * fp_dist_ * (roi_ymax - 0.5)); // far plane lower height
float fp_w_l = float(2 * std::tan(hfov_rad / 2) * fp_dist_ * (roi_xmin - 0.5) * (-1)); // far plane left width
float fp_w_r = float(2 * std::tan(hfov_rad / 2) * fp_dist_ * (roi_xmax - 0.5)); // far plane right width
float np_h_u = float(2 * std::tan(fov_lower_bound_rad) * np_dist_ * (roi_ymin - 0.5)); // near plane upper height
Copy link
Member

Choose a reason for hiding this comment

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

I just noticed something strange: np_h_u seems to be switched with np_h_d, and fp_h_u seems to be switched with fp_h_d. Then later, while computing fp_tl, fp_tr, np_br, ... (the corners), there shouldn't be any minus signs because the upper/lower height and left/right width are already signed. Could you look into this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, but this seems another big issue, maybe dealing with it in another PR is better.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I am fine with that

Copy link
Member

@mvieth mvieth left a comment

Choose a reason for hiding this comment

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

Thank you again for implementing this!

@mvieth mvieth merged commit 4dad5b3 into PointCloudLibrary:master Sep 26, 2022
@keineahnung2345 keineahnung2345 deleted the frustum_asymmetrical branch September 26, 2022 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: new feature Meta-information for changelog generation module: filters

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants