[Bf-blender-cvs] [0a096f2be23] blender-v3.3-release: Fix T98781: OBJ exporter wrongly writing default material socket values when textures are present

Aras Pranckevicius noreply at git.blender.org
Thu Aug 11 14:51:43 CEST 2022


Commit: 0a096f2be233b6faca468a1add615fb72e57909e
Author: Aras Pranckevicius
Date:   Thu Aug 11 15:51:36 2022 +0300
Branches: blender-v3.3-release
https://developer.blender.org/rB0a096f2be233b6faca468a1add615fb72e57909e

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.

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

M	source/blender/io/wavefront_obj/exporter/obj_export_file_writer.cc
M	source/blender/io/wavefront_obj/exporter/obj_export_mtl.hh
M	source/blender/io/wavefront_obj/importer/obj_import_mtl.cc
M	source/blender/io/wavefront_obj/tests/obj_importer_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 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<eMTLSyntaxElement::Ns>(mtl_material.Ns);
-  fmt_handler_.write<eMTLSyntaxElement::Ka>(
-      mtl_material.Ka.x, mtl_material.Ka.y, mtl_material.Ka.z);
-  fmt_handler_.write<eMTLSyntaxElement::Kd>(
-      mtl_material.Kd.x, mtl_material.Kd.y, mtl_material.Kd.z);
-  fmt_handler_.write<eMTLSyntaxElement::Ks>(
-      mtl_material.Ks.x, mtl_material.Ks.y, mtl_material.Ks.z);
-  fmt_handler_.write<eMTLSyntaxElement::Ke>(
-      mtl_material.Ke.x, mtl_material.Ke.y, mtl_material.Ke.z);
-  fmt_handler_.write<eMTLSyntaxElement::Ni>(mtl_material.Ni);
-  fmt_handler_.write<eMTLSyntaxElement::d>(mtl_material.d);
-  fmt_handler_.write<eMTLSyntaxElement::illum>(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<eMTLSyntaxElement::Ns>(mtl.Ns);
+  }
+  fmt_handler_.write<eMTLSyntaxElement::Ka>(mtl.Ka.x, mtl.Ka.y, mtl.Ka.z);
+  if (!mtl.tex_map_of_type(eMTLSyntaxElement::map_Kd).is_valid()) {
+    fmt_handler_.write<eMTLSyntaxElement::Kd>(mtl.Kd.x, mtl.Kd.y, mtl.Kd.z);
+  }
+  if (!mtl.tex_map_of_type(eMTLSyntaxElement::map_Ks).is_valid()) {
+    fmt_handler_.write<eMTLSyntaxElement::Ks>(mtl.Ks.x, mtl.Ks.y, mtl.Ks.z);
+  }
+  if (!mtl.tex_map_of_type(eMTLSyntaxElement::map_Ke).is_valid()) {
+    fmt_handler_.write<eMTLSyntaxElement::Ke>(mtl.Ke.x, mtl.Ke.y, mtl.Ke.z);
+  }
+  fmt_handler_.write<eMTLSyntaxElement::Ni>(mtl.Ni);
+  if (!mtl.tex_map_of_type(eMTLSyntaxElement::map_d).is_valid()) {
+    fmt_handler_.write<eMTLSyntaxElement::d>(mtl.d);
+  }
+  fmt_handler_.write<eMTLSyntaxElement::illum>(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, &copy_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, &copy_set); \
-    /* Always emit forward slashes for cross-platform compatibility. */ \
-    std::replace(path.begin(), path.end(), '\\', '/'); \
     fmt_handler_.write<eMTLSyntaxElement>(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<eMTLSyntaxElement::newmtl>(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<io::obj::eMTLSyntaxElement> {
 }  // 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<const eMTLSyntaxElement, tex_map_XX>::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)



More information about the Bf-blender-cvs mailing list