[Bf-blender-cvs] [33518f9da16] master: Fix T97417: OBJ: support tab and other whitespace characters after obj/mtl keywords

Aras Pranckevicius noreply at git.blender.org
Sun May 1 19:07:14 CEST 2022


Commit: 33518f9da160cf2d9e91dd7a69cd5788303dcca4
Author: Aras Pranckevicius
Date:   Sun May 1 20:07:03 2022 +0300
Branches: master
https://developer.blender.org/rB33518f9da160cf2d9e91dd7a69cd5788303dcca4

Fix T97417: OBJ: support tab and other whitespace characters after obj/mtl keywords

Even if available OBJ/MTL format documentations don't explicitly specify
which characters can possibly separate keywords & arguments, turns out
some files out there in the wild use TAB character after the line
keywords. Which is something the new 3.2 importer was not quite
expecting (T97417).

Fix this by factoring out a utility function that checks if line starts
with a keyword followed by any whitespace, and using that across the
importer. Also fix some other "possible whitespace around name-like
parts" of obj/mtl parser as pointed out by the repro files in T97417.

Reviewed By: Howard Trickey
Differential Revision: https://developer.blender.org/D14782

===================================================================

M	source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
M	source/blender/io/wavefront_obj/tests/obj_mtl_parser_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 3e722f8a0dd..56806449733 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
@@ -310,6 +310,25 @@ OBJParser::~OBJParser()
   }
 }
 
+/* If line starts with keyword followed by whitespace, returns true and drops it from the line. */
+static bool parse_keyword(StringRef &line, StringRef keyword)
+{
+  const size_t keyword_len = keyword.size();
+  if (line.size() < keyword_len + 1) {
+    return false;
+  }
+  if (!line.startswith(keyword)) {
+    return false;
+  }
+  /* Treat any ASCII control character as whitespace; don't use isspace() for performance reasons.
+   */
+  if (line[keyword_len] > ' ') {
+    return false;
+  }
+  line = line.drop_prefix(keyword_len + 1);
+  return true;
+}
+
 void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
                       GlobalVertices &r_global_vertices)
 {
@@ -386,22 +405,18 @@ 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 (line.startswith("v ")) {
-          line = line.drop_prefix(2);
+        if (parse_keyword(line, "v")) {
           geom_add_vertex(curr_geom, line, r_global_vertices);
         }
-        else if (line.startswith("vn ")) {
-          line = line.drop_prefix(3);
+        else if (parse_keyword(line, "vn")) {
           geom_add_vertex_normal(curr_geom, line, r_global_vertices);
         }
-        else if (line.startswith("vt ")) {
-          line = line.drop_prefix(3);
+        else if (parse_keyword(line, "vt")) {
           geom_add_uv_vertex(line, r_global_vertices);
         }
       }
       /* Faces. */
-      else if (line.startswith("f ")) {
-        line = line.drop_prefix(2);
+      else if (parse_keyword(line, "f")) {
         geom_add_polygon(curr_geom,
                          line,
                          r_global_vertices,
@@ -411,23 +426,20 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
                          state_shaded_smooth);
       }
       /* Faces. */
-      else if (line.startswith("l ")) {
-        line = line.drop_prefix(2);
+      else if (parse_keyword(line, "l")) {
         geom_add_edge(curr_geom, line, offsets, r_global_vertices);
       }
       /* Objects. */
-      else if (line.startswith("o ")) {
-        line = line.drop_prefix(2);
+      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, r_global_vertices, r_all_geometries, offsets);
+            curr_geom, GEOM_MESH, line.trim(), r_global_vertices, r_all_geometries, offsets);
       }
       /* Groups. */
-      else if (line.startswith("g ")) {
-        line = line.drop_prefix(2);
-        geom_update_group(line, state_group_name);
+      else if (parse_keyword(line, "g")) {
+        geom_update_group(line.trim(), state_group_name);
         int new_index = curr_geom->group_indices_.size();
         state_group_index = curr_geom->group_indices_.lookup_or_add(state_group_name, new_index);
         if (new_index == state_group_index) {
@@ -435,14 +447,12 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
         }
       }
       /* Smoothing groups. */
-      else if (line.startswith("s ")) {
-        line = line.drop_prefix(2);
+      else if (parse_keyword(line, "s")) {
         geom_update_smooth_group(line, state_shaded_smooth);
       }
       /* Materials and their libraries. */
-      else if (line.startswith("usemtl ")) {
-        line = line.drop_prefix(7);
-        state_material_name = line;
+      else if (parse_keyword(line, "usemtl")) {
+        state_material_name = line.trim();
         int new_mat_index = curr_geom->material_indices_.size();
         state_material_index = curr_geom->material_indices_.lookup_or_add(state_material_name,
                                                                           new_mat_index);
@@ -450,37 +460,35 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
           curr_geom->material_order_.append(state_material_name);
         }
       }
-      else if (line.startswith("mtllib ")) {
-        line = line.drop_prefix(7);
-        mtl_libraries_.append(string(line));
+      else if (parse_keyword(line, "mtllib")) {
+        mtl_libraries_.append(line.trim());
       }
       /* Comments. */
       else if (line.startswith("#")) {
         /* Nothing to do. */
       }
       /* Curve related things. */
-      else if (line.startswith("cstype ")) {
-        line = line.drop_prefix(7);
+      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);
       }
-      else if (line.startswith("deg ")) {
-        line = line.drop_prefix(4);
+      else if (parse_keyword(line, "deg")) {
         geom_set_curve_degree(curr_geom, line);
       }
-      else if (line.startswith("curv ")) {
-        line = line.drop_prefix(5);
+      else if (parse_keyword(line, "curv")) {
         geom_add_curve_vertex_indices(curr_geom, line, r_global_vertices);
       }
-      else if (line.startswith("parm ")) {
-        line = line.drop_prefix(5);
+      else if (parse_keyword(line, "parm")) {
         geom_add_curve_parameters(curr_geom, line);
       }
       else if (line.startswith("end")) {
         /* End of curve definition, nothing else to do. */
       }
+      else if (line.front() <= ' ') {
+        /* Just whitespace, skip. */
+      }
       else {
-        std::cout << "OBJ element not recognised: '" << line << "'" << std::endl;
+        std::cout << "OBJ element not recognized: '" << line << "'" << std::endl;
       }
     }
 
@@ -494,77 +502,64 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
 
 static eMTLSyntaxElement mtl_line_start_to_enum(StringRef &line)
 {
-  if (line.startswith("map_Kd")) {
-    line = line.drop_prefix(6);
+  if (parse_keyword(line, "map_Kd")) {
     return eMTLSyntaxElement::map_Kd;
   }
-  if (line.startswith("map_Ks")) {
-    line = line.drop_prefix(6);
+  if (parse_keyword(line, "map_Ks")) {
     return eMTLSyntaxElement::map_Ks;
   }
-  if (line.startswith("map_Ns")) {
-    line = line.drop_prefix(6);
+  if (parse_keyword(line, "map_Ns")) {
     return eMTLSyntaxElement::map_Ns;
   }
-  if (line.startswith("map_d")) {
-    line = line.drop_prefix(5);
+  if (parse_keyword(line, "map_d")) {
     return eMTLSyntaxElement::map_d;
   }
-  if (line.startswith("refl")) {
-    line = line.drop_prefix(4);
+  if (parse_keyword(line, "refl")) {
     return eMTLSyntaxElement::map_refl;
   }
-  if (line.startswith("map_refl")) {
-    line = line.drop_prefix(8);
+  if (parse_keyword(line, "map_refl")) {
     return eMTLSyntaxElement::map_refl;
   }
-  if (line.startswith("map_Ke")) {
-    line = line.drop_prefix(6);
+  if (parse_keyword(line, "map_Ke")) {
     return eMTLSyntaxElement::map_Ke;
   }
-  if (line.startswith("bump")) {
-    line = line.drop_prefix(4);
+  if (parse_keyword(line, "bump")) {
     return eMTLSyntaxElement::map_Bump;
   }
-  if (line.startswith("map_Bump") || line.startswith("map_bump")) {
-    line = line.drop_prefix(8);
+  if (parse_keyword(line, "map_Bump") || parse_keyword(line, "map_bump")) {
     return eMTLSyntaxElement::map_Bump;
   }
   return eMTLSyntaxElement::string;
 }
 
-static const std::pair<const char *, int> unsupported_texture_options[] = {
-    {"-blendu ", 1},
-    {"-blendv ", 1},
-    {"-boost ", 1},
-    {"-cc ", 1},
-    {"-clamp ", 1},
-    {"-imfchan ", 1},
-    {"-mm ", 2},
-    {"-t ", 3},
-    {"-texres ", 1},
+static const std::pair<StringRef, int> unsupported_texture_options[] = {
+    {"-blendu", 1},
+    {"-blendv", 1},
+    {"-boost", 1},
+    {"-cc", 1},
+    {"-clamp", 1},
+    {"-imfchan", 1},
+    {"-mm", 2},
+    {"-t", 3},
+    {"-texres", 1},
 };
 
 static bool parse_texture_option(StringRef &line, MTLMaterial *material, tex_map_XX &tex_map)
 {
   line = drop_whitespace(line);
-  if (line.startswith("-o ")) {
-    line = line.drop_prefix(3);
+  if (parse_keyword(line, "-o")) {
     line = parse_floats(line, 0.0f, tex_map.translation, 3);
     return true;
   }
-  if (line.startswith("-s ")) {
-    line = line.drop_prefix(3);
+  if (parse_keyword(line, "-s")) {
     line = parse_floats(line, 1.0f, tex_map.scale, 3);
     return true;
   }
-  if (line.startswith("-bm ")) {
-    line = line.drop_prefix(4);
+  if (parse_keyword(line, "-bm")) {
     line = parse_float(line, 0.0f, material->map_Bump_strength);
     return true;
   }
-  if (line.startswith("-type ")) {
-    line = line.drop_prefix(6);
+  if (parse_keyword(line, "-type")) {
     line = drop_whitespace(line);
     /* Only sphere is supported. */
     tex_map.projection_type = SHD_PROJ_SPHERE;
@@ -577,9 +572,7 @@ static bool parse_texture_option(StringRef &line, MTLMaterial *material, tex_map
   }
   /* Check for unsupported options and skip them. */
   for (const auto &opt : unsupported_texture_options) {
-    if (line.startswith(opt.first)) {
-      /* Drop the option name. */
-      line = line.drop_known_prefix(opt.first);
+    if (parse_keyword(line, opt.first)) {
       /* Drop the arguments. */
       for (int i = 0; i < opt.second; ++i) {
         line = drop_whitespace(line);
@@ -645,12 +638,13 @@ void MTLParser::parse_and_store(Map<string, std::unique_ptr<MTLMaterial>> &r_mat
   StringRef buffer_str{(const char *)buffer, (int64_t)buffer_len};
   while (!buffer_str.is_empty()) {
     StringRef line = read_next_line(buffer_str);
+    line = drop_whitespace(line);
     if (line.is_empty()) {
       continue;
     }
 
-    if (line.startswith("newmtl ")) {
-      line = line.drop_prefix(7);
+    if (parse_keyword(line, "newmtl")) {
+      line = line.trim();
       if

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list