[Bf-blender-cvs] [fad857f4731] master: Fix T99532: New OBJ importer in some cases fails to import faces
Aras Pranckevicius
noreply at git.blender.org
Sun Jul 10 19:09:24 CEST 2022
Commit: fad857f47319166d3ff97029385b50059731a576
Author: Aras Pranckevicius
Date: Sun Jul 10 20:09:29 2022 +0300
Branches: master
https://developer.blender.org/rBfad857f47319166d3ff97029385b50059731a576
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
===================================================================
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 6eb1bbc51f3..a32fd90594d 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
@@ -22,32 +22,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();
- g->vertex_color_start_ = global_vertices.vertex_colors.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) {
@@ -70,15 +62,11 @@ static Geometry *create_geometry(Geometry *const prev_geometry,
return new_geometry();
}
-static void geom_add_vertex(Geometry *geom,
- const char *p,
- const char *end,
- GlobalVertices &r_global_vertices)
+static void geom_add_vertex(const char *p, const char *end, GlobalVertices &r_global_vertices)
{
float3 vert;
p = parse_floats(p, end, 0.0f, vert, 3);
r_global_vertices.vertices.append(vert);
- geom->vertex_count_++;
/* OBJ extension: `xyzrgb` vertex colors, when the vertex position
* is followed by 3 more RGB color components. See
* http://paulbourke.net/dataformats/obj/colour.html */
@@ -88,16 +76,22 @@ static void geom_add_vertex(Geometry *geom,
if (srgb.x >= 0 && srgb.y >= 0 && srgb.z >= 0) {
float3 linear;
srgb_to_linearrgb_v3_v3(linear, srgb);
- r_global_vertices.vertex_colors.append(linear);
- geom->vertex_color_count_++;
+
+ auto &blocks = r_global_vertices.vertex_colors;
+ /* If we don't have vertex colors yet, or the previous vertex
+ * was without color, we need to start a new vertex colors block. */
+ if (blocks.is_empty() || (blocks.last().start_vertex_index + blocks.last().colors.size() !=
+ r_global_vertices.vertices.size() - 1)) {
+ GlobalVertices::VertexColorsBlock block;
+ block.start_vertex_index = r_global_vertices.vertices.size() - 1;
+ blocks.append(block);
+ }
+ blocks.last().colors.append(linear);
}
}
}
-static void geom_add_mrgb_colors(Geometry *geom,
- const char *p,
- const char *end,
- GlobalVertices &r_global_vertices)
+static void geom_add_mrgb_colors(const char *p, const char *end, GlobalVertices &r_global_vertices)
{
/* MRGB color extension, in the form of
* "#MRGB MMRRGGBBMMRRGGBB ..."
@@ -117,14 +111,26 @@ static void geom_add_mrgb_colors(Geometry *geom,
srgb[3] = 0xFF;
float linear[4];
srgb_to_linearrgb_uchar4(linear, srgb);
- r_global_vertices.vertex_colors.append({linear[0], linear[1], linear[2]});
- geom->vertex_color_count_++;
+
+ auto &blocks = r_global_vertices.vertex_colors;
+ /* If we don't have vertex colors yet, or the previous vertex
+ * was without color, we need to start a new vertex colors block. */
+ if (blocks.is_empty() || (blocks.last().start_vertex_index + blocks.last().colors.size() !=
+ r_global_vertices.vertices.size())) {
+ GlobalVertices::VertexColorsBlock block;
+ block.start_vertex_index = r_global_vertices.vertices.size();
+ blocks.append(block);
+ }
+ blocks.last().colors.append({linear[0], linear[1], linear[2]});
+ /* MRGB colors are specified after vertex positions; each new color
+ * "pushes" the vertex colors block further back into which vertices it is for. */
+ blocks.last().start_vertex_index--;
+
p += mrgb_length;
}
}
-static void geom_add_vertex_normal(Geometry *geom,
- const char *p,
+static void geom_add_vertex_normal(const char *p,
const char *end,
GlobalVertices &r_global_vertices)
{
@@ -135,7 +141,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 char *p, const char *end, GlobalVertices &r_global_vertices)
@@ -148,24 +153,24 @@ static void geom_add_uv_vertex(const char *p, const char *end, GlobalVertices &r
static void geom_add_edge(Geometry *geom,
const char *p,
const char *end,
- const VertexIndexOffset &offsets,
GlobalVertices &r_global_vertices)
{
int edge_v1, edge_v2;
p = parse_int(p, end, -1, edge_v1);
p = parse_int(p, end, -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,
const char *p,
const char *end,
const GlobalVertices &global_vertices,
- const VertexIndexOffset &offsets,
const int material_index,
const int group_index,
const bool shaded_smooth)
@@ -204,8 +209,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",
@@ -213,6 +217,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()) {
@@ -226,7 +233,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;
@@ -260,9 +267,7 @@ static void geom_add_polygon(Geometry *geom,
static Geometry *geom_set_curve_type(Geometry *geom,
const char *p,
const char *end,
- const GlobalVertices &global_vertices,
const StringRef group_name,
- VertexIndexOffset &r_offsets,
Vector<std::unique_ptr<Geometry>> &r_all_geometries)
{
p = drop_whitespace(p, end);
@@ -270,8 +275,7 @@ static Geometry *geom_set_curve_type(Geometry *geom,
std::cerr << "Curve type not supported: '" << std::string(p, end) << "'" << 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;
}
@@ -402,9 +406,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, "");
-
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list