From 9185bc8c901bf8839af2e25ed960f9b5e7b9c66f Mon Sep 17 00:00:00 2001 From: Markus Vieth Date: Wed, 20 Nov 2024 21:01:54 +0100 Subject: [PATCH] Misc small fixes and improvements - remove unused variables in dominant_plane_segmentation.hpp, integral_image_normal.hpp, our_cvfh.hpp - remove unnecessary parenthesis in pcl_macros.h (otherwise the deprecation messages are "It will be removed in PCL (1).15") - ply_io.cpp: catch exception when parsing bad ply files - add includes for KdTree, OrganizedNeighbor, eigen33 (the files use these classes/functions, so the headers should be included; currently they rely on transitive includes) - in random_sample.h: use srand_deterministic on OpenBSD to properly use seed and fix failing test on OpenBSD - in pcl_visualizer.cpp: replace 5 with better value. The Boost doc only says that digest_type is at least 16 bytes large (in the current code it is 20 bytes large). Additionally, there is no guarantee that unsigned int is 4 bytes large (even if that is the most common situation) --- .../pcl/apps/impl/dominant_plane_segmentation.hpp | 4 ---- common/include/pcl/pcl_macros.h | 4 +++- features/include/pcl/features/impl/cppf.hpp | 1 + features/include/pcl/features/impl/cvfh.hpp | 1 + features/include/pcl/features/impl/flare.hpp | 2 ++ .../pcl/features/impl/integral_image_normal.hpp | 2 +- .../pcl/features/impl/intensity_gradient.hpp | 1 + features/include/pcl/features/impl/our_cvfh.hpp | 2 +- features/include/pcl/features/impl/ppf.hpp | 1 + features/include/pcl/features/impl/ppfrgb.hpp | 1 + .../pcl/features/impl/principal_curvatures.hpp | 1 + filters/include/pcl/filters/convolution_3d.h | 2 +- filters/include/pcl/filters/impl/random_sample.hpp | 4 ++++ .../include/pcl/filters/sampling_surface_normal.h | 2 +- filters/src/random_sample.cpp | 4 ++++ io/src/ply_io.cpp | 12 ++++++++++-- .../impl/outofcore_depth_first_iterator.hpp | 2 +- .../registration/impl/pyramid_feature_matching.hpp | 2 +- registration/include/pcl/registration/ndt.h | 2 +- .../sample_consensus/impl/sac_model_ellipse3d.hpp | 5 ++--- .../pcl/sample_consensus/impl/sac_model_torus.hpp | 2 +- .../include/pcl/surface/impl/grid_projection.hpp | 4 +--- .../pcl/visualization/impl/pcl_visualizer.hpp | 4 +--- .../impl/point_cloud_color_handlers.hpp | 14 ++++---------- visualization/src/pcl_visualizer.cpp | 4 ++-- 25 files changed, 47 insertions(+), 36 deletions(-) diff --git a/apps/include/pcl/apps/impl/dominant_plane_segmentation.hpp b/apps/include/pcl/apps/impl/dominant_plane_segmentation.hpp index 549f893bcff..119c9f3b9d2 100644 --- a/apps/include/pcl/apps/impl/dominant_plane_segmentation.hpp +++ b/apps/include/pcl/apps/impl/dominant_plane_segmentation.hpp @@ -120,7 +120,6 @@ pcl::apps::DominantPlaneSegmentation::compute_table_plane() // Compute the plane coefficients Eigen::Vector4f model_coefficients; - EIGEN_ALIGN16 Eigen::Matrix3f covariance_matrix; model_coefficients[0] = table_coefficients_->values[0]; model_coefficients[1] = table_coefficients_->values[1]; @@ -251,7 +250,6 @@ pcl::apps::DominantPlaneSegmentation::compute_fast( // Compute the plane coefficients Eigen::Vector4f model_coefficients; - EIGEN_ALIGN16 Eigen::Matrix3f covariance_matrix; model_coefficients[0] = table_coefficients_->values[0]; model_coefficients[1] = table_coefficients_->values[1]; @@ -626,7 +624,6 @@ pcl::apps::DominantPlaneSegmentation::compute( // Compute the plane coefficients Eigen::Vector4f model_coefficients; - EIGEN_ALIGN16 Eigen::Matrix3f covariance_matrix; model_coefficients[0] = table_coefficients_->values[0]; model_coefficients[1] = table_coefficients_->values[1]; @@ -795,7 +792,6 @@ pcl::apps::DominantPlaneSegmentation::compute_full( // Compute the plane coefficients Eigen::Vector4f model_coefficients; - EIGEN_ALIGN16 Eigen::Matrix3f covariance_matrix; model_coefficients[0] = table_coefficients_->values[0]; model_coefficients[1] = table_coefficients_->values[1]; diff --git a/common/include/pcl/pcl_macros.h b/common/include/pcl/pcl_macros.h index 0e2740965ee..eebc14decd0 100644 --- a/common/include/pcl/pcl_macros.h +++ b/common/include/pcl/pcl_macros.h @@ -116,11 +116,13 @@ #define _PCL_DEPRECATED_HEADER_IMPL(Message) #endif +// NOLINTBEGIN(bugprone-macro-parentheses) /** * \brief A handy way to inform the user of the removal deadline */ #define _PCL_PREPARE_REMOVAL_MESSAGE(Major, Minor, Msg) \ - Msg " (It will be removed in PCL " BOOST_PP_STRINGIZE((Major).Minor) ")" + Msg " (It will be removed in PCL " BOOST_PP_STRINGIZE(Major.Minor) ")" +// NOLINTEND(bugprone-macro-parentheses) /** * \brief Tests for Minor < PCL_MINOR_VERSION diff --git a/features/include/pcl/features/impl/cppf.hpp b/features/include/pcl/features/impl/cppf.hpp index 975bb98cc81..94f9c3a61f3 100755 --- a/features/include/pcl/features/impl/cppf.hpp +++ b/features/include/pcl/features/impl/cppf.hpp @@ -42,6 +42,7 @@ #define PCL_FEATURES_IMPL_CPPF_H_ #include +#include // for KdTree ////////////////////////////////////////////////////////////////////////////////////////////// template diff --git a/features/include/pcl/features/impl/cvfh.hpp b/features/include/pcl/features/impl/cvfh.hpp index de4fbc17eb3..55173305456 100644 --- a/features/include/pcl/features/impl/cvfh.hpp +++ b/features/include/pcl/features/impl/cvfh.hpp @@ -44,6 +44,7 @@ #include #include #include +#include // for KdTree ////////////////////////////////////////////////////////////////////////////////////////////// template void diff --git a/features/include/pcl/features/impl/flare.hpp b/features/include/pcl/features/impl/flare.hpp index 2f8d3c9577d..e00f242f422 100644 --- a/features/include/pcl/features/impl/flare.hpp +++ b/features/include/pcl/features/impl/flare.hpp @@ -41,6 +41,8 @@ #include #include +#include // for KdTree +#include // for OrganizedNeighbor ////////////////////////////////////////////////////////////////////////////////////////////// template bool diff --git a/features/include/pcl/features/impl/integral_image_normal.hpp b/features/include/pcl/features/impl/integral_image_normal.hpp index f76765a1bcc..612a6df80a1 100644 --- a/features/include/pcl/features/impl/integral_image_normal.hpp +++ b/features/include/pcl/features/impl/integral_image_normal.hpp @@ -36,6 +36,7 @@ * */ #pragma once +#include // for eigen33 #include #include @@ -491,7 +492,6 @@ pcl::IntegralImageNormalEstimation::computePointNormalMirro Eigen::Vector3f center; typename IntegralImage2D::SecondOrderType so_elements; typename IntegralImage2D::ElementType tmp_center; - typename IntegralImage2D::SecondOrderType tmp_so_elements; center[0] = 0; center[1] = 0; diff --git a/features/include/pcl/features/impl/intensity_gradient.hpp b/features/include/pcl/features/impl/intensity_gradient.hpp index df54ae8f9e1..fcb8c85f4f2 100644 --- a/features/include/pcl/features/impl/intensity_gradient.hpp +++ b/features/include/pcl/features/impl/intensity_gradient.hpp @@ -43,6 +43,7 @@ #include #include // for pcl::isFinite +#include // for eigen33 ////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/features/include/pcl/features/impl/our_cvfh.hpp b/features/include/pcl/features/impl/our_cvfh.hpp index b0f0bc01a2e..15fab154805 100644 --- a/features/include/pcl/features/impl/our_cvfh.hpp +++ b/features/include/pcl/features/impl/our_cvfh.hpp @@ -47,6 +47,7 @@ #include // for copyPointCloud #include // for getMaxDistance #include +#include // for KdTree ////////////////////////////////////////////////////////////////////////////////////////////// template void @@ -623,7 +624,6 @@ pcl::OURCVFHEstimation::computeFeature (PointCloud { pcl::PointIndices pi; - pcl::PointIndices pi_cvfh; pcl::PointIndices pi_filtered; clusters_.push_back (pi); diff --git a/features/include/pcl/features/impl/ppf.hpp b/features/include/pcl/features/impl/ppf.hpp index 8cdb914a975..7fb8818a645 100644 --- a/features/include/pcl/features/impl/ppf.hpp +++ b/features/include/pcl/features/impl/ppf.hpp @@ -43,6 +43,7 @@ #include #include #include // for computePairFeatures +#include // for KdTree ////////////////////////////////////////////////////////////////////////////////////////////// template diff --git a/features/include/pcl/features/impl/ppfrgb.hpp b/features/include/pcl/features/impl/ppfrgb.hpp index 87f74548a62..193d54d6f56 100644 --- a/features/include/pcl/features/impl/ppfrgb.hpp +++ b/features/include/pcl/features/impl/ppfrgb.hpp @@ -40,6 +40,7 @@ #include #include +#include // for KdTree ////////////////////////////////////////////////////////////////////////////////////////////// template diff --git a/features/include/pcl/features/impl/principal_curvatures.hpp b/features/include/pcl/features/impl/principal_curvatures.hpp index 0d2ddd34ebb..8c0561c41d5 100644 --- a/features/include/pcl/features/impl/principal_curvatures.hpp +++ b/features/include/pcl/features/impl/principal_curvatures.hpp @@ -43,6 +43,7 @@ #include #include // for pcl::isFinite +#include // for eigen33 /////////////////////////////////////////////////////////////////////////////////////////// template void diff --git a/filters/include/pcl/filters/convolution_3d.h b/filters/include/pcl/filters/convolution_3d.h index 31a5a8093e0..8cb21bdee3f 100644 --- a/filters/include/pcl/filters/convolution_3d.h +++ b/filters/include/pcl/filters/convolution_3d.h @@ -272,7 +272,7 @@ namespace pcl double search_radius_; /** \brief number of threads */ - unsigned int threads_; + unsigned int threads_{1}; /** \brief convlving kernel */ KernelT kernel_; diff --git a/filters/include/pcl/filters/impl/random_sample.hpp b/filters/include/pcl/filters/impl/random_sample.hpp index 24a0c571a67..f4138afcf3c 100644 --- a/filters/include/pcl/filters/impl/random_sample.hpp +++ b/filters/include/pcl/filters/impl/random_sample.hpp @@ -63,7 +63,11 @@ pcl::RandomSample::applyFilter (Indices &indices) removed_indices_->resize (N - sample_size); // Set random seed so derived indices are the same each time the filter runs +#ifdef __OpenBSD__ + srand_deterministic (seed_); // OpenBSD only offers repeatable sequences with this function +#else std::srand (seed_); +#endif // __OpenBSD__ // Algorithm S std::size_t i = 0; diff --git a/filters/include/pcl/filters/sampling_surface_normal.h b/filters/include/pcl/filters/sampling_surface_normal.h index 3c2e8397404..25253747a51 100644 --- a/filters/include/pcl/filters/sampling_surface_normal.h +++ b/filters/include/pcl/filters/sampling_surface_normal.h @@ -61,7 +61,7 @@ namespace pcl using PointCloudPtr = typename PointCloud::Ptr; using PointCloudConstPtr = typename PointCloud::ConstPtr; - using Vector = Eigen::Matrix; + using Vector = Eigen::Matrix; public: diff --git a/filters/src/random_sample.cpp b/filters/src/random_sample.cpp index f271d8d4da2..97a75a6dbf1 100644 --- a/filters/src/random_sample.cpp +++ b/filters/src/random_sample.cpp @@ -117,7 +117,11 @@ pcl::RandomSample::applyFilter (Indices &indices) removed_indices_->resize (N - sample_size); // Set random seed so derived indices are the same each time the filter runs +#ifdef __OpenBSD__ + srand_deterministic (seed_); // OpenBSD only offers repeatable sequences with this function +#else std::srand (seed_); +#endif // __OpenBSD__ // Algorithm S std::size_t i = 0; diff --git a/io/src/ply_io.cpp b/io/src/ply_io.cpp index af58106a2cb..936e836fb0d 100644 --- a/io/src/ply_io.cpp +++ b/io/src/ply_io.cpp @@ -409,8 +409,16 @@ void pcl::PLYReader::vertexIntensityCallback (pcl::io::ply::uint8 intensity) { pcl::io::ply::float32 intensity_ (intensity); - cloud_->at(vertex_count_, vertex_offset_before_) = intensity_; - vertex_offset_before_ += static_cast (sizeof (pcl::io::ply::float32)); + try + { + cloud_->at(vertex_count_, vertex_offset_before_) = intensity_; + vertex_offset_before_ += static_cast (sizeof (pcl::io::ply::float32)); + } + catch(const std::out_of_range&) + { + PCL_WARN ("[pcl::PLYReader::vertexIntensityCallback] Incorrect data index specified (%lu)!\n", vertex_count_ * cloud_->point_step + vertex_offset_before_); + assert(false); + } } void diff --git a/outofcore/include/pcl/outofcore/impl/outofcore_depth_first_iterator.hpp b/outofcore/include/pcl/outofcore/impl/outofcore_depth_first_iterator.hpp index a7d15386619..b5744d52503 100644 --- a/outofcore/include/pcl/outofcore/impl/outofcore_depth_first_iterator.hpp +++ b/outofcore/include/pcl/outofcore/impl/outofcore_depth_first_iterator.hpp @@ -107,10 +107,10 @@ namespace pcl if (!stack_.empty ()) { std::pair*, unsigned char>& stackEntry = stack_.back (); - stack_.pop_back (); this->currentNode_ = stackEntry.first; currentChildIdx_ = stackEntry.second; + stack_.pop_back (); // stackEntry is a reference, so pop_back after accessing it! //don't do anything with the keys here... this->currentOctreeDepth_--; diff --git a/registration/include/pcl/registration/impl/pyramid_feature_matching.hpp b/registration/include/pcl/registration/impl/pyramid_feature_matching.hpp index ba7b4eebd0a..4ced722e45a 100644 --- a/registration/include/pcl/registration/impl/pyramid_feature_matching.hpp +++ b/registration/include/pcl/registration/impl/pyramid_feature_matching.hpp @@ -299,8 +299,8 @@ PyramidFeatureHistogram::compute() if (!initializeHistogram()) return; + std::vector feature_vector; // put here to reuse memory for (const auto& point : *input_) { - std::vector feature_vector; // NaN is converted to very high number that gives out of bound exception. if (!pcl::isFinite(point)) continue; diff --git a/registration/include/pcl/registration/ndt.h b/registration/include/pcl/registration/ndt.h index e19c6d391ba..26115717bd7 100644 --- a/registration/include/pcl/registration/ndt.h +++ b/registration/include/pcl/registration/ndt.h @@ -126,7 +126,7 @@ class NormalDistributionsTransform // Prevents unnecessary voxel initiations if (resolution_ != resolution) { resolution_ = resolution; - if (input_) { + if (target_) { // init() needs target_ init(); } } diff --git a/sample_consensus/include/pcl/sample_consensus/impl/sac_model_ellipse3d.hpp b/sample_consensus/include/pcl/sample_consensus/impl/sac_model_ellipse3d.hpp index 33740e44904..abb0d32c9c4 100644 --- a/sample_consensus/include/pcl/sample_consensus/impl/sac_model_ellipse3d.hpp +++ b/sample_consensus/include/pcl/sample_consensus/impl/sac_model_ellipse3d.hpp @@ -451,10 +451,9 @@ pcl::SampleConsensusModelEllipse3D::optimizeModelCoefficients ( OptimizationFunctor functor(this, inliers); Eigen::NumericalDiff num_diff(functor); Eigen::LevenbergMarquardt, double> lm(num_diff); - Eigen::VectorXd coeff; + Eigen::VectorXd coeff = model_coefficients.cast(); int info = lm.minimize(coeff); - for (Eigen::Index i = 0; i < coeff.size (); ++i) - optimized_coefficients[i] = static_cast (coeff[i]); + optimized_coefficients = coeff.cast(); // Compute the L2 norm of the residuals PCL_DEBUG ("[pcl::SampleConsensusModelEllipse3D::optimizeModelCoefficients] LM solver finished with exit code %i, having a residual norm of %g. \nInitial solution: %g %g %g %g %g %g %g %g %g %g %g\nFinal solution: %g %g %g %g %g %g %g %g %g %g %g\n", diff --git a/sample_consensus/include/pcl/sample_consensus/impl/sac_model_torus.hpp b/sample_consensus/include/pcl/sample_consensus/impl/sac_model_torus.hpp index dd9e7699c4d..81cd1070c1e 100644 --- a/sample_consensus/include/pcl/sample_consensus/impl/sac_model_torus.hpp +++ b/sample_consensus/include/pcl/sample_consensus/impl/sac_model_torus.hpp @@ -86,7 +86,7 @@ pcl::SampleConsensusModelTorus::isSampleGood( ////////////////////////////////////////////////////////////////////////////////////////////////////////////////// float -crossDot(Eigen::Vector3f v1, Eigen::Vector3f v2, Eigen::Vector3f v3) +crossDot(const Eigen::Vector3f& v1, const Eigen::Vector3f& v2, const Eigen::Vector3f& v3) { return v1.cross(v2).dot(v3); } diff --git a/surface/include/pcl/surface/impl/grid_projection.hpp b/surface/include/pcl/surface/impl/grid_projection.hpp index c5e232fa606..6147e51fea2 100644 --- a/surface/include/pcl/surface/impl/grid_projection.hpp +++ b/surface/include/pcl/surface/impl/grid_projection.hpp @@ -630,9 +630,7 @@ pcl::GridProjection::reconstructPolygons (std::vector &p for (pcl::index_t cp = 0; cp < static_cast (data_->size ()); ++cp) { // Check if the point is invalid - if (!std::isfinite ((*data_)[cp].x) || - !std::isfinite ((*data_)[cp].y) || - !std::isfinite ((*data_)[cp].z)) + if (!pcl::isXYZFinite((*data_)[cp])) continue; Eigen::Vector3i index_3d; diff --git a/visualization/include/pcl/visualization/impl/pcl_visualizer.hpp b/visualization/include/pcl/visualization/impl/pcl_visualizer.hpp index 08c81f08e23..f8060868d75 100644 --- a/visualization/include/pcl/visualization/impl/pcl_visualizer.hpp +++ b/visualization/include/pcl/visualization/impl/pcl_visualizer.hpp @@ -259,9 +259,7 @@ pcl::visualization::PCLVisualizer::convertPointCloudToVTKPolyData ( for (vtkIdType i = 0; i < nr_points; ++i) { // Check if the point is invalid - if (!std::isfinite ((*cloud)[i].x) || - !std::isfinite ((*cloud)[i].y) || - !std::isfinite ((*cloud)[i].z)) + if (!pcl::isXYZFinite((*cloud)[i])) continue; std::copy (&(*cloud)[i].x, &(*cloud)[i].x + 3, &data[ptr]); diff --git a/visualization/include/pcl/visualization/impl/point_cloud_color_handlers.hpp b/visualization/include/pcl/visualization/impl/point_cloud_color_handlers.hpp index 991991db08f..7441dd8a4a9 100644 --- a/visualization/include/pcl/visualization/impl/point_cloud_color_handlers.hpp +++ b/visualization/include/pcl/visualization/impl/point_cloud_color_handlers.hpp @@ -171,9 +171,7 @@ PointCloudColorHandlerRGBField::getColor () const for (vtkIdType cp = 0; cp < nr_points; ++cp) { // Copy the value at the specified field - if (!std::isfinite ((*cloud_)[cp].x) || - !std::isfinite ((*cloud_)[cp].y) || - !std::isfinite ((*cloud_)[cp].z)) + if (!pcl::isXYZFinite((*cloud_)[cp])) continue; memcpy (&rgb, (reinterpret_cast (&(*cloud_)[cp])) + rgba_offset, sizeof (pcl::RGB)); colors[j ] = rgb.r; @@ -256,9 +254,7 @@ PointCloudColorHandlerHSVField::getColor () const for (vtkIdType cp = 0; cp < nr_points; ++cp) { // Copy the value at the specified field - if (!std::isfinite ((*cloud_)[cp].x) || - !std::isfinite ((*cloud_)[cp].y) || - !std::isfinite ((*cloud_)[cp].z)) + if (!pcl::isXYZFinite((*cloud_)[cp])) continue; ///@todo do this with the point_types_conversion in common, first template it! @@ -409,7 +405,7 @@ PointCloudColorHandlerGenericField::getColor () const for (vtkIdType cp = 0; cp < nr_points; ++cp) { // Copy the value at the specified field - if (!std::isfinite ((*cloud_)[cp].x) || !std::isfinite ((*cloud_)[cp].y) || !std::isfinite ((*cloud_)[cp].z)) + if (!pcl::isXYZFinite((*cloud_)[cp])) continue; const std::uint8_t* pt_data = reinterpret_cast (&(*cloud_)[cp]); @@ -479,9 +475,7 @@ PointCloudColorHandlerRGBAField::getColor () const for (vtkIdType cp = 0; cp < nr_points; ++cp) { // Copy the value at the specified field - if (!std::isfinite ((*cloud_)[cp].x) || - !std::isfinite ((*cloud_)[cp].y) || - !std::isfinite ((*cloud_)[cp].z)) + if (!pcl::isXYZFinite((*cloud_)[cp])) continue; colors[j ] = (*cloud_)[cp].r; diff --git a/visualization/src/pcl_visualizer.cpp b/visualization/src/pcl_visualizer.cpp index 64df8564a1c..902ee201657 100644 --- a/visualization/src/pcl_visualizer.cpp +++ b/visualization/src/pcl_visualizer.cpp @@ -4358,7 +4358,7 @@ int pcl::visualization::PCLVisualizer::getGeometryHandlerIndex (const std::string &id) { auto am_it = style_->getCloudActorMap ()->find (id); - if (am_it != cloud_actor_map_->end ()) + if (am_it == cloud_actor_map_->end ()) return (-1); return (am_it->second.geometry_handler_index_); @@ -4593,7 +4593,7 @@ pcl::visualization::PCLVisualizer::getUniqueCameraFile (int argc, char **argv) boost::uuids::detail::sha1::digest_type digest; sha1.get_digest (digest); sstream << "."; - for (int i = 0; i < 5; ++i) { + for (int i = 0; i < static_cast(sizeof(digest)/sizeof(unsigned int)); ++i) { sstream << std::hex << *(reinterpret_cast(&digest[0]) + i); } sstream << ".cam";