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

[filters] Add a filter accepting a functor to reduce boiler plate code for simple filters #3890

Merged
merged 33 commits into from
Jul 9, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
3cf3edb
Proposed lambda filter
kunaltyagi Apr 8, 2020
a44d993
Adding complicated vodoo
kunaltyagi Apr 8, 2020
dc10149
Correct comparison
kunaltyagi Apr 23, 2020
997c935
move traits to common header
kunaltyagi Jun 5, 2020
fc2cce3
Simplify and integrate design
kunaltyagi Jun 6, 2020
cfff428
Boost HOF isn't available on Ubuntu 16.04
kunaltyagi Jun 6, 2020
877f265
Update filters/include/pcl/filters/lambda_filter_indices.h
kunaltyagi Jun 8, 2020
65a3c78
Update filters/include/pcl/filters/lambda_filter_indices.h
kunaltyagi Jun 8, 2020
557012c
Adding most of the suggestions
kunaltyagi Jun 8, 2020
81215f0
Adding to CMakeLists
kunaltyagi Jun 8, 2020
a093e0c
minor improvement
kunaltyagi Jun 8, 2020
a3a374c
oopsie
kunaltyagi Jun 8, 2020
49bef76
forgot static on variable
kunaltyagi Jun 8, 2020
027550f
Adding tests
kunaltyagi Jun 8, 2020
d76ecde
minor improvements, less license
kunaltyagi Jun 8, 2020
f5c7fb2
typo
kunaltyagi Jun 11, 2020
5977f67
Removing cruft and adding sergio's suggestion
kunaltyagi Jun 11, 2020
2e92983
Move out of detail ns and check other conditions
kunaltyagi Jun 11, 2020
50430a0
Correct error msg
kunaltyagi Jun 11, 2020
7a4ec01
Simplifying implementation
kunaltyagi Jun 12, 2020
0661536
Post sergio's partial review
kunaltyagi Jun 12, 2020
87fb62e
Adding a lil test and more review
kunaltyagi Jun 12, 2020
dd00eec
Adding more tests
kunaltyagi Jun 13, 2020
1dcbf28
Test commit to show error with Sergio's suggestion
kunaltyagi Jun 14, 2020
d43f3ba
Rename and add a non-const overload
kunaltyagi Jun 14, 2020
993ce0b
Make test more deterministic
kunaltyagi Jun 15, 2020
683806e
Using multiple seeds
kunaltyagi Jun 15, 2020
c046b3f
Using Typed Test
kunaltyagi Jun 15, 2020
16286c3
For older gtest
kunaltyagi Jun 15, 2020
8632d05
Adding more macros for older gtest
kunaltyagi Jun 15, 2020
7984f97
Typos
kunaltyagi Jun 15, 2020
3b97d4e
Undefine local macro
kunaltyagi Jun 26, 2020
115b185
Measure the sizes directly
kunaltyagi Jul 4, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions common/include/pcl/type_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@

#include <cstddef> // for std::size_t
#include <cstdint> // for std::uint8_t
#include <string> // for std::string

#include <functional> // for std::function, needed till C++17
#include <string> // for std::string
#include <type_traits> // for std::false_type, std::true_type

namespace pcl
Expand Down Expand Up @@ -260,5 +262,32 @@ namespace pcl
template <typename T> struct has_custom_allocator<T, void_t<typename T::_custom_allocator_type_trait>> : std::true_type {};

#endif
}

/**
* \todo: Remove in C++17
*/
#ifndef __cpp_lib_is_invocable
template <typename F, typename... Args>
constexpr bool is_invocable_v =
std::is_constructible<std::function<void(Args...)>,
std::reference_wrapper<std::remove_reference_t<F>>>::value;

template <typename R, typename F, typename... Args>
constexpr bool is_invocable_r_v =
std::is_constructible<std::function<R(Args...)>,
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
std::reference_wrapper<std::remove_reference_t<F>>>::value;
#else
using std::is_invocable_v;
using std::is_invocable_r_v;
#endif

/**
* \todo: Remove in C++17
*/
#ifndef __cpp_lib_remove_cvref
template <typename T>
using remove_cvref_t = std::remove_cv_t<std::remove_reference_t<T>>;
#else
using std::remove_cvref_t;
#endif
}
166 changes: 166 additions & 0 deletions filters/include/pcl/filters/lambda_filter_indices.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
/*
* Software License Agreement (BSD License)
*
* Point Cloud Library (PCL) - www.pointclouds.org
* Copyright (c) 2020-, Open Perception
*
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution.
* * Neither the name of the copyright holder(s) nor the names of its
* contributors may be used to endorse or promote products derived
* from this software without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
* CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*
* $Id$
*
*/

#pragma once

#include <pcl/filters/filter_indices.h>
#include <pcl/pcl_macros.h>
#include <pcl/type_traits.h> // for is_invocable

#ifndef __cpp_lib_is_invocable
#include <boost/hof/is_invocable.hpp>
#endif
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved

#include <type_traits>

#include <utility>

namespace pcl {
namespace detail {
template <typename PointT, typename Function>
using PointFilterLambda =
std::enable_if_t<pcl::is_invocable_r_v<bool,
Function,
const pcl::remove_cvref_t<PointT>&,
pcl::index_t>,
bool>;
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved

// can't use this for SFINAE since Derived isn't properly defined
// but this can be used after the class definition to test it
/*
template <class Base, class Derived>
constexpr auto IsValidLambdaFilter = std::enable_if_t<
std::is_base_of<Base, Derived>::value &&
pcl::is_invocable_v<std::declval<Derived>().get_lambda, void> &&
std::is_same<PointFilterLambda<std::declval<Derived>().get_lambda()>,
bool>, bool>;
*/

/**
* \brief LambdaFilterIndices filters point clouds and indices based on a
* function pointer passed to filter command \ingroup filters
*/
template <typename PointT, typename Derived>
struct LambdaFilterIndicesImpl : public FilterIndices<PointT> {
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
private:
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
using Base = FilterIndices<PointT>;
using PCLBase = pcl::PCLBase<PointT>;

protected:
using PointCloud = typename FilterIndices<PointT>::PointCloud;
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
using PointCloudPtr = typename PointCloud::Ptr;
using PointCloudConstPtr = typename PointCloud::ConstPtr;
using FieldList = typename pcl::traits::fieldList<PointT>::type;

using Base::extract_removed_indices_;
using Base::removed_indices_;
using PCLBase::indices_;
using PCLBase::input_;

protected:
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
// to prevent instantiation of impl class
LambdaFilterIndicesImpl(bool extract_removed_indices)
: FilterIndices<PointT>(extract_removed_indices)
{}
/** \brief Filtered results are indexed by an indices array.
* \param[out] indices The resultant indices.
*/
void
applyFilter(std::vector<int>& indices) override
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
{
indices.clear();
indices.reserve(input_->points.size());
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
if (extract_removed_indices_) {
removed_indices_->clear();
removed_indices_->reserve(input_->points.size());
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
}

const auto& lambda = static_cast<Derived*>(this)->get_lambda();

for (const auto index : *indices_) {
// lambda returns true for points that shoud be selected
if (!(this->negative_) == lambda(input_->points[index], index)) {
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
indices.push_back(index);
}
else {
if (extract_removed_indices_) {
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
removed_indices_->push_back(index);
}
}
}
}
};
} // namespace detail

template <typename PointT, typename Functor>
struct LambdaFilterIndices
: public detail::LambdaFilterIndicesImpl<PointT, LambdaFilterIndices<PointT, Functor>> {
private:
using Self = LambdaFilterIndices<PointT, Functor>;
using Base = detail::LambdaFilterIndicesImpl<PointT, Self>;

public:
using FunctorT = Functor;
// using in type would complicate signature
static_assert(std::is_same<detail::PointFilterLambda<PointT, FunctorT>, bool>::value,
"Functor needs to be able to satisfy the callable constraint");
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved

protected:
using Base::filter_name_;
// need to hold a value because lambdas can only be copy or move constructed
const FunctorT lambda_;
SergioRAgostinho marked this conversation as resolved.
Show resolved Hide resolved

public:
/** \brief Constructor.
* \param[in] extract_removed_indices Set to true if you want to be able to
* extract the indices of points being removed (default = false).
*/
LambdaFilterIndices(FunctorT lambda, bool extract_removed_indices = false)
: Base(extract_removed_indices), lambda_(lambda)
{
filter_name_ = "lambda_filter_indices";
}

const FunctorT&
get_lambda() const noexcept
kunaltyagi marked this conversation as resolved.
Show resolved Hide resolved
{
return lambda_;
}
};
} // namespace pcl