[Bf-blender-cvs] [d120a083da1] master: Fix T96763: New OBJ Exporter Incorrectly saving the materials in the MTL file

[Aras Pranckevicius]

Commit: d120a083da1f4bbead4895209dd064d1455bc7d6
Author: Aras Pranckevicius
Date:   Thu Mar 31 13:38:59 2022 +0300
Branches: master
https://developer.blender.org/rBd120a083da1f4bbead4895209dd064d1455bc7d6

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

===================================================================

M	source/blender/io/wavefront_obj/exporter/obj_export_file_writer.cc
M	source/blender/io/wavefront_obj/exporter/obj_export_io.hh
M	source/blender/io/wavefront_obj/exporter/obj_export_mtl.cc
M	source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc

===================================================================

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<const eMTLSyntaxElement, tex_map_XX>::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<eMTLSyntaxElement>(translation + scale + map_bump_strength, \
-                                          texture_map.value.image_path); \
+    fmt_handler_.write<eMTLSyntaxElement>(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<T...>};
     }
-    /* 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<T...>};
+      return {"map_Kd{} {}\n", 2, is_type_string_related<T...>};
     }
     case eMTLSyntaxElement::map_Ks: {
-      return {"map_Ks {} {}\n", 2, is_type_string_related<T...>};
+      return {"map_Ks{} {}\n", 2, is_type_string_related<T...>};
     }
     case eMTLSyntaxElement::map_Ns: {
-      return {"map_Ns {} {}\n", 2, is_type_string_related<T...>};
+      return {"map_Ns{} {}\n", 2, is_type_string_related<T...>};
     }
     case eMTLSyntaxElement::map_d: {
-      return {"map_d {} {}\n", 2, is_type_string_related<T...>};
+      return {"map_d{} {}\n", 2, is_type_string_related<T...>};
     }
     case eMTLSyntaxElement::map_refl: {
-      return {"map_refl {} {}\n", 2, is_type_string_related<T...>};
+      return {"map_refl{} {}\n", 2, is_type_string_related<T...>};
     }
     case eMTLSyntaxElement::map_Ke: {
-      return {"map_Ke {} {}\n", 2, is_type_string_related<T...>};
+      return {"map_Ke{} {}\n", 2, is_type_string_related<T...>};
     }
     case eMTLSyntaxElement::map_Bump: {
-      return {"map_Bump {} {}\n", 2, is_type_string_related<T...>};
+      return {"map_Bump{} {}\n", 2, is_type_string_related<T...>};
     }
     case eMTLSyntaxElement::string: {
       return {"{}", 1, is_type_string_related<T...>};
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;