[Bf-blender-cvs] [5f5e7ac317d] blender-v3.2-release: Fix T97757: Some MTL import correctness issues in the new OBJ importer
Aras Pranckevicius
noreply at git.blender.org
Tue May 3 13:49:06 CEST 2022
Commit: 5f5e7ac317ddef5fed1c85fc8568c3ce09141fc1
Author: Aras Pranckevicius
Date: Tue May 3 14:44:49 2022 +0300
Branches: blender-v3.2-release
https://developer.blender.org/rB5f5e7ac317ddef5fed1c85fc8568c3ce09141fc1
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
===================================================================
M source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
M source/blender/io/wavefront_obj/importer/obj_import_file_reader.hh
M source/blender/io/wavefront_obj/importer/obj_import_mesh.cc
M source/blender/io/wavefront_obj/importer/obj_import_mesh.hh
M source/blender/io/wavefront_obj/importer/obj_import_mtl.cc
M source/blender/io/wavefront_obj/importer/obj_importer.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 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
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list