From d120a083da1f4bbead4895209dd064d1455bc7d6 Mon Sep 17 00:00:00 2001 From: Aras Pranckevicius Date: Thu, 31 Mar 2022 13:38:59 +0300 Subject: Fix T96763: New OBJ Exporter Incorrectly saving the materials in the MTL file Original report (T96763) only reported the issue of double-space before the texture path, but while adding test coverage I found some other issues that I fixed while at it: - Incorrectly emits two spaces between `map_Xx` keyword and the texture path, leading to some 3rd party software not finding the textures, - Emissive texture map (`map_Ke`) was not exported, - When Mapping node is used on the texture UVs, the "Location" and "Scale" values were mixed up (location written as "scale", scale written as "location). Added gtest coverage. Reviewed By: Howard Trickey Differential Revision: https://developer.blender.org/D14519 --- .../io/wavefront_obj/exporter/obj_export_file_writer.cc | 15 ++++++--------- .../blender/io/wavefront_obj/exporter/obj_export_io.hh | 16 ++++++++-------- .../blender/io/wavefront_obj/exporter/obj_export_mtl.cc | 15 ++++++++------- .../blender/io/wavefront_obj/tests/obj_exporter_tests.cc | 13 +++++++++++++ 4 files changed, 35 insertions(+), 24 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 acfdaa29b52..f78ef334d4d 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 @@ -525,24 +525,21 @@ void MTLWriter::write_texture_map( const MTLMaterial &mtl_material, const Map::Item &texture_map) { - std::string translation; - std::string scale; - std::string map_bump_strength; - /* Optional strings should have their own leading spaces. */ + std::string options; + /* Option strings should have their own leading spaces. */ if (texture_map.value.translation != float3{0.0f, 0.0f, 0.0f}) { - translation.append(" -s ").append(float3_to_string(texture_map.value.translation)); + options.append(" -o ").append(float3_to_string(texture_map.value.translation)); } if (texture_map.value.scale != float3{1.0f, 1.0f, 1.0f}) { - scale.append(" -o ").append(float3_to_string(texture_map.value.scale)); + options.append(" -s ").append(float3_to_string(texture_map.value.scale)); } if (texture_map.key == eMTLSyntaxElement::map_Bump && mtl_material.map_Bump_strength > 0.0001f) { - map_bump_strength.append(" -bm ").append(std::to_string(mtl_material.map_Bump_strength)); + options.append(" -bm ").append(std::to_string(mtl_material.map_Bump_strength)); } #define SYNTAX_DISPATCH(eMTLSyntaxElement) \ if (texture_map.key == eMTLSyntaxElement) { \ - fmt_handler_.write(translation + scale + map_bump_strength, \ - texture_map.value.image_path); \ + fmt_handler_.write(options, texture_map.value.image_path); \ return; \ } diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_io.hh b/source/blender/io/wavefront_obj/exporter/obj_export_io.hh index 0095384609a..0d7c089ec14 100644 --- a/source/blender/io/wavefront_obj/exporter/obj_export_io.hh +++ b/source/blender/io/wavefront_obj/exporter/obj_export_io.hh @@ -236,27 +236,27 @@ constexpr FormattingSyntax syntax_elem_to_formatting(const eMTLSyntaxElement key case eMTLSyntaxElement::Ke: { return {"Ke {:.6f} {:.6f} {:.6f}\n", 3, is_type_float}; } - /* Keep only one space between options since filepaths may have leading spaces too. */ + /* Note: first texture map related argument, if present, will have its own leading space. */ case eMTLSyntaxElement::map_Kd: { - return {"map_Kd {} {}\n", 2, is_type_string_related}; + return {"map_Kd{} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_Ks: { - return {"map_Ks {} {}\n", 2, is_type_string_related}; + return {"map_Ks{} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_Ns: { - return {"map_Ns {} {}\n", 2, is_type_string_related}; + return {"map_Ns{} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_d: { - return {"map_d {} {}\n", 2, is_type_string_related}; + return {"map_d{} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_refl: { - return {"map_refl {} {}\n", 2, is_type_string_related}; + return {"map_refl{} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_Ke: { - return {"map_Ke {} {}\n", 2, is_type_string_related}; + return {"map_Ke{} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::map_Bump: { - return {"map_Bump {} {}\n", 2, is_type_string_related}; + return {"map_Bump{} {}\n", 2, is_type_string_related}; } case eMTLSyntaxElement::string: { return {"{}", 1, is_type_string_related}; diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_mtl.cc b/source/blender/io/wavefront_obj/exporter/obj_export_mtl.cc index b8814b1bdd3..c48d5a5f7f0 100644 --- a/source/blender/io/wavefront_obj/exporter/obj_export_mtl.cc +++ b/source/blender/io/wavefront_obj/exporter/obj_export_mtl.cc @@ -287,14 +287,15 @@ static void store_image_textures(const nodes::NodeRef *bsdf_node, /* Find sockets linked to "Color" socket in normal map node. */ linked_sockets_to_dest_id(normal_map_node, *node_tree, "Color", linked_sockets); } - else if (texture_map.key == eMTLSyntaxElement::map_Ke) { - float emission_strength = 0.0f; - copy_property_from_node(SOCK_FLOAT, bnode, "Emission Strength", {&emission_strength, 1}); - if (emission_strength == 0.0f) { - continue; - } - } else { + /* Skip emission map if emission strength is zero. */ + if (texture_map.key == eMTLSyntaxElement::map_Ke) { + float emission_strength = 0.0f; + copy_property_from_node(SOCK_FLOAT, bnode, "Emission Strength", {&emission_strength, 1}); + if (emission_strength == 0.0f) { + continue; + } + } /* Find sockets linked to the destination socket of interest, in P-BSDF node. */ linked_sockets_to_dest_id( bnode, *node_tree, texture_map.value.dest_socket_id, linked_sockets); diff --git a/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc b/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc index 7e3a9228c3b..a3512595fa7 100644 --- a/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc +++ b/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc @@ -486,6 +486,19 @@ TEST_F(obj_exporter_regression_test, cubes_positioned) _export.params); } +/* Note: texture paths in the resulting mtl file currently are always + * as they are stored in the source .blend file; not relative to where + * the export is done. When that is properly fixed, the expected .mtl + * file should be updated. */ +TEST_F(obj_exporter_regression_test, cubes_with_textures) +{ + OBJExportParamsDefault _export; + compare_obj_export_to_golden("io_tests/blend_geometry/cubes_with_textures.blend", + "io_tests/obj/cubes_with_textures.obj", + "io_tests/obj/cubes_with_textures.mtl", + _export.params); +} + TEST_F(obj_exporter_regression_test, suzanne_all_data) { OBJExportParamsDefault _export; -- cgit v1.2.3