Fix T85155: Vertex groups from object don't transfer to next nodes modifier

Because the the vertex group name-to-index map is stored in the object
rather than object data, the object info node has to replace the
map when it replaces the mesh component on the geometry set with mesh
data from another object.

This normally works fine as a way to use the vertex groups from the
input mesh, but when passing this mesh to the next modifier, the entire
mesh component was replaced, removing the vertex group name map.

This commit adds a function to replace only the mesh data in mesh
component, uses it in the modifier code, and updates the relevant
comments.

Note that the fact that vertex group names are stored in object data
is a legacy design decision that should be reevaluated at some point.

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

3 files changed, 40 insertions, 14 deletions

diff --git a/source/blender/blenkernel/BKE_geometry_set.hh b/source/blender/blenkernel/BKE_geometry_set.hh
index 3b093ae730a..31739465afd 100644
--- a/source/blender/blenkernel/BKE_geometry_set.hh
+++ b/source/blender/blenkernel/BKE_geometry_set.hh
@@ -349,6 +349,8 @@ class MeshComponent : public GeometryComponent {
   void clear();
   bool has_mesh() const;
   void replace(Mesh *mesh, GeometryOwnershipType ownership = GeometryOwnershipType::Owned);
+  void replace_mesh_but_keep_vertex_group_names(
+      Mesh *mesh, GeometryOwnershipType ownership = GeometryOwnershipType::Owned);
   Mesh *release();
 
   void copy_vertex_group_names_from_object(const struct Object &object);
diff --git a/source/blender/blenkernel/intern/DerivedMesh.cc b/source/blender/blenkernel/intern/DerivedMesh.cc
index d79413cfaa0..ecc41e3f0df 100644
--- a/source/blender/blenkernel/intern/DerivedMesh.cc
+++ b/source/blender/blenkernel/intern/DerivedMesh.cc
@@ -883,16 +883,15 @@ void BKE_mesh_wrapper_deferred_finalize(Mesh *me_eval,
 }
 
 /**
- * Modifies the given mesh and geometry set. The geometry set is expect to have NO mesh component.
- * After this function ends, the geometry set will still have NO mesh component. Instead, an input
- * mesh is passed separately and is returned separately.
+ * Modifies the given mesh and geometry set. The mesh is not passed as part of the mesh component
+ * in the \a geometry_set input, it is only passed in \a input_mesh and returned in the return
+ * value.
  *
- * The purpose of the geometry set is to store all non-mesh geometry components that are generated
- * by modifiers.
+ * The purpose of the geometry set is to store all geometry components that are generated
+ * by modifiers to allow outputting non-mesh data from modifiers.
  */
 static Mesh *modifier_modify_mesh_and_geometry_set(ModifierData *md,
                                                    const ModifierEvalContext &mectx,
-                                                   Object *ob,
                                                    Mesh *input_mesh,
                                                    GeometrySet &geometry_set)
 {
@@ -907,10 +906,12 @@ static Mesh *modifier_modify_mesh_and_geometry_set(ModifierData *md,
     BKE_mesh_wrapper_ensure_mdata(input_mesh);
 
     /* Adds a new mesh component to the geometry set based on the #input_mesh. */
-    BLI_assert(!geometry_set.has<MeshComponent>());
     MeshComponent &mesh_component = geometry_set.get_component_for_write<MeshComponent>();
-    mesh_component.replace(input_mesh, GeometryOwnershipType::Editable);
-    mesh_component.copy_vertex_group_names_from_object(*ob);
+    /* Replace only the mesh rather than the whole component, because the entire #MeshComponent
+     * might have been replaced by data from a different object in the node tree, which means the
+     * component contains vertex group name data for that object that should not be removed. */
+    mesh_component.replace_mesh_but_keep_vertex_group_names(input_mesh,
+                                                            GeometryOwnershipType::Editable);
 
     /* Let the modifier change the geometry set. */
     mti->modifyGeometrySet(md, &mectx, &geometry_set);
@@ -919,7 +920,6 @@ static Mesh *modifier_modify_mesh_and_geometry_set(ModifierData *md,
     if (geometry_set.has<MeshComponent>()) {
       MeshComponent &mesh_component = geometry_set.get_component_for_write<MeshComponent>();
       mesh_output = mesh_component.release();
-      geometry_set.remove<MeshComponent>();
     }
 
     /* Return an empty mesh instead of null.  */
@@ -954,6 +954,13 @@ static void mesh_calc_modifiers(struct Depsgraph *depsgraph,
   Mesh *mesh_deform = nullptr;
   /* This geometry set contains the non-mesh data that might be generated by modifiers. */
   GeometrySet geometry_set_final;
+
+  /* Add the initial mesh component, with a copy of the vertex group names from the object,
+   * since they need to be stored in the geometry set for evaluation. */
+  MeshComponent &initial_mesh_component =
+      geometry_set_final.get_component_for_write<MeshComponent>();
+  initial_mesh_component.copy_vertex_group_names_from_object(*ob);
+
   BLI_assert((mesh_input->id.tag & LIB_TAG_COPIED_ON_WRITE_EVAL_RESULT) == 0);
 
   /* Deformed vertex locations array. Deform only modifier need this type of
@@ -1252,7 +1259,7 @@ static void mesh_calc_modifiers(struct Depsgraph *depsgraph,
       }
 
       Mesh *mesh_next = modifier_modify_mesh_and_geometry_set(
-          md, mectx, ob, mesh_final, geometry_set_final);
+          md, mectx, mesh_final, geometry_set_final);
       ASSERT_IS_VALID_MESH(mesh_next);
 
       if (mesh_next) {
@@ -1717,7 +1724,7 @@ static void editbmesh_calc_modifiers(struct Depsgraph *depsgraph,
       }
 
       Mesh *mesh_next = modifier_modify_mesh_and_geometry_set(
-          md, mectx, ob, mesh_final, geometry_set_final);
+          md, mectx, mesh_final, geometry_set_final);
       ASSERT_IS_VALID_MESH(mesh_next);
 
       if (mesh_next) {
@@ -1893,9 +1900,9 @@ static void mesh_build_data(struct Depsgraph *depsgraph,
   BKE_object_eval_assign_data(ob, &mesh_eval->id, is_mesh_eval_owned);
 
   /* Add the final mesh as read-only non-owning component to the geometry set. */
-  BLI_assert(!geometry_set_eval->has<MeshComponent>());
   MeshComponent &mesh_component = geometry_set_eval->get_component_for_write<MeshComponent>();
-  mesh_component.replace(mesh_eval, GeometryOwnershipType::ReadOnly);
+  mesh_component.replace_mesh_but_keep_vertex_group_names(mesh_eval,
+                                                          GeometryOwnershipType::ReadOnly);
   ob->runtime.geometry_set_eval = geometry_set_eval;
 
   ob->runtime.mesh_deform_eval = mesh_deform_eval;
diff --git a/source/blender/blenkernel/intern/geometry_set.cc b/source/blender/blenkernel/intern/geometry_set.cc
index bb315bc0289..a47a3dbc872 100644
--- a/source/blender/blenkernel/intern/geometry_set.cc
+++ b/source/blender/blenkernel/intern/geometry_set.cc
@@ -315,6 +315,23 @@ void MeshComponent::replace(Mesh *mesh, GeometryOwnershipType ownership)
   ownership_ = ownership;
 }
 
+/* This function exists for the same reason as #vertex_group_names_. Non-nodes modifiers need to
+ * be able to replace the mesh data without losing the vertex group names, which may have come
+ * from another object. */
+void MeshComponent::replace_mesh_but_keep_vertex_group_names(Mesh *mesh,
+                                                             GeometryOwnershipType ownership)
+{
+  BLI_assert(this->is_mutable());
+  if (mesh_ != nullptr) {
+    if (ownership_ == GeometryOwnershipType::Owned) {
+      BKE_id_free(nullptr, mesh_);
+    }
+    mesh_ = nullptr;
+  }
+  mesh_ = mesh;
+  ownership_ = ownership;
+}
+
 /* Return the mesh and clear the component. The caller takes over responsibility for freeing the
  * mesh (if the component was responsible before). */
 Mesh *MeshComponent::release()