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:
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
|
@ -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; \
|
||||
}
|
||||
|
||||
|
|
|
@ -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...>};
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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;
|
||||
|
|
Loading…
Reference in New Issue