Skip to content

Commit

Permalink
Remove unnecessary calls to std::isnan and std::isfinite. This fix MS…
Browse files Browse the repository at this point in the history
…VC compile issue due to missing IntegralType overloading.
  • Loading branch information
Heiko Thiel committed Jan 24, 2019
1 parent 2c6b624 commit d6b7134
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 112 deletions.
54 changes: 24 additions & 30 deletions common/include/pcl/common/point_tests.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,29 +63,30 @@ namespace pcl
}
#endif

template<> inline bool isFinite<pcl::RGB> (const pcl::RGB&) { return (true); }
template<> inline bool isFinite<pcl::Label> (const pcl::Label&) { return (true); }
template<> inline bool isFinite<pcl::Axis> (const pcl::Axis&) { return (true); }
template<> inline bool isFinite<pcl::Intensity> (const pcl::Intensity&) { return (true); }
template<> inline bool isFinite<pcl::MomentInvariants> (const pcl::MomentInvariants&) { return (true); }
template<> inline bool isFinite<pcl::PrincipalRadiiRSD> (const pcl::PrincipalRadiiRSD&) { return (true); }
template<> inline bool isFinite<pcl::Boundary> (const pcl::Boundary&) { return (true); }
template<> inline bool isFinite<pcl::PrincipalCurvatures> (const pcl::PrincipalCurvatures&) { return (true); }
template<> inline bool isFinite<pcl::SHOT352> (const pcl::SHOT352&) { return (true); }
template<> inline bool isFinite<pcl::SHOT1344> (const pcl::SHOT1344&) { return (true); }
template<> inline bool isFinite<pcl::ReferenceFrame> (const pcl::ReferenceFrame&) { return (true); }
template<> inline bool isFinite<pcl::ShapeContext1980> (const pcl::ShapeContext1980&) { return (true); }
template<> inline bool isFinite<pcl::UniqueShapeContext1960> (const pcl::UniqueShapeContext1960&) { return (true); }
template<> inline bool isFinite<pcl::PFHSignature125> (const pcl::PFHSignature125&) { return (true); }
template<> inline bool isFinite<pcl::PFHRGBSignature250> (const pcl::PFHRGBSignature250&) { return (true); }
template<> inline bool isFinite<pcl::PPFSignature> (const pcl::PPFSignature&) { return (true); }
template<> inline bool isFinite<pcl::PPFRGBSignature> (const pcl::PPFRGBSignature&) { return (true); }
template<> inline bool isFinite<pcl::NormalBasedSignature12> (const pcl::NormalBasedSignature12&) { return (true); }
template<> inline bool isFinite<pcl::FPFHSignature33> (const pcl::FPFHSignature33&) { return (true); }
template<> inline bool isFinite<pcl::VFHSignature308> (const pcl::VFHSignature308&) { return (true); }
template<> inline bool isFinite<pcl::ESFSignature640> (const pcl::ESFSignature640&) { return (true); }
template<> inline bool isFinite<pcl::IntensityGradient> (const pcl::IntensityGradient&) { return (true); }
template<> inline bool isFinite<pcl::BRISKSignature512> (const pcl::BRISKSignature512&) { return (true); }
template<> inline bool isFinite<pcl::Axis>(const pcl::Axis&) { return (true); }
template<> inline bool isFinite<pcl::BRISKSignature512>(const pcl::BRISKSignature512&) { return (true); }
template<> inline bool isFinite<pcl::BorderDescription>(const pcl::BorderDescription &p) { return true; }
template<> inline bool isFinite<pcl::Boundary>(const pcl::Boundary&) { return (true); }
template<> inline bool isFinite<pcl::ESFSignature640>(const pcl::ESFSignature640&) { return (true); }
template<> inline bool isFinite<pcl::FPFHSignature33>(const pcl::FPFHSignature33&) { return (true); }
template<> inline bool isFinite<pcl::Intensity>(const pcl::Intensity&) { return (true); }
template<> inline bool isFinite<pcl::IntensityGradient>(const pcl::IntensityGradient&) { return (true); }
template<> inline bool isFinite<pcl::Label>(const pcl::Label&) { return (true); }
template<> inline bool isFinite<pcl::MomentInvariants>(const pcl::MomentInvariants&) { return (true); }
template<> inline bool isFinite<pcl::NormalBasedSignature12>(const pcl::NormalBasedSignature12&) { return (true); }
template<> inline bool isFinite<pcl::PFHRGBSignature250>(const pcl::PFHRGBSignature250&) { return (true); }
template<> inline bool isFinite<pcl::PFHSignature125>(const pcl::PFHSignature125&) { return (true); }
template<> inline bool isFinite<pcl::PPFRGBSignature>(const pcl::PPFRGBSignature&) { return (true); }
template<> inline bool isFinite<pcl::PPFSignature>(const pcl::PPFSignature&) { return (true); }
template<> inline bool isFinite<pcl::PrincipalCurvatures>(const pcl::PrincipalCurvatures&) { return (true); }
template<> inline bool isFinite<pcl::PrincipalRadiiRSD>(const pcl::PrincipalRadiiRSD&) { return (true); }
template<> inline bool isFinite<pcl::RGB>(const pcl::RGB&) { return (true); }
template<> inline bool isFinite<pcl::ReferenceFrame>(const pcl::ReferenceFrame&) { return (true); }
template<> inline bool isFinite<pcl::SHOT1344>(const pcl::SHOT1344&) { return (true); }
template<> inline bool isFinite<pcl::SHOT352>(const pcl::SHOT352&) { return (true); }
template<> inline bool isFinite<pcl::ShapeContext1980>(const pcl::ShapeContext1980&) { return (true); }
template<> inline bool isFinite<pcl::UniqueShapeContext1960>(const pcl::UniqueShapeContext1960&) { return (true); }
template<> inline bool isFinite<pcl::VFHSignature308>(const pcl::VFHSignature308&) { return (true); }

// specification for pcl::PointXY
template <> inline bool
Expand All @@ -94,13 +95,6 @@ namespace pcl
return (std::isfinite (p.x) && std::isfinite (p.y));
}

// specification for pcl::BorderDescription
template <> inline bool
isFinite<pcl::BorderDescription> (const pcl::BorderDescription &p)
{
return (std::isfinite (p.x) && std::isfinite (p.y));
}

// specification for pcl::Normal
template <> inline bool
isFinite<pcl::Normal> (const pcl::Normal &n)
Expand Down
63 changes: 42 additions & 21 deletions io/include/pcl/io/file_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,8 @@ namespace pcl
* \param[in] fields_count the current fields count
* \param[out] stream the ostringstream to copy into
*/
template <typename Type> inline void
template <typename Type> inline
std::enable_if_t<std::is_floating_point<Type>::value>
copyValueString (const pcl::PCLPointCloud2 &cloud,
const unsigned int point_index,
const int point_size,
Expand All @@ -244,8 +245,23 @@ namespace pcl
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<Type>(value);
stream << value;
}

template <typename Type> inline
std::enable_if_t<std::is_integral<Type>::value>
copyValueString (const pcl::PCLPointCloud2 &cloud,
const unsigned int point_index,
const int point_size,
const unsigned int field_idx,
const unsigned int fields_count,
std::ostream &stream)
{
Type value;
memcpy (&value, &cloud.data[point_index * point_size + cloud.fields[field_idx].offset + fields_count * sizeof (Type)], sizeof (Type));
stream << value;
}

template <> inline void
copyValueString<int8_t> (const pcl::PCLPointCloud2 &cloud,
const unsigned int point_index,
Expand All @@ -256,27 +272,22 @@ namespace pcl
{
int8_t value;
memcpy (&value, &cloud.data[point_index * point_size + cloud.fields[field_idx].offset + fields_count * sizeof (int8_t)], sizeof (int8_t));
if (std::isnan (value))
stream << "nan";
else
// Numeric cast doesn't give us what we want for int8_t
stream << boost::numeric_cast<int>(value);
//Cast to int to prevent value is handled as char
stream << boost::numeric_cast<int>(value);
}

template <> inline void
copyValueString<uint8_t> (const pcl::PCLPointCloud2 &cloud,
const unsigned int point_index,
const int point_size,
const unsigned int field_idx,
const unsigned int fields_count,
std::ostream &stream)
const unsigned int point_index,
const int point_size,
const unsigned int field_idx,
const unsigned int fields_count,
std::ostream &stream)
{
uint8_t value;
memcpy (&value, &cloud.data[point_index * point_size + cloud.fields[field_idx].offset + fields_count * sizeof (uint8_t)], sizeof (uint8_t));
if (std::isnan (value))
stream << "nan";
else
// Numeric cast doesn't give us what we want for uint8_t
stream << boost::numeric_cast<int>(value);
//Cast to unsigned int to prevent value is handled as char
stream << boost::numeric_cast<unsigned int>(value);
}

/** \brief Check whether a given value of type Type (uchar, char, uint, int, float, double, ...) is finite or not
Expand All @@ -289,7 +300,8 @@ namespace pcl
*
* \return true if the value is finite, false otherwise
*/
template <typename Type> inline bool
template <typename Type> inline
std::enable_if_t<std::is_floating_point<Type>::value, bool>
isValueFinite (const pcl::PCLPointCloud2 &cloud,
const unsigned int point_index,
const int point_size,
Expand All @@ -298,9 +310,18 @@ namespace pcl
{
Type value;
memcpy (&value, &cloud.data[point_index * point_size + cloud.fields[field_idx].offset + fields_count * sizeof (Type)], sizeof (Type));
if (!std::isfinite (value))
return (false);
return (true);
return std::isfinite (value);
}

template <typename Type> inline
std::enable_if_t<std::is_integral<Type>::value, bool>
isValueFinite (const pcl::PCLPointCloud2 &cloud,
const unsigned int point_index,
const int point_size,
const unsigned int field_idx,
const unsigned int fields_count)
{
return true;
}

/** \brief Copy one single value of type T (uchar, char, uint, int, float, double, ...) from a string
Expand Down
70 changes: 14 additions & 56 deletions io/include/pcl/io/impl/pcd_io.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -494,60 +494,42 @@ pcl::PCDWriter::writeASCII (const std::string &file_name, const pcl::PointCloud<
{
int8_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[i]) + fields[d].offset + c * sizeof (int8_t), sizeof (int8_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<uint32_t>(value);
stream << boost::numeric_cast<int32_t>(value);
break;
}
case pcl::PCLPointField::UINT8:
{
uint8_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[i]) + fields[d].offset + c * sizeof (uint8_t), sizeof (uint8_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<uint32_t>(value);
stream << boost::numeric_cast<uint32_t>(value);
break;
}
case pcl::PCLPointField::INT16:
{
int16_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[i]) + fields[d].offset + c * sizeof (int16_t), sizeof (int16_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<int16_t>(value);
stream << boost::numeric_cast<int16_t>(value);
break;
}
case pcl::PCLPointField::UINT16:
{
uint16_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[i]) + fields[d].offset + c * sizeof (uint16_t), sizeof (uint16_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<uint16_t>(value);
stream << boost::numeric_cast<uint16_t>(value);
break;
}
case pcl::PCLPointField::INT32:
{
int32_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[i]) + fields[d].offset + c * sizeof (int32_t), sizeof (int32_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<int32_t>(value);
stream << boost::numeric_cast<int32_t>(value);
break;
}
case pcl::PCLPointField::UINT32:
{
uint32_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[i]) + fields[d].offset + c * sizeof (uint32_t), sizeof (uint32_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<uint32_t>(value);
stream << boost::numeric_cast<uint32_t>(value);
break;
}
case pcl::PCLPointField::FLOAT32:
Expand All @@ -561,10 +543,7 @@ pcl::PCDWriter::writeASCII (const std::string &file_name, const pcl::PointCloud<
{
uint32_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[i]) + fields[d].offset + c * sizeof (float), sizeof (float));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<uint32_t>(value);
stream << boost::numeric_cast<uint32_t>(value);
break;
}
else
Expand Down Expand Up @@ -803,60 +782,42 @@ pcl::PCDWriter::writeASCII (const std::string &file_name,
{
int8_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[indices[i]]) + fields[d].offset + c * sizeof (int8_t), sizeof (int8_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<uint32_t>(value);
stream << boost::numeric_cast<int32_t>(value);
break;
}
case pcl::PCLPointField::UINT8:
{
uint8_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[indices[i]]) + fields[d].offset + c * sizeof (uint8_t), sizeof (uint8_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<uint32_t>(value);
stream << boost::numeric_cast<uint32_t>(value);
break;
}
case pcl::PCLPointField::INT16:
{
int16_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[indices[i]]) + fields[d].offset + c * sizeof (int16_t), sizeof (int16_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<int16_t>(value);
stream << boost::numeric_cast<int16_t>(value);
break;
}
case pcl::PCLPointField::UINT16:
{
uint16_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[indices[i]]) + fields[d].offset + c * sizeof (uint16_t), sizeof (uint16_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<uint16_t>(value);
stream << boost::numeric_cast<uint16_t>(value);
break;
}
case pcl::PCLPointField::INT32:
{
int32_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[indices[i]]) + fields[d].offset + c * sizeof (int32_t), sizeof (int32_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<int32_t>(value);
stream << boost::numeric_cast<int32_t>(value);
break;
}
case pcl::PCLPointField::UINT32:
{
uint32_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[indices[i]]) + fields[d].offset + c * sizeof (uint32_t), sizeof (uint32_t));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<uint32_t>(value);
stream << boost::numeric_cast<uint32_t>(value);
break;
}
case pcl::PCLPointField::FLOAT32:
Expand All @@ -870,10 +831,7 @@ pcl::PCDWriter::writeASCII (const std::string &file_name,
{
uint32_t value;
memcpy (&value, reinterpret_cast<const char*> (&cloud.points[indices[i]]) + fields[d].offset + c * sizeof (float), sizeof (float));
if (std::isnan (value))
stream << "nan";
else
stream << boost::numeric_cast<uint32_t>(value);
stream << boost::numeric_cast<uint32_t>(value);
}
else
{
Expand Down
21 changes: 18 additions & 3 deletions io/src/ply_io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,25 @@ namespace pcl
return (0);
}

template<typename T> inline
std::enable_if_t<std::is_floating_point<T>::value>
unsetDenseFlagIfNotFinite(T value, PCLPointCloud2& cloud)
{
if (!std::isfinite(value))
cloud.is_dense = false;
}

template<typename T> inline
std::enable_if_t<std::is_integral<T>::value>
unsetDenseFlagIfNotFinite(T value, PCLPointCloud2& cloud)
{
}

template<typename Scalar> void
PLYReader::vertexScalarPropertyCallback (Scalar value)
{
if (!std::isfinite (value))
cloud_->is_dense = false;
//MSVC is missing bool std::isfinite(IntegralType arg); variant, so we implement an own template specialization for this
unsetDenseFlagIfNotFinite(value, cloud_);

memcpy (&cloud_->data[vertex_count_ * cloud_->point_step + vertex_offset_before_],
&value,
Expand All @@ -294,7 +308,8 @@ namespace pcl
template<typename ContentType> void
PLYReader::vertexListPropertyContentCallback (ContentType value)
{
if (!std::isfinite (value))
//MSVC is missing bool std::isfinite(IntegralType arg); variant, so we need to cast ourself to double
if (!std::isfinite (static_cast<double>(value)))
cloud_->is_dense = false;

memcpy (&cloud_->data[vertex_count_ * cloud_->point_step + vertex_offset_before_],
Expand Down
5 changes: 3 additions & 2 deletions test/io/test_buffers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,9 @@ class BuffersTest : public ::testing::Test
memcpy (d.data (), dptr, buffer.size () * sizeof (T));
buffer.push (d);
for (size_t j = 0; j < buffer.size (); ++j)
if (std::isnan (eptr[j]))
EXPECT_TRUE (std::isnan (buffer[j]));
//MSVC is missing bool std::isnan(IntegralType arg); variant, so we need to cast ourself to double
if (std::isnan (static_cast<double>(eptr[j])))
EXPECT_TRUE (std::isnan (static_cast<double>(buffer[j])));
else
EXPECT_EQ (eptr[j], buffer[j]);
dptr += buffer.size ();
Expand Down

0 comments on commit d6b7134

Please sign in to comment.