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. @@ _______________________________________________ Bf-blender-cvs mailing list Bf-blender-cvs@blender.org List details, subscription details or unsubscribe: https://lists.blender.org/mailman/listinfo/bf-blender-cvs