Page MenuHome

Fix T100863, T100875: Vertex group reading broken for recently saved files
ClosedPublic

Authored by Hans Goudey (HooglyBoogly) on Sep 7 2022, 5:20 PM.

Details

Summary

This patch consists of two related fixes. The first is a simple fix for forward compatibility,
setting the Mesh.dvert pointer when writing a file allows old Blender versions to read
vertex groups from newly saved files.

The second part is a bit uglier and more complex. Normally mesh vertex group data is read
in mesh_blend_read_data, for backward compatibility with very old files. However, after
05952aa94d33eeb50 the mesh.dvert pointer was not set, so the data was not read.
Reading vertex group layers when reading custom data is enough to fix that issue.
We need to read the data from *both* places, but BKE_defvert_blend_read cannot
run twice without memory leaks, so first try reading from custom data, then read the
pointer if that fails.

Diff Detail

Repository
rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Sep 7 2022, 5:20 PM
Hans Goudey (HooglyBoogly) created this revision.
Bastien Montagne (mont29) requested changes to this revision.Sep 7 2022, 6:08 PM

Write part is fine afaict.

For read part, I would rather do this the other way around:

  • First read customdata layers, including potential CD_MDEFORMVERT one.
  • Try to get dvert pointer from cdlayers, and only if null, try to read from mesh->dvert

That way no need for special weird option in customdata reading code.

This revision now requires changes to proceed.Sep 7 2022, 6:08 PM

Try to get dvert pointer from cdlayers, and only if null, try to read from mesh->dvert

Great idea!

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)

Simpler logic by switching the order

LGTM, also checked it fixes T100875

source/blender/blenkernel/intern/mesh.cc
328–329

missing * on start of second line ;)

This revision is now accepted and ready to land.Sep 7 2022, 9:19 PM
source/blender/blenkernel/intern/mesh.cc
327–331

Wait actually, aren't you supposed to create an CD_MDEFORMVERT layer to take ownership of that data in that case???

Also I believe the commit message (aka description of this patch) needs to be adjusted?

Bastien Montagne (mont29) requested changes to this revision.Sep 7 2022, 9:29 PM
This revision now requires changes to proceed.Sep 7 2022, 9:29 PM
Bastien Montagne (mont29) added inline comments.
source/blender/blenkernel/intern/mesh.cc
327–331

Scratch that. it's late here... this is done since 2.42 versioning code!

This revision is now accepted and ready to land.Sep 7 2022, 9:32 PM
Hans Goudey (HooglyBoogly) marked an inline comment as done.Sep 7 2022, 9:33 PM