Fix T96824: New 3.1 OBJ exporter writes incorrect polygon/vertex groups in some cases

The new 3.1 OBJ exporter code had incorrect code to determine which vertex group a polygon belongs to -- for each vertex, it was only looking at the first vertex group it has, and not using the group weight either.

This 99% fixes T96824, but not 100% on the user's submitted mesh -- exactly two faces from that mesh get assigned a different group compared to the old exporter. Either choice is "correct" given that on these two faces there are two vertex groups with equal contribution. The old Python exporter was picking the group based on internal python group name map order, whereas the new C++ exporter is picking the group with the lowest index, in case of ties. I'm not sure if it's possible to fix this TBH, will have to wait until the importer is also C++.

While at it, the new vertex group calculation code was doing a lot of redundant work for each and every face (traversing group lists several times, allocating & freeing memory), so I fixed that. Exporting a 6-level subdivided Monkey mesh with 30 vertex groups was taking 810ms, now takes 330ms.

Reviewed By: Howard Trickey
Differential Revision: https://developer.blender.org/D14500

source/blender/io/wavefront_obj/exporter/obj_export_file_writer.cc

@@ -9,6 +9,7 @@
 
 #include "BKE_blender_version.h"
 
+#include "BLI_enumerable_thread_specific.hh"
 #include "BLI_path_util.h"
 #include "BLI_task.hh"
 
@@ -308,6 +309,9 @@ void OBJWriter::write_poly_elements(FormatHandler<eFileType::OBJ> &fh,
       obj_mesh_data.tot_uv_vertices());
 
   const int tot_polygons = obj_mesh_data.tot_polygons();
+  const int tot_deform_groups = obj_mesh_data.tot_deform_groups();
+  threading::EnumerableThreadSpecific<Vector<float>> group_weights;
+
   obj_parallel_chunked_output(fh, tot_polygons, [&](FormatHandler<eFileType::OBJ> &buf, int idx) {
     /* Polygon order for writing into the file is not necessarily the same
      * as order in the mesh; it will be sorted by material indices. Remap current
@@ -330,9 +334,12 @@ void OBJWriter::write_poly_elements(FormatHandler<eFileType::OBJ> &fh,
 
     /* Write vertex group if different from previous. */
     if (export_params_.export_vertex_groups) {
+      Vector<float> &local_weights = group_weights.local();
+      local_weights.resize(tot_deform_groups);
       const int16_t prev_group = idx == 0 ? NEGATIVE_INIT :
-                                            obj_mesh_data.get_poly_deform_group_index(prev_i);
-      const int16_t group = obj_mesh_data.get_poly_deform_group_index(i);
+                                            obj_mesh_data.get_poly_deform_group_index(
+                                                prev_i, local_weights);
+      const int16_t group = obj_mesh_data.get_poly_deform_group_index(i, local_weights);
       if (group != prev_group) {
         buf.write<eOBJSyntaxElement::object_group>(
             group == NOT_FOUND ? DEFORM_GROUP_DISABLED :

source/blender/io/wavefront_obj/exporter/obj_export_mesh.cc

@@ -426,6 +426,9 @@ void OBJMesh::store_normal_coords_and_indices()
 
 Vector<int> OBJMesh::calc_poly_normal_indices(const int poly_index) const
 {
+  if (loop_to_normal_index_.is_empty()) {
+    return {};
+  }
   const MPoly &mpoly = export_mesh_eval_->mpoly[poly_index];
   const int totloop = mpoly.totloop;
   Vector<int> r_poly_normal_indices(totloop);
@@ -436,46 +439,47 @@ Vector<int> OBJMesh::calc_poly_normal_indices(const int poly_index) const
   return r_poly_normal_indices;
 }
 
-int16_t OBJMesh::get_poly_deform_group_index(const int poly_index) const
+int OBJMesh::tot_deform_groups() const
+{
+  if (!BKE_object_supports_vertex_groups(&export_object_eval_)) {
+    return 0;
+  }
+  return BKE_object_defgroup_count(&export_object_eval_);
+}
+
+int16_t OBJMesh::get_poly_deform_group_index(const int poly_index,
+                                             MutableSpan<float> group_weights) const
 {
   BLI_assert(poly_index < export_mesh_eval_->totpoly);
-  const MPoly &mpoly = export_mesh_eval_->mpoly[poly_index];
-  const MLoop *mloop = &export_mesh_eval_->mloop[mpoly.loopstart];
-  const Object *obj = &export_object_eval_;
-  const int tot_deform_groups = BKE_object_defgroup_count(obj);
-  /* Indices of the vector index into deform groups of an object; values are the]
-   * number of vertex members in one deform group. */
-  Vector<int16_t> deform_group_members(tot_deform_groups, 0);
-  /* Whether at least one vertex in the polygon belongs to any group. */
-  bool found_group = false;
+  BLI_assert(group_weights.size() == BKE_object_defgroup_count(&export_object_eval_));
 
-  const MDeformVert *dvert_orig = static_cast<MDeformVert *>(
+  const MDeformVert *dvert_layer = static_cast<MDeformVert *>(
       CustomData_get_layer(&export_mesh_eval_->vdata, CD_MDEFORMVERT));
-  if (!dvert_orig) {
+  if (!dvert_layer) {
     return NOT_FOUND;
   }
 
-  const MDeformWeight *curr_weight = nullptr;
-  const MDeformVert *dvert = nullptr;
-  for (int loop_index = 0; loop_index < mpoly.totloop; loop_index++) {
-    dvert = &dvert_orig[(mloop + loop_index)->v];
-    curr_weight = dvert->dw;
-    if (curr_weight) {
-      bDeformGroup *vertex_group = static_cast<bDeformGroup *>(
-          BLI_findlink(BKE_object_defgroup_list(obj), curr_weight->def_nr));
-      if (vertex_group) {
-        deform_group_members[curr_weight->def_nr] += 1;
-        found_group = true;
+  group_weights.fill(0);
+  bool found_any_group = false;
+  const MPoly &mpoly = export_mesh_eval_->mpoly[poly_index];
+  const MLoop *mloop = &export_mesh_eval_->mloop[mpoly.loopstart];
+  for (int loop_i = 0; loop_i < mpoly.totloop; ++loop_i, ++mloop) {
+    const MDeformVert &dvert = dvert_layer[mloop->v];
+    for (int weight_i = 0; weight_i < dvert.totweight; ++weight_i) {
+      const auto group = dvert.dw[weight_i].def_nr;
+      if (group < group_weights.size()) {
+        group_weights[group] += dvert.dw[weight_i].weight;
+        found_any_group = true;
       }
     }
   }
 
-  if (!found_group) {
+  if (!found_any_group) {
     return NOT_FOUND;
   }
   /* Index of the group with maximum vertices. */
-  int16_t max_idx = std::max_element(deform_group_members.begin(), deform_group_members.end()) -
-                    deform_group_members.begin();
+  int16_t max_idx = std::max_element(group_weights.begin(), group_weights.end()) -
+                    group_weights.begin();
   return max_idx;
 }
 

source/blender/io/wavefront_obj/exporter/obj_export_mesh.hh

@@ -116,6 +116,7 @@ class OBJMesh : NonCopyable {
   int tot_uv_vertices() const;
   int tot_normal_indices() const;
   int tot_edges() const;
+  int tot_deform_groups() const;
   bool is_mirrored_transform() const
   {
     return mirrored_transform_;
@@ -204,13 +205,15 @@ class OBJMesh : NonCopyable {
    */
   Vector<int> calc_poly_normal_indices(int poly_index) const;
   /**
-   * Find the index of the vertex group with the maximum number of vertices in a polygon.
-   * The index indices into the #Object.defbase.
+   * Find the most representative vertex group of a polygon.
    *
-   * If two or more groups have the same number of vertices (maximum), group name depends on the
-   * implementation of #std::max_element.
+   * This adds up vertex group weights, and the group with the largest
+   * weight sum across the polygon is the one returned.
+   *
+   * group_weights is temporary storage to avoid reallocations, it must
+   * be the size of amount of vertex groups in the object.
    */
-  int16_t get_poly_deform_group_index(int poly_index) const;
+  int16_t get_poly_deform_group_index(int poly_index, MutableSpan<float> group_weights) const;
   /**
    * Find the name of the vertex deform group at the given index.
    * The index indices into the #Object.defbase.

source/blender/io/wavefront_obj/tests/obj_exporter_tests.cc

@@ -467,6 +467,19 @@ TEST_F(obj_exporter_regression_test, cube_normal_edit)
                                _export.params);
 }
 
+TEST_F(obj_exporter_regression_test, cube_vertex_groups)
+{
+  OBJExportParamsDefault _export;
+  _export.params.export_materials = false;
+  _export.params.export_normals = false;
+  _export.params.export_uv = false;
+  _export.params.export_vertex_groups = true;
+  compare_obj_export_to_golden("io_tests/blend_geometry/cube_vertex_groups.blend",
+                               "io_tests/obj/cube_vertex_groups.obj",
+                               "",
+                               _export.params);
+}
+
 TEST_F(obj_exporter_regression_test, cubes_positioned)
 {
   OBJExportParamsDefault _export;