Fix T98874: new obj importer missing an option to import vertex groups

The old Python OBJ importer had a (somewhat confusingly named) "Keep
Vertex Order -> Poly Groups" option, that imported OBJ groups as
"vertex groups" on the resulting mesh. All vertices of any face were
assigned the vertex group, with a 1.0 weight.

The new C++ importer did not have this option. It was trying to do
something with vertex groups, but failing to actually achieve
anything :) -- the vertex groups were created on the wrong object
(later on overwritten by "nomain mesh to main mesh" operation);
vertex weights were set to 1.0/vertex_count, and each vertex was only
set to be in one group, even when it belongs to multiple faces from
different groups. End result was that to the user, vertex groups were
not visible/present at all (see T98874).

This patch adds the import option (named "Vertex Groups"), which is
off by default, and fixes the import code logic to actually do the
right thing. Tested on file from T98874; vertex groups are imported
just like with the Python importer.

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

7 files changed, 31 insertions, 28 deletions

diff --git a/source/blender/editors/io/io_obj.c b/source/blender/editors/io/io_obj.c
index 4819ae09785..53fa4788d0e 100644
--- a/source/blender/editors/io/io_obj.c
+++ b/source/blender/editors/io/io_obj.c
@@ -392,6 +392,7 @@ static int wm_obj_import_exec(bContext *C, wmOperator *op)
   import_params.clamp_size = RNA_float_get(op->ptr, "clamp_size");
   import_params.forward_axis = RNA_enum_get(op->ptr, "forward_axis");
   import_params.up_axis = RNA_enum_get(op->ptr, "up_axis");
+  import_params.import_vertex_groups = RNA_boolean_get(op->ptr, "import_vertex_groups");
   import_params.validate_meshes = RNA_boolean_get(op->ptr, "validate_meshes");
 
   OBJ_import(C, &import_params);
@@ -422,6 +423,7 @@ static void ui_obj_import_settings(uiLayout *layout, PointerRNA *imfptr)
   box = uiLayoutBox(layout);
   uiItemL(box, IFACE_("Options"), ICON_EXPORT);
   col = uiLayoutColumn(box, false);
+  uiItemR(col, imfptr, "import_vertex_groups", 0, NULL, ICON_NONE);
   uiItemR(col, imfptr, "validate_meshes", 0, NULL, ICON_NONE);
 }
 
@@ -468,6 +470,11 @@ void WM_OT_obj_import(struct wmOperatorType *ot)
       ot->srna, "forward_axis", io_transform_axis, IO_AXIS_NEGATIVE_Z, "Forward Axis", "");
   RNA_def_enum(ot->srna, "up_axis", io_transform_axis, IO_AXIS_Y, "Up Axis", "");
   RNA_def_boolean(ot->srna,
+                  "import_vertex_groups",
+                  false,
+                  "Vertex Groups",
+                  "Import OBJ groups as vertex groups");
+  RNA_def_boolean(ot->srna,
                   "validate_meshes",
                   false,
                   "Validate Meshes",
diff --git a/source/blender/io/wavefront_obj/IO_wavefront_obj.h b/source/blender/io/wavefront_obj/IO_wavefront_obj.h
index a719dff2126..b4a00deb99c 100644
--- a/source/blender/io/wavefront_obj/IO_wavefront_obj.h
+++ b/source/blender/io/wavefront_obj/IO_wavefront_obj.h
@@ -72,6 +72,7 @@ struct OBJImportParams {
   float clamp_size;
   eIOAxis forward_axis;
   eIOAxis up_axis;
+  bool import_vertex_groups;
   bool validate_meshes;
 };
 
diff --git a/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc b/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
index 13e28be5668..3cc17e7d8e6 100644
--- a/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
+++ b/source/blender/io/wavefront_obj/importer/obj_import_file_reader.cc
@@ -175,7 +175,7 @@ static void geom_add_polygon(Geometry *geom,
   curr_face.material_index = material_index;
   if (group_index >= 0) {
     curr_face.vertex_group_index = group_index;
-    geom->use_vertex_groups_ = true;
+    geom->has_vertex_groups_ = true;
   }
 
   const int orig_corners_size = geom->face_corners_.size();
diff --git a/source/blender/io/wavefront_obj/importer/obj_import_mesh.cc b/source/blender/io/wavefront_obj/importer/obj_import_mesh.cc
index acc35ad46e1..51318331c76 100644
--- a/source/blender/io/wavefront_obj/importer/obj_import_mesh.cc
+++ b/source/blender/io/wavefront_obj/importer/obj_import_mesh.cc
@@ -10,6 +10,7 @@
 
 #include "BKE_attribute.h"
 #include "BKE_customdata.h"
+#include "BKE_deform.h"
 #include "BKE_material.h"
 #include "BKE_mesh.h"
 #include "BKE_node_tree_update.h"
@@ -47,7 +48,7 @@ Object *MeshFromGeometry::create_mesh(Main *bmain,
   obj->data = BKE_object_obdata_add_from_type(bmain, OB_MESH, ob_name.c_str());
 
   create_vertices(mesh);
-  create_polys_loops(obj, mesh);
+  create_polys_loops(obj, mesh, import_params.import_vertex_groups);
   create_edges(mesh);
   create_uv_verts(mesh);
   create_normals(mesh);
@@ -69,6 +70,9 @@ Object *MeshFromGeometry::create_mesh(Main *bmain,
   BKE_mesh_nomain_to_mesh(mesh, dst, obj, &CD_MASK_EVERYTHING, true);
   dst->flag |= autosmooth;
 
+  /* Note: vertex groups have to be created after final mesh is assigned to the object. */
+  create_vertex_groups(obj);
+
   return obj;
 }
 
@@ -163,19 +167,13 @@ void MeshFromGeometry::create_vertices(Mesh *mesh)
   }
 }
 
-void MeshFromGeometry::create_polys_loops(Object *obj, Mesh *mesh)
+void MeshFromGeometry::create_polys_loops(Object *obj, Mesh *mesh, bool use_vertex_groups)
 {
-  /* Will not be used if vertex groups are not imported. */
   mesh->dvert = nullptr;
-  float weight = 0.0f;
   const int64_t total_verts = mesh_geometry_.vertex_count_;
-  if (total_verts && mesh_geometry_.use_vertex_groups_) {
+  if (use_vertex_groups && total_verts && mesh_geometry_.has_vertex_groups_) {
     mesh->dvert = static_cast<MDeformVert *>(
         CustomData_add_layer(&mesh->vdata, CD_MDEFORMVERT, CD_CALLOC, nullptr, total_verts));
-    weight = 1.0f / total_verts;
-  }
-  else {
-    UNUSED_VARS(weight);
   }
 
   const int64_t tot_face_elems{mesh->totpoly};
@@ -208,28 +206,23 @@ void MeshFromGeometry::create_polys_loops(Object *obj, Mesh *mesh)
       tot_loop_idx++;
       mloop.v = curr_corner.vert_index;
 
+      /* Setup vertex group data, if needed. */
       if (!mesh->dvert) {
         continue;
       }
-      /* Iterating over mloop results in finding the same vertex multiple times.
-       * Another way is to allocate memory for dvert while creating vertices and fill them here.
-       */
-      MDeformVert &def_vert = mesh->dvert[mloop.v];
-      if (!def_vert.dw) {
-        def_vert.dw = static_cast<MDeformWeight *>(
-            MEM_callocN(sizeof(MDeformWeight), "OBJ Import Deform Weight"));
-      }
-      /* Every vertex in a face is assigned the same deform group. */
-      int group_idx = curr_face.vertex_group_index;
-      /* Deform group number (def_nr) must behave like an index into the names' list. */
-      *(def_vert.dw) = {static_cast<unsigned int>(group_idx), weight};
+      const int group_index = curr_face.vertex_group_index;
+      MDeformWeight *dw = BKE_defvert_ensure_index(mesh->dvert + mloop.v, group_index);
+      dw->weight = 1.0f;
     }
   }
+}
 
-  if (!mesh->dvert) {
+void MeshFromGeometry::create_vertex_groups(Object *obj)
+{
+  Mesh *mesh = static_cast<Mesh *>(obj->data);
+  if (mesh->dvert == nullptr) {
     return;
   }
-  /* Add deform group names. */
   for (const std::string &name : mesh_geometry_.group_order_) {
     BKE_object_defgroup_add_name(obj, name.data());
   }
diff --git a/source/blender/io/wavefront_obj/importer/obj_import_mesh.hh b/source/blender/io/wavefront_obj/importer/obj_import_mesh.hh
index 216717f3578..c6773cfa0cb 100644
--- a/source/blender/io/wavefront_obj/importer/obj_import_mesh.hh
+++ b/source/blender/io/wavefront_obj/importer/obj_import_mesh.hh
@@ -45,10 +45,9 @@ class MeshFromGeometry : NonMovable, NonCopyable {
   void fixup_invalid_faces();
   void create_vertices(Mesh *mesh);
   /**
-   * Create polygons for the Mesh, set smooth shading flags, deform group names,
-   * Materials.
+   * Create polygons for the Mesh, set smooth shading flags, Materials.
    */
-  void create_polys_loops(Object *obj, Mesh *mesh);
+  void create_polys_loops(Object *obj, Mesh *mesh, bool use_vertex_groups);
   /**
    * Add explicitly imported OBJ edges to the mesh.
    */
@@ -66,6 +65,7 @@ class MeshFromGeometry : NonMovable, NonCopyable {
                         Object *obj);
   void create_normals(Mesh *mesh);
   void create_colors(Mesh *mesh);
+  void create_vertex_groups(Object *obj);
 };
 
 }  // namespace blender::io::obj
diff --git a/source/blender/io/wavefront_obj/importer/obj_import_objects.hh b/source/blender/io/wavefront_obj/importer/obj_import_objects.hh
index 69babc26bb0..3d6733d661e 100644
--- a/source/blender/io/wavefront_obj/importer/obj_import_objects.hh
+++ b/source/blender/io/wavefront_obj/importer/obj_import_objects.hh
@@ -113,7 +113,7 @@ struct Geometry {
 
   bool has_invalid_polys_ = false;
   bool has_vertex_normals_ = false;
-  bool use_vertex_groups_ = false;
+  bool has_vertex_groups_ = false;
   NurbsElement nurbs_element_;
   int total_loops_ = 0;
 };
diff --git a/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc b/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc
index b67adbc9753..eeb81f5e23e 100644
--- a/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc
+++ b/source/blender/io/wavefront_obj/tests/obj_importer_tests.cc
@@ -58,6 +58,8 @@ class obj_importer_test : public BlendfileLoadingBaseTest {
     params.clamp_size = 0;
     params.forward_axis = IO_AXIS_NEGATIVE_Z;
     params.up_axis = IO_AXIS_Y;
+    params.validate_meshes = true;
+    params.import_vertex_groups = false;
 
     std::string obj_path = blender::tests::flags_test_asset_dir() + "/io_tests/obj/" + path;
     strncpy(params.filepath, obj_path.c_str(), FILE_MAX - 1);