[Bf-blender-cvs] [f11dc58667f] blender-v3.2-release: Fix T99532: New OBJ importer in some cases fails to import faces
Aras Pranckevicius
noreply at git.blender.org
Thu Jul 28 16:20:47 CEST 2022
Commit: f11dc58667fbcabe6b9f0c7f5df736fc5f8d52f2
Author: Aras Pranckevicius
Date: Sun Jul 10 20:09:29 2022 +0300
Branches: blender-v3.2-release
https://developer.blender.org/rBf11dc58667fbcabe6b9f0c7f5df736fc5f8d52f2
Fix T99532: New OBJ importer in some cases fails to import faces
The importer code was written under incorrect assumption that vertex
data (v, vn, vt commands etc.) are grouped by object, i.e. follow the
o command, and that each object has its own vertex data commands. This
is not the case -- all the vertex data in the whole OBJ file is
"global", with no relation to any objects/groups; it's just that the
faces belong to the object, and then they pull in any vertices they
like.
This patch fixes this incorrect assumption in the importer:
- Vertex data is now properly global; no need to track some sort of
"offsets" per object like it was doing before.
- For each object, face definitions track the minimum & maximum vertex
indices referenced by the object, and then all that vertex range is
created in the final Blender object. Note: it might be (unusual, but
possible) that an object does not reference a sequential range of
vertices, e.g. just a single face with vertex indices 1, 10, 100 --
the resulting Blender mesh will have all the 100 vertices (some
"loose" without belonging to a face). It should be possible to track
the used vertices exactly (e.g. with a vector set), but I haven't
done that for performance reasons.
Reviewed By: Howard Trickey
Differential Revision: https://developer.blender.org/D15410
# Conflicts:
# source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
# source/blender/io/wavefront_obj/importer/obj_import_mesh.cc
# source/blender/io/wavefront_obj/importer/obj_import_objects.hh
# source/blender/io/wavefront_obj/tests/obj_importer_tests.cc
===================================================================
M source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
M source/blender/io/wavefront_obj/importer/obj_import_mesh.cc
M source/blender/io/wavefront_obj/importer/obj_import_objects.hh
M source/blender/io/wavefront_obj/tests/obj_importer_tests.cc
===================================================================
diff --git a/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc b/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
index f801bb1d3fa..6dd696d0c21 100644
--- a/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
+++ b/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
@@ -20,31 +20,24 @@ using std::string;
/**
* Based on the properties of the given Geometry instance, create a new Geometry instance
* or return the previous one.
- *
- * Also update index offsets which should always happen if a new Geometry instance is created.
*/
static Geometry *create_geometry(Geometry *const prev_geometry,
const eGeometryType new_type,
StringRef name,
- const GlobalVertices &global_vertices,
- Vector<std::unique_ptr<Geometry>> &r_all_geometries,
- VertexIndexOffset &r_offset)
+ Vector<std::unique_ptr<Geometry>> &r_all_geometries)
{
auto new_geometry = [&]() {
r_all_geometries.append(std::make_unique<Geometry>());
Geometry *g = r_all_geometries.last().get();
g->geom_type_ = new_type;
g->geometry_name_ = name.is_empty() ? "New object" : name;
- g->vertex_start_ = global_vertices.vertices.size();
- r_offset.set_index_offset(g->vertex_start_);
return g;
};
if (prev_geometry && prev_geometry->geom_type_ == GEOM_MESH) {
/* After the creation of a Geometry instance, at least one element has been found in the OBJ
- * file that indicates that it is a mesh (basically anything but the vertex positions). */
- if (!prev_geometry->face_elements_.is_empty() || prev_geometry->has_vertex_normals_ ||
- !prev_geometry->edges_.is_empty()) {
+ * file that indicates that it is a mesh (faces or edges). */
+ if (!prev_geometry->face_elements_.is_empty() || !prev_geometry->edges_.is_empty()) {
return new_geometry();
}
if (new_type == GEOM_MESH) {
@@ -67,19 +60,14 @@ static Geometry *create_geometry(Geometry *const prev_geometry,
return new_geometry();
}
-static void geom_add_vertex(Geometry *geom,
- const StringRef line,
- GlobalVertices &r_global_vertices)
+static void geom_add_vertex(const StringRef line, GlobalVertices &r_global_vertices)
{
float3 vert;
parse_floats(line, 0.0f, vert, 3);
r_global_vertices.vertices.append(vert);
- geom->vertex_count_++;
}
-static void geom_add_vertex_normal(Geometry *geom,
- const StringRef line,
- GlobalVertices &r_global_vertices)
+static void geom_add_vertex_normal(const StringRef line, GlobalVertices &r_global_vertices)
{
float3 normal;
parse_floats(line, 0.0f, normal, 3);
@@ -88,7 +76,6 @@ static void geom_add_vertex_normal(Geometry *geom,
* normalized. */
normalize_v3(normal);
r_global_vertices.vertex_normals.append(normal);
- geom->has_vertex_normals_ = true;
}
static void geom_add_uv_vertex(const StringRef line, GlobalVertices &r_global_vertices)
@@ -98,25 +85,23 @@ static void geom_add_uv_vertex(const StringRef line, GlobalVertices &r_global_ve
r_global_vertices.uv_vertices.append(uv);
}
-static void geom_add_edge(Geometry *geom,
- StringRef line,
- const VertexIndexOffset &offsets,
- GlobalVertices &r_global_vertices)
+static void geom_add_edge(Geometry *geom, StringRef line, GlobalVertices &r_global_vertices)
{
int edge_v1, edge_v2;
line = parse_int(line, -1, edge_v1);
line = parse_int(line, -1, edge_v2);
/* Always keep stored indices non-negative and zero-based. */
- edge_v1 += edge_v1 < 0 ? r_global_vertices.vertices.size() : -offsets.get_index_offset() - 1;
- edge_v2 += edge_v2 < 0 ? r_global_vertices.vertices.size() : -offsets.get_index_offset() - 1;
+ edge_v1 += edge_v1 < 0 ? r_global_vertices.vertices.size() : -1;
+ edge_v2 += edge_v2 < 0 ? r_global_vertices.vertices.size() : -1;
BLI_assert(edge_v1 >= 0 && edge_v2 >= 0);
geom->edges_.append({static_cast<uint>(edge_v1), static_cast<uint>(edge_v2)});
+ geom->track_vertex_index(edge_v1);
+ geom->track_vertex_index(edge_v2);
}
static void geom_add_polygon(Geometry *geom,
StringRef line,
const GlobalVertices &global_vertices,
- const VertexIndexOffset &offsets,
const int material_index,
const int group_index,
const bool shaded_smooth)
@@ -155,8 +140,7 @@ static void geom_add_polygon(Geometry *geom,
}
}
/* Always keep stored indices non-negative and zero-based. */
- corner.vert_index += corner.vert_index < 0 ? global_vertices.vertices.size() :
- -offsets.get_index_offset() - 1;
+ corner.vert_index += corner.vert_index < 0 ? global_vertices.vertices.size() : -1;
if (corner.vert_index < 0 || corner.vert_index >= global_vertices.vertices.size()) {
fprintf(stderr,
"Invalid vertex index %i (valid range [0, %zu)), ignoring face\n",
@@ -164,6 +148,9 @@ static void geom_add_polygon(Geometry *geom,
(size_t)global_vertices.vertices.size());
face_valid = false;
}
+ else {
+ geom->track_vertex_index(corner.vert_index);
+ }
if (got_uv) {
corner.uv_vert_index += corner.uv_vert_index < 0 ? global_vertices.uv_vertices.size() : -1;
if (corner.uv_vert_index < 0 || corner.uv_vert_index >= global_vertices.uv_vertices.size()) {
@@ -177,7 +164,7 @@ static void geom_add_polygon(Geometry *geom,
/* Ignore corner normal index, if the geometry does not have any normals.
* Some obj files out there do have face definitions that refer to normal indices,
* without any normals being present (T98782). */
- if (got_normal && geom->has_vertex_normals_) {
+ if (got_normal && !global_vertices.vertex_normals.is_empty()) {
corner.vertex_normal_index += corner.vertex_normal_index < 0 ?
global_vertices.vertex_normals.size() :
-1;
@@ -210,17 +197,14 @@ static void geom_add_polygon(Geometry *geom,
static Geometry *geom_set_curve_type(Geometry *geom,
const StringRef rest_line,
- const GlobalVertices &global_vertices,
const StringRef group_name,
- VertexIndexOffset &r_offsets,
Vector<std::unique_ptr<Geometry>> &r_all_geometries)
{
if (rest_line.find("bspline") == StringRef::not_found) {
std::cerr << "Curve type not supported:'" << rest_line << "'" << std::endl;
return geom;
}
- geom = create_geometry(
- geom, GEOM_CURVE, group_name, global_vertices, r_all_geometries, r_offsets);
+ geom = create_geometry(geom, GEOM_CURVE, group_name, r_all_geometries);
geom->nurbs_element_.group_ = group_name;
return geom;
}
@@ -350,9 +334,7 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
BLI_strncpy(ob_name, BLI_path_basename(import_params_.filepath), FILE_MAXFILE);
BLI_path_extension_replace(ob_name, FILE_MAXFILE, "");
- VertexIndexOffset offsets;
- Geometry *curr_geom = create_geometry(
- nullptr, GEOM_MESH, ob_name, r_global_vertices, r_all_geometries, offsets);
+ Geometry *curr_geom = create_geometry(nullptr, GEOM_MESH, ob_name, r_all_geometries);
/* State variables: once set, they remain the same for the remaining
* elements in the object. */
@@ -424,10 +406,10 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
/* Most common things that start with 'v': vertices, normals, UVs. */
if (line[0] == 'v') {
if (parse_keyword(line, "v")) {
- geom_add_vertex(curr_geom, line, r_global_vertices);
+ geom_add_vertex(line, r_global_vertices);
}
else if (parse_keyword(line, "vn")) {
- geom_add_vertex_normal(curr_geom, line, r_global_vertices);
+ geom_add_vertex_normal(line, r_global_vertices);
}
else if (parse_keyword(line, "vt")) {
geom_add_uv_vertex(line, r_global_vertices);
@@ -438,22 +420,20 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
geom_add_polygon(curr_geom,
line,
r_global_vertices,
- offsets,
state_material_index,
- state_group_index, /* TODO was wrongly material name! */
+ state_group_index,
state_shaded_smooth);
}
/* Faces. */
else if (parse_keyword(line, "l")) {
- geom_add_edge(curr_geom, line, offsets, r_global_vertices);
+ geom_add_edge(curr_geom, line, r_global_vertices);
}
/* Objects. */
else if (parse_keyword(line, "o")) {
state_shaded_smooth = false;
state_group_name = "";
state_material_name = "";
- curr_geom = create_geometry(
- curr_geom, GEOM_MESH, line.trim(), r_global_vertices, r_all_geometries, offsets);
+ curr_geom = create_geometry(curr_geom, GEOM_MESH, line.trim(), r_all_geometries);
}
/* Groups. */
else if (parse_keyword(line, "g")) {
@@ -487,8 +467,7 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
}
/* Curve related things. */
else if (parse_keyword(line, "cstype")) {
- curr_geom = geom_set_curve_type(
- curr_geom, line, r_global_vertices, state_group_name, offsets, r_all_geometries);
+ curr_geom = geom_set_curve_type(curr_geom, line, state_group_name, r_all_geometries);
}
else if (parse_keyword(line, "deg")) {
geom_set_curve_degree(curr_
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list