Fix T97757: Some MTL import correctness issues in the new OBJ importer

Fix several correctness issues where the new OBJ/MTL importer was not
producing the same results as the old one, mostly because the code for
some reason had slightly different logic. Fixes T97757:

- When .obj file tries to use a material that does not exist, the code
  was continuing to use the previous material, instead of creating new
  default one, as the previous importer did.
- Previous importer was always searching/parsing "foo.mtl" for a
  "foo.obj" file, even if the file itself does not contain
  "mtllib foo.mtl" statement. One file from T97757 repros happens to
  depend on that, so resurrect that behavior.
- When IOR (Ni) or Alpha (d) are not specified in .mtl file, do not
  wrongly set -1 values to the blender material.
- When base (Kd) or emissive (Ke) colors are not specified in the .mtl
  file, do not set them on the blender material.
- Roughness and metallic values used by viewport shading were not set
  onto blender material.
- The logic for when metallic was set to zero was incorrect; it should
  be set to zero when "not using reflection", not when "mtl file does
  not contain metallic".
- Do not produce a warning when illum value is not spelled out in .mtl
  file, treat as default (1).
- Parse illum as a float just like python importer does, as to not
  reintroduce part of T60135.

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

6 files changed, 106 insertions, 74 deletions

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 56806449733..3724eaaa361 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
@@ -461,7 +461,7 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
         }
       }
       else if (parse_keyword(line, "mtllib")) {
-        mtl_libraries_.append(line.trim());
+        add_mtl_library(line.trim());
       }
       /* Comments. */
       else if (line.startswith("#")) {
@@ -498,6 +498,8 @@ void OBJParser::parse(Vector<std::unique_ptr<Geometry>> &r_all_geometries,
     memmove(buffer.data(), buffer.data() + last_nl, left_size);
     buffer_offset = left_size;
   }
+
+  add_default_mtl_library();
 }
 
 static eMTLSyntaxElement mtl_line_start_to_enum(StringRef &line)
@@ -616,6 +618,30 @@ Span<std::string> OBJParser::mtl_libraries() const
   return mtl_libraries_;
 }
 
+void OBJParser::add_mtl_library(const std::string &path)
+{
+  if (!mtl_libraries_.contains(path)) {
+    mtl_libraries_.append(path);
+  }
+}
+
+void OBJParser::add_default_mtl_library()
+{
+  /* Add any existing .mtl file that's with the same base name as the .obj file
+   * into candidate .mtl files to search through. This is not technically following the
+   * spec, but the old python importer was doing it, and there are user files out there
+   * that contain "mtllib bar.mtl" for a foo.obj, and depend on finding materials
+   * from foo.mtl (see T97757). */
+  char mtl_file_path[FILE_MAX];
+  BLI_strncpy(mtl_file_path, import_params_.filepath, sizeof(mtl_file_path));
+  BLI_path_extension_replace(mtl_file_path, sizeof(mtl_file_path), ".mtl");
+  if (BLI_exists(mtl_file_path)) {
+    char mtl_file_base[FILE_MAX];
+    BLI_split_file_part(mtl_file_path, mtl_file_base, sizeof(mtl_file_base));
+    add_mtl_library(mtl_file_base);
+  }
+}
+
 MTLParser::MTLParser(StringRef mtl_library, StringRefNull obj_filepath)
 {
   char obj_file_dir[FILE_MAXDIR];
@@ -645,11 +671,12 @@ void MTLParser::parse_and_store(Map<string, std::unique_ptr<MTLMaterial>> &r_mat
 
     if (parse_keyword(line, "newmtl")) {
       line = line.trim();
-      if (r_materials.remove_as(line)) {
-        std::cerr << "Duplicate material found:'" << line
-                  << "', using the last encountered Material definition." << std::endl;
+      if (r_materials.contains(line)) {
+        material = nullptr;
+      }
+      else {
+        material = r_materials.lookup_or_add(string(line), std::make_unique<MTLMaterial>()).get();
       }
-      material = r_materials.lookup_or_add(string(line), std::make_unique<MTLMaterial>()).get();
     }
     else if (material != nullptr) {
       if (parse_keyword(line, "Ns")) {
@@ -674,7 +701,10 @@ void MTLParser::parse_and_store(Map<string, std::unique_ptr<MTLMaterial>> &r_mat
         parse_float(line, 1.0f, material->d);
       }
       else if (parse_keyword(line, "illum")) {
-        parse_int(line, 2, material->illum);
+        /* Some files incorrectly use a float (T60135). */
+        float val;
+        parse_float(line, 1.0f, val);
+        material->illum = val;
       }
       else {
         parse_texture_map(line, material, mtl_dir_path_);
diff --git a/source/blender/io/wavefront_obj/importer/obj_import_file_reader.hh b/source/blender/io/wavefront_obj/importer/obj_import_file_reader.hh
index 8093417fcda..8dd60d17100 100644
--- a/source/blender/io/wavefront_obj/importer/obj_import_file_reader.hh
+++ b/source/blender/io/wavefront_obj/importer/obj_import_file_reader.hh
@@ -39,6 +39,10 @@ class OBJParser {
    * Return a list of all material library filepaths referenced by the OBJ file.
    */
   Span<std::string> mtl_libraries() const;
+
+ private:
+  void add_mtl_library(const std::string &path);
+  void add_default_mtl_library();
 };
 
 class MTLParser {
diff --git a/source/blender/io/wavefront_obj/importer/obj_import_mesh.cc b/source/blender/io/wavefront_obj/importer/obj_import_mesh.cc
index 01a2d63927e..fc40333c24d 100644
--- a/source/blender/io/wavefront_obj/importer/obj_import_mesh.cc
+++ b/source/blender/io/wavefront_obj/importer/obj_import_mesh.cc
@@ -24,11 +24,10 @@
 
 namespace blender::io::obj {
 
-Object *MeshFromGeometry::create_mesh(
-    Main *bmain,
-    const Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
-    Map<std::string, Material *> &created_materials,
-    const OBJImportParams &import_params)
+Object *MeshFromGeometry::create_mesh(Main *bmain,
+                                      Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
+                                      Map<std::string, Material *> &created_materials,
+                                      const OBJImportParams &import_params)
 {
   std::string ob_name{mesh_geometry_.geometry_name_};
   if (ob_name.empty()) {
@@ -276,11 +275,10 @@ void MeshFromGeometry::create_uv_verts(Mesh *mesh)
   }
 }
 
-static Material *get_or_create_material(
-    Main *bmain,
-    const std::string &name,
-    const Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
-    Map<std::string, Material *> &created_materials)
+static Material *get_or_create_material(Main *bmain,
+                                        const std::string &name,
+                                        Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
+                                        Map<std::string, Material *> &created_materials)
 {
   /* Have we created this material already? */
   Material **found_mat = created_materials.lookup_ptr(name);
@@ -288,23 +286,13 @@ static Material *get_or_create_material(
     return *found_mat;
   }
 
-  /* We have not, will have to create it. */
-  if (!materials.contains(name)) {
-    std::cerr << "Material named '" << name << "' not found in material library." << std::endl;
-    return nullptr;
-  }
+  /* We have not, will have to create it. Create a new default
+   * MTLMaterial too, in case the OBJ file tries to use a material
+   * that was not in the MTL file. */
+  const MTLMaterial &mtl = *materials.lookup_or_add(name, std::make_unique<MTLMaterial>()).get();
 
   Material *mat = BKE_material_add(bmain, name.c_str());
-  const MTLMaterial &mtl = *materials.lookup(name);
   ShaderNodetreeWrap mat_wrap{bmain, mtl, mat};
-
-  /* Viewport shading uses legacy r,g,b material values. */
-  if (mtl.Kd[0] >= 0 && mtl.Kd[1] >= 0 && mtl.Kd[2] >= 0) {
-    mat->r = mtl.Kd[0];
-    mat->g = mtl.Kd[1];
-    mat->b = mtl.Kd[2];
-  }
-
   mat->use_nodes = true;
   mat->nodetree = mat_wrap.get_nodetree();
   BKE_ntree_update_main_tree(bmain, mat->nodetree, nullptr);
@@ -313,11 +301,10 @@ static Material *get_or_create_material(
   return mat;
 }
 
-void MeshFromGeometry::create_materials(
-    Main *bmain,
-    const Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
-    Map<std::string, Material *> &created_materials,
-    Object *obj)
+void MeshFromGeometry::create_materials(Main *bmain,
+                                        Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
+                                        Map<std::string, Material *> &created_materials,
+                                        Object *obj)
 {
   for (const std::string &name : mesh_geometry_.material_order_) {
     Material *mat = get_or_create_material(bmain, name, materials, created_materials);
diff --git a/source/blender/io/wavefront_obj/importer/obj_import_mesh.hh b/source/blender/io/wavefront_obj/importer/obj_import_mesh.hh
index 7cc7ed25ad1..cf4a2aee394 100644
--- a/source/blender/io/wavefront_obj/importer/obj_import_mesh.hh
+++ b/source/blender/io/wavefront_obj/importer/obj_import_mesh.hh
@@ -32,7 +32,7 @@ class MeshFromGeometry : NonMovable, NonCopyable {
   }
 
   Object *create_mesh(Main *bmain,
-                      const Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
+                      Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
                       Map<std::string, Material *> &created_materials,
                       const OBJImportParams &import_params);
 
@@ -61,7 +61,7 @@ class MeshFromGeometry : NonMovable, NonCopyable {
    * Add materials and the node-tree to the Mesh Object.
    */
   void create_materials(Main *bmain,
-                        const Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
+                        Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
                         Map<std::string, Material *> &created_materials,
                         Object *obj);
   void create_normals(Mesh *mesh);
diff --git a/source/blender/io/wavefront_obj/importer/obj_import_mtl.cc b/source/blender/io/wavefront_obj/importer/obj_import_mtl.cc
index 56e3a062cb6..c2ecd8a37de 100644
--- a/source/blender/io/wavefront_obj/importer/obj_import_mtl.cc
+++ b/source/blender/io/wavefront_obj/importer/obj_import_mtl.cc
@@ -197,10 +197,10 @@ void ShaderNodetreeWrap::set_bsdf_socket_values(Material *mat)
   bool do_glass = false;
   /* See https://wikipedia.org/wiki/Wavefront_.obj_file for possible values of illum. */
   switch (illum) {
-    case 1: {
+    case -1:
+    case 1:
       /* Base color on, ambient on. */
       break;
-    }
     case 2: {
       /* Highlight on. */
       do_highlight = true;
@@ -257,28 +257,30 @@ void ShaderNodetreeWrap::set_bsdf_socket_values(Material *mat)
    * Principled BSDF: */
   /* Specular: average of Ks components. */
   float specular = (mtl_mat_.Ks[0] + mtl_mat_.Ks[1] + mtl_mat_.Ks[2]) / 3;
-  /* Roughness: map 0..1000 range to 1..0 and apply non-linearity. */
-  float clamped_ns = std::max(0.0f, std::min(1000.0f, mtl_mat_.Ns));
-  float roughness = 1.0f - sqrt(clamped_ns / 1000.0f);
-  /* Metallic: average of Ka components. */
-  float metallic = (mtl_mat_.Ka[0] + mtl_mat_.Ka[1] + mtl_mat_.Ka[2]) / 3;
-  float ior = mtl_mat_.Ni;
-  float alpha = mtl_mat_.d;
-
   if (specular < 0.0f) {
-    specular = static_cast<float>(do_highlight);
+    specular = do_highlight ? 1.0f : 0.0f;
   }
+  /* Roughness: map 0..1000 range to 1..0 and apply non-linearity. */
+  float roughness;
   if (mtl_mat_.Ns < 0.0f) {
-    roughness = static_cast<float>(!do_highlight);
+    roughness = do_highlight ? 0.0f : 1.0f;
   }
-  if (metallic < 0.0f) {
-    if (do_reflection) {
+  else {
+    float clamped_ns = std::max(0.0f, std::min(1000.0f, mtl_mat_.Ns));
+    roughness = 1.0f - sqrt(clamped_ns / 1000.0f);
+  }
+  /* Metallic: average of Ka components. */
+  float metallic = (mtl_mat_.Ka[0] + mtl_mat_.Ka[1] + mtl_mat_.Ka[2]) / 3;
+  if (do_reflection) {
+    if (metallic < 0.0f) {
       metallic = 1.0f;
     }
   }
   else {
     metallic = 0.0f;
   }
+
+  float ior = mtl_mat_.Ni;
   if (ior < 0) {
     if (do_tranparency) {
       ior = 1.0f;
@@ -287,28 +289,38 @@ void ShaderNodetreeWrap::set_bsdf_socket_values(Material *mat)
       ior = 1.5f;
     }
   }
-  if (alpha < 0) {
-    if (do_tranparency) {
-      alpha = 1.0f;
-    }
+  float alpha = mtl_mat_.d;
+  if (do_tranparency && alpha < 0) {
+    alpha = 1.0f;
   }
-  float3 base_color = {std::max(0.0f, mtl_mat_.Kd[0]),
-                       std::max(0.0f, mtl_mat_.Kd[1]),
-                       std::max(0.0f, mtl_mat_.Kd[2])};
-  float3 emission_color = {std::max(0.0f, mtl_mat_.Ke[0]),
-                           std::max(0.0f, mtl_mat_.Ke[1]),
-                           std::max(0.0f, mtl_mat_.Ke[2])};
 
-  set_property_of_socket(SOCK_RGBA, "Base Color", {base_color, 3}, bsdf_);
-  set_property_of_socket(SOCK_RGBA, "Emission", {emission_color, 3}, bsdf_);
+  float3 base_color = {mtl_mat_.Kd[0], mtl_mat_.Kd[1], mtl_mat_.Kd[2]};
+  if (base_color.x >= 0 && base_color.y >= 0 && base_color.z >= 0) {
+    set_property_of_socket(SOCK_RGBA, "Base Color", {base_color, 3}, bsdf_);
+    /* Viewport shading uses legacy r,g,b base color. */
+    mat->r = base_color.x;
+    mat->g = base_color.y;
+    mat->b = base_color.z;
+  }
+
+  float3 emission_color = {mtl_mat_.Ke[0], mtl_mat_.Ke[1], mtl_mat_.Ke[2]};
+  if (emission_color.x >= 0 && emission_color.y >= 0 && emission_color.z >= 0) {
+    set_property_of_socket(SOCK_RGBA, "Emission", {emission_color, 3}, bsdf_);
+  }
   if (mtl_mat_.texture_maps.contains_as(eMTLSyntaxElement::map_Ke)) {
     set_property_of_socket(SOCK_FLOAT, "Emission Strength", {1.0f}, bsdf_);
   }
   set_property_of_socket(SOCK_FLOAT, "Specular", {specular}, bsdf_);
   set_property_of_socket(SOCK_FLOAT, "Roughness", {roughness}, bsdf_);
+  mat->roughness = roughness;
   set_property_of_socket(SOCK_FLOAT, "Metallic", {metallic}, bsdf_);
-  set_property_of_socket(SOCK_FLOAT, "IOR", {ior}, bsdf_);
-  set_property_of_socket(SOCK_FLOAT, "Alpha", {alpha}, bsdf_);
+  mat->metallic = metallic;
+  if (ior != -1) {
+    set_property_of_socket(SOCK_FLOAT, "IOR", {ior}, bsdf_);
+  }
+  if (alpha != -1) {
+    set_property_of_socket(SOCK_FLOAT, "Alpha", {alpha}, bsdf_);
+  }
   if (do_tranparency) {
     mat->blend_method = MA_BM_BLEND;
   }
diff --git a/source/blender/io/wavefront_obj/importer/obj_importer.cc b/source/blender/io/wavefront_obj/importer/obj_importer.cc
index c21d2d9583c..d9c033e1f77 100644
--- a/source/blender/io/wavefront_obj/importer/obj_importer.cc
+++ b/source/blender/io/wavefront_obj/importer/obj_importer.cc
@@ -29,15 +29,14 @@ namespace blender::io::obj {
 /**
  * Make Blender Mesh, Curve etc from Geometry and add them to the import collection.
  */
-static void geometry_to_blender_objects(
-    Main *bmain,
-    Scene *scene,
-    ViewLayer *view_layer,
-    const OBJImportParams &import_params,
-    Vector<std::unique_ptr<Geometry>> &all_geometries,
-    const GlobalVertices &global_vertices,
-    const Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
-    Map<std::string, Material *> &created_materials)
+static void geometry_to_blender_objects(Main *bmain,
+                                        Scene *scene,
+                                        ViewLayer *view_layer,
+                                        const OBJImportParams &import_params,
+                                        Vector<std::unique_ptr<Geometry>> &all_geometries,
+                                        const GlobalVertices &global_vertices,
+                                        Map<std::string, std::unique_ptr<MTLMaterial>> &materials,
+                                        Map<std::string, Material *> &created_materials)
 {
   BKE_view_layer_base_deselect_all(view_layer);
   LayerCollection *lc = BKE_layer_collection_get_active(view_layer);