From 5f5e7ac317ddef5fed1c85fc8568c3ce09141fc1 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Tue, 3 May 2022 14:44:49 +0300 Subject: 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 --- .../importer/obj_import_file_reader.cc | 42 +++++++++++-- .../importer/obj_import_file_reader.hh | 4 ++ .../io/wavefront_obj/importer/obj_import_mesh.cc | 45 +++++--------- .../io/wavefront_obj/importer/obj_import_mesh.hh | 4 +- .../io/wavefront_obj/importer/obj_import_mtl.cc | 68 +++++++++++++--------- .../io/wavefront_obj/importer/obj_importer.cc | 17 +++--- 6 files changed, 106 insertions(+), 74 deletions(-) (limited to 'source/blender') 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> &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> &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 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> &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()).get(); } - material = r_materials.lookup_or_add(string(line), std::make_unique()).get(); } else if (material != nullptr) { if (parse_keyword(line, "Ns")) { @@ -674,7 +701,10 @@ void MTLParser::parse_and_store(Map> &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 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> &materials, - Map &created_materials, - const OBJImportParams &import_params) +Object *MeshFromGeometry::create_mesh(Main *bmain, + Map> &materials, + Map &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> &materials, - Map &created_materials) +static Material *get_or_create_material(Main *bmain, + const std::string &name, + Map> &materials, + Map &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()).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> &materials, - Map &created_materials, - Object *obj) +void MeshFromGeometry::create_materials(Main *bmain, + Map> &materials, + Map &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> &materials, + Map> &materials, Map &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> &materials, + Map> &materials, Map &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(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(!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> &all_geometries, - const GlobalVertices &global_vertices, - const Map> &materials, - Map &created_materials) +static void geometry_to_blender_objects(Main *bmain, + Scene *scene, + ViewLayer *view_layer, + const OBJImportParams &import_params, + Vector> &all_geometries, + const GlobalVertices &global_vertices, + Map> &materials, + Map &created_materials) { BKE_view_layer_base_deselect_all(view_layer); LayerCollection *lc = BKE_layer_collection_get_active(view_layer); -- cgit v1.2.3