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
This commit is contained in:
Aras Pranckevicius 2022-03-31 13:38:59 +03:00
parent b073f58a8e
commit d120a083da
Notes: blender-bot 2024-05-08 11:36:44 +02:00
Referenced by issue #96763, Regression: New OBJ Exporter Incorrectly saving the materials in the MTL (materials) file, extra space before texture files
4 changed files with 35 additions and 24 deletions

View File

@ -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; \
}

View File

@ -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...>};

View File

@ -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);

View File

@ -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;