From cf8922ef577083668a22d52b9b01fc545bafd206 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Sun, 19 Jun 2022 17:38:32 +0300 Subject: Fix T97820: new OBJ importer wrongly producing "sharp" edges in some cases The new OBJ importer is producing "sharp" edges on some meshes that should be completely smooth. Only observed on UV-Sphere type meshes so far (see T97820). I'm not 100% sure what is the root cause, but my theory was that maybe due to limited number of float digits that are printed for vertex normals in the file, the normals that are read in are not always exactly 1.0 length. And then the Blender's "set custom loop normals" function (which expects normalized inputs) wrongly marks some edges as sharp. Adding explicit normalization for the normals that are read from the file fixes the wrongly-sharp edges in test cases from T97820. I have not observed measurable performance impact in importing large models (e.g. 6-level subdivided Monkey) that contain vertex normals. Reviewed By: Howard Trickey Differential Revision: https://developer.blender.org/D15202 --- source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'source/blender/io/wavefront_obj/importer/obj_import_file_reader.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 ee55dd1e45a..13e28be5668 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 @@ -6,6 +6,7 @@ #include "BLI_map.hh" #include "BLI_math_color.h" +#include "BLI_math_vector.h" #include "BLI_string_ref.hh" #include "BLI_vector.hh" @@ -129,6 +130,10 @@ static void geom_add_vertex_normal(Geometry *geom, { float3 normal; parse_floats(p, end, 0.0f, normal, 3); + /* Normals can be printed with only several digits in the file, + * making them ever-so-slightly non unit length. Make sure they are + * normalized. */ + normalize_v3(normal); r_global_vertices.vertex_normals.append(normal); geom->has_vertex_normals_ = true; } -- cgit v1.2.3 From 91b52545989ce29c6cd350a99191accf23d3d9dd Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Sun, 19 Jun 2022 17:39:54 +0300 Subject: Fix T98874: new obj importer missing an option to import vertex groups The old Python OBJ importer had a (somewhat confusingly named) "Keep Vertex Order -> Poly Groups" option, that imported OBJ groups as "vertex groups" on the resulting mesh. All vertices of any face were assigned the vertex group, with a 1.0 weight. The new C++ importer did not have this option. It was trying to do something with vertex groups, but failing to actually achieve anything :) -- the vertex groups were created on the wrong object (later on overwritten by "nomain mesh to main mesh" operation); vertex weights were set to 1.0/vertex_count, and each vertex was only set to be in one group, even when it belongs to multiple faces from different groups. End result was that to the user, vertex groups were not visible/present at all (see T98874). This patch adds the import option (named "Vertex Groups"), which is off by default, and fixes the import code logic to actually do the right thing. Tested on file from T98874; vertex groups are imported just like with the Python importer. Reviewed By: Howard Trickey Differential Revision: https://developer.blender.org/D15200 --- source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source/blender/io/wavefront_obj/importer/obj_import_file_reader.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 13e28be5668..3cc17e7d8e6 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 @@ -175,7 +175,7 @@ static void geom_add_polygon(Geometry *geom, curr_face.material_index = material_index; if (group_index >= 0) { curr_face.vertex_group_index = group_index; - geom->use_vertex_groups_ = true; + geom->has_vertex_groups_ = true; } const int orig_corners_size = geom->face_corners_.size(); -- cgit v1.2.3 From 50f9c1c09ce331c7ce09115016ba3e4407691701 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Thu, 7 Jul 2022 11:34:04 +0300 Subject: OBJ: more robust .mtl texture offset/scale parsing (T89421) As pointed out in a comment on T89421, if a MTL file contained something like: `map_Ka -o 1 2.png` then it was parsed as having offset `1 2` and the texture filename just a `.png`. Make it so that mtl option numbers are parsed in a way where the number is only accepted only if it's followed by whitespace. Differential Revision: https://developer.blender.org/D15385 --- source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'source/blender/io/wavefront_obj/importer/obj_import_file_reader.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 3cc17e7d8e6..c2aa96713b1 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 @@ -626,15 +626,15 @@ static bool parse_texture_option(const char *&p, { p = drop_whitespace(p, end); if (parse_keyword(p, end, "-o")) { - p = parse_floats(p, end, 0.0f, tex_map.translation, 3); + p = parse_floats(p, end, 0.0f, tex_map.translation, 3, true); return true; } if (parse_keyword(p, end, "-s")) { - p = parse_floats(p, end, 1.0f, tex_map.scale, 3); + p = parse_floats(p, end, 1.0f, tex_map.scale, 3, true); return true; } if (parse_keyword(p, end, "-bm")) { - p = parse_float(p, end, 1.0f, material->map_Bump_strength); + p = parse_float(p, end, 1.0f, material->map_Bump_strength, true, true); return true; } if (parse_keyword(p, end, "-type")) { -- cgit v1.2.3 From 4114ace6169282390b199ea593c93445af748903 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Sun, 10 Jul 2022 18:27:07 +0300 Subject: Fix T99536: new 3.2 OBJ importer fails with trailing space after wrapped lines Address the issue by re-working line continuation handling: stop trying to parse sequences like "backslash, newline" (which is the bug: it should also handle "backslash, possible whitespace, newline") during parsing. Instead, fixup line continuations after reading chunks of input file data - turn backslash and the following newline into spaces. The rest of parsing code does not have to be aware of them at all then. Makes the file attached to T99536 load correctly now. Also will extend one of the test files in subversion tests repo to contain backslashes followed by newlines. --- .../blender/io/wavefront_obj/importer/obj_import_file_reader.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'source/blender/io/wavefront_obj/importer/obj_import_file_reader.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 c2aa96713b1..6eb1bbc51f3 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 @@ -427,6 +427,11 @@ void OBJParser::parse(Vector> &r_all_geometries, break; /* No more data to read. */ } + /* Take care of line continuations now (turn them into spaces); + * the rest of the parsing code does not need to worry about them anymore. */ + fixup_line_continuations(buffer.data() + buffer_offset, + buffer.data() + buffer_offset + bytes_read); + /* Ensure buffer ends in a newline. */ if (bytes_read < read_buffer_size_) { if (bytes_read == 0 || buffer[buffer_offset + bytes_read - 1] != '\n') { @@ -445,9 +450,7 @@ void OBJParser::parse(Vector> &r_all_geometries, while (last_nl > 0) { --last_nl; if (buffer[last_nl] == '\n') { - if (last_nl < 1 || buffer[last_nl - 1] != '\\') { - break; - } + break; } } if (buffer[last_nl] != '\n') { -- cgit v1.2.3 From fad857f47319166d3ff97029385b50059731a576 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Sun, 10 Jul 2022 20:09:29 +0300 Subject: 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 --- .../importer/obj_import_file_reader.cc | 106 ++++++++++----------- 1 file changed, 51 insertions(+), 55 deletions(-) (limited to 'source/blender/io/wavefront_obj/importer/obj_import_file_reader.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> &r_all_geometries, - VertexIndexOffset &r_offset) + Vector> &r_all_geometries) { auto new_geometry = [&]() { r_all_geometries.append(std::make_unique()); 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(edge_v1), static_cast(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> &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> &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. */ @@ -477,10 +479,10 @@ void OBJParser::parse(Vector> &r_all_geometries, /* Most common things that start with 'v': vertices, normals, UVs. */ if (*p == 'v') { if (parse_keyword(p, end, "v")) { - geom_add_vertex(curr_geom, p, end, r_global_vertices); + geom_add_vertex(p, end, r_global_vertices); } else if (parse_keyword(p, end, "vn")) { - geom_add_vertex_normal(curr_geom, p, end, r_global_vertices); + geom_add_vertex_normal(p, end, r_global_vertices); } else if (parse_keyword(p, end, "vt")) { geom_add_uv_vertex(p, end, r_global_vertices); @@ -492,26 +494,21 @@ void OBJParser::parse(Vector> &r_all_geometries, p, end, 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(p, end, "l")) { - geom_add_edge(curr_geom, p, end, offsets, r_global_vertices); + geom_add_edge(curr_geom, p, end, r_global_vertices); } /* Objects. */ else if (parse_keyword(p, end, "o")) { state_shaded_smooth = false; state_group_name = ""; state_material_name = ""; - curr_geom = create_geometry(curr_geom, - GEOM_MESH, - StringRef(p, end).trim(), - r_global_vertices, - r_all_geometries, - offsets); + curr_geom = create_geometry( + curr_geom, GEOM_MESH, StringRef(p, end).trim(), r_all_geometries); } /* Groups. */ else if (parse_keyword(p, end, "g")) { @@ -540,7 +537,7 @@ void OBJParser::parse(Vector> &r_all_geometries, add_mtl_library(StringRef(p, end).trim()); } else if (parse_keyword(p, end, "#MRGB")) { - geom_add_mrgb_colors(curr_geom, p, end, r_global_vertices); + geom_add_mrgb_colors(p, end, r_global_vertices); } /* Comments. */ else if (*p == '#') { @@ -548,8 +545,7 @@ void OBJParser::parse(Vector> &r_all_geometries, } /* Curve related things. */ else if (parse_keyword(p, end, "cstype")) { - curr_geom = geom_set_curve_type( - curr_geom, p, end, r_global_vertices, state_group_name, offsets, r_all_geometries); + curr_geom = geom_set_curve_type(curr_geom, p, end, state_group_name, r_all_geometries); } else if (parse_keyword(p, end, "deg")) { geom_set_curve_degree(curr_geom, p, end); -- cgit v1.2.3 From c49717a82473e078ea4e58ef7b42d425a1b7d45a Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Thu, 28 Jul 2022 16:39:42 +0300 Subject: Fix T100017: OBJ: new importer does not import vertices that aren't part of any face The Python based importer had a special case handling of "no faces in the whole file at all", where it ended up treating the whole file as essentially a point-cloud-like object (just loose vertices, no faces or edges). The new importer code was missing this special case. Fixes T100017. Added gtest coverage that was failing without the fix. --- .../wavefront_obj/importer/obj_import_file_reader.cc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'source/blender/io/wavefront_obj/importer/obj_import_file_reader.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 a32fd90594d..8594603867f 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 @@ -13,6 +13,7 @@ #include "obj_import_file_reader.hh" #include "obj_import_string_utils.hh" +#include #include namespace blender::io::obj { @@ -394,6 +395,23 @@ static bool parse_keyword(const char *&p, const char *end, StringRef keyword) return true; } +/* Special case: if there were no faces/edges in any geometries, + * treat all the vertices as a point cloud. */ +static void use_all_vertices_if_no_faces(Geometry *geom, + const Vector> &all_geometries, + const GlobalVertices &global_vertices) +{ + if (!global_vertices.vertices.is_empty() && geom && geom->geom_type_ == GEOM_MESH) { + if (std::all_of( + all_geometries.begin(), all_geometries.end(), [](const std::unique_ptr &g) { + return g->get_vertex_count() == 0; + })) { + geom->track_vertex_index(0); + geom->track_vertex_index(global_vertices.vertices.size() - 1); + } + } +} + void OBJParser::parse(Vector> &r_all_geometries, GlobalVertices &r_global_vertices) { @@ -571,6 +589,7 @@ void OBJParser::parse(Vector> &r_all_geometries, buffer_offset = left_size; } + use_all_vertices_if_no_faces(curr_geom, r_all_geometries, r_global_vertices); add_default_mtl_library(); } -- cgit v1.2.3 From d76583cb4a16315c196f07f4acb9340f341bee47 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Wed, 10 Aug 2022 13:34:42 +0300 Subject: Fix T100302: New OBJ importer produces too many vertices when faces don't span a continuous range As part of the previous fix (D15410), the importer got code to track min & max vertex indices used as part of the mesh faces. However, if faces refer to a "sparse" (i.e. non-contiguous) subset of all vertices, then the imported mesh would contain all the vertices between min & max range. Replace that with proper tracking of actually used vertex indices for each imported mesh. Fixes T100302. This does affect import performance a tiny bit, e.g. importing Blender 3.0 splash scene goes 21.7s -> 22.1s, and importing rungholt.obj goes 2.37s -> 2.48s. Importer related tests have a bunch of vertex changes in them, since now vertices are added in the order that the faces are referring to them. Which incidentally matches the order that the Python based importer was creating them too. --- source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'source/blender/io/wavefront_obj/importer/obj_import_file_reader.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 8594603867f..7069e1185e0 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 @@ -406,8 +406,7 @@ static void use_all_vertices_if_no_faces(Geometry *geom, all_geometries.begin(), all_geometries.end(), [](const std::unique_ptr &g) { return g->get_vertex_count() == 0; })) { - geom->track_vertex_index(0); - geom->track_vertex_index(global_vertices.vertices.size() - 1); + geom->track_all_vertices(global_vertices.vertices.size()); } } } -- cgit v1.2.3 From 28750bcf7e8b73d9da015898a3c0f21ef5d761f2 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 28 Aug 2022 20:52:28 +1000 Subject: Cleanup: replace NULL with nullptr for C++ files --- source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source/blender/io/wavefront_obj/importer/obj_import_file_reader.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 7069e1185e0..633f70b2e38 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 @@ -748,7 +748,7 @@ MTLParser::MTLParser(StringRefNull mtl_library, StringRefNull obj_filepath) { char obj_file_dir[FILE_MAXDIR]; BLI_split_dir_part(obj_filepath.data(), obj_file_dir, FILE_MAXDIR); - BLI_path_join(mtl_file_path_, FILE_MAX, obj_file_dir, mtl_library.data(), NULL); + BLI_path_join(mtl_file_path_, FILE_MAX, obj_file_dir, mtl_library.data(), nullptr); BLI_split_dir_part(mtl_file_path_, mtl_dir_path_, FILE_MAXDIR); } -- cgit v1.2.3 From 68f234b8ab2ef4f69498650ece54015d174bea07 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Mon, 29 Aug 2022 21:10:16 +0300 Subject: Cleanup: obj: simplify import/export syntax handling code I want to add support for PBR materials extension to OBJ, but the way current I/O code syntax handling was done made it quite cumbersome to extend the number of MTL textures/parameters. Simplify all that by removing FormatHandler template on "syntax" that gets routed through keyword enums, and instead just have simple `write_obj_*` and `write_mtl_*` functions. Simplify MTLMaterial to not contain a map of textures (that is always fully filled with all possible textures), instead now there's a simple array. Rename `tex_map_XX` to `MTLTexMap`. All this does not affect behavior or performance, but it does result in 170 fewer lines of code, and saves a couple kilobytes of executable size. --- .../importer/obj_import_file_reader.cc | 37 ++++++++++------------ 1 file changed, 16 insertions(+), 21 deletions(-) (limited to 'source/blender/io/wavefront_obj/importer/obj_import_file_reader.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 633f70b2e38..088784b4194 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 @@ -592,36 +592,31 @@ void OBJParser::parse(Vector> &r_all_geometries, add_default_mtl_library(); } -static eMTLSyntaxElement mtl_line_start_to_enum(const char *&p, const char *end) +static MTLTexMapType mtl_line_start_to_texture_type(const char *&p, const char *end) { if (parse_keyword(p, end, "map_Kd")) { - return eMTLSyntaxElement::map_Kd; + return MTLTexMapType::Kd; } if (parse_keyword(p, end, "map_Ks")) { - return eMTLSyntaxElement::map_Ks; + return MTLTexMapType::Ks; } if (parse_keyword(p, end, "map_Ns")) { - return eMTLSyntaxElement::map_Ns; + return MTLTexMapType::Ns; } if (parse_keyword(p, end, "map_d")) { - return eMTLSyntaxElement::map_d; + return MTLTexMapType::d; } - if (parse_keyword(p, end, "refl")) { - return eMTLSyntaxElement::map_refl; - } - if (parse_keyword(p, end, "map_refl")) { - return eMTLSyntaxElement::map_refl; + if (parse_keyword(p, end, "refl") || parse_keyword(p, end, "map_refl")) { + return MTLTexMapType::refl; } if (parse_keyword(p, end, "map_Ke")) { - return eMTLSyntaxElement::map_Ke; - } - if (parse_keyword(p, end, "bump")) { - return eMTLSyntaxElement::map_Bump; + return MTLTexMapType::Ke; } - if (parse_keyword(p, end, "map_Bump") || parse_keyword(p, end, "map_bump")) { - return eMTLSyntaxElement::map_Bump; + if (parse_keyword(p, end, "bump") || parse_keyword(p, end, "map_Bump") || + parse_keyword(p, end, "map_bump")) { + return MTLTexMapType::bump; } - return eMTLSyntaxElement::string; + return MTLTexMapType::Count; } static const std::pair unsupported_texture_options[] = { @@ -639,7 +634,7 @@ static const std::pair unsupported_texture_options[] = { static bool parse_texture_option(const char *&p, const char *end, MTLMaterial *material, - tex_map_XX &tex_map) + MTLTexMap &tex_map) { p = drop_whitespace(p, end); if (parse_keyword(p, end, "-o")) { @@ -693,13 +688,13 @@ static void parse_texture_map(const char *p, if (!is_map && !is_refl && !is_bump) { return; } - eMTLSyntaxElement key = mtl_line_start_to_enum(p, end); - if (key == eMTLSyntaxElement::string || !material->texture_maps.contains(key)) { + MTLTexMapType key = mtl_line_start_to_texture_type(p, end); + if (key == MTLTexMapType::Count) { /* No supported texture map found. */ std::cerr << "OBJ import: MTL texture map type not supported: '" << line << "'" << std::endl; return; } - tex_map_XX &tex_map = material->texture_maps.lookup(key); + MTLTexMap &tex_map = material->tex_map_of_type(key); tex_map.mtl_dir_path = mtl_dir_path; /* Parse texture map options. */ -- cgit v1.2.3 From fa40013009463703a22a8ae05d116723084966b7 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Thu, 1 Sep 2022 21:19:31 +0300 Subject: Cleanup: obj: simplify material node tree creation As pointed out in D15827 comment, the unique_ptr usage in ShaderNodetreeWrap related code does not sound very useful. Looking at it, whole ShaderNodetreeWrap does not make much sense - it's only ever created, and then immediately just one thing is fetched from it. This very much sounds like "a function", so make it just that - header file contains just a `create_mtl_node_tree` function, and the whole implementation is hidden from the users. Which I've also simplified into just a handful of freestanding functions. No functionality or performance changes, but the code does get ~80 lines shorter. --- source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'source/blender/io/wavefront_obj/importer/obj_import_file_reader.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 088784b4194..2ad8a09bd90 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 @@ -10,6 +10,7 @@ #include "BLI_string_ref.hh" #include "BLI_vector.hh" +#include "obj_export_mtl.hh" #include "obj_import_file_reader.hh" #include "obj_import_string_utils.hh" -- cgit v1.2.3