Skip to content

Commit

Permalink
Fix regression in OBJ loader
Browse files Browse the repository at this point in the history
Regression happened in #6047
If a OBJ file only defines vertices and vertex normals (same number), but no faces, then vertices and vertex normals should be associated one-to-one in the order they appear.
This worked before, but since the mentioned pull request, normals would be zero in this case.
  • Loading branch information
mvieth committed Feb 4, 2025
1 parent 6e4eb4e commit 460d9fc
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 2 deletions.
28 changes: 26 additions & 2 deletions io/src/obj_io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,7 @@ pcl::OBJReader::read (const std::string &file_name, pcl::PCLPointCloud2 &cloud,

//vector[idx of vertex]<accumulated normals{x, y, z}>
std::vector<Eigen::Vector3f> normal_mapping;
bool normal_mapping_used = false;

// std::size_t rgba_field = 0;
for (std::size_t i = 0; i < cloud.fields.size (); ++i)
Expand Down Expand Up @@ -662,6 +663,7 @@ pcl::OBJReader::read (const std::string &file_name, pcl::PCLPointCloud2 &cloud,
n = (n < 0) ? normal_idx + n : n - 1;

normal_mapping[v] += normals[n];
normal_mapping_used = true;
}
}
continue;
Expand All @@ -675,7 +677,7 @@ pcl::OBJReader::read (const std::string &file_name, pcl::PCLPointCloud2 &cloud,
return (-1);
}

if (!normal_mapping.empty())
if (normal_mapping_used && !normal_mapping.empty())
{
for (uindex_t i = 0, main_offset = 0; i < cloud.width; ++i, main_offset += cloud.point_step)
{
Expand All @@ -685,6 +687,16 @@ pcl::OBJReader::read (const std::string &file_name, pcl::PCLPointCloud2 &cloud,
memcpy(&cloud.data[main_offset + cloud.fields[f].offset], &normal_mapping[i][j], sizeof(float));
}
}
else if (cloud.width == normals.size())
{
// if obj file contains vertex normals (same number as vertices), but does not define faces,
// then associate vertices and vertex normals one-to-one
for (uindex_t i = 0, main_offset = 0; i < cloud.width; ++i, main_offset += cloud.point_step)
{
for (int j = 0, f = normal_x_field; j < 3; ++j, ++f)
memcpy(&cloud.data[main_offset + cloud.fields[f].offset], &normals[i][j], sizeof(float));
}
}

double total_time = tt.toc ();
PCL_DEBUG ("[pcl::OBJReader::read] Loaded %s as a dense cloud in %g ms with %d points. Available dimensions: %s.\n",
Expand Down Expand Up @@ -983,6 +995,7 @@ pcl::OBJReader::read (const std::string &file_name, pcl::PolygonMesh &mesh,

//vector[idx of vertex]<accumulated normals{x, y, z}>
std::vector<Eigen::Vector3f> normal_mapping;
bool normal_mapping_used = false;

// std::size_t rgba_field = 0;
for (std::size_t i = 0; i < mesh.cloud.fields.size (); ++i)
Expand Down Expand Up @@ -1093,6 +1106,7 @@ pcl::OBJReader::read (const std::string &file_name, pcl::PolygonMesh &mesh,
n = (n < 0) ? vn_idx + n : n - 1;

normal_mapping[v] += normals[n];
normal_mapping_used = true;
}
}
mesh.polygons.push_back (face_vertices);
Expand All @@ -1107,7 +1121,7 @@ pcl::OBJReader::read (const std::string &file_name, pcl::PolygonMesh &mesh,
return (-1);
}

if (!normal_mapping.empty())
if (normal_mapping_used && !normal_mapping.empty())
{
for (uindex_t i = 0, main_offset = 0; i < mesh.cloud.width; ++i, main_offset += mesh.cloud.point_step)
{
Expand All @@ -1117,6 +1131,16 @@ pcl::OBJReader::read (const std::string &file_name, pcl::PolygonMesh &mesh,
memcpy(&mesh.cloud.data[main_offset + mesh.cloud.fields[f].offset], &normal_mapping[i][j], sizeof(float));
}
}
else if (mesh.cloud.width == normals.size())
{
// if obj file contains vertex normals (same number as vertices), but does not define faces,
// then associate vertices and vertex normals one-to-one
for (uindex_t i = 0, main_offset = 0; i < mesh.cloud.width; ++i, main_offset += mesh.cloud.point_step)
{
for (int j = 0, f = normal_x_field; j < 3; ++j, ++f)
memcpy(&mesh.cloud.data[main_offset + mesh.cloud.fields[f].offset], &normals[i][j], sizeof(float));
}
}

double total_time = tt.toc ();
PCL_DEBUG ("[pcl::OBJReader::read] Loaded %s as a PolygonMesh in %g ms with %zu points and %zu polygons.\n",
Expand Down
26 changes: 26 additions & 0 deletions test/io/test_io.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1144,6 +1144,32 @@ TEST(PCL, OBJRead)
remove ("test_obj.mtl");
}

TEST(PCL, loadOBJWithoutFaces)
{
std::ofstream fs;
fs.open ("test_obj.obj");
fs << "v -1.000000 -1.000000 1.000000\n"
"v -1.000000 1.000000 1.000000\n"
"v 1.000000 -1.000000 1.000000\n"
"v 1.000000 1.000000 1.000000\n"
"vn -1.0000 0.0000 0.0000\n"
"vn 0.0000 0.0000 -1.0000\n"
"vn 1.0000 0.0000 0.0000\n"
"vn 0.0000 1.0000 0.0000\n";

fs.close ();
pcl::PointCloud<pcl::PointNormal> point_cloud;
pcl::io::loadOBJFile("test_obj.obj", point_cloud);
EXPECT_EQ(point_cloud.size(), 4);
EXPECT_NEAR(point_cloud[0].normal_x, -1.0, 1e-5);
EXPECT_NEAR(point_cloud[1].normal_z, -1.0, 1e-5);
EXPECT_NEAR(point_cloud[2].normal_x, 1.0, 1e-5);
EXPECT_NEAR(point_cloud[3].normal_y, 1.0, 1e-5);
EXPECT_NEAR(point_cloud[1].normal_y, 0.0, 1e-5);
EXPECT_NEAR(point_cloud[3].normal_z, 0.0, 1e-5);
remove ("test_obj.obj");
}

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

struct PointXYZFPFH33
Expand Down

0 comments on commit 460d9fc

Please sign in to comment.