Fix: Mesh SoA format conversion skips versioning
ClosedPublic
Actions

Authored by Hans Goudey (HooglyBoogly) on Sep 8 2022, 8:43 PM.

Details

Reviewers

Campbell Barton (campbellbarton)
Sergey Sharybin (sergey)
Jacques Lucke (JacquesLucke)

Commits

rB818c9e524d82: Fix: Mesh SoA format conversion skips versioning

Summary

Converting to the SoA format (T95965) immediately when reading meshes means that none
of the changes from versioning would be applied first. This means important fixes like
rBf14995aba70a aren't properly applied, so modifications could be done to invalid CustomData.
To fix this, we move the SoA changes into versioning code, in a new versioning_400.cc file.

Another option is moving all CustomDataversioning changes to readfile code, but that
doesn't scale well and mixes the purpose of the two areas.

I noticed this problem when working on D15795.
To reproduce, check out refactor-mesh-selection-generic and run ctest -R physics_cloth.

Diff Detail

Repository: rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Sep 8 2022, 8:43 PM

Hans Goudey (HooglyBoogly) created this revision.

Hans Goudey (HooglyBoogly) added a child revision: D15795: Mesh: Move selection flags to generic attributes.Sep 11 2022, 12:17 AM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Sep 12 2022, 4:21 AM

Jacques Lucke (JacquesLucke) added inline comments.Sep 12 2022, 9:33 AM

source/blender/blenloader/intern/versioning_300.c
3375 ↗	(On Diff #55454)	If we allow this kind of versioning code that always runs, it should definitely be moved to a separate function. Also I'm not convinced it is correct to do this in `versioning_300.c`, because when the version number increases to 4.0, this code will suddenly run between other versioning which is not what we want. I know this should become permanent in 4.0 but maybe not right after the version bump, and it would have to be moved to `versioning_400.c` anyway. Currently I think this kind of code should be moved to a separate function called from `do_versions` after `blo_do_versions_300`.

Move versioning to versioning_400.c as @Sergey Sharybin (sergey) suggested in chat. This avoids generalizing this behavior, which I don't think is necessary, and is the planned permanent home of the versioning anyway.

Add missing file to diff

Hans Goudey (HooglyBoogly) retitled this revision from Fix: Move mesh legacy conversions to out of reading code to Fix: Mesh SoA format conversion skips versioning.Sep 13 2022, 2:24 AM

Sergey Sharybin (sergey) accepted this revision.Sep 13 2022, 9:53 AM

This revision is now accepted and ready to land.Sep 13 2022, 9:53 AM

Jacques Lucke (JacquesLucke) accepted this revision.Sep 13 2022, 5:04 PM

Hans Goudey (HooglyBoogly) added a child revision: D15927: Mesh: Move edge crease out of MEdge.Sep 13 2022, 7:15 PM

Closed by commit rB818c9e524d82: Fix: Mesh SoA format conversion skips versioning (authored by Hans Goudey (HooglyBoogly)). · Explain WhySep 14 2022, 6:34 PM

This revision was automatically updated to reflect the committed changes.

Hans Goudey (HooglyBoogly) added a commit: rB818c9e524d82: Fix: Mesh SoA format conversion skips versioning.

The slight changes in the committed version come from Campbell commenting in chat that it looked like a mistake that it wasn't in a version bump. I made that look more explicit here.