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

[tracking] Use SFINAE instead of relying on macro PCL_TRACKING_NORMAL_SUPPORTED #4643

Merged
merged 3 commits into from
Mar 16, 2021

Conversation

kunaltyagi
Copy link
Member

@kunaltyagi kunaltyagi commented Mar 11, 2021

Eliminates the need for macro PCL_TRACKING_NORMAL_SUPPORTED

Possibly fixes #4630

@kunaltyagi kunaltyagi requested a review from mvieth March 11, 2021 03:34
@kunaltyagi kunaltyagi marked this pull request as ready for review March 11, 2021 03:42
@kunaltyagi kunaltyagi added changelog: ABI break Meta-information for changelog generation module: tracking changelog: API break Meta-information for changelog generation labels Mar 11, 2021
@kunaltyagi kunaltyagi changed the title Use SFINAE instead of relying on macros [tracking] Use SFINAE instead of relying on macro PCL_TRACKING_NORMAL_SUPPORTED Mar 11, 2021
@kunaltyagi
Copy link
Member Author

kunaltyagi commented Mar 11, 2021

Any ideas on how to fix a linker error on MacOS?

No issues. Reproduced the error (in apps module)

@kunaltyagi
Copy link
Member Author

Yay!! All tests pass

@kunaltyagi kunaltyagi requested review from koide3 and larshg March 14, 2021 02:33
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.

if constexpr would have been another nice solution, but since we are still at C++14 👍
An option would be to call computeTransformedPointCloudWithoutNormal inside the HasNoNormal variant (in addition to the warning) and/or to only allow setting use_normal_ to true if the point actually has a normal inside setUseNormal, but that is not a must.

@kunaltyagi
Copy link
Member Author

Nice suggestion! That can be a future addition. Creating an issue from it

@kunaltyagi kunaltyagi merged commit 810a77f into PointCloudLibrary:master Mar 16, 2021
@kunaltyagi kunaltyagi deleted the tracking.sfinae branch March 16, 2021 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: ABI break Meta-information for changelog generation changelog: API break Meta-information for changelog generation module: tracking
Projects
None yet
3 participants