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

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).

Cherry-picked from 8aa365745a78, with minor conflict fixes.

Reviewed By: Howard Trickey

Differential Revision: https://developer.blender.org/D14343

5 files changed, 114 insertions, 28 deletions

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 38163af64b6..4eea2e51521 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
@@ -54,56 +54,109 @@ const char *MATERIAL_GROUP_DISABLED = "";
 
 void OBJWriter::write_vert_uv_normal_indices(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 = index_offsets_.vertex_offset + 1;
+  const int uv_offset = index_offsets_.uv_vertex_offset + 1;
+  const int normal_offset = index_offsets_.normal_offset + 1;
+  const int n = vert_indices.size();
   file_handler_->write<eOBJSyntaxElement::poly_element_begin>();
-  for (int j = 0; j < vert_indices.size(); j++) {
-    file_handler_->write<eOBJSyntaxElement::vertex_uv_normal_indices>(
-        vert_indices[j] + index_offsets_.vertex_offset + 1,
-        uv_indices[j] + index_offsets_.uv_vertex_offset + 1,
-        normal_indices[j] + index_offsets_.normal_offset + 1);
+  if (!flip) {
+    for (int j = 0; j < n; ++j) {
+      file_handler_->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;
+      file_handler_->write<eOBJSyntaxElement::vertex_uv_normal_indices>(
+          vert_indices[j] + vertex_offset,
+          uv_indices[j] + uv_offset,
+          normal_indices[j] + normal_offset);
+    }
   }
   file_handler_->write<eOBJSyntaxElement::poly_element_end>();
 }
 
 void OBJWriter::write_vert_normal_indices(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 = index_offsets_.vertex_offset + 1;
+  const int normal_offset = index_offsets_.normal_offset + 1;
+  const int n = vert_indices.size();
   file_handler_->write<eOBJSyntaxElement::poly_element_begin>();
-  for (int j = 0; j < vert_indices.size(); j++) {
-    file_handler_->write<eOBJSyntaxElement::vertex_normal_indices>(
-        vert_indices[j] + index_offsets_.vertex_offset + 1,
-        normal_indices[j] + index_offsets_.normal_offset + 1);
+  if (!flip) {
+    for (int j = 0; j < n; ++j) {
+      file_handler_->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;
+      file_handler_->write<eOBJSyntaxElement::vertex_normal_indices>(
+          vert_indices[j] + vertex_offset, normal_indices[j] + normal_offset);
+    }
   }
   file_handler_->write<eOBJSyntaxElement::poly_element_end>();
 }
 
 void OBJWriter::write_vert_uv_indices(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 = index_offsets_.vertex_offset + 1;
+  const int uv_offset = index_offsets_.uv_vertex_offset + 1;
+  const int n = vert_indices.size();
   file_handler_->write<eOBJSyntaxElement::poly_element_begin>();
-  for (int j = 0; j < vert_indices.size(); j++) {
-    file_handler_->write<eOBJSyntaxElement::vertex_uv_indices>(
-        vert_indices[j] + index_offsets_.vertex_offset + 1,
-        uv_indices[j] + index_offsets_.uv_vertex_offset + 1);
+  if (!flip) {
+    for (int j = 0; j < n; ++j) {
+      file_handler_->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;
+      file_handler_->write<eOBJSyntaxElement::vertex_uv_indices>(vert_indices[j] + vertex_offset,
+                                                                 uv_indices[j] + uv_offset);
+    }
   }
   file_handler_->write<eOBJSyntaxElement::poly_element_end>();
 }
 
 void OBJWriter::write_vert_indices(Span<int> vert_indices,
                                    Span<int> /*uv_indices*/,
-                                   Span<int> /*normal_indices*/) const
+                                   Span<int> /*normal_indices*/,
+                                   bool flip) const
 {
+  const int vertex_offset = index_offsets_.vertex_offset + 1;
+  const int n = vert_indices.size();
   file_handler_->write<eOBJSyntaxElement::poly_element_begin>();
-  for (const int vert_index : vert_indices) {
-    file_handler_->write<eOBJSyntaxElement::vertex_indices>(vert_index +
-                                                            index_offsets_.vertex_offset + 1);
+  if (!flip) {
+    for (int j = 0; j < n; ++j) {
+      file_handler_->write<eOBJSyntaxElement::vertex_indices>(vert_indices[j] + vertex_offset);
+    }
+  }
+  else {
+    for (int k = 0; k < n; ++k) {
+      int j = k == 0 ? 0 : n - k;
+      file_handler_->write<eOBJSyntaxElement::vertex_indices>(vert_indices[j] + vertex_offset);
+    }
   }
   file_handler_->write<eOBJSyntaxElement::poly_element_end>();
 }
@@ -297,7 +350,10 @@ void OBJWriter::write_poly_elements(const OBJMesh &obj_mesh_data,
     last_poly_smooth_group = write_smooth_group(obj_mesh_data, i, last_poly_smooth_group);
     last_poly_vertex_group = write_vertex_group(obj_mesh_data, i, last_poly_vertex_group);
     last_poly_mat_nr = write_poly_material(obj_mesh_data, i, last_poly_mat_nr, matname_fn);
-    (this->*poly_element_writer)(poly_vertex_indices, poly_uv_indices, poly_normal_indices);
+    (this->*poly_element_writer)(poly_vertex_indices,
+                                 poly_uv_indices,
+                                 poly_normal_indices,
+                                 obj_mesh_data.is_mirrored_transform());
   }
 }
 
diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_file_writer.hh b/source/blender/io/wavefront_obj/exporter/obj_export_file_writer.hh
index 7385d9fabe2..1bbdb7a848c 100644
--- a/source/blender/io/wavefront_obj/exporter/obj_export_file_writer.hh
+++ b/source/blender/io/wavefront_obj/exporter/obj_export_file_writer.hh
@@ -135,7 +135,8 @@ class OBJWriter : NonMovable, NonCopyable {
  private:
   using func_vert_uv_normal_indices = void (OBJWriter::*)(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.
    */
@@ -146,25 +147,29 @@ class OBJWriter : NonMovable, NonCopyable {
    */
   void write_vert_uv_normal_indices(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 ...".
    */
   void write_vert_normal_indices(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 ...".
    */
   void write_vert_uv_indices(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 ...".
    */
   void write_vert_indices(Span<int> vert_indices,
                           Span<int> /*uv_indices*/,
-                          Span<int> /*normal_indices*/) const;
+                          Span<int> /*normal_indices*/,
+                          bool flip) const;
 };
 
 /**
diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc b/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc
index 66f10009875..c1b9b0a8ccf 100644
--- a/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc
+++ b/source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc
@@ -125,6 +125,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
@@ -322,7 +329,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;
 }
 
@@ -367,7 +375,8 @@ void OBJMesh::store_normal_coords_and_indices(Vector<float3> &r_normal_coords)
         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) {
diff --git a/source/blender/io/wavefront_obj/exporter/obj_export_mesh.hh b/source/blender/io/wavefront_obj/exporter/obj_export_mesh.hh
index f3ace140006..3ec670d2203 100644
--- a/source/blender/io/wavefront_obj/exporter/obj_export_mesh.hh
+++ b/source/blender/io/wavefront_obj/exporter/obj_export_mesh.hh
@@ -73,6 +73,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.
@@ -113,6 +115,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.
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 48340b9eea8..7e60492bac2 100644
--- a/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc
+++ b/source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc
@@ -373,6 +373,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;