Skip to content

Commit

Permalink
Fixes PLY parsing of char and uchar scalars
Browse files Browse the repository at this point in the history 
WIP of PointCloudLibrary#1381. int8 and uint8 types were being evaluated as char and
unsigned char, their actual original type, so whenever a lexical_cast
was performed from a multi-digit string to an (unsigned) char, a
bad_lexical_cast exception was thrown and the value set to a quiet
NaN. Functionally-wise, this PR will make color parsing work.
  • Loading branch information
@SergioRAgostinho
SergioRAgostinho committed Nov 30, 2015
1 parent bf51cf4 commit 1df66e9
Show file tree
Hide file tree
Showing 4 changed files with 195 additions and 20 deletions.
26 changes: 14 additions & 12 deletions io/include/pcl/io/ply/ply.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,23 +73,25 @@ namespace pcl
# error
#endif

#define PLY_TYPE_TRAITS(TYPE, NAME, OLD_NAME) \
#define PLY_TYPE_TRAITS(TYPE, PARSE_TYPE, NAME, OLD_NAME) \
template <> \
struct type_traits<TYPE> \
{ \
typedef TYPE type; \
static const char* name() { return NAME; } \
static const char* old_name() { return OLD_NAME; } \
}
typedef PARSE_TYPE parse_type; \
static const char* name () { return NAME; } \
static const char* old_name () { return OLD_NAME; } \
};

PLY_TYPE_TRAITS(int8, int16, "int8", "char");
PLY_TYPE_TRAITS(int16, int16, "int16", "short");
PLY_TYPE_TRAITS(int32, int32, "int32", "int");
PLY_TYPE_TRAITS(uint8, uint16, "uint8", "uchar");
PLY_TYPE_TRAITS(uint16, uint16, "uint16", "ushort");
PLY_TYPE_TRAITS(uint32, uint32, "uint32", "uint");
PLY_TYPE_TRAITS(float32, float32, "float32", "float");
PLY_TYPE_TRAITS(float64, float64, "float64", "double");

PLY_TYPE_TRAITS(int8, "int8", "char");
PLY_TYPE_TRAITS(int16, "int16", "short");
PLY_TYPE_TRAITS(int32, "int32", "int");
PLY_TYPE_TRAITS(uint8, "uint8", "uchar");
PLY_TYPE_TRAITS(uint16, "uint16", "ushort");
PLY_TYPE_TRAITS(uint32, "uint32", "uint");
PLY_TYPE_TRAITS(float32, "float32", "float");
PLY_TYPE_TRAITS(float64, "float64", "double");

#undef PLY_TYPE_TRAITS

Expand Down
4 changes: 2 additions & 2 deletions io/include/pcl/io/ply/ply_parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ inline bool pcl::io::ply::ply_parser::parse_scalar_property (format_type format,
istream >> value_s;
try
{
value = boost::lexical_cast<scalar_type> (value_s);
value = static_cast<scalar_type> (boost::lexical_cast<typename pcl::io::ply::type_traits<scalar_type>::parse_type> (value_s));
}
catch (boost::bad_lexical_cast &)
{
Expand Down Expand Up @@ -641,7 +641,7 @@ inline bool pcl::io::ply::ply_parser::parse_list_property (format_type format, s
istream >> value_s;
try
{
value = boost::lexical_cast<scalar_type> (value_s);
value = static_cast<scalar_type> (boost::lexical_cast<typename pcl::io::ply::type_traits<scalar_type>::parse_type> (value_s));
}
catch (boost::bad_lexical_cast &)
{
Expand Down
9 changes: 4 additions & 5 deletions test/io/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
if(WITH_OPENNI)
PCL_ADD_TEST(io_io test_io
FILES test_io.cpp
LINK_WITH pcl_gtest pcl_io)
endif()
PCL_ADD_TEST(io_io test_io
FILES test_io.cpp
LINK_WITH pcl_gtest pcl_io)

PCL_ADD_TEST(io_iterators test_iterators
FILES test_iterators.cpp
LINK_WITH pcl_gtest pcl_io)
Expand Down
176 changes: 175 additions & 1 deletion test/io/test_io.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,6 @@ TEST (PCL, PCDReaderWriterASCIIColorPrecision)
}
}

///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
TEST (PCL, ASCIIReader)
{
Expand Down Expand Up @@ -865,6 +864,181 @@ TEST (PCL, PLYReaderWriter)
}
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
class PLYTest : public ::testing::Test
{
protected:

PLYTest () : mesh_file_ply_("ply_color_mesh.ply")
{
std::ofstream fs;
fs.open (mesh_file_ply_);
fs << "ply\n"
"format ascii 1.0\n"
"element vertex 4\n"
"property float x\n"
"property float y\n"
"property float z\n"
"property uchar red\n"
"property uchar green\n"
"property uchar blue\n"
"property uchar alpha\n"
"element face 2\n"
"property list uchar int vertex_indices\n"
"end_header\n"
"4.23607 0 1.61803 255 0 0 255\n"
"2.61803 2.61803 2.61803 0 255 0 0\n"
"0 1.61803 4.23607 0 0 255 128\n"
"0 -1.61803 4.23607 255 255 255 128\n"
"3 0 1 2\n"
"3 1 2 3\n";
fs.close ();

// Test colors from ply_benchmark.ply
rgba_1_ = static_cast<uint32_t> (255) << 24 | static_cast<uint32_t> (255) << 16 |
static_cast<uint32_t> (0) << 8 | static_cast<uint32_t> (0);
rgba_2_ = static_cast<uint32_t> (0) << 24 | static_cast<uint32_t> (0) << 16 |
static_cast<uint32_t> (255) << 8 | static_cast<uint32_t> (0);
rgba_3_ = static_cast<uint32_t> (128) << 24 | static_cast<uint32_t> (0) << 16 |
static_cast<uint32_t> (0) << 8 | static_cast<uint32_t> (255);
rgba_4_ = static_cast<uint32_t> (128) << 24 | static_cast<uint32_t> (255) << 16 |
static_cast<uint32_t> (255) << 8 | static_cast<uint32_t> (255);
}

virtual
~PLYTest () { remove (mesh_file_ply_.c_str ()); }

std::string mesh_file_ply_;
uint32_t rgba_1_;
uint32_t rgba_2_;
uint32_t rgba_3_;
uint32_t rgba_4_;
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
TEST_F (PLYTest, LoadPLYFileColoredASCIIIntoBlob)
{
int res;
uint32_t rgba;

PCLPointCloud2 cloud_blob;
uint32_t ps;
int32_t offset = -1;

// check if loading is ok
res = loadPLYFile (mesh_file_ply_, cloud_blob);
ASSERT_EQ (res, 0);

// blob has proper structure
EXPECT_EQ (cloud_blob.height, 1);
EXPECT_EQ (cloud_blob.width, 4);
EXPECT_EQ (cloud_blob.fields.size(), 4);
EXPECT_FALSE (cloud_blob.is_bigendian);
EXPECT_EQ (cloud_blob.point_step, 16);
EXPECT_EQ (cloud_blob.row_step, 16 * 4);
EXPECT_EQ (cloud_blob.data.size(), 16 * 4);
// EXPECT_TRUE (cloud_blob.is_dense); // this is failing and it shouldnt?

// scope blob data
ps = cloud_blob.point_step;
for (size_t i = 0; i < cloud_blob.fields.size (); ++i)
if (cloud_blob.fields[i].name == std::string("rgba"))
offset = static_cast<int32_t> (cloud_blob.fields[i].offset);

ASSERT_GE (offset, 0);

// 1st point
rgba = *reinterpret_cast<uint32_t *> (&cloud_blob.data[offset]);
ASSERT_EQ (rgba, rgba_1_);

// 2th point
rgba = *reinterpret_cast<uint32_t *> (&cloud_blob.data[ps + offset]);
ASSERT_EQ (rgba, rgba_2_);

// 3th point
rgba = *reinterpret_cast<uint32_t *> (&cloud_blob.data[2 * ps + offset]);
ASSERT_EQ (rgba, rgba_3_);

// 4th point
rgba = *reinterpret_cast<uint32_t *> (&cloud_blob.data[3 * ps + offset]);
ASSERT_EQ (rgba, rgba_4_);
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
TEST_F (PLYTest, LoadPLYFileColoredASCIIIntoPolygonMesh)
{
int res;
uint32_t rgba;
PolygonMesh mesh;
uint32_t ps;
int32_t offset = -1;

// check if loading is ok
res = loadPLYFile (mesh_file_ply_, mesh);
ASSERT_EQ (res, 0);

// blob has proper structure
EXPECT_EQ (mesh.cloud.height, 1);
EXPECT_EQ (mesh.cloud.width, 4);
EXPECT_EQ (mesh.cloud.fields.size(), 4);
EXPECT_FALSE (mesh.cloud.is_bigendian);
EXPECT_EQ (mesh.cloud.point_step, 16);
EXPECT_EQ (mesh.cloud.row_step, 16 * 4);
EXPECT_EQ (mesh.cloud.data.size(), 16 * 4);
// EXPECT_TRUE (mesh.cloud.is_dense); // this is failing and it shouldnt?

// scope blob data
ps = mesh.cloud.point_step;
for (size_t i = 0; i < mesh.cloud.fields.size (); ++i)
if (mesh.cloud.fields[i].name == std::string("rgba"))
offset = static_cast<int32_t> (mesh.cloud.fields[i].offset);

ASSERT_GE (offset, 0);

// 1st point
rgba = *reinterpret_cast<uint32_t *> (&mesh.cloud.data[offset]);
ASSERT_EQ (rgba, rgba_1_);

// 2th point
rgba = *reinterpret_cast<uint32_t *> (&mesh.cloud.data[ps + offset]);
ASSERT_EQ (rgba, rgba_2_);

// 3th point
rgba = *reinterpret_cast<uint32_t *> (&mesh.cloud.data[2 * ps + offset]);
ASSERT_EQ (rgba, rgba_3_);

// 4th point
rgba = *reinterpret_cast<uint32_t *> (&mesh.cloud.data[3 * ps + offset]);
ASSERT_EQ (rgba, rgba_4_);
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
template <typename T> class PLYPointCloudTest : public PLYTest { };
typedef ::testing::Types<BOOST_PP_SEQ_ENUM (PCL_RGB_POINT_TYPES)> RGBPointTypes;
TYPED_TEST_CASE (PLYPointCloudTest, RGBPointTypes);
TYPED_TEST (PLYPointCloudTest, LoadPLYFileColoredASCIIIntoPointCloud)
{
int res;
uint32_t rgba;
PointCloud<TypeParam> cloud_rgb;

// check if loading is ok
res = loadPLYFile (PLYTest::mesh_file_ply_, cloud_rgb);
ASSERT_EQ (res, 0);

// cloud has proper structure
EXPECT_EQ (cloud_rgb.height, 1);
EXPECT_EQ (cloud_rgb.width, 4);
EXPECT_EQ (cloud_rgb.points.size(), 4);
// EXPECT_TRUE (cloud_rgb.is_dense); // this is failing and it shouldnt?

// scope cloud data
ASSERT_EQ (cloud_rgb[0].rgba, PLYTest::rgba_1_);
ASSERT_EQ (cloud_rgb[1].rgba, PLYTest::rgba_2_);
ASSERT_EQ (cloud_rgb[2].rgba, PLYTest::rgba_3_);
ASSERT_EQ (cloud_rgb[3].rgba, PLYTest::rgba_4_);
}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////

struct PointXYZFPFH33
Expand Down

0 comments on commit 1df66e9

Please sign in to comment.