Page MenuHome

Fix T69822: Undo is making objects to disappear in sculpt mode
ClosedPublic

Authored by Campbell Barton (campbellbarton) on Wed, Nov 6, 6:04 AM.

Details

Summary

The bug T69822: Undo is making objects to disappear in sculpt mode is caused by edits not being flushed from sculpt mode to object data.

A simple fix for this is to always run ED_editors_flush_edits before writing memfile undo blocks, however this will often do unnecessary work. Also, there may be many objects in sculpt mode at once.

This patch tags editmode & sculpt mode objects which are out of sync, and syncs only these data-blocks before writing memfile undo steps.

Note:

  • While the report is about sculpt mode, the same kind of issue can happen in edit-mode with multiple windows & view layers.
  • Undo read/write is used to tag the edit-data as needing to be flushed, this could be changed but seems OK.
  • Armature and MetaBall's don't have edit-mode data which means the flag needs to be cleared on file load (see readfile.c for e.g.).

Diff Detail

Repository
rB Blender
Branch
TEMP-SCULPT-UNDO-FIX (branched from master)
Build Status
Buildable 5579
Build 5579: arc lint + arc unit

Event Timeline

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)
  • Rename 'Main' variable
  • Cleanup, use number instead of boolean for 'char' variables
  • Use undo for edit-mesh undo/redo tagging

Am a bit off of undo code since some time now, so not 100% sure i fully got the logic, but from what I understand this change looks good and sane.

One thing am confused about are the changes regarding the sculpt mode (am not familiar with it at all), if I understand things properly it is now fully switched to memfile undo?

Also, for edit mode of things like meshes, and sculpt mode, it means we do the custom edit/sculpt optimized undo, and flush edited results to the 'regular' obdata to be stored in memfile undo as well?

Juts trying to understand things better here, otherwise code itself looks good, besides note below.

source/blender/blenloader/intern/readfile.c
3865

Add a comment explaining why we do that for armatures and metaballs (because they never actually need flushing...)?

source/blender/editors/mesh/editmesh_undo.c
737–740

Why no bmain->is_memfile_undo_flush_needed = true; line for editmesh undo/redo? Or is it 'hidden' somewhere else? All other obedit undo code do it in both encode and decode steps. If this is not a mistake then would need a comment explaining why in any case. ;)

Campbell Barton (campbellbarton) added inline comments.
source/blender/blenloader/intern/readfile.c
3865

These don't have edit-data (moving to edit-data would be good, but thats a bigger refactor).

Since these values are stored in DNA, clearing these on read is just for good hygene - incase the DNA is written on un-flushed data.

source/blender/editors/mesh/editmesh_undo.c
737–740

It's a mistake, fixed.

Campbell Barton (campbellbarton) marked 2 inline comments as done and an inline comment as not done.Wed, Nov 6, 8:26 PM

For the records, patch seems to work (for sculpt). I never got the "objects disappear" before, but I did get an all-or-nothing undo when it comes to the other objects.

That said I just wanted to add that I get plenty of warnings when sculpting:
'BM_mesh_elem_index_ensure_ex' Invalid Index: at //source/blender/bmesh/intern/bmesh_mesh.c:1717, BM_mesh_elem_index_ensure_ex, vert[558] invalid index 26, 'Should Never Fail!', 'BM_mesh_elem_index_ensure_ex'

But I got the same before the patch, so nothing related to it.

Noticed this issue with BM_mesh_elem_index_ensure_ex too, quite sure it's unrelated to the fix.

This revision is now accepted and ready to land.Thu, Nov 7, 10:47 AM