Details

Reviewers

Maniphest Tasks

T69822: Undo is making objects to disappear in sculpt mode

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 5570
Build 5570: arc lint + arc unit

Event Timeline

Campbell Barton (campbellbarton) created this revision.Nov 6 2019, 6:04 AM

Harbormaster completed remote builds in B5570: Diff 19422.Nov 6 2019, 6:04 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Nov 6 2019, 6:11 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

Rename 'Main' variable

Harbormaster completed remote builds in B5571: Diff 19423.Nov 6 2019, 6:19 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Nov 6 2019, 6:19 AM

Cleanup, use number instead of boolean for 'char' variables

Harbormaster completed remote builds in B5572: Diff 19424.Nov 6 2019, 6:22 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Nov 6 2019, 6:23 AM

Use undo for edit-mesh undo/redo tagging

Harbormaster completed remote builds in B5573: Diff 19425.Nov 6 2019, 6:26 AM

Campbell Barton (campbellbarton) added a task: T69822: Undo is making objects to disappear in sculpt mode.Nov 6 2019, 6:32 AM

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
738 ↗	(On Diff #19425)	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. ;)

Update based on feedback from @Bastien Montagne (mont29)

Harbormaster completed remote builds in B5579: Diff 19433.Nov 6 2019, 12:28 PM

Campbell Barton (campbellbarton) marked an inline comment as done.Nov 6 2019, 12:37 PM

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
738 ↗	(On Diff #19425)	It's a mistake, fixed.

Campbell Barton (campbellbarton) marked 2 inline comments as done and an inline comment as not done.Nov 6 2019, 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.

LGTM now.

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

Committed rB79b703bb635ea719bbe31c1ece9884d2d298eaef

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

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 19422

source/blender/blenkernel/BKE_editmesh.h

source/blender/blenkernel/BKE_font.h

source/blender/blenkernel/BKE_main.h

source/blender/blenkernel/BKE_object.h

source/blender/blenkernel/BKE_paint.h

source/blender/blenkernel/intern/object.c

source/blender/blenloader/intern/readfile.c

source/blender/editors/armature/editarmature_undo.c

source/blender/editors/curve/editcurve_undo.c

source/blender/editors/curve/editfont_undo.c

source/blender/editors/include/ED_util.h

source/blender/editors/lattice/editlattice_undo.c

source/blender/editors/mesh/editmesh_utils.c

source/blender/editors/metaball/editmball_undo.c

source/blender/editors/object/object_edit.c

source/blender/editors/sculpt_paint/sculpt_undo.c

source/blender/editors/undo/ed_undo.c

source/blender/editors/undo/memfile_undo.c

source/blender/editors/util/ed_util.c

source/blender/makesdna/DNA_armature_types.h

source/blender/makesdna/DNA_curve_types.h

source/blender/makesdna/DNA_lattice_types.h

source/blender/makesdna/DNA_meta_types.h

Fix T69822: Undo is making objects to disappear in sculpt modeClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 19422

source/blender/blenkernel/BKE_editmesh.h

source/blender/blenkernel/BKE_font.h

source/blender/blenkernel/BKE_main.h

source/blender/blenkernel/BKE_object.h

source/blender/blenkernel/BKE_paint.h

source/blender/blenkernel/intern/object.c

source/blender/blenloader/intern/readfile.c

source/blender/editors/armature/editarmature_undo.c

source/blender/editors/curve/editcurve_undo.c

source/blender/editors/curve/editfont_undo.c

source/blender/editors/include/ED_util.h

source/blender/editors/lattice/editlattice_undo.c

source/blender/editors/mesh/editmesh_utils.c

source/blender/editors/metaball/editmball_undo.c

source/blender/editors/object/object_edit.c

source/blender/editors/sculpt_paint/sculpt_undo.c

source/blender/editors/undo/ed_undo.c

source/blender/editors/undo/memfile_undo.c

source/blender/editors/util/ed_util.c

source/blender/makesdna/DNA_armature_types.h

source/blender/makesdna/DNA_curve_types.h

source/blender/makesdna/DNA_lattice_types.h

source/blender/makesdna/DNA_meta_types.h

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

Revision Contents
Changeset List