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.

Sign up for GitHub

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

Jump to bottom

Improve routing for transformPointCloud #4398

Merged
merged 1 commit into from
Sep 13, 2020
Merged

Improve routing for transformPointCloud #4398

merged 1 commit into from
Sep 13, 2020

Conversation

mvieth
Copy link
Member

@mvieth mvieth commented Sep 12, 2020

  • Change implementations to work with matrix instead of affine transform since Transformer wants a 4 by 4 matrix
  • Route the affine transform variant through the matrix variant
  • Change routing of other variants to point directly to implementations (avoid detours)

Also: are the specializations for float in transforms.h actually necessary?

@mvieth
Change routing for transform cloud functions
3b66ab3 
- Change implementations to work with matrix instead of affine transform since Transformer wants a 4 by 4 matrix
- Route the affine transform variant through the matrix variant
- Change routing of other variants to point directly to implementations (avoid detours)
kunaltyagi
kunaltyagi reviewed Sep 12, 2020
View reviewed changes
Copy link
Member

@kunaltyagi kunaltyagi left a comment

Choose a reason for hiding this comment

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

Regarding the float specialization, perhaps reduced compilation time... Other than that, no idea

common/include/pcl/common/impl/transforms.hpp Show resolved Hide resolved
@kunaltyagi kunaltyagi added module: common needs: author reply Specify why not closed/merged yet and removed needs: author reply Specify why not closed/merged yet labels Sep 12, 2020
@kunaltyagi
Copy link
Member

How should I label this as? Any advantages over current approach (so it'd be enhancement)? Or just unlabelled?

@kunaltyagi kunaltyagi added this to the pcl-1.12.0 milestone Sep 12, 2020
@kunaltyagi kunaltyagi added the needs: author reply Specify why not closed/merged yet label Sep 12, 2020
@mvieth
Copy link
Member Author

mvieth commented Sep 12, 2020

How should I label this as? Any advantages over current approach (so it'd be enhancement)? Or just unlabelled?

It might bring a slight increase in speed since there are fewer detours and fewer conversions between Eigen::Transform and Eigen::Matrix

@kunaltyagi kunaltyagi added the changelog: enhancement Meta-information for changelog generation label Sep 12, 2020
@kunaltyagi kunaltyagi changed the title Change routing for transform cloud functions Improve routing for transformPointCloud Sep 12, 2020
@kunaltyagi kunaltyagi merged commit 603316e into PointCloudLibrary:master Sep 13, 2020
@mvieth mvieth deleted the transform branch September 14, 2020 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kunaltyagi kunaltyagi kunaltyagi approved these changes

Assignees
No one assigned
Labels
changelog: enhancement Meta-information for changelog generation module: common needs: author reply Specify why not closed/merged yet
Projects
None yet
Milestone
pcl-1.12.0
Development

Successfully merging this pull request may close these issues.
2 participants
@mvieth @kunaltyagi