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.

Sign up for GitHub

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

Fix PLYReader is_dense behavior #2133

Merged
merged 5 commits into from
Mar 6, 2018
Merged

Fix PLYReader is_dense behavior #2133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
72b83ea
Fix is_dense flag not properly set
VictorLamoine Dec 2, 2015
782eca3
Re-enable is_dense check in PLY io tests
VictorLamoine Dec 2, 2015
dc68e36
Add finite check to list values
SergioRAgostinho Dec 8, 2017
d58e79c
Clarify is_dense definition
SergioRAgostinho Dec 8, 2017
316c26d
Add unit tests validating is_dense inference by PLYReader
SergioRAgostinho Dec 8, 2017
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
2 changes: 1 addition & 1 deletion common/include/pcl/point_cloud.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ namespace pcl
/** \brief The point cloud height (if organized as an image-structure). */
uint32_t height;

/** \brief True if no points are invalid (e.g., have NaN or Inf values). */
/** \brief True if no points are invalid (e.g., have NaN or Inf values in any of their floating point fields). */
bool is_dense;

/** \brief Sensor acquisition pose (origin/translation). */
Expand Down
8 changes: 7 additions & 1 deletion io/src/ply_io.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pcl::PLYReader::elementDefinitionCallback (const std::string& element_name, std:
cloud_->width = static_cast<uint32_t> (count);
cloud_->height = 1;
}
cloud_->is_dense = false;
cloud_->is_dense = true;
cloud_->point_step = 0;
cloud_->row_step = 0;
vertex_count_ = 0;
Expand Down Expand Up @@ -267,6 +267,9 @@ namespace pcl
template<typename Scalar> void
PLYReader::vertexScalarPropertyCallback (Scalar value)
{
if (!pcl_isfinite (value))
cloud_->is_dense = false;

memcpy (&cloud_->data[vertex_count_ * cloud_->point_step + vertex_offset_before_],
&value,
sizeof (Scalar));
Expand All @@ -291,6 +294,9 @@ namespace pcl
template<typename ContentType> void
PLYReader::vertexListPropertyContentCallback (ContentType value)
{
if (!pcl_isfinite (value))
cloud_->is_dense = false;

memcpy (&cloud_->data[vertex_count_ * cloud_->point_step + vertex_offset_before_],
&value,
sizeof (ContentType));
Expand Down
5 changes: 5 additions & 0 deletions test/io/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ if (build)
FILES test_grabbers.cpp
LINK_WITH pcl_gtest pcl_io
ARGUMENTS "${PCL_SOURCE_DIR}/test/grabber_sequences")

PCL_ADD_TEST(io_ply_io test_ply_io
FILES test_ply_io.cpp
LINK_WITH pcl_gtest pcl_io)

# Uses VTK readers to verify
if (VTK_FOUND AND NOT ANDROID)
include_directories(${VTK_INCLUDE_DIRS})
Expand Down
234 changes: 0 additions & 234 deletions test/io/test_io.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,240 +808,6 @@ TEST (PCL, ASCIIReader)

}

//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
TEST (PCL, PLYReaderWriter)
{
pcl::PCLPointCloud2 cloud_blob, cloud_blob2;
PointCloud<PointXYZI> cloud, cloud2;

cloud.width = 640;
cloud.height = 480;
cloud.resize (cloud.width * cloud.height);
cloud.is_dense = true;

srand (static_cast<unsigned int> (time (NULL)));
size_t nr_p = cloud.size ();
// Randomly create a new point cloud
for (size_t i = 0; i < nr_p; ++i)
{
cloud[i].x = static_cast<float> (1024 * rand () / (RAND_MAX + 1.0));
cloud[i].y = static_cast<float> (1024 * rand () / (RAND_MAX + 1.0));
cloud[i].z = static_cast<float> (1024 * rand () / (RAND_MAX + 1.0));
cloud[i].intensity = static_cast<float> (i);
}

// Convert from data type to blob
toPCLPointCloud2 (cloud, cloud_blob);

EXPECT_EQ (cloud_blob.width, cloud.width); // test for toPCLPointCloud2 ()
EXPECT_EQ (cloud_blob.height, cloud.height); // test for toPCLPointCloud2 ()
EXPECT_EQ (cloud_blob.is_dense, cloud.is_dense); // test for toPCLPointCloud2 ()
EXPECT_EQ (cloud_blob.data.size (),
cloud_blob.width * cloud_blob.height * sizeof (PointXYZI));

// test for toPCLPointCloud2 ()
PLYWriter writer;
writer.write ("test_pcl_io.ply", cloud_blob, Eigen::Vector4f::Zero (), Eigen::Quaternionf::Identity (), true, true);

PLYReader reader;
reader.read ("test_pcl_io.ply", cloud_blob2);
//PLY DOES preserve organiziation
EXPECT_EQ (cloud_blob.width * cloud_blob.height, cloud_blob2.width * cloud_blob2.height);
EXPECT_EQ (cloud_blob.is_dense, cloud.is_dense);
EXPECT_EQ (size_t (cloud_blob2.data.size ()), // PointXYZI is 16*2 (XYZ+1, Intensity+3)
cloud_blob2.width * cloud_blob2.height * sizeof (PointXYZ)); // test for loadPLYFile ()

// Convert from blob to data type
fromPCLPointCloud2 (cloud_blob2, cloud2);

// EXPECT_EQ (cloud.width, cloud2.width); // test for fromPCLPointCloud2 ()
// EXPECT_EQ (cloud.height, cloud2.height); // test for fromPCLPointCloud2 ()
// EXPECT_EQ (cloud.is_dense, cloud2.is_dense); // test for fromPCLPointCloud2 ()
EXPECT_EQ (cloud.size (), cloud2.size ()); // test for fromPCLPointCloud2 ()

for (uint32_t counter = 0; counter < cloud.size (); ++counter)
{
EXPECT_FLOAT_EQ (cloud[counter].x, cloud2[counter].x); // test for fromPCLPointCloud2 ()
EXPECT_FLOAT_EQ (cloud[counter].y, cloud2[counter].y); // test for fromPCLPointCloud2 ()
EXPECT_FLOAT_EQ (cloud[counter].z, cloud2[counter].z); // test for fromPCLPointCloud2 ()
EXPECT_FLOAT_EQ (cloud[counter].intensity, cloud2[counter].intensity); // test for fromPCLPointCloud2 ()
}
}

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

PLYTest () : mesh_file_ply_("ply_color_mesh.ply")
{
std::ofstream fs;
fs.open (mesh_file_ply_.c_str ());
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 shouldn't?

// 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 shouldn't?

// 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;
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 shouldn't?

// 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
Loading