From 0a096f2be233b6faca468a1add615fb72e57909e Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Thu, 11 Aug 2022 15:51:36 +0300 Subject: Fix T98781: OBJ exporter wrongly writing default material socket values when textures are present Report T98781 and part of T97642: the MTLMaterial info only captures image nodes and the default socket values. When the image information is present, do not emit the socket defaults - the .MTL spec states they are multiplied together, but the default value is not used in blender when the socket is connected. Also contains svn tests repository update to extend the test coverage, and update test expectation outputs. --- .../exporter/obj_export_file_writer.cc | 47 +++++++++++++--------- .../io/wavefront_obj/exporter/obj_export_mtl.hh | 26 ++++++------ .../io/wavefront_obj/importer/obj_import_mtl.cc | 4 +- .../io/wavefront_obj/tests/obj_importer_tests.cc | 12 +++++- 4 files changed, 55 insertions(+), 34 deletions(-) diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_file_writer.cc b/source/blender/io/wavefront_obj/exporter/obj_export_file_writer.cc index 36a9cf1b9ae..53aa80700cc 100644 --- a/source/blender/io/wavefront_obj/exporter/obj_export_file_writer.cc +++ b/source/blender/io/wavefront_obj/exporter/obj_export_file_writer.cc @@ -547,20 +547,29 @@ StringRefNull MTLWriter::mtl_file_path() const return mtl_filepath_; } -void MTLWriter::write_bsdf_properties(const MTLMaterial &mtl_material) +void MTLWriter::write_bsdf_properties(const MTLMaterial &mtl) { - fmt_handler_.write(mtl_material.Ns); - fmt_handler_.write( - mtl_material.Ka.x, mtl_material.Ka.y, mtl_material.Ka.z); - fmt_handler_.write( - mtl_material.Kd.x, mtl_material.Kd.y, mtl_material.Kd.z); - fmt_handler_.write( - mtl_material.Ks.x, mtl_material.Ks.y, mtl_material.Ks.z); - fmt_handler_.write( - mtl_material.Ke.x, mtl_material.Ke.y, mtl_material.Ke.z); - fmt_handler_.write(mtl_material.Ni); - fmt_handler_.write(mtl_material.d); - fmt_handler_.write(mtl_material.illum); + /* For various material properties, we only capture information + * coming from the texture, or the default value of the socket. + * When the texture is present, do not emit the default value. */ + if (!mtl.tex_map_of_type(eMTLSyntaxElement::map_Ns).is_valid()) { + fmt_handler_.write(mtl.Ns); + } + fmt_handler_.write(mtl.Ka.x, mtl.Ka.y, mtl.Ka.z); + if (!mtl.tex_map_of_type(eMTLSyntaxElement::map_Kd).is_valid()) { + fmt_handler_.write(mtl.Kd.x, mtl.Kd.y, mtl.Kd.z); + } + if (!mtl.tex_map_of_type(eMTLSyntaxElement::map_Ks).is_valid()) { + fmt_handler_.write(mtl.Ks.x, mtl.Ks.y, mtl.Ks.z); + } + if (!mtl.tex_map_of_type(eMTLSyntaxElement::map_Ke).is_valid()) { + fmt_handler_.write(mtl.Ke.x, mtl.Ke.y, mtl.Ke.z); + } + fmt_handler_.write(mtl.Ni); + if (!mtl.tex_map_of_type(eMTLSyntaxElement::map_d).is_valid()) { + fmt_handler_.write(mtl.d); + } + fmt_handler_.write(mtl.illum); } void MTLWriter::write_texture_map( @@ -583,12 +592,13 @@ void MTLWriter::write_texture_map( options.append(" -bm ").append(std::to_string(mtl_material.map_Bump_strength)); } + std::string path = path_reference( + texture_map.value.image_path.c_str(), blen_filedir, dest_dir, path_mode, ©_set); + /* Always emit forward slashes for cross-platform compatibility. */ + std::replace(path.begin(), path.end(), '\\', '/'); + #define SYNTAX_DISPATCH(eMTLSyntaxElement) \ if (texture_map.key == eMTLSyntaxElement) { \ - std::string path = path_reference( \ - texture_map.value.image_path.c_str(), blen_filedir, dest_dir, path_mode, ©_set); \ - /* Always emit forward slashes for cross-platform compatibility. */ \ - std::replace(path.begin(), path.end(), '\\', '/'); \ fmt_handler_.write(options, path.c_str()); \ return; \ } @@ -600,6 +610,7 @@ void MTLWriter::write_texture_map( SYNTAX_DISPATCH(eMTLSyntaxElement::map_refl); SYNTAX_DISPATCH(eMTLSyntaxElement::map_Ke); SYNTAX_DISPATCH(eMTLSyntaxElement::map_Bump); +#undef SYNTAX_DISPATCH BLI_assert(!"This map type was not written to the file."); } @@ -626,7 +637,7 @@ void MTLWriter::write_materials(const char *blen_filepath, fmt_handler_.write(mtlmat.name); write_bsdf_properties(mtlmat); for (const auto &tex : mtlmat.texture_maps.items()) { - if (tex.value.image_path.empty()) { + if (!tex.value.is_valid()) { continue; } write_texture_map(mtlmat, tex, blen_filedir, dest_dir, path_mode, copy_set); diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_mtl.hh b/source/blender/io/wavefront_obj/exporter/obj_export_mtl.hh index 80e3127f69f..f83b3b49bf5 100644 --- a/source/blender/io/wavefront_obj/exporter/obj_export_mtl.hh +++ b/source/blender/io/wavefront_obj/exporter/obj_export_mtl.hh @@ -8,8 +8,6 @@ #include "BLI_map.hh" #include "BLI_math_vec_types.hh" -#include "BLI_string_ref.hh" -#include "BLI_vector.hh" #include "DNA_node_types.h" #include "obj_export_io.hh" @@ -25,19 +23,22 @@ template<> struct DefaultHash { } // namespace blender namespace blender::io::obj { -class OBJMesh; /** * Generic container for texture node properties. */ struct tex_map_XX { tex_map_XX(StringRef to_socket_id) : dest_socket_id(to_socket_id){}; + bool is_valid() const + { + return !image_path.empty(); + } - /** Target socket which this texture node connects to. */ + /* Target socket which this texture node connects to. */ const std::string dest_socket_id; float3 translation{0.0f}; float3 scale{1.0f}; - /* Only Flat and Smooth projections are supported. */ + /* Only Flat and Sphere projections are supported. */ int projection_type = SHD_PROJ_FLAT; std::string image_path; std::string mtl_dir_path; @@ -58,16 +59,15 @@ struct MTLMaterial { texture_maps.add(eMTLSyntaxElement::map_Bump, tex_map_XX("Normal")); } - /** - * Caller must ensure that the given lookup key exists in the Map. - * \return Texture map corresponding to the given ID. - */ + const tex_map_XX &tex_map_of_type(const eMTLSyntaxElement key) const + { + BLI_assert(texture_maps.contains(key)); + return texture_maps.lookup(key); + } tex_map_XX &tex_map_of_type(const eMTLSyntaxElement key) { - { - BLI_assert(texture_maps.contains_as(key)); - return texture_maps.lookup_as(key); - } + BLI_assert(texture_maps.contains(key)); + return texture_maps.lookup(key); } std::string name; 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 27bb5aa0d71..02e09a77a5d 100644 --- a/source/blender/io/wavefront_obj/importer/obj_import_mtl.cc +++ b/source/blender/io/wavefront_obj/importer/obj_import_mtl.cc @@ -342,7 +342,7 @@ void ShaderNodetreeWrap::set_bsdf_socket_values(Material *mat) 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)) { + if (mtl_mat_.tex_map_of_type(eMTLSyntaxElement::map_Ke).is_valid()) { set_property_of_socket(SOCK_FLOAT, "Emission Strength", {1.0f}, bsdf_); } set_property_of_socket(SOCK_FLOAT, "Specular", {specular}, bsdf_); @@ -365,7 +365,7 @@ void ShaderNodetreeWrap::add_image_textures(Main *bmain, Material *mat, bool rel { for (const Map::Item texture_map : mtl_mat_.texture_maps.items()) { - if (texture_map.value.image_path.empty()) { + if (!texture_map.value.is_valid()) { /* No Image texture node of this map type can be added to this material. */ continue; } diff --git a/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc b/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc index 63c827d385f..f97546fe4f5 100644 --- a/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc +++ b/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc @@ -325,6 +325,16 @@ TEST_F(obj_importer_test, import_cubes_with_textures_rel) float3(1, -1, -1), float3(0, 1, 0), float2(0.9935f, 0.0020f)}, + {"OBCubeTexMul", + OB_MESH, + 8, + 12, + 6, + 24, + float3(4, -2, -1), + float3(4, -4, -1), + float3(0, 1, 0), + float2(0.9935f, 0.0020f)}, {"OBCubeTiledTex", OB_MESH, 8, @@ -346,7 +356,7 @@ TEST_F(obj_importer_test, import_cubes_with_textures_rel) float3(0, 1, 0), float2(0.9935f, 0.0020f)}, }; - import_and_check("cubes_with_textures_rel.obj", expect, std::size(expect), 3, 4); + import_and_check("cubes_with_textures_rel.obj", expect, std::size(expect), 4, 4); } TEST_F(obj_importer_test, import_faces_invalid_or_with_holes) -- cgit v1.2.3