Fix T96430: new OBJ exporter wrong normals for non-uniform scale, and wrong face order for negative scale

This applies patch D14343 from Aras Pranckevicius, with a description:

The new 3.1+ OBJ exporter did not have correct logic when faced with
non-uniform & mirrored (negative on odd number of axes) object scale:

- Normals were not transformed correctly (should use inverse transpose of the matrix),
 and were not normalized,
- Face order was not "flipped" when transform has negative scale on odd number of axes
 (visible when using "face orientation" viewport overlay).
This commit is contained in:
Aras Pranckevicius 2022-03-19 16:20:22 -04:00 committed by Howard Trickey
parent 873801d25e
commit 8aa365745a
Notes: blender-bot 2023-02-13 16:05:01 +01:00
Referenced by issue #96727, OBJ EXPORTER - Regression
Referenced by issue #96430, OBJ Export: wrong normals with scaled object
Referenced by issue #96241, 3.1: Potential candidates for corrective releases
5 changed files with 114 additions and 27 deletions

View File

@ -43,16 +43,33 @@ void OBJWriter::write_vert_uv_normal_indices(FormatHandler<eFileType::OBJ> &fh,
const IndexOffsets &offsets,
Span<int> vert_indices,
Span<int> uv_indices,
Span<int> normal_indices) const
Span<int> normal_indices,
bool flip) const
{
BLI_assert(vert_indices.size() == uv_indices.size() &&
vert_indices.size() == normal_indices.size());
const int vertex_offset = offsets.vertex_offset + 1;
const int uv_offset = offsets.uv_vertex_offset + 1;
const int normal_offset = offsets.normal_offset + 1;
const int n = vert_indices.size();
fh.write<eOBJSyntaxElement::poly_element_begin>();
for (int j = 0; j < vert_indices.size(); j++) {
fh.write<eOBJSyntaxElement::vertex_uv_normal_indices>(
vert_indices[j] + offsets.vertex_offset + 1,
uv_indices[j] + offsets.uv_vertex_offset + 1,
normal_indices[j] + offsets.normal_offset + 1);
if (!flip) {
for (int j = 0; j < n; ++j) {
fh.write<eOBJSyntaxElement::vertex_uv_normal_indices>(vert_indices[j] + vertex_offset,
uv_indices[j] + uv_offset,
normal_indices[j] + normal_offset);
}
}
else {
/* For a transform that is mirrored (negative scale on odd number of axes),
* we want to flip the face index order. Start from the same index, and
* then go backwards. Same logic in other write_*_indices functions below. */
for (int k = 0; k < n; ++k) {
int j = k == 0 ? 0 : n - k;
fh.write<eOBJSyntaxElement::vertex_uv_normal_indices>(vert_indices[j] + vertex_offset,
uv_indices[j] + uv_offset,
normal_indices[j] + normal_offset);
}
}
fh.write<eOBJSyntaxElement::poly_element_end>();
}
@ -61,14 +78,26 @@ void OBJWriter::write_vert_normal_indices(FormatHandler<eFileType::OBJ> &fh,
const IndexOffsets &offsets,
Span<int> vert_indices,
Span<int> /*uv_indices*/,
Span<int> normal_indices) const
Span<int> normal_indices,
bool flip) const
{
BLI_assert(vert_indices.size() == normal_indices.size());
const int vertex_offset = offsets.vertex_offset + 1;
const int normal_offset = offsets.normal_offset + 1;
const int n = vert_indices.size();
fh.write<eOBJSyntaxElement::poly_element_begin>();
for (int j = 0; j < vert_indices.size(); j++) {
fh.write<eOBJSyntaxElement::vertex_normal_indices>(vert_indices[j] + offsets.vertex_offset + 1,
normal_indices[j] + offsets.normal_offset +
1);
if (!flip) {
for (int j = 0; j < n; ++j) {
fh.write<eOBJSyntaxElement::vertex_normal_indices>(vert_indices[j] + vertex_offset,
normal_indices[j] + normal_offset);
}
}
else {
for (int k = 0; k < n; ++k) {
int j = k == 0 ? 0 : n - k;
fh.write<eOBJSyntaxElement::vertex_normal_indices>(vert_indices[j] + vertex_offset,
normal_indices[j] + normal_offset);
}
}
fh.write<eOBJSyntaxElement::poly_element_end>();
}
@ -77,13 +106,26 @@ void OBJWriter::write_vert_uv_indices(FormatHandler<eFileType::OBJ> &fh,
const IndexOffsets &offsets,
Span<int> vert_indices,
Span<int> uv_indices,
Span<int> /*normal_indices*/) const
Span<int> /*normal_indices*/,
bool flip) const
{
BLI_assert(vert_indices.size() == uv_indices.size());
const int vertex_offset = offsets.vertex_offset + 1;
const int uv_offset = offsets.uv_vertex_offset + 1;
const int n = vert_indices.size();
fh.write<eOBJSyntaxElement::poly_element_begin>();
for (int j = 0; j < vert_indices.size(); j++) {
fh.write<eOBJSyntaxElement::vertex_uv_indices>(vert_indices[j] + offsets.vertex_offset + 1,
uv_indices[j] + offsets.uv_vertex_offset + 1);
if (!flip) {
for (int j = 0; j < n; ++j) {
fh.write<eOBJSyntaxElement::vertex_uv_indices>(vert_indices[j] + vertex_offset,
uv_indices[j] + uv_offset);
}
}
else {
for (int k = 0; k < n; ++k) {
int j = k == 0 ? 0 : n - k;
fh.write<eOBJSyntaxElement::vertex_uv_indices>(vert_indices[j] + vertex_offset,
uv_indices[j] + uv_offset);
}
}
fh.write<eOBJSyntaxElement::poly_element_end>();
}
@ -92,11 +134,22 @@ void OBJWriter::write_vert_indices(FormatHandler<eFileType::OBJ> &fh,
const IndexOffsets &offsets,
Span<int> vert_indices,
Span<int> /*uv_indices*/,
Span<int> /*normal_indices*/) const
Span<int> /*normal_indices*/,
bool flip) const
{
const int vertex_offset = offsets.vertex_offset + 1;
const int n = vert_indices.size();
fh.write<eOBJSyntaxElement::poly_element_begin>();
for (const int vert_index : vert_indices) {
fh.write<eOBJSyntaxElement::vertex_indices>(vert_index + offsets.vertex_offset + 1);
if (!flip) {
for (int j = 0; j < n; ++j) {
fh.write<eOBJSyntaxElement::vertex_indices>(vert_indices[j] + vertex_offset);
}
}
else {
for (int k = 0; k < n; ++k) {
int j = k == 0 ? 0 : n - k;
fh.write<eOBJSyntaxElement::vertex_indices>(vert_indices[j] + vertex_offset);
}
}
fh.write<eOBJSyntaxElement::poly_element_end>();
}
@ -323,8 +376,12 @@ void OBJWriter::write_poly_elements(FormatHandler<eFileType::OBJ> &fh,
}
/* Write polygon elements. */
(this->*poly_element_writer)(
buf, offsets, poly_vertex_indices, poly_uv_indices, poly_normal_indices);
(this->*poly_element_writer)(buf,
offsets,
poly_vertex_indices,
poly_uv_indices,
poly_normal_indices,
obj_mesh_data.is_mirrored_transform());
});
}

View File

@ -115,7 +115,8 @@ class OBJWriter : NonMovable, NonCopyable {
const IndexOffsets &offsets,
Span<int> vert_indices,
Span<int> uv_indices,
Span<int> normal_indices) const;
Span<int> normal_indices,
bool flip) const;
/**
* \return Writer function with appropriate polygon-element syntax.
*/
@ -128,7 +129,8 @@ class OBJWriter : NonMovable, NonCopyable {
const IndexOffsets &offsets,
Span<int> vert_indices,
Span<int> uv_indices,
Span<int> normal_indices) const;
Span<int> normal_indices,
bool flip) const;
/**
* Write one line of polygon indices as "f v1//vn1 v2//vn2 ...".
*/
@ -136,7 +138,8 @@ class OBJWriter : NonMovable, NonCopyable {
const IndexOffsets &offsets,
Span<int> vert_indices,
Span<int> /*uv_indices*/,
Span<int> normal_indices) const;
Span<int> normal_indices,
bool flip) const;
/**
* Write one line of polygon indices as "f v1/vt1 v2/vt2 ...".
*/
@ -144,7 +147,8 @@ class OBJWriter : NonMovable, NonCopyable {
const IndexOffsets &offsets,
Span<int> vert_indices,
Span<int> uv_indices,
Span<int> /*normal_indices*/) const;
Span<int> /*normal_indices*/,
bool flip) const;
/**
* Write one line of polygon indices as "f v1 v2 ...".
*/
@ -152,7 +156,8 @@ class OBJWriter : NonMovable, NonCopyable {
const IndexOffsets &offsets,
Span<int> vert_indices,
Span<int> /*uv_indices*/,
Span<int> /*normal_indices*/) const;
Span<int> /*normal_indices*/,
bool flip) const;
};
/**

View File

@ -123,6 +123,13 @@ void OBJMesh::set_world_axes_transform(const eTransformAxisForward forward,
/* mul_m4_m3m4 does not transform last row of obmat, i.e. location data. */
mul_v3_m3v3(world_and_axes_transform_[3], axes_transform, export_object_eval_.obmat[3]);
world_and_axes_transform_[3][3] = export_object_eval_.obmat[3][3];
/* Normals need inverse transpose of the regular matrix to handle non-uniform scale. */
float normal_matrix[3][3];
copy_m3_m4(normal_matrix, world_and_axes_transform_);
invert_m3_m3(world_and_axes_normal_transform_, normal_matrix);
transpose_m3(world_and_axes_normal_transform_);
mirrored_transform_ = determinant_m3_array(world_and_axes_normal_transform_) < 0;
}
int OBJMesh::tot_vertices() const
@ -319,7 +326,8 @@ float3 OBJMesh::calc_poly_normal(const int poly_index) const
const MLoop &mloop = export_mesh_eval_->mloop[poly.loopstart];
const MVert &mvert = *(export_mesh_eval_->mvert);
BKE_mesh_calc_poly_normal(&poly, &mloop, &mvert, r_poly_normal);
mul_mat3_m4_v3(world_and_axes_transform_, r_poly_normal);
mul_m3_v3(world_and_axes_normal_transform_, r_poly_normal);
normalize_v3(r_poly_normal);
return r_poly_normal;
}
@ -364,7 +372,8 @@ void OBJMesh::store_normal_coords_and_indices()
int loop_index = mpoly.loopstart + loop_of_poly;
BLI_assert(loop_index < export_mesh_eval_->totloop);
copy_v3_v3(loop_normal, lnors[loop_index]);
mul_mat3_m4_v3(world_and_axes_transform_, loop_normal);
mul_m3_v3(world_and_axes_normal_transform_, loop_normal);
normalize_v3(loop_normal);
float3 rounded_loop_normal = round_float3_to_n_digits(loop_normal, round_digits);
int loop_norm_index = normal_to_index.lookup_default(rounded_loop_normal, -1);
if (loop_norm_index == -1) {

View File

@ -59,6 +59,8 @@ class OBJMesh : NonCopyable {
* object's world transform matrix.
*/
float world_and_axes_transform_[4][4];
float world_and_axes_normal_transform_[3][3];
bool mirrored_transform_;
/**
* Total UV vertices in a mesh's texture map.
@ -110,6 +112,10 @@ class OBJMesh : NonCopyable {
int tot_uv_vertices() const;
int tot_normal_indices() const;
int tot_edges() const;
bool is_mirrored_transform() const
{
return mirrored_transform_;
}
/**
* \return Total materials in the object.

View File

@ -405,6 +405,16 @@ TEST_F(obj_exporter_regression_test, vertices)
"io_tests/blend_geometry/vertices.blend", "io_tests/obj/vertices.obj", "", _export.params);
}
TEST_F(obj_exporter_regression_test, non_uniform_scale)
{
OBJExportParamsDefault _export;
_export.params.export_materials = false;
compare_obj_export_to_golden("io_tests/blend_geometry/non_uniform_scale.blend",
"io_tests/obj/non_uniform_scale.obj",
"",
_export.params);
}
TEST_F(obj_exporter_regression_test, nurbs_as_nurbs)
{
OBJExportParamsDefault _export;