Page MenuHome

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

Authored by Hans Goudey (HooglyBoogly) on Jan 30 2021, 2:35 AM.

Details

Summary

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

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 is replaced, and the vertex group name map is lost.

Normally I'm not sure I would go to this length to make up for very
weird code architecture, but the changes in DerivedMesh.c actually
seem pretty reasonable to me. Rather than assuming the vertex groups
are for the original object and replacing the map for every modifier,
we only replace the mesh data in the mesh component.

On the other hand, I'm not so happy with the idea of exposing a
replace_mesh function just for the geometry component. Maybe that
behavior could be exposed a bit differently.

I looked into moving the vertex group name list to object data, but I
think I should only have one large refactor patch in review at a time.

Diff Detail

Repository
rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Jan 30 2021, 2:35 AM
Hans Goudey (HooglyBoogly) created this revision.

Note that this would only fix the issue for nodes modifiers. Any other modifier that wanted to use the vertex groups from the other object wouldn't be able to do it.
That's expected though, and to me is a fine compromise.

Separately though, I think a valid response to this patch would be "It's not worth it".
If we thought a bit about a plan for moving vertex groups to mesh/lattice/gpencil data and decided to do that sometime in the future, I would be happy too.

  • Improve comment wording

This breaks the contract described in the comment above of modifier_modify_mesh_and_geometry_set. So the comment would have to be updated.

source/blender/blenkernel/BKE_geometry_set.hh
352

Maybe we should be more specific with the naming? replace_mesh_but_keep_vertex_group_names?

source/blender/blenkernel/intern/DerivedMesh.cc
894–895

Unused parameter.

Hans Goudey (HooglyBoogly) marked 2 inline comments as done.Feb 2 2021, 5:09 AM
Hans Goudey (HooglyBoogly) added inline comments.
source/blender/blenkernel/BKE_geometry_set.hh
352

Good idea. Best to be honest that this function exists for a very specific reason.

Hans Goudey (HooglyBoogly) marked an inline comment as done.Feb 2 2021, 5:09 AM
  • Fix incorrect comment
  • Rename function, add comment
Jacques Lucke (JacquesLucke) added inline comments.
source/blender/blenkernel/intern/DerivedMesh.cc
912

constains

This revision is now accepted and ready to land.Feb 2 2021, 2:38 PM