Fixes PLY parsing of char and uchar scalars

SergioRAgostinho · SergioRAgostinho · commit ed1bdb3f3d3c · 2015-11-30T00:02:08.000+01:00
WIP of #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.
diff --git a/io/include/pcl/io/ply/ply.h b/io/include/pcl/io/ply/ply.h
@@ -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
       
diff --git a/io/include/pcl/io/ply/ply_parser.h b/io/include/pcl/io/ply/ply_parser.h
@@ -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 &)
     {
@@ -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 &)
       {
diff --git a/test/io/CMakeLists.txt b/test/io/CMakeLists.txt
@@ -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)
diff --git a/test/io/test_io.cpp b/test/io/test_io.cpp
@@ -761,7 +761,6 @@ TEST (PCL, PCDReaderWriterASCIIColorPrecision)
   }
 }
 
-///////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 TEST (PCL, ASCIIReader)
 {
@@ -865,6 +864,193 @@ 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 ();
+  }
+
+  virtual
+  ~PLYTest () { remove (mesh_file_ply_.c_str ()); }
+
+  std::string mesh_file_ply_;
+};
+
+//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+TEST_F (PLYTest, LoadPLYFileColoredASCII)
+{
+  int res;
+  uint32_t rgba, rgba_1, rgba_2, rgba_3, rgba_4;
+
+  // 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);
+
+  // Blobs
+  {
+    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);
+  }
+
+  // PointCloud<PointXYZRGB>
+  {
+    PointCloud<PointXYZRGB> cloud_rgb;
+
+    // check if loading is ok
+    res = loadPLYFile (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, rgba_1);
+    ASSERT_EQ (cloud_rgb[1].rgba, rgba_2);
+    ASSERT_EQ (cloud_rgb[2].rgba, rgba_3);
+    ASSERT_EQ (cloud_rgb[3].rgba, rgba_4);
+  }
+
+  //PointCloud<PointXYZRGBA>
+  {
+    PointCloud<PointXYZRGBA> cloud_rgba;
+
+    // check if loading is ok
+    res = loadPLYFile (mesh_file_ply_, cloud_rgba);
+    ASSERT_EQ (res, 0);
+
+    // cloud has proper structure
+    EXPECT_EQ (cloud_rgba.height, 1);
+    EXPECT_EQ (cloud_rgba.width, 4);
+    EXPECT_EQ (cloud_rgba.points.size(), 4);
+    // EXPECT_TRUE (cloud_rgba.is_dense); // this is failing and it shouldnt?
+
+    // scope cloud data
+    ASSERT_EQ (cloud_rgba[0].rgba, rgba_1);
+    ASSERT_EQ (cloud_rgba[1].rgba, rgba_2);
+    ASSERT_EQ (cloud_rgba[2].rgba, rgba_3);
+    ASSERT_EQ (cloud_rgba[3].rgba, rgba_4);
+  }
+
+  //PolygonMesh
+  {
+    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);
+  }
+}
+
 //////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 struct PointXYZFPFH33

Original file line number	Diff line number	Diff line change
`@@ -565,7 +565,7 @@ inline bool pcl::io::ply::ply_parser::parse_scalar_property (format_type format,`
`565`	`565`	`istream >> value_s;`
`566`	`566`	`try`
`567`	`567`	`{`
`568`		`- value = boost::lexical_cast<scalar_type> (value_s);`
	`568`	`+ value = static_cast<scalar_type> (boost::lexical_cast<typename pcl::io::ply::type_traits<scalar_type>::parse_type> (value_s));`
`569`	`569`	`}`
`570`	`570`	`catch (boost::bad_lexical_cast &)`
`571`	`571`	`{`
`@@ -641,7 +641,7 @@ inline bool pcl::io::ply::ply_parser::parse_list_property (format_type format, s`
`641`	`641`	`istream >> value_s;`
`642`	`642`	`try`
`643`	`643`	`{`
`644`		`- value = boost::lexical_cast<scalar_type> (value_s);`
	`644`	`+ value = static_cast<scalar_type> (boost::lexical_cast<typename pcl::io::ply::type_traits<scalar_type>::parse_type> (value_s));`
`645`	`645`	`}`
`646`	`646`	`catch (boost::bad_lexical_cast &)`
`647`	`647`	`{`