Harden OBJ and DRC Import Against Malformed Files (#15948)

* Validate OBJ Texture-Coordinate Indices

load_obj read the texture coordinates of a face without checking the
vt index, so a face referencing a vt past the end of the list read out
of bounds and crashed, and a face vertex with no vt read index -1.
Out-of-range or missing indices now fall back to a zero UV. The face
keeps its entry in the per-face UV list, so the following faces stay
aligned, and the geometry loads as before.

Negative (relative) vt indices were also rebased by dividing the float
count by 3, but each vt stores two floats.

* Reject DRC Meshes Without Positions or with Invalid Face Indices

load_drc dereferenced the POSITION attribute without checking that the
mesh has one, and trusted the decoded face indices, which the Draco
decoder does not check against the point count. Both now fail the load
cleanly. A failed vertex conversion is treated the same way.

The libslic3r tests link Draco so they can encode the malformed meshes
in-test.

* Keep OBJ Texture Coordinates That Carry a W Component

The vt parser stopped reading the optional third component when texture
coordinates were cut down to u and v, but the check that nothing is left
on the line stayed. A legal "vt u v w" line was therefore rejected and
silently dropped, shifting every later texture index. The w component is
parsed again and discarded.

The texture coordinate stride is now a named constant, OBJ_TEXCOORD_LENGTH,
used by the parser and the importer, so the relative-index rebase cannot
drift from the storage layout again.
This commit is contained in:
HanifKoh
2026-09-29 12:25:16 +08:00
committed by GitHub
parent 7d8318f275
commit 8ffd3e514e
7 changed files with 234 additions and 19 deletions
+21 -8
View File
@@ -48,24 +48,37 @@ bool load_drc(const char *path, TriangleMesh *meshptr)
indexed_triangle_set its;
const PointAttribute *const positions = dracoMesh.GetNamedAttribute(GeometryAttribute::POSITION);
if (positions == nullptr) {
BOOST_LOG_TRIVIAL(error) << "load_drc: the mesh has no POSITION attribute";
return false;
}
size_t num_vertices = positions->size();
its.vertices.reserve(num_vertices);
for (AttributeValueIndex i(0); i < num_vertices; ++ i) {
float pos[3];
positions->ConvertValue<float>(i, 3, pos);
if (!positions->ConvertValue<float>(i, 3, pos)) {
BOOST_LOG_TRIVIAL(error) << "load_drc: invalid vertex position";
return false;
}
its.vertices.emplace_back(pos[0], pos[1], pos[2]);
}
// The Draco decoder does not check face indices against the point count.
const uint32_t num_points = dracoMesh.num_points();
size_t num_faces = dracoMesh.num_faces();
its.indices.reserve(num_faces);
for (FaceIndex i(0); i < num_faces; ++ i) {
Mesh::Face face = dracoMesh.face(i);
its.indices.emplace_back(
positions->mapped_index(face[0]).value(),
positions->mapped_index(face[1]).value(),
positions->mapped_index(face[2]).value()
);
const Mesh::Face &face = dracoMesh.face(i);
stl_triangle_vertex_indices facet;
for (int k = 0; k < 3; ++ k) {
const size_t vertex_idx = face[k].value() < num_points ? positions->mapped_index(face[k]).value() : num_vertices;
if (vertex_idx >= num_vertices) {
BOOST_LOG_TRIVIAL(error) << "load_drc: invalid vertex index";
return false;
}
facet[k] = static_cast<int>(vertex_idx);
}
its.indices.emplace_back(facet);
}
*meshptr = TriangleMesh(std::move(its));
+9 -4
View File
@@ -166,10 +166,15 @@ bool load_obj(const char *path, TriangleMesh *meshptr, ObjInfo& obj_info, std::s
obj_info.uv_map_pngs[face_index] = png_name;
}
if (data.textureCoordinates.size() > 0) {
Vec2f uv0(data.textureCoordinates[uvs[0] * 2], data.textureCoordinates[uvs[0] * 2 + 1]);
Vec2f uv1(data.textureCoordinates[uvs[1] * 2], data.textureCoordinates[uvs[1] * 2 + 1]);
Vec2f uv2(data.textureCoordinates[uvs[2] * 2], data.textureCoordinates[uvs[2] * 2 + 1]);
std::array<Vec2f, 3> uv_array{uv0, uv1, uv2};
// A face vertex may omit vt or reference a missing one. Fall back to (0, 0) rather than
// skipping the face, so obj_info.uvs stays aligned with the face indices.
const int uv_count = static_cast<int>(data.textureCoordinates.size() / OBJ_TEXCOORD_LENGTH);
auto uv_at = [&data, uv_count](int idx) -> Vec2f {
if (idx < 0 || idx >= uv_count)
return Vec2f::Zero();
return Vec2f(data.textureCoordinates[idx * OBJ_TEXCOORD_LENGTH], data.textureCoordinates[idx * OBJ_TEXCOORD_LENGTH + 1]);
};
std::array<Vec2f, 3> uv_array{uv_at(uvs[0]), uv_at(uvs[1]), uv_at(uvs[2])};
obj_info.uvs.emplace_back(uv_array);
}
obj_info.face_colors.emplace_back(face_color);
+4 -5
View File
@@ -51,19 +51,18 @@ static bool obj_parseline(const char *line, ObjData &data)
line = endptr;
EATWS();
}
/*double w = 0;
// The optional w is accepted but not stored: only u and v are used.
if (*line != 0) {
w = strtod(line, &endptr);
strtod(line, &endptr);
if (endptr == 0 || (*endptr != ' ' && *endptr != '\t' && *endptr != 0))
return false;
line = endptr;
EATWS();
}*/
}
if (*line != 0)
return false;
data.textureCoordinates.push_back((float)u);
data.textureCoordinates.push_back((float)v);
//data.textureCoordinates.push_back((float)w);
break;
}
case 'n':
@@ -245,7 +244,7 @@ static bool obj_parseline(const char *line, ObjData &data)
else
-- vertex.normalIdx;
if (vertex.textureCoordIdx < 0)
vertex.textureCoordIdx += (int)data.textureCoordinates.size() / 3;
vertex.textureCoordIdx += (int)data.textureCoordinates.size() / OBJ_TEXCOORD_LENGTH;
else
-- vertex.textureCoordIdx;
data.vertices.push_back(vertex);
+2 -1
View File
@@ -92,6 +92,7 @@ inline bool operator==(const ObjSmoothingGroup &v1, const ObjSmoothingGroup &v2)
}
#define OBJ_VERTEX_COLOR_ALPHA 6
#define OBJ_VERTEX_LENGTH 7 // x, y, z, color_x,color_y,color_z,color_w
#define OBJ_TEXCOORD_LENGTH 2 // u, v
#define ONE_FACE_SIZE 4//ONE_FACE format: f 8/4/6 7/3/6 6/2/6 -1/-1/-1
struct ObjData {
// Version of the data structure for load / store in the private binary format.
@@ -100,7 +101,7 @@ struct ObjData {
// x, y, z, color_x,color_y,color_z,color_w
std::vector<float> coordinates;
bool has_vertex_color{false};
// u, v, w
// u, v
std::vector<float> textureCoordinates;
// x, y, z
std::vector<float> normals;