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

Added new color traits in point_types.hpp #4761

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ueqri
Copy link
Contributor

@ueqri ueqri commented May 17, 2021

A follow up work of #4706 (comment) #4706 (comment).

New color traits:

  • has_Lab
  • has_HSV
  • has_rgb
  • has_any_color

Add deprecated comments for has_color.

@ueqri
Copy link
Contributor Author

ueqri commented May 17, 2021

I'm not very sure about the 1.15 as the deprecated version here. If needed, I'd be glad to change it. 😊

/** Deprecated: Metafunction to check if a given point type has either rgb or rgba field. */
template <typename PointT>
struct PCL_DEPRECATED(1, 15, "Please use has_rgb instead.")
has_color : has_any_field<PointT, boost::mpl::vector<pcl::fields::rgb,
pcl::fields::rgba> >
{ };

For CI build on Ubuntu 18.04, the PCL_WARNINGS_ARE_ERRORS is open by default, and there is no exclusive rules like -Wno-error=deprecated-declarations in CMAKE_CXX_FLAGS. So the errors occurs in CI checks.

18.04 GCC: # oldest LTS
CONTAINER: env1804
CC: gcc
CXX: g++
BUILD_GPU: ON
CMAKE_ARGS: '-DPCL_WARNINGS_ARE_ERRORS=ON'

This PR has been built and tested successfully in local env18.04 container with -Wno-error=deprecated-declarations flag. 😀

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.

Should we change the existing use of has_color and friends to has_rgb?

For the deprecation, perhaps putting the struct in a namespace and deprecating only the using has_color = details::has_color; might do the trick

@ueqri
Copy link
Contributor Author

ueqri commented Jun 1, 2021

Thanks for your kind review!

Should we change the existing use of has_color and friends to has_rgb?

I'd be glad to change the existing use of has_color has_color_v HasColor HasNoColor to has_rgb families.

For the deprecation, perhaps putting the struct in a namespace and deprecating only the using has_color = details::has_color; might do the trick

I'd move the struct to namespace detail and deprecate the using has_color = details::has_color;. Thanks for your advice! 😀

UPDATE: Still not very sure about how to implement the deprecating only the using has_color = details::has_color;. I just deprecated the struct in namespace detail in 8dd75c4 commit. I'd be grateful if you could give me more hints about this.

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.

LGTM, minor clarification needed

common/include/pcl/impl/point_types.hpp Outdated Show resolved Hide resolved
common/include/pcl/impl/point_types.hpp Outdated Show resolved Hide resolved
common/include/pcl/impl/point_types.hpp Outdated Show resolved Hide resolved
common/include/pcl/impl/point_types.hpp Show resolved Hide resolved
@ueqri
Copy link
Contributor Author

ueqri commented Jun 12, 2021

Since has_color family is the same as has_rgb currently, I was wondering if we could remove the test codes of has_color family in test/common/test_type_traits.cpp of this PR branch. In this way, we could avoid lots of unnecessary deprecated warnings during PCL sources build.

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.

2 participants